diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentRepositoryBase.cs index 845006891d..77d114abe1 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentRepositoryBase.cs @@ -980,6 +980,77 @@ namespace Umbraco.Core.Persistence.Repositories.Implement } + /// + /// Inserts property values for the content entity + /// + /// + /// + /// + /// + /// + /// Used when creating a new entity + /// + protected void InsertPropertyValues(TEntity entity, int publishedVersionId, out bool edited, out HashSet editedCultures) + { + // persist the property data + var propertyDataDtos = PropertyFactory.BuildDtos(entity.ContentType.Variations, entity.VersionId, publishedVersionId, entity.Properties, LanguageRepository, out edited, out editedCultures); + foreach (var propertyDataDto in propertyDataDtos) + { + Database.Insert(propertyDataDto); + } + // TODO: we can speed this up: Use BulkInsert and then do one SELECT to re-retrieve the property data inserted with assigned IDs. + // This is a perfect thing to benchmark with Benchmark.NET to compare perf between Nuget releases. + } + + /// + /// Used to atomically replace the property values for the entity version specified + /// + /// + /// + /// + /// + /// + protected void ReplacePropertyValues(TEntity entity, int versionId, int publishedVersionId, out bool edited, out HashSet editedCultures) + { + // Replace the property data. + // Lookup the data to update with a UPDLOCK (using ForUpdate()) this is because we need to be atomic + // and handle DB concurrency. Doing a clear and then re-insert is prone to concurrency issues. + + var propDataSql = SqlContext.Sql().Select("*").From().Where(x => x.VersionId == versionId).ForUpdate(); + var existingPropData = Database.Fetch(propDataSql); + var propertyTypeToPropertyData = new Dictionary<(int propertyTypeId, int versionId), PropertyDataDto>(); + var existingPropDataIds = new List(); + foreach (var p in existingPropData) + { + existingPropDataIds.Add(p.Id); + propertyTypeToPropertyData[(p.PropertyTypeId, p.VersionId)] = p; + } + var propertyDataDtos = PropertyFactory.BuildDtos(entity.ContentType.Variations, entity.VersionId, publishedVersionId, entity.Properties, LanguageRepository, out edited, out editedCultures); + foreach (var propertyDataDto in propertyDataDtos) + { + // Check if this already exists and update, else insert a new one + if (propertyTypeToPropertyData.TryGetValue((propertyDataDto.PropertyTypeId, propertyDataDto.VersionId), out var propData)) + { + propertyDataDto.Id = propData.Id; + Database.Update(propertyDataDto); + } + else + { + // TODO: we can speed this up: Use BulkInsert and then do one SELECT to re-retrieve the property data inserted with assigned IDs. + // This is a perfect thing to benchmark with Benchmark.NET to compare perf between Nuget releases. + Database.Insert(propertyDataDto); + } + + // track which ones have been processed + existingPropDataIds.Remove(propertyDataDto.Id); + } + // For any remaining that haven't been processed they need to be deleted + if (existingPropDataIds.Count > 0) + { + Database.Execute(SqlContext.Sql().Delete().WhereIn(x => x.Id, existingPropDataIds)); + } + } + private class NodeIdKey { [Column("id")] diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs index e4ef939870..c230d9f894 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs @@ -610,17 +610,11 @@ namespace Umbraco.Core.Persistence.Repositories.Implement Database.Insert(documentVersionDto); } - // replace the property data (rather than updating) + // replace the property data (rather than updating) // only need to delete for the version that existed, the new version (if any) has no property data yet - var versionToDelete = publishing ? entity.PublishedVersionId : entity.VersionId; - var deletePropertyDataSql = Sql().Delete().Where(x => x.VersionId == versionToDelete); - Database.Execute(deletePropertyDataSql); - - // insert property data - var propertyDataDtos = PropertyFactory.BuildDtos(entity.ContentType.Variations, entity.VersionId, publishing ? entity.PublishedVersionId : 0, - entity.Properties, LanguageRepository, out var edited, out var editedCultures); - foreach (var propertyDataDto in propertyDataDtos) - Database.Insert(propertyDataDto); + var versionToDelete = publishing ? entity.PublishedVersionId : entity.VersionId; + // insert property data + ReplacePropertyValues(entity, versionToDelete, publishing ? entity.PublishedVersionId : 0, out var edited, out var editedCultures); // if !publishing, we may have a new name != current publish name, // also impacts 'edited' diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/MediaRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/MediaRepository.cs index 242f21c749..83198d4428 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/MediaRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/MediaRepository.cs @@ -281,9 +281,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement Database.Insert(mediaVersionDto); // persist the property data - var propertyDataDtos = PropertyFactory.BuildDtos(entity.ContentType.Variations, entity.VersionId, 0, entity.Properties, LanguageRepository, out _, out _); - foreach (var propertyDataDto in propertyDataDtos) - Database.Insert(propertyDataDto); + InsertPropertyValues(entity, 0, out _, out _); // set tags SetEntityTags(entity, _tagRepository); @@ -346,11 +344,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement Database.Update(mediaVersionDto); // replace the property data - var deletePropertyDataSql = SqlContext.Sql().Delete().Where(x => x.VersionId == entity.VersionId); - Database.Execute(deletePropertyDataSql); - var propertyDataDtos = PropertyFactory.BuildDtos(entity.ContentType.Variations, entity.VersionId, 0, entity.Properties, LanguageRepository, out _, out _); - foreach (var propertyDataDto in propertyDataDtos) - Database.Insert(propertyDataDto); + ReplacePropertyValues(entity, entity.VersionId, 0, out _, out _); SetEntityTags(entity, _tagRepository); diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/MemberRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/MemberRepository.cs index 687e35aef3..78dbbe317a 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/MemberRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/MemberRepository.cs @@ -245,8 +245,6 @@ namespace Umbraco.Core.Persistence.Repositories.Implement } entity.AddingEntity(); - var member = (Member) entity; - // ensure that strings don't contain characters that are invalid in xml // TODO: do we really want to keep doing this here? entity.SanitizeEntityPropertiesForXmlStorage(); @@ -304,7 +302,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement contentVersionDto.NodeId = nodeDto.NodeId; contentVersionDto.Current = true; Database.Insert(contentVersionDto); - member.VersionId = contentVersionDto.Id; + entity.VersionId = contentVersionDto.Id; // persist the member dto dto.NodeId = nodeDto.NodeId; @@ -321,9 +319,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement Database.Insert(dto); // persist the property data - var propertyDataDtos = PropertyFactory.BuildDtos(member.ContentType.Variations, member.VersionId, 0, entity.Properties, LanguageRepository, out _, out _); - foreach (var propertyDataDto in propertyDataDtos) - Database.Insert(propertyDataDto); + InsertPropertyValues(entity, 0, out _, out _); SetEntityTags(entity, _tagRepository); @@ -335,11 +331,9 @@ namespace Umbraco.Core.Persistence.Repositories.Implement } protected override void PersistUpdatedItem(IMember entity) - { - var member = (Member) entity; - + { // update - member.UpdatingEntity(); + entity.UpdatingEntity(); // ensure that strings don't contain characters that are invalid in xml // TODO: do we really want to keep doing this here? @@ -385,27 +379,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement if (changedCols.Count > 0) Database.Update(dto, changedCols); - // Replace the property data - // Lookup the data to update with a UPDLOCK (using ForUpdate()) this is because we have another method that doesn't take an explicit WriteLock - // in SetLastLogin which is called very often and we want to avoid the lock timeout for the explicit lock table but we still need to ensure atomic - // operations between that method and this one. - - var propDataSql = SqlContext.Sql().Select("*").From().Where(x => x.VersionId == member.VersionId).ForUpdate(); - var existingPropData = Database.Fetch(propDataSql).ToDictionary(x => x.PropertyTypeId); - var propertyDataDtos = PropertyFactory.BuildDtos(member.ContentType.Variations, member.VersionId, 0, entity.Properties, LanguageRepository, out _, out _); - foreach (var propertyDataDto in propertyDataDtos) - { - // Check if this already exists and update, else insert a new one - if (existingPropData.TryGetValue(propertyDataDto.PropertyTypeId, out var propData)) - { - propertyDataDto.Id = propData.Id; - Database.Update(propertyDataDto); - } - else - { - Database.Insert(propertyDataDto); - } - } + ReplacePropertyValues(entity, entity.VersionId, 0, out _, out _); SetEntityTags(entity, _tagRepository); diff --git a/src/Umbraco.Examine/ContentIndexPopulator.cs b/src/Umbraco.Examine/ContentIndexPopulator.cs index af30b56038..837c230313 100644 --- a/src/Umbraco.Examine/ContentIndexPopulator.cs +++ b/src/Umbraco.Examine/ContentIndexPopulator.cs @@ -4,6 +4,7 @@ using System.Linq; using Examine; using Umbraco.Core; using Umbraco.Core.Models; +using Umbraco.Core.Models.Blocks; using Umbraco.Core.Services; using Umbraco.Core.Persistence; using Umbraco.Core.Persistence.DatabaseModelDefinitions; @@ -76,31 +77,89 @@ namespace Umbraco.Examine { contentParentId = _parentId.Value; } + + if (_publishedValuesOnly) + { + IndexPublishedContent(contentParentId, pageIndex, pageSize, indexes); + } + else + { + IndexAllContent(contentParentId, pageIndex, pageSize, indexes); + } + } + + protected void IndexAllContent(int contentParentId, int pageIndex, int pageSize, IReadOnlyList indexes) + { IContent[] content; do { - if (!_publishedValuesOnly) - { - content = _contentService.GetPagedDescendants(contentParentId, pageIndex, pageSize, out _).ToArray(); - } - else - { - //add the published filter - //note: We will filter for published variants in the validator - content = _contentService.GetPagedDescendants(contentParentId, pageIndex, pageSize, out _, - _publishedQuery, Ordering.By("Path", Direction.Ascending)).ToArray(); - } + content = _contentService.GetPagedDescendants(contentParentId, pageIndex, pageSize, out _).ToArray(); if (content.Length > 0) { + var valueSets = _contentValueSetBuilder.GetValueSets(content).ToList(); + // ReSharper disable once PossibleMultipleEnumeration foreach (var index in indexes) - index.IndexItems(_contentValueSetBuilder.GetValueSets(content)); + { + index.IndexItems(valueSets); + } + } + + pageIndex++; + } while (content.Length == pageSize); + } + + protected void IndexPublishedContent(int contentParentId, int pageIndex, int pageSize, + IReadOnlyList indexes) + { + IContent[] content; + + var publishedPages = new HashSet(); + + do + { + //add the published filter + //note: We will filter for published variants in the validator + content = _contentService.GetPagedDescendants(contentParentId, pageIndex, pageSize, out _, _publishedQuery, + Ordering.By("Path", Direction.Ascending)).ToArray(); + + + if (content.Length > 0) + { + var indexableContent = new List(); + + foreach (var item in content) + { + if (item.Level == 1) + { + // first level pages are always published so no need to filter them + indexableContent.Add(item); + publishedPages.Add(item.Id); + } + else + { + if (publishedPages.Contains(item.ParentId)) + { + // only index when parent is published + publishedPages.Add(item.Id); + indexableContent.Add(item); + } + } + } + + var valueSets = _contentValueSetBuilder.GetValueSets(indexableContent.ToArray()).ToList(); + + // ReSharper disable once PossibleMultipleEnumeration + foreach (var index in indexes) + index.IndexItems(valueSets); } pageIndex++; } while (content.Length == pageSize); } } + + } diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index 041dabe7d2..bfbb9a0d95 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -1537,6 +1537,53 @@ namespace Umbraco.Tests.Services Assert.That(content.HasIdentity, Is.True); } + [Test] + public void Can_Update_Content_Property_Values() + { + IContentType contentType = MockedContentTypes.CreateSimpleContentType(); + ServiceContext.ContentTypeService.Save(contentType); + IContent content = MockedContent.CreateSimpleContent(contentType, "hello"); + content.SetValue("title", "title of mine"); + content.SetValue("bodyText", "hello world"); + ServiceContext.ContentService.SaveAndPublish(content); + + // re-get + content = ServiceContext.ContentService.GetById(content.Id); + content.SetValue("title", "another title of mine"); // Change a value + content.SetValue("bodyText", null); // Clear a value + content.SetValue("author", "new author"); // Add a value + ServiceContext.ContentService.SaveAndPublish(content); + + // re-get + content = ServiceContext.ContentService.GetById(content.Id); + Assert.AreEqual("another title of mine", content.GetValue("title")); + Assert.IsNull(content.GetValue("bodyText")); + Assert.AreEqual("new author", content.GetValue("author")); + + content.SetValue("title", "new title"); + content.SetValue("bodyText", "new body text"); + content.SetValue("author", "new author text"); + ServiceContext.ContentService.Save(content); // new non-published version + + // re-get + content = ServiceContext.ContentService.GetById(content.Id); + content.SetValue("title", null); // Clear a value + content.SetValue("bodyText", null); // Clear a value + ServiceContext.ContentService.Save(content); // saving non-published version + + // re-get + content = ServiceContext.ContentService.GetById(content.Id); + Assert.IsNull(content.GetValue("title")); // Test clearing the value worked with the non-published version + Assert.IsNull(content.GetValue("bodyText")); + Assert.AreEqual("new author text", content.GetValue("author")); + + // make sure that the published version remained the same + var publishedContent = ServiceContext.ContentService.GetVersion(content.PublishedVersionId); + Assert.AreEqual("another title of mine", publishedContent.GetValue("title")); + Assert.IsNull(publishedContent.GetValue("bodyText")); + Assert.AreEqual("new author", publishedContent.GetValue("author")); + } + [Test] public void Can_Bulk_Save_Content() { diff --git a/src/Umbraco.Tests/Services/MediaServiceTests.cs b/src/Umbraco.Tests/Services/MediaServiceTests.cs index b3dc274c5e..52f26ecb4d 100644 --- a/src/Umbraco.Tests/Services/MediaServiceTests.cs +++ b/src/Umbraco.Tests/Services/MediaServiceTests.cs @@ -26,6 +26,30 @@ namespace Umbraco.Tests.Services [UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest, PublishedRepositoryEvents = true)] public class MediaServiceTests : TestWithSomeContentBase { + [Test] + public void Can_Update_Media_Property_Values() + { + IMediaType mediaType = MockedContentTypes.CreateSimpleMediaType("test", "Test"); + ServiceContext.MediaTypeService.Save(mediaType); + IMedia media = MockedMedia.CreateSimpleMedia(mediaType, "hello", -1); + media.SetValue("title", "title of mine"); + media.SetValue("bodyText", "hello world"); + ServiceContext.MediaService.Save(media); + + // re-get + media = ServiceContext.MediaService.GetById(media.Id); + media.SetValue("title", "another title of mine"); // Change a value + media.SetValue("bodyText", null); // Clear a value + media.SetValue("author", "new author"); // Add a value + ServiceContext.MediaService.Save(media); + + // re-get + media = ServiceContext.MediaService.GetById(media.Id); + Assert.AreEqual("another title of mine", media.GetValue("title")); + Assert.IsNull(media.GetValue("bodyText")); + Assert.AreEqual("new author", media.GetValue("author")); + } + /// /// Used to list out all ambiguous events that will require dispatching with a name /// diff --git a/src/Umbraco.Tests/Services/MemberServiceTests.cs b/src/Umbraco.Tests/Services/MemberServiceTests.cs index 816bc6d7dd..61bc3cf1c1 100644 --- a/src/Umbraco.Tests/Services/MemberServiceTests.cs +++ b/src/Umbraco.Tests/Services/MemberServiceTests.cs @@ -49,22 +49,27 @@ namespace Umbraco.Tests.Services } [Test] - public void Can_Update_Member_Property_Value() + public void Can_Update_Member_Property_Values() { IMemberType memberType = MockedContentTypes.CreateSimpleMemberType(); ServiceContext.MemberTypeService.Save(memberType); IMember member = MockedMember.CreateSimpleMember(memberType, "hello", "helloworld@test123.com", "hello", "hello"); member.SetValue("title", "title of mine"); + member.SetValue("bodyText", "hello world"); ServiceContext.MemberService.Save(member); // re-get member = ServiceContext.MemberService.GetById(member.Id); - member.SetValue("title", "another title of mine"); + member.SetValue("title", "another title of mine"); // Change a value + member.SetValue("bodyText", null); // Clear a value + member.SetValue("author", "new author"); // Add a value ServiceContext.MemberService.Save(member); // re-get member = ServiceContext.MemberService.GetById(member.Id); Assert.AreEqual("another title of mine", member.GetValue("title")); + Assert.IsNull(member.GetValue("bodyText")); + Assert.AreEqual("new author", member.GetValue("author")); } [Test] diff --git a/src/Umbraco.Tests/UmbracoExamine/IndexInitializer.cs b/src/Umbraco.Tests/UmbracoExamine/IndexInitializer.cs index e9f18d8947..1a288b423c 100644 --- a/src/Umbraco.Tests/UmbracoExamine/IndexInitializer.cs +++ b/src/Umbraco.Tests/UmbracoExamine/IndexInitializer.cs @@ -45,7 +45,7 @@ namespace Umbraco.Tests.UmbracoExamine public static ContentIndexPopulator GetContentIndexRebuilder(PropertyEditorCollection propertyEditors, IContentService contentService, IScopeProvider scopeProvider, bool publishedValuesOnly) { var contentValueSetBuilder = GetContentValueSetBuilder(propertyEditors, scopeProvider, publishedValuesOnly); - var contentIndexDataSource = new ContentIndexPopulator(true, null, contentService, scopeProvider.SqlContext, contentValueSetBuilder); + var contentIndexDataSource = new ContentIndexPopulator(publishedValuesOnly, null, contentService, scopeProvider.SqlContext, contentValueSetBuilder); return contentIndexDataSource; } diff --git a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj index 5d9bd8c343..2ccece8f49 100644 --- a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj +++ b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj @@ -349,6 +349,9 @@ 8900 / http://localhost:8900 + 8660 + / + http://localhost:8660 False False diff --git a/src/Umbraco.Web/Search/ExamineComponent.cs b/src/Umbraco.Web/Search/ExamineComponent.cs index ca16aaee65..c9d7b7cf56 100644 --- a/src/Umbraco.Web/Search/ExamineComponent.cs +++ b/src/Umbraco.Web/Search/ExamineComponent.cs @@ -269,6 +269,14 @@ namespace Umbraco.Web.Search DeleteIndexForEntity(c4.Id, false); } break; + case MessageType.RefreshByPayload: + var payload = (MemberCacheRefresher.JsonPayload[])args.MessageObject; + var members = payload.Select(x => _services.MemberService.GetById(x.Id)); + foreach(var m in members) + { + ReIndexForMember(m); + } + break; case MessageType.RefreshAll: case MessageType.RefreshByJson: default: @@ -746,6 +754,6 @@ namespace Umbraco.Web.Search } #endregion - + } }