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

Passes test! Now to add more tests/assertions and then the logic to renormalize based on name changes.

Browse Source
This commit is contained in:
Shannon
2019-08-01 15:54:44 +10:00
parent 87e7cec02e
commit ff952a6df1
4 changed files with 95 additions and 63 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
src/Umbraco.Core/Persistence/Factories/PropertyFactory.cs
View File

@@ -104,10 +104,9 @@ namespace Umbraco.Core.Persistence.Factories
/// <param name="properties">The properties to map</param>
/// <param name="languageRepository"></param>
/// <param name="edited">out parameter indicating that one or more properties have been edited</param>
/// <param name="editedCultures">out parameter containing a collection of edited cultures when the contentVariation varies by culture</param>
/// <param name="persistedEditedCultures">
/// out parameter containing a collection of edited cultures that are currently persisted in the database which is used to maintain the edited state
/// of each culture value (in cases where variance is being switched)
/// <param name="editedCultures">
/// Out parameter containing a collection of edited cultures when the contentVariation varies by culture.
/// The value of this will be used to populate the edited cultures in the umbracoDocumentCultureVariation table.
/// </param>
/// <returns></returns>
public static IEnumerable<PropertyDataDto> BuildDtos(ContentVariation contentVariation, int currentVersionId, int publishedVersionId, IEnumerable<Property> properties,
@@ -146,14 +145,16 @@ namespace Umbraco.Core.Persistence.Factories
if (propertyValue.EditedValue != null)
propertyDataDtos.Add(BuildDto(currentVersionId, property, languageRepository.GetIdByIsoCode(propertyValue.Culture), propertyValue.Segment, propertyValue.EditedValue));
//// property.Values will contain ALL of it's values, both variant and invariant which will be populated if the administrator has previously
//// changed the property type to be variant vs invariant.
//// We need to check for this scenario here because otherwise the editedCultures and edited flags
//// will end up incorrectly so here we need to only process edited cultures based on the
//// current value type and how the property varies.
// property.Values will contain ALL of it's values, both variant and invariant which will be populated if the
// administrator has previously changed the property type to be variant vs invariant.
// We need to check for this scenario here because otherwise the editedCultures and edited flags
// will end up incorrectly set in the umbracoDocumentCultureVariation table so here we need to
// only process edited cultures based on the current value type and how the property varies.
// The above logic will still persist the currently saved property value for each culture in case the admin
// decides to swap the property's variance again, in which case the edited flag will be recalculated.
//if (property.PropertyType.VariesByCulture() && isInvariantValue) continue;
//if (!property.PropertyType.VariesByCulture() && isCultureValue) continue;
if (property.PropertyType.VariesByCulture() && isInvariantValue || !property.PropertyType.VariesByCulture() && isCultureValue)
continue;
// use explicit equals here, else object comparison fails at comparing eg strings
var sameValues = propertyValue.PublishedValue == null ? propertyValue.EditedValue == null : propertyValue.PublishedValue.Equals(propertyValue.EditedValue);

112
src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs
View File

@@ -648,14 +648,14 @@ AND umbracoNode.id <> @id",
case ContentVariation.Culture:
CopyPropertyData(null, defaultLanguageId, propertyTypeIds, impactedL);
CopyTagData(null, defaultLanguageId, propertyTypeIds, impactedL);
RenormalizeDocumentCultureVariations(propertyTypeIds, impactedL);
RenormalizeDocumentEditedFlags(propertyTypeIds, impactedL);
//TODO: Here we need to normalize the umbracoDocumentCultureVariation table for it's edited flags which are calculated based
//on changed property or name values
break;
case ContentVariation.Nothing:
CopyPropertyData(defaultLanguageId, null, propertyTypeIds, impactedL);
CopyTagData(defaultLanguageId, null, propertyTypeIds, impactedL);
RenormalizeDocumentCultureVariations(propertyTypeIds, impactedL);
RenormalizeDocumentEditedFlags(propertyTypeIds, impactedL);
//TODO: Here we need to normalize the umbracoDocumentCultureVariation table for it's edited flags which are calculated based
//on changed property or name values
break;
@@ -1024,19 +1024,17 @@ AND umbracoNode.id <> @id",
}
/// <summary>
/// Re-normalizes the edited value in the umbracoDocumentCultureVariation table when property variations are changed
/// Re-normalizes the edited value in the umbracoDocumentCultureVariation and umbracoDocument table when variations are changed
/// </summary>
/// <param name="propertyTypeIds"></param>
/// <param name="contentTypeIds"></param>
/// <remarks>
/// If this is not done, then in some cases the "edited" value for a particular culture for a document will remain true when it should be false
/// if the property was changed to invariant. In order to do this we need to recalculate this value based on the values stored for each
/// property, culture and current/published version. The end result is to update the edited value in the umbracoDocumentCultureVariation table so we
/// make sure to join this table with the lookups so that only relevant data is returned.
/// property, culture and current/published version.
/// </remarks>
private void RenormalizeDocumentCultureVariations(IReadOnlyCollection<int> propertyTypeIds, IReadOnlyCollection<int> contentTypeIds = null)
private void RenormalizeDocumentEditedFlags(IReadOnlyCollection<int> propertyTypeIds, IReadOnlyCollection<int> contentTypeIds = null)
{
var defaultLang = LanguageRepository.GetDefaultId();
//This will build up a query to get the property values of both the current and the published version so that we can check
@@ -1073,14 +1071,16 @@ AND umbracoNode.id <> @id",
.OrderBy<ContentVersionDto>(x => x.NodeId)
.OrderBy<PropertyDataDto>(x => x.PropertyTypeId, x => x.LanguageId, x => x.VersionId);
//keep track of this node/lang to mark or unmark as edited
var editedVersions = new Dictionary<(int nodeId, int? langId), bool>();
//keep track of this node/lang to mark or unmark a culture as edited
var editedLanguageVersions = new Dictionary<(int nodeId, int? langId), bool>();
//keep track of which node to mark or unmark as edited
var editedDocument = new Dictionary<int, bool>();
var nodeId = -1;
var propertyTypeId = -1;
PropertyValueVersionDto pubRow = null;
//This is a QUERY we are not fetching this all into memory so we cannot make any changes during this iteration, we are just collecting data.
//This is a reader (Query), we are not fetching this all into memory so we cannot make any changes during this iteration, we are just collecting data.
//Published data will always come before Current data based on the version id sort.
//There will only be one published row (max) and one current row per property.
foreach (var row in Database.Query<PropertyValueVersionDto>(propertySql))
@@ -1105,19 +1105,24 @@ AND umbracoNode.id <> @id",
|| propVariations.VariesByCulture() && !row.LanguageId.HasValue)
{
//Flag this as not edited for this node/lang if the key doesn't exist
if (!editedVersions.TryGetValue((row.NodeId, row.LanguageId), out _))
editedVersions.Add((row.NodeId, row.LanguageId), false);
if (!editedLanguageVersions.TryGetValue((row.NodeId, row.LanguageId), out _))
editedLanguageVersions.Add((row.NodeId, row.LanguageId), false);
//mark as false if the item doesn't exist, else coerce to true
editedDocument[row.NodeId] = editedDocument.TryGetValue(row.NodeId, out var edited) ? (edited |= false) : false;
}
else if (pubRow == null)
{
//this would mean that that this property is 'edited' since there is no published version
editedVersions.Add((row.NodeId, row.LanguageId), true);
editedLanguageVersions.Add((row.NodeId, row.LanguageId), true);
editedDocument[row.NodeId] = true;
}
//compare the property values, if they differ from versions then flag the current version as edited
else if (IsPropertyValueChanged(pubRow, row))
{
//Here we would check if the property is invariant, in which case the edited language should be indicated by the default lang
editedVersions[(row.NodeId, !propVariations.VariesByCulture() ? defaultLang : row.LanguageId)] = true;
editedLanguageVersions[(row.NodeId, !propVariations.VariesByCulture() ? defaultLang : row.LanguageId)] = true;
editedDocument[row.NodeId] = true;
}
//reset
@@ -1125,32 +1130,6 @@ AND umbracoNode.id <> @id",
}
}
//lookup all matching rows in umbracoDocumentCultureVariation
var docCultureVariationsToUpdate = Database.Fetch<DocumentCultureVariationDto>(
Sql().Select<DocumentCultureVariationDto>().From<DocumentCultureVariationDto>()
.WhereIn<DocumentCultureVariationDto>(x => x.LanguageId, editedVersions.Keys.Select(x => x.langId).ToList())
.WhereIn<DocumentCultureVariationDto>(x => x.NodeId, editedVersions.Keys.Select(x => x.nodeId)))
//convert to dictionary with the same key type
.ToDictionary(x => (x.NodeId, (int?)x.LanguageId), x => x);
foreach (var ev in editedVersions)
{
if (docCultureVariationsToUpdate.TryGetValue(ev.Key, out var docVariations))
{
//check if it needs updating
if (docVariations.Edited != ev.Value)
{
docVariations.Edited = ev.Value;
Database.Update(docVariations);
}
}
else
{
//the row doesn't exist but needs creating
//TODO: Does this ever happen?? Need to see if we can test this
}
}
////Generate SQL to lookup the current name vs the publish name for each language
//var nameSql = Sql()
// .Select<ContentVersionDto>("cv1", x => x.NodeId, x => Alias(x.Id, "currentVersion"))
@@ -1172,8 +1151,57 @@ AND umbracoNode.id <> @id",
// .Where<ContentVersionDto>(x => x.Current, "cv1")
// .OrderBy("cv1.nodeId, cvcv1.versionId, cvcv1.languageId");
//var names = Database.Fetch<NameCompareDto>(nameSql);
////This is a reader (Query), we are not fetching this all into memory so we cannot make any changes during this iteration, we are just collecting data.
//foreach (var name in Database.Query<NameCompareDto>(nameSql))
//{
// if (name.CurrentName != name.PublishedName)
// {
// }
//}
//lookup all matching rows in umbracoDocumentCultureVariation
var docCultureVariationsToUpdate = editedLanguageVersions.InGroupsOf(2000)
.SelectMany(_ => Database.Fetch<DocumentCultureVariationDto>(
Sql().Select<DocumentCultureVariationDto>().From<DocumentCultureVariationDto>()
.WhereIn<DocumentCultureVariationDto>(x => x.LanguageId, editedLanguageVersions.Keys.Select(x => x.langId).ToList())
.WhereIn<DocumentCultureVariationDto>(x => x.NodeId, editedLanguageVersions.Keys.Select(x => x.nodeId))))
//convert to dictionary with the same key type
.ToDictionary(x => (x.NodeId, (int?)x.LanguageId), x => x);
var toUpdate = new List<DocumentCultureVariationDto>();
foreach (var ev in editedLanguageVersions)
{
if (docCultureVariationsToUpdate.TryGetValue(ev.Key, out var docVariations))
{
//check if it needs updating
if (docVariations.Edited != ev.Value)
{
docVariations.Edited = ev.Value;
toUpdate.Add(docVariations);
}
}
else
{
//the row doesn't exist but needs creating
//TODO: Does this ever happen?? Need to see if we can test this
throw new PanicException($"The existing DocumentCultureVariationDto was not found for node {ev.Key.nodeId} and language {ev.Key.langId}");
}
}
//Now bulk update the table DocumentCultureVariationDto, once for edited = true, another for edited = false
foreach (var editValue in toUpdate.GroupBy(x => x.Edited))
{
Database.Execute(Sql().Update<DocumentCultureVariationDto>(u => u.Set(x => x.Edited, editValue.Key))
.WhereIn<DocumentCultureVariationDto>(x => x.Id, editValue.Select(x => x.Id)));
}
//Now bulk update the umbracoDocument table
foreach(var editValue in editedDocument.GroupBy(x => x.Value))
{
Database.Execute(Sql().Update<DocumentDto>(u => u.Set(x => x.Edited, editValue.Key))
.WhereIn<DocumentDto>(x => x.NodeId, editValue.Select(x => x.Key)));
}
}
private static bool IsPropertyValueChanged(PropertyValueVersionDto pubRow, PropertyValueVersionDto row)

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

