yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fixes regression issue with property values being updated in content repositories (#9146)

Browse Source
This commit is contained in:
Shannon Deminick
2020-10-22 05:30:35 +11:00
committed by GitHub
parent b96776949f
commit 657f1fcdc7
8 changed files with 172 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files

71
src/Umbraco.Core/Persistence/Repositories/Implement/ContentRepositoryBase.cs
View File

@@ -863,6 +863,77 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
}
/// <summary>
/// Inserts property values for the content entity
/// </summary>
/// <param name="entity"></param>
/// <param name="publishedVersionId"></param>
/// <param name="edited"></param>
/// <param name="editedCultures"></param>
/// <remarks>
/// Used when creating a new entity
/// </remarks>
protected void InsertPropertyValues(TEntity entity, int publishedVersionId, out bool edited, out HashSet<string> 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.
}
/// <summary>
/// Used to atomically replace the property values for the entity version specified
/// </summary>
/// <param name="entity"></param>
/// <param name="versionId"></param>
/// <param name="publishedVersionId"></param>
/// <param name="edited"></param>
/// <param name="editedCultures"></param>
protected void ReplacePropertyValues(TEntity entity, int versionId, int publishedVersionId, out bool edited, out HashSet<string> 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<PropertyDataDto>().Where<PropertyDataDto>(x => x.VersionId == versionId).ForUpdate();
var existingPropData = Database.Fetch<PropertyDataDto>(propDataSql);
var propertyTypeToPropertyData = new Dictionary<(int propertyTypeId, int versionId), PropertyDataDto>();
var existingPropDataIds = new List<int>();
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<PropertyDataDto>().WhereIn<PropertyDataDto>(x => x.Id, existingPropDataIds));
}
}
private class NodeIdKey
{
[Column("id")]

16
src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs
View File

@@ -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<PropertyDataDto>().Where<PropertyDataDto>(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'

19
src/Umbraco.Core/Persistence/Repositories/Implement/MediaRepository.cs
View File

@@ -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<PropertyDataDto>().Where<PropertyDataDto>(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);

36
src/Umbraco.Core/Persistence/Repositories/Implement/MemberRepository.cs
View File

@@ -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<PropertyDataDto>().Where<PropertyDataDto>(x => x.VersionId == member.VersionId).ForUpdate();
var existingPropData = Database.Fetch<PropertyDataDto>(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);

47
src/Umbraco.Tests/Services/ContentServiceTests.cs
View File

@@ -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()
{

24
src/Umbraco.Tests/Services/MediaServiceTests.cs
View File

@@ -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"));
}
/// <summary>
/// Used to list out all ambiguous events that will require dispatching with a name
/// </summary>

9
src/Umbraco.Tests/Services/MemberServiceTests.cs
View File

@@ -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]

10
src/Umbraco.Web/Search/ExamineComponent.cs
View File

@@ -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
}
}