diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentRepositoryBase.cs index 13b687eb4e..2ea4914aaa 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentRepositoryBase.cs @@ -863,6 +863,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 a34aadd70f..2fdfc9afff 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs @@ -519,8 +519,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement protected override void PersistUpdatedItem(IContent entity) { - var entityBase = entity as EntityBase; - var isEntityDirty = entityBase != null && entityBase.IsDirty(); + var isEntityDirty = entity.IsDirty(); // check if we need to make any database changes at all if ((entity.PublishedState == PublishedState.Published || entity.PublishedState == PublishedState.Unpublished) @@ -597,17 +596,10 @@ namespace Umbraco.Core.Persistence.Repositories.Implement Database.Insert(documentVersionDto); } - // replace the property data (rather than updating) + // replace the property data // 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; + 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 081efcfdf6..63cee80a8b 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/MediaRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/MediaRepository.cs @@ -219,7 +219,6 @@ namespace Umbraco.Core.Persistence.Repositories.Implement protected override void PersistNewItem(IMedia entity) { - var media = (Models.Media) entity; entity.AddingEntity(); // ensure unique name on the same level @@ -274,17 +273,15 @@ namespace Umbraco.Core.Persistence.Repositories.Implement contentVersionDto.NodeId = nodeDto.NodeId; contentVersionDto.Current = true; Database.Insert(contentVersionDto); - media.VersionId = contentVersionDto.Id; + entity.VersionId = contentVersionDto.Id; // persist the media version dto var mediaVersionDto = dto.MediaVersionDto; - mediaVersionDto.Id = media.VersionId; + mediaVersionDto.Id = entity.VersionId; Database.Insert(mediaVersionDto); // persist the property data - var propertyDataDtos = PropertyFactory.BuildDtos(media.ContentType.Variations, media.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); @@ -298,10 +295,8 @@ namespace Umbraco.Core.Persistence.Repositories.Implement protected override void PersistUpdatedItem(IMedia entity) { - var media = (Models.Media) entity; - // update - media.UpdatingEntity(); + entity.UpdatingEntity(); // ensure unique name on the same level entity.Name = EnsureUniqueNodeName(entity.ParentId, entity.Name, entity.Id); @@ -339,11 +334,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement Database.Update(mediaVersionDto); // replace the property data - var deletePropertyDataSql = SqlContext.Sql().Delete().Where(x => x.VersionId == media.VersionId); - Database.Execute(deletePropertyDataSql); - var propertyDataDtos = PropertyFactory.BuildDtos(media.ContentType.Variations, media.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.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index 92d2a68472..c0d9b47c37 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 ce84c2701b..00bfe59fa3 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.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 - + } }