@@ -511,7 +511,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
entity.VersionId = documentVersionDto.Id = contentVersionDto.Id; // get the new id
documentVersionDto.Published = false; // non-published version
Database.Insert(documentVersionDto);
Database.Insert(documentVersionDto);
}
// replace the property data (rather than updating)

21
src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs
View File

@@ -828,12 +828,14 @@ namespace Umbraco.Tests.Services
Assert.AreEqual("doc1en", document.GetCultureName("en"));
Assert.AreEqual("doc1fr", document.GetCultureName("fr"));
Assert.AreEqual("v1en", document.GetValue("value1", "en"));
Assert.AreEqual("v1en-init", document.GetValue("value1", "en", published: true));
Assert.AreEqual("v1fr", document.GetValue("value1", "fr"));
Assert.AreEqual("v1fr-init", document.GetValue("value1", "fr", published: true));
Assert.IsTrue(document.IsCultureEdited("en")); //This will be true because the edited value isn't the same as the published value
Assert.IsTrue(document.IsCultureEdited("fr")); //This will be true because the edited value isn't the same as the published value
Assert.IsTrue(document.Edited);
// switch property type to Nothing
// switch property type to Invariant
contentType.PropertyTypes.First(x => x.Alias == "value1").Variations = ContentVariation.Nothing;
ServiceContext.ContentTypeService.Save(contentType); //This is going to have to re-normalize the "Edited" flag
@@ -852,9 +854,12 @@ namespace Umbraco.Tests.Services
Assert.AreEqual("doc1fr", document.GetCultureName("fr"));
Assert.IsNull(document.GetValue("value1", "en")); //The values are there but the business logic returns null
Assert.IsNull(document.GetValue("value1", "fr")); //The values are there but the business logic returns null
Assert.IsNull(document.GetValue("value1", "en", published: true)); //The values are there but the business logic returns null
Assert.IsNull(document.GetValue("value1", "fr", published: true)); //The values are there but the business logic returns null
Assert.AreEqual("v1inv", document.GetValue("value1"));
Assert.IsFalse(document.IsCultureEdited("en")); //This returns false, everything is published - however in the DB this is not the case for the "en" version of the property
Assert.IsFalse(document.IsCultureEdited("fr")); //This returns false, everything is published
Assert.AreEqual("v1inv", document.GetValue("value1", published: true));
Assert.IsFalse(document.IsCultureEdited("en")); //This returns false, everything is published
Assert.IsFalse(document.IsCultureEdited("fr")); //This will be false because nothing has changed for this culture and the property no longer reflects variant changes
Assert.IsFalse(document.Edited);
// switch property back to Culture
@@ -863,14 +868,12 @@ namespace Umbraco.Tests.Services
document = ServiceContext.ContentService.GetById(document.Id);
Assert.AreEqual("v1inv", document.GetValue("value1", "en")); //The invariant property value gets copied over to the default language
Assert.AreEqual("v1fr", document.GetValue("value1", "fr"));
Assert.AreEqual("v1inv", document.GetValue("value1", "en", published: true));
Assert.AreEqual("v1fr", document.GetValue("value1", "fr")); //values are still retained
Assert.AreEqual("v1fr-init", document.GetValue("value1", "fr", published: true)); //values are still retained
Assert.IsFalse(document.IsCultureEdited("en")); //The invariant published AND edited values are copied over to the default language
//TODO: This fails - for some reason in the DB, the DocumentCultureVariationDto Edited flag is set to false for the FR culture
// I'm assuming this is somehow done during the ContentTypeService.Save method when changing variation. It should not be false but instead
// true because the values for it's edited vs published version are different. Perhaps it's false because that is the default boolean value and
// this row gets deleted somewhere along the way?
Assert.IsTrue(document.IsCultureEdited("fr")); //The previously existing french values are there and there is no published value
Assert.IsTrue(document.Edited);
Assert.IsTrue(document.Edited); //Will be flagged edited again because the french culture had pending changes
// publish again
document.SetValue("value1", "v1en2", "en"); //update the value now that it's variant again