From 365ad2981d7cfcf7bfd01fa620bb53f1208e94db Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 30 Jul 2019 19:02:03 +1000 Subject: [PATCH 01/12] Fixing issue when changing a property type from invariant to variant and back again and the document always reporting pending changes. --- .../Persistence/Factories/PropertyFactory.cs | 43 ++++++++++++------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Factories/PropertyFactory.cs b/src/Umbraco.Core/Persistence/Factories/PropertyFactory.cs index f1473b5888..f0937a3781 100644 --- a/src/Umbraco.Core/Persistence/Factories/PropertyFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/PropertyFactory.cs @@ -12,7 +12,7 @@ namespace Umbraco.Core.Persistence.Factories public static IEnumerable BuildEntities(PropertyType[] propertyTypes, IReadOnlyCollection dtos, int publishedVersionId, ILanguageRepository languageRepository) { var properties = new List(); - var xdtos = dtos.GroupBy(x => x.PropertyTypeId).ToDictionary(x => x.Key, x => (IEnumerable) x); + var xdtos = dtos.GroupBy(x => x.PropertyTypeId).ToDictionary(x => x.Key, x => (IEnumerable)x); foreach (var propertyType in propertyTypes) { @@ -130,6 +130,9 @@ namespace Umbraco.Core.Persistence.Factories // publishing = deal with edit and published values foreach (var propertyValue in property.Values) { + var isInvariantValue = propertyValue.Culture == null; + var isCultureValue = propertyValue.Culture != null && propertyValue.Segment == null; + // deal with published value if (propertyValue.PublishedValue != null && publishedVersionId > 0) propertyDataDtos.Add(BuildDto(publishedVersionId, property, languageRepository.GetIdByIsoCode(propertyValue.Culture), propertyValue.Segment, propertyValue.PublishedValue)); @@ -138,26 +141,34 @@ 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. + + if (property.PropertyType.VariesByCulture() && isInvariantValue) continue; + if (!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); + edited |= !sameValues; - if (entityVariesByCulture // cultures can be edited, ie CultureNeutral is supported - && propertyValue.Culture != null && propertyValue.Segment == null // and value is CultureNeutral - && !sameValues) // and edited and published are different + if (entityVariesByCulture && !sameValues) { - editedCultures.Add(propertyValue.Culture); // report culture as edited - } + if (isCultureValue) + { + editedCultures.Add(propertyValue.Culture); // report culture as edited + } + else if (isInvariantValue) + { + // flag culture as edited if it contains an edited invariant property + if (defaultCulture == null) + defaultCulture = languageRepository.GetDefaultIsoCode(); - // flag culture as edited if it contains an edited invariant property - if (propertyValue.Culture == null //invariant property - && !sameValues // and edited and published are different - && entityVariesByCulture) //only when the entity is variant - { - if (defaultCulture == null) - defaultCulture = languageRepository.GetDefaultIsoCode(); - - editedCultures.Add(defaultCulture); + editedCultures.Add(defaultCulture); + } } } } @@ -167,7 +178,7 @@ namespace Umbraco.Core.Persistence.Factories { // not publishing = only deal with edit values if (propertyValue.EditedValue != null) - propertyDataDtos.Add(BuildDto(currentVersionId, property, languageRepository.GetIdByIsoCode(propertyValue.Culture), propertyValue.Segment, propertyValue.EditedValue)); + propertyDataDtos.Add(BuildDto(currentVersionId, property, languageRepository.GetIdByIsoCode(propertyValue.Culture), propertyValue.Segment, propertyValue.EditedValue)); } edited = true; } From 6482d849e31a949c40deaa8418d9ec6fc8a07321 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 30 Jul 2019 22:59:33 +1000 Subject: [PATCH 02/12] Moves test to different fixture --- .../Services/ContentTypeServiceTests.cs | 355 +---------------- .../ContentTypeServiceVariantsTests.cs | 356 ++++++++++++++++++ 2 files changed, 357 insertions(+), 354 deletions(-) diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs index a0b5f01a1f..f2a4368ae4 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs @@ -53,360 +53,7 @@ namespace Umbraco.Tests.Services Assert.IsTrue(contentType.IsElement); } - [Test] - public void Change_Content_Type_Variation_Clears_Redirects() - { - //create content type with a property type that varies by culture - var contentType = MockedContentTypes.CreateBasicContentType(); - contentType.Variations = ContentVariation.Nothing; - var contentCollection = new PropertyTypeCollection(true); - contentCollection.Add(new PropertyType("test", ValueStorageType.Ntext) - { - Alias = "title", - Name = "Title", - Description = "", - Mandatory = false, - SortOrder = 1, - DataTypeId = -88, - Variations = ContentVariation.Nothing - }); - contentType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Content", SortOrder = 1 }); - ServiceContext.ContentTypeService.Save(contentType); - var contentType2 = MockedContentTypes.CreateBasicContentType("test"); - ServiceContext.ContentTypeService.Save(contentType2); - - //create some content of this content type - IContent doc = MockedContent.CreateBasicContent(contentType); - doc.Name = "Hello1"; - ServiceContext.ContentService.Save(doc); - - IContent doc2 = MockedContent.CreateBasicContent(contentType2); - ServiceContext.ContentService.Save(doc2); - - ServiceContext.RedirectUrlService.Register("hello/world", doc.Key); - ServiceContext.RedirectUrlService.Register("hello2/world2", doc2.Key); - - Assert.AreEqual(1, ServiceContext.RedirectUrlService.GetContentRedirectUrls(doc.Key).Count()); - Assert.AreEqual(1, ServiceContext.RedirectUrlService.GetContentRedirectUrls(doc2.Key).Count()); - - //change variation - contentType.Variations = ContentVariation.Culture; - ServiceContext.ContentTypeService.Save(contentType); - - Assert.AreEqual(0, ServiceContext.RedirectUrlService.GetContentRedirectUrls(doc.Key).Count()); - Assert.AreEqual(1, ServiceContext.RedirectUrlService.GetContentRedirectUrls(doc2.Key).Count()); - - } - - [Test] - public void Change_Content_Type_From_Invariant_Variant() - { - //create content type with a property type that varies by culture - var contentType = MockedContentTypes.CreateBasicContentType(); - contentType.Variations = ContentVariation.Nothing; - var contentCollection = new PropertyTypeCollection(true); - contentCollection.Add(new PropertyType("test", ValueStorageType.Ntext) - { - Alias = "title", - Name = "Title", - Description = "", - Mandatory = false, - SortOrder = 1, - DataTypeId = -88, - Variations = ContentVariation.Nothing - }); - contentType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Content", SortOrder = 1 }); - ServiceContext.ContentTypeService.Save(contentType); - - //create some content of this content type - IContent doc = MockedContent.CreateBasicContent(contentType); - doc.Name = "Hello1"; - doc.SetValue("title", "hello world"); - ServiceContext.ContentService.Save(doc); - - Assert.AreEqual("Hello1", doc.Name); - Assert.AreEqual("hello world", doc.GetValue("title")); - - //change the content type to be variant, we will also update the name here to detect the copy changes - doc.Name = "Hello2"; - ServiceContext.ContentService.Save(doc); - contentType.Variations = ContentVariation.Culture; - ServiceContext.ContentTypeService.Save(contentType); - doc = ServiceContext.ContentService.GetById(doc.Id); //re-get - - Assert.AreEqual("Hello2", doc.GetCultureName("en-US")); - Assert.AreEqual("hello world", doc.GetValue("title")); //We are not checking against en-US here because properties will remain invariant - - //change back property type to be invariant, we will also update the name here to detect the copy changes - doc.SetCultureName("Hello3", "en-US"); - ServiceContext.ContentService.Save(doc); - contentType.Variations = ContentVariation.Nothing; - ServiceContext.ContentTypeService.Save(contentType); - doc = ServiceContext.ContentService.GetById(doc.Id); //re-get - - Assert.AreEqual("Hello3", doc.Name); - Assert.AreEqual("hello world", doc.GetValue("title")); - } - - [Test] - public void Change_Content_Type_From_Variant_Invariant() - { - //create content type with a property type that varies by culture - var contentType = MockedContentTypes.CreateBasicContentType(); - contentType.Variations = ContentVariation.Culture; - var contentCollection = new PropertyTypeCollection(true); - contentCollection.Add(new PropertyType("test", ValueStorageType.Ntext) - { - Alias = "title", - Name = "Title", - Description = "", - Mandatory = false, - SortOrder = 1, - DataTypeId = -88, - Variations = ContentVariation.Culture - }); - contentType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Content", SortOrder = 1 }); - ServiceContext.ContentTypeService.Save(contentType); - - //create some content of this content type - IContent doc = MockedContent.CreateBasicContent(contentType); - doc.SetCultureName("Hello1", "en-US"); - doc.SetValue("title", "hello world", "en-US"); - ServiceContext.ContentService.Save(doc); - - Assert.AreEqual("Hello1", doc.GetCultureName("en-US")); - Assert.AreEqual("hello world", doc.GetValue("title", "en-US")); - - //change the content type to be invariant, we will also update the name here to detect the copy changes - doc.SetCultureName("Hello2", "en-US"); - ServiceContext.ContentService.Save(doc); - contentType.Variations = ContentVariation.Nothing; - ServiceContext.ContentTypeService.Save(contentType); - doc = ServiceContext.ContentService.GetById(doc.Id); //re-get - - Assert.AreEqual("Hello2", doc.Name); - Assert.AreEqual("hello world", doc.GetValue("title")); - - //change back property type to be variant, we will also update the name here to detect the copy changes - doc.Name = "Hello3"; - ServiceContext.ContentService.Save(doc); - contentType.Variations = ContentVariation.Culture; - ServiceContext.ContentTypeService.Save(contentType); - doc = ServiceContext.ContentService.GetById(doc.Id); //re-get - - //at this stage all property types were switched to invariant so even though the variant value - //exists it will not be returned because the property type is invariant, - //so this check proves that null will be returned - Assert.IsNull(doc.GetValue("title", "en-US")); - - //we can now switch the property type to be variant and the value can be returned again - contentType.PropertyTypes.First().Variations = ContentVariation.Culture; - ServiceContext.ContentTypeService.Save(contentType); - doc = ServiceContext.ContentService.GetById(doc.Id); //re-get - - Assert.AreEqual("Hello3", doc.GetCultureName("en-US")); - Assert.AreEqual("hello world", doc.GetValue("title", "en-US")); - - } - - [Test] - public void Change_Property_Type_From_Invariant_Variant() - { - //create content type with a property type that varies by culture - var contentType = MockedContentTypes.CreateBasicContentType(); - contentType.Variations = ContentVariation.Nothing; - var contentCollection = new PropertyTypeCollection(true); - contentCollection.Add(new PropertyType("test", ValueStorageType.Ntext) - { - Alias = "title", - Name = "Title", - Description = "", - Mandatory = false, - SortOrder = 1, - DataTypeId = -88, - Variations = ContentVariation.Nothing - }); - contentType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Content", SortOrder = 1 }); - ServiceContext.ContentTypeService.Save(contentType); - - //create some content of this content type - IContent doc = MockedContent.CreateBasicContent(contentType); - doc.Name = "Home"; - doc.SetValue("title", "hello world"); - ServiceContext.ContentService.Save(doc); - - Assert.AreEqual("hello world", doc.GetValue("title")); - - //change the property type to be variant - contentType.PropertyTypes.First().Variations = ContentVariation.Culture; - ServiceContext.ContentTypeService.Save(contentType); - doc = ServiceContext.ContentService.GetById(doc.Id); //re-get - - Assert.AreEqual("hello world", doc.GetValue("title", "en-US")); - - //change back property type to be invariant - contentType.PropertyTypes.First().Variations = ContentVariation.Nothing; - ServiceContext.ContentTypeService.Save(contentType); - doc = ServiceContext.ContentService.GetById(doc.Id); //re-get - - Assert.AreEqual("hello world", doc.GetValue("title")); - } - - [Test] - public void Change_Property_Type_From_Variant_Invariant() - { - //create content type with a property type that varies by culture - var contentType = MockedContentTypes.CreateBasicContentType(); - contentType.Variations = ContentVariation.Culture; - var contentCollection = new PropertyTypeCollection(true); - contentCollection.Add(new PropertyType("test", ValueStorageType.Ntext) - { - Alias = "title", - Name = "Title", - Description = "", - Mandatory = false, - SortOrder = 1, - DataTypeId = -88, - Variations = ContentVariation.Culture - }); - contentType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Content", SortOrder = 1 }); - ServiceContext.ContentTypeService.Save(contentType); - - //create some content of this content type - IContent doc = MockedContent.CreateBasicContent(contentType); - doc.SetCultureName("Home", "en-US"); - doc.SetValue("title", "hello world", "en-US"); - ServiceContext.ContentService.Save(doc); - - Assert.AreEqual("hello world", doc.GetValue("title", "en-US")); - - //change the property type to be invariant - contentType.PropertyTypes.First().Variations = ContentVariation.Nothing; - ServiceContext.ContentTypeService.Save(contentType); - doc = ServiceContext.ContentService.GetById(doc.Id); //re-get - - Assert.AreEqual("hello world", doc.GetValue("title")); - - //change back property type to be variant - contentType.PropertyTypes.First().Variations = ContentVariation.Culture; - ServiceContext.ContentTypeService.Save(contentType); - doc = ServiceContext.ContentService.GetById(doc.Id); //re-get - - Assert.AreEqual("hello world", doc.GetValue("title", "en-US")); - } - - [Test] - public void Change_Property_Type_From_Variant_Invariant_On_A_Composition() - { - //create content type with a property type that varies by culture - var contentType = MockedContentTypes.CreateBasicContentType(); - contentType.Variations = ContentVariation.Culture; - var contentCollection = new PropertyTypeCollection(true); - contentCollection.Add(new PropertyType("test", ValueStorageType.Ntext) - { - Alias = "title", - Name = "Title", - Description = "", - Mandatory = false, - SortOrder = 1, - DataTypeId = -88, - Variations = ContentVariation.Culture - }); - contentType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Content", SortOrder = 1 }); - ServiceContext.ContentTypeService.Save(contentType); - - //compose this from the other one - var contentType2 = MockedContentTypes.CreateBasicContentType("test"); - contentType2.Variations = ContentVariation.Culture; - contentType2.AddContentType(contentType); - ServiceContext.ContentTypeService.Save(contentType2); - - //create some content of this content type - IContent doc = MockedContent.CreateBasicContent(contentType); - doc.SetCultureName("Home", "en-US"); - doc.SetValue("title", "hello world", "en-US"); - ServiceContext.ContentService.Save(doc); - - IContent doc2 = MockedContent.CreateBasicContent(contentType2); - doc2.SetCultureName("Home", "en-US"); - doc2.SetValue("title", "hello world", "en-US"); - ServiceContext.ContentService.Save(doc2); - - //change the property type to be invariant - contentType.PropertyTypes.First().Variations = ContentVariation.Nothing; - ServiceContext.ContentTypeService.Save(contentType); - doc = ServiceContext.ContentService.GetById(doc.Id); //re-get - doc2 = ServiceContext.ContentService.GetById(doc2.Id); //re-get - - Assert.AreEqual("hello world", doc.GetValue("title")); - Assert.AreEqual("hello world", doc2.GetValue("title")); - - //change back property type to be variant - contentType.PropertyTypes.First().Variations = ContentVariation.Culture; - ServiceContext.ContentTypeService.Save(contentType); - doc = ServiceContext.ContentService.GetById(doc.Id); //re-get - doc2 = ServiceContext.ContentService.GetById(doc2.Id); //re-get - - Assert.AreEqual("hello world", doc.GetValue("title", "en-US")); - Assert.AreEqual("hello world", doc2.GetValue("title", "en-US")); - } - - [Test] - public void Change_Content_Type_From_Variant_Invariant_On_A_Composition() - { - //create content type with a property type that varies by culture - var contentType = MockedContentTypes.CreateBasicContentType(); - contentType.Variations = ContentVariation.Culture; - var contentCollection = new PropertyTypeCollection(true); - contentCollection.Add(new PropertyType("test", ValueStorageType.Ntext) - { - Alias = "title", - Name = "Title", - Description = "", - Mandatory = false, - SortOrder = 1, - DataTypeId = -88, - Variations = ContentVariation.Culture - }); - contentType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Content", SortOrder = 1 }); - ServiceContext.ContentTypeService.Save(contentType); - - //compose this from the other one - var contentType2 = MockedContentTypes.CreateBasicContentType("test"); - contentType2.Variations = ContentVariation.Culture; - contentType2.AddContentType(contentType); - ServiceContext.ContentTypeService.Save(contentType2); - - //create some content of this content type - IContent doc = MockedContent.CreateBasicContent(contentType); - doc.SetCultureName("Home", "en-US"); - doc.SetValue("title", "hello world", "en-US"); - ServiceContext.ContentService.Save(doc); - - IContent doc2 = MockedContent.CreateBasicContent(contentType2); - doc2.SetCultureName("Home", "en-US"); - doc2.SetValue("title", "hello world", "en-US"); - ServiceContext.ContentService.Save(doc2); - - //change the content type to be invariant - contentType.Variations = ContentVariation.Nothing; - ServiceContext.ContentTypeService.Save(contentType); - doc = ServiceContext.ContentService.GetById(doc.Id); //re-get - doc2 = ServiceContext.ContentService.GetById(doc2.Id); //re-get - - Assert.AreEqual("hello world", doc.GetValue("title")); - Assert.AreEqual("hello world", doc2.GetValue("title")); - - //change back content type to be variant - contentType.Variations = ContentVariation.Culture; - ServiceContext.ContentTypeService.Save(contentType); - doc = ServiceContext.ContentService.GetById(doc.Id); //re-get - doc2 = ServiceContext.ContentService.GetById(doc2.Id); //re-get - - //this will be null because the doc type was changed back to variant but it's property types don't get changed back - Assert.IsNull(doc.GetValue("title", "en-US")); - Assert.IsNull(doc2.GetValue("title", "en-US")); - } + [Test] public void Deleting_Content_Type_With_Hierarchy_Of_Content_Items_Moves_Orphaned_Content_To_Recycle_Bin() diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs index 18ea95cd98..2748502e2c 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs @@ -17,6 +17,7 @@ using Umbraco.Core.PropertyEditors; using Umbraco.Core.Services; using Umbraco.Core.Strings; using Umbraco.Core.Sync; +using Umbraco.Tests.TestHelpers.Entities; using Umbraco.Tests.Testing; using Umbraco.Web.PublishedCache; using Umbraco.Web.PublishedCache.NuCache; @@ -106,6 +107,361 @@ namespace Umbraco.Tests.Services } } + [Test] + public void Change_Content_Type_Variation_Clears_Redirects() + { + //create content type with a property type that varies by culture + var contentType = MockedContentTypes.CreateBasicContentType(); + contentType.Variations = ContentVariation.Nothing; + var contentCollection = new PropertyTypeCollection(true); + contentCollection.Add(new PropertyType("test", ValueStorageType.Ntext) + { + Alias = "title", + Name = "Title", + Description = "", + Mandatory = false, + SortOrder = 1, + DataTypeId = -88, + Variations = ContentVariation.Nothing + }); + contentType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Content", SortOrder = 1 }); + ServiceContext.ContentTypeService.Save(contentType); + var contentType2 = MockedContentTypes.CreateBasicContentType("test"); + ServiceContext.ContentTypeService.Save(contentType2); + + //create some content of this content type + IContent doc = MockedContent.CreateBasicContent(contentType); + doc.Name = "Hello1"; + ServiceContext.ContentService.Save(doc); + + IContent doc2 = MockedContent.CreateBasicContent(contentType2); + ServiceContext.ContentService.Save(doc2); + + ServiceContext.RedirectUrlService.Register("hello/world", doc.Key); + ServiceContext.RedirectUrlService.Register("hello2/world2", doc2.Key); + + Assert.AreEqual(1, ServiceContext.RedirectUrlService.GetContentRedirectUrls(doc.Key).Count()); + Assert.AreEqual(1, ServiceContext.RedirectUrlService.GetContentRedirectUrls(doc2.Key).Count()); + + //change variation + contentType.Variations = ContentVariation.Culture; + ServiceContext.ContentTypeService.Save(contentType); + + Assert.AreEqual(0, ServiceContext.RedirectUrlService.GetContentRedirectUrls(doc.Key).Count()); + Assert.AreEqual(1, ServiceContext.RedirectUrlService.GetContentRedirectUrls(doc2.Key).Count()); + + } + + [Test] + public void Change_Content_Type_From_Invariant_Variant() + { + //create content type with a property type that varies by culture + var contentType = MockedContentTypes.CreateBasicContentType(); + contentType.Variations = ContentVariation.Nothing; + var contentCollection = new PropertyTypeCollection(true); + contentCollection.Add(new PropertyType("test", ValueStorageType.Ntext) + { + Alias = "title", + Name = "Title", + Description = "", + Mandatory = false, + SortOrder = 1, + DataTypeId = -88, + Variations = ContentVariation.Nothing + }); + contentType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Content", SortOrder = 1 }); + ServiceContext.ContentTypeService.Save(contentType); + + //create some content of this content type + IContent doc = MockedContent.CreateBasicContent(contentType); + doc.Name = "Hello1"; + doc.SetValue("title", "hello world"); + ServiceContext.ContentService.Save(doc); + + Assert.AreEqual("Hello1", doc.Name); + Assert.AreEqual("hello world", doc.GetValue("title")); + + //change the content type to be variant, we will also update the name here to detect the copy changes + doc.Name = "Hello2"; + ServiceContext.ContentService.Save(doc); + contentType.Variations = ContentVariation.Culture; + ServiceContext.ContentTypeService.Save(contentType); + doc = ServiceContext.ContentService.GetById(doc.Id); //re-get + + Assert.AreEqual("Hello2", doc.GetCultureName("en-US")); + Assert.AreEqual("hello world", doc.GetValue("title")); //We are not checking against en-US here because properties will remain invariant + + //change back property type to be invariant, we will also update the name here to detect the copy changes + doc.SetCultureName("Hello3", "en-US"); + ServiceContext.ContentService.Save(doc); + contentType.Variations = ContentVariation.Nothing; + ServiceContext.ContentTypeService.Save(contentType); + doc = ServiceContext.ContentService.GetById(doc.Id); //re-get + + Assert.AreEqual("Hello3", doc.Name); + Assert.AreEqual("hello world", doc.GetValue("title")); + } + + [Test] + public void Change_Content_Type_From_Variant_Invariant() + { + //create content type with a property type that varies by culture + var contentType = MockedContentTypes.CreateBasicContentType(); + contentType.Variations = ContentVariation.Culture; + var contentCollection = new PropertyTypeCollection(true); + contentCollection.Add(new PropertyType("test", ValueStorageType.Ntext) + { + Alias = "title", + Name = "Title", + Description = "", + Mandatory = false, + SortOrder = 1, + DataTypeId = -88, + Variations = ContentVariation.Culture + }); + contentType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Content", SortOrder = 1 }); + ServiceContext.ContentTypeService.Save(contentType); + + //create some content of this content type + IContent doc = MockedContent.CreateBasicContent(contentType); + doc.SetCultureName("Hello1", "en-US"); + doc.SetValue("title", "hello world", "en-US"); + ServiceContext.ContentService.Save(doc); + + Assert.AreEqual("Hello1", doc.GetCultureName("en-US")); + Assert.AreEqual("hello world", doc.GetValue("title", "en-US")); + + //change the content type to be invariant, we will also update the name here to detect the copy changes + doc.SetCultureName("Hello2", "en-US"); + ServiceContext.ContentService.Save(doc); + contentType.Variations = ContentVariation.Nothing; + ServiceContext.ContentTypeService.Save(contentType); + doc = ServiceContext.ContentService.GetById(doc.Id); //re-get + + Assert.AreEqual("Hello2", doc.Name); + Assert.AreEqual("hello world", doc.GetValue("title")); + + //change back property type to be variant, we will also update the name here to detect the copy changes + doc.Name = "Hello3"; + ServiceContext.ContentService.Save(doc); + contentType.Variations = ContentVariation.Culture; + ServiceContext.ContentTypeService.Save(contentType); + doc = ServiceContext.ContentService.GetById(doc.Id); //re-get + + //at this stage all property types were switched to invariant so even though the variant value + //exists it will not be returned because the property type is invariant, + //so this check proves that null will be returned + Assert.IsNull(doc.GetValue("title", "en-US")); + + //we can now switch the property type to be variant and the value can be returned again + contentType.PropertyTypes.First().Variations = ContentVariation.Culture; + ServiceContext.ContentTypeService.Save(contentType); + doc = ServiceContext.ContentService.GetById(doc.Id); //re-get + + Assert.AreEqual("Hello3", doc.GetCultureName("en-US")); + Assert.AreEqual("hello world", doc.GetValue("title", "en-US")); + + } + + [Test] + public void Change_Property_Type_From_Invariant_Variant() + { + //create content type with a property type that varies by culture + var contentType = MockedContentTypes.CreateBasicContentType(); + contentType.Variations = ContentVariation.Nothing; + var contentCollection = new PropertyTypeCollection(true); + contentCollection.Add(new PropertyType("test", ValueStorageType.Ntext) + { + Alias = "title", + Name = "Title", + Description = "", + Mandatory = false, + SortOrder = 1, + DataTypeId = -88, + Variations = ContentVariation.Nothing + }); + contentType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Content", SortOrder = 1 }); + ServiceContext.ContentTypeService.Save(contentType); + + //create some content of this content type + IContent doc = MockedContent.CreateBasicContent(contentType); + doc.Name = "Home"; + doc.SetValue("title", "hello world"); + ServiceContext.ContentService.Save(doc); + + Assert.AreEqual("hello world", doc.GetValue("title")); + + //change the property type to be variant + contentType.PropertyTypes.First().Variations = ContentVariation.Culture; + ServiceContext.ContentTypeService.Save(contentType); + doc = ServiceContext.ContentService.GetById(doc.Id); //re-get + + Assert.AreEqual("hello world", doc.GetValue("title", "en-US")); + + //change back property type to be invariant + contentType.PropertyTypes.First().Variations = ContentVariation.Nothing; + ServiceContext.ContentTypeService.Save(contentType); + doc = ServiceContext.ContentService.GetById(doc.Id); //re-get + + Assert.AreEqual("hello world", doc.GetValue("title")); + } + + [Test] + public void Change_Property_Type_From_Variant_Invariant() + { + //create content type with a property type that varies by culture + var contentType = MockedContentTypes.CreateBasicContentType(); + contentType.Variations = ContentVariation.Culture; + var contentCollection = new PropertyTypeCollection(true); + contentCollection.Add(new PropertyType("test", ValueStorageType.Ntext) + { + Alias = "title", + Name = "Title", + Description = "", + Mandatory = false, + SortOrder = 1, + DataTypeId = -88, + Variations = ContentVariation.Culture + }); + contentType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Content", SortOrder = 1 }); + ServiceContext.ContentTypeService.Save(contentType); + + //create some content of this content type + IContent doc = MockedContent.CreateBasicContent(contentType); + doc.SetCultureName("Home", "en-US"); + doc.SetValue("title", "hello world", "en-US"); + ServiceContext.ContentService.Save(doc); + + Assert.AreEqual("hello world", doc.GetValue("title", "en-US")); + + //change the property type to be invariant + contentType.PropertyTypes.First().Variations = ContentVariation.Nothing; + ServiceContext.ContentTypeService.Save(contentType); + doc = ServiceContext.ContentService.GetById(doc.Id); //re-get + + Assert.AreEqual("hello world", doc.GetValue("title")); + + //change back property type to be variant + contentType.PropertyTypes.First().Variations = ContentVariation.Culture; + ServiceContext.ContentTypeService.Save(contentType); + doc = ServiceContext.ContentService.GetById(doc.Id); //re-get + + Assert.AreEqual("hello world", doc.GetValue("title", "en-US")); + } + + [Test] + public void Change_Property_Type_From_Variant_Invariant_On_A_Composition() + { + //create content type with a property type that varies by culture + var contentType = MockedContentTypes.CreateBasicContentType(); + contentType.Variations = ContentVariation.Culture; + var contentCollection = new PropertyTypeCollection(true); + contentCollection.Add(new PropertyType("test", ValueStorageType.Ntext) + { + Alias = "title", + Name = "Title", + Description = "", + Mandatory = false, + SortOrder = 1, + DataTypeId = -88, + Variations = ContentVariation.Culture + }); + contentType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Content", SortOrder = 1 }); + ServiceContext.ContentTypeService.Save(contentType); + + //compose this from the other one + var contentType2 = MockedContentTypes.CreateBasicContentType("test"); + contentType2.Variations = ContentVariation.Culture; + contentType2.AddContentType(contentType); + ServiceContext.ContentTypeService.Save(contentType2); + + //create some content of this content type + IContent doc = MockedContent.CreateBasicContent(contentType); + doc.SetCultureName("Home", "en-US"); + doc.SetValue("title", "hello world", "en-US"); + ServiceContext.ContentService.Save(doc); + + IContent doc2 = MockedContent.CreateBasicContent(contentType2); + doc2.SetCultureName("Home", "en-US"); + doc2.SetValue("title", "hello world", "en-US"); + ServiceContext.ContentService.Save(doc2); + + //change the property type to be invariant + contentType.PropertyTypes.First().Variations = ContentVariation.Nothing; + ServiceContext.ContentTypeService.Save(contentType); + doc = ServiceContext.ContentService.GetById(doc.Id); //re-get + doc2 = ServiceContext.ContentService.GetById(doc2.Id); //re-get + + Assert.AreEqual("hello world", doc.GetValue("title")); + Assert.AreEqual("hello world", doc2.GetValue("title")); + + //change back property type to be variant + contentType.PropertyTypes.First().Variations = ContentVariation.Culture; + ServiceContext.ContentTypeService.Save(contentType); + doc = ServiceContext.ContentService.GetById(doc.Id); //re-get + doc2 = ServiceContext.ContentService.GetById(doc2.Id); //re-get + + Assert.AreEqual("hello world", doc.GetValue("title", "en-US")); + Assert.AreEqual("hello world", doc2.GetValue("title", "en-US")); + } + + [Test] + public void Change_Content_Type_From_Variant_Invariant_On_A_Composition() + { + //create content type with a property type that varies by culture + var contentType = MockedContentTypes.CreateBasicContentType(); + contentType.Variations = ContentVariation.Culture; + var contentCollection = new PropertyTypeCollection(true); + contentCollection.Add(new PropertyType("test", ValueStorageType.Ntext) + { + Alias = "title", + Name = "Title", + Description = "", + Mandatory = false, + SortOrder = 1, + DataTypeId = -88, + Variations = ContentVariation.Culture + }); + contentType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Content", SortOrder = 1 }); + ServiceContext.ContentTypeService.Save(contentType); + + //compose this from the other one + var contentType2 = MockedContentTypes.CreateBasicContentType("test"); + contentType2.Variations = ContentVariation.Culture; + contentType2.AddContentType(contentType); + ServiceContext.ContentTypeService.Save(contentType2); + + //create some content of this content type + IContent doc = MockedContent.CreateBasicContent(contentType); + doc.SetCultureName("Home", "en-US"); + doc.SetValue("title", "hello world", "en-US"); + ServiceContext.ContentService.Save(doc); + + IContent doc2 = MockedContent.CreateBasicContent(contentType2); + doc2.SetCultureName("Home", "en-US"); + doc2.SetValue("title", "hello world", "en-US"); + ServiceContext.ContentService.Save(doc2); + + //change the content type to be invariant + contentType.Variations = ContentVariation.Nothing; + ServiceContext.ContentTypeService.Save(contentType); + doc = ServiceContext.ContentService.GetById(doc.Id); //re-get + doc2 = ServiceContext.ContentService.GetById(doc2.Id); //re-get + + Assert.AreEqual("hello world", doc.GetValue("title")); + Assert.AreEqual("hello world", doc2.GetValue("title")); + + //change back content type to be variant + contentType.Variations = ContentVariation.Culture; + ServiceContext.ContentTypeService.Save(contentType); + doc = ServiceContext.ContentService.GetById(doc.Id); //re-get + doc2 = ServiceContext.ContentService.GetById(doc2.Id); //re-get + + //this will be null because the doc type was changed back to variant but it's property types don't get changed back + Assert.IsNull(doc.GetValue("title", "en-US")); + Assert.IsNull(doc2.GetValue("title", "en-US")); + } + [Test] public void Change_Variations_SimpleContentType_VariantToInvariantAndBack() { From 5c8cd6027518acf45e2d960b66b28f564952a33b Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 31 Jul 2019 00:13:40 +1000 Subject: [PATCH 03/12] Writes up a test to show the issue with edited culture flags, this shows that the current fix works but there's still an issue --- .../ContentTypeServiceVariantsTests.cs | 138 +++++++++++++++--- 1 file changed, 121 insertions(+), 17 deletions(-) diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs index 2748502e2c..f5fa4e8795 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs @@ -468,10 +468,7 @@ namespace Umbraco.Tests.Services // one simple content type, variant, with both variant and invariant properties // can change it to invariant and back - var languageEn = new Language("en") { IsDefault = true }; - ServiceContext.LocalizationService.Save(languageEn); - var languageFr = new Language("fr"); - ServiceContext.LocalizationService.Save(languageFr); + CreateFrenchAndEnglishLangs(); var contentType = new ContentType(-1) { @@ -499,7 +496,7 @@ namespace Umbraco.Tests.Services contentType.PropertyGroups.Add(new PropertyGroup(properties) { Name = "Content" }); ServiceContext.ContentTypeService.Save(contentType); - var document = (IContent) new Content("document", -1, contentType); + var document = (IContent)new Content("document", -1, contentType); document.SetCultureName("doc1en", "en"); document.SetCultureName("doc1fr", "fr"); document.SetValue("value1", "v1en", "en"); @@ -682,10 +679,7 @@ namespace Umbraco.Tests.Services // one simple content type, variant, with both variant and invariant properties // can change an invariant property to variant and back - var languageEn = new Language("en") { IsDefault = true }; - ServiceContext.LocalizationService.Save(languageEn); - var languageFr = new Language("fr"); - ServiceContext.LocalizationService.Save(languageFr); + CreateFrenchAndEnglishLangs(); var contentType = new ContentType(-1) { @@ -785,6 +779,114 @@ namespace Umbraco.Tests.Services "{'properties':{'value1':[{'culture':'en','seg':'','val':'v1en'},{'culture':'fr','seg':'','val':'v1fr'}],'value2':[{'culture':'en','seg':'','val':'v2'}]},'cultureData':"); } + [Test] + public void Change_Variations_SimpleContentType_VariantPropertyToInvariantAndBack_While_Publishing() + { + // one simple content type, variant, with both variant and invariant properties + // can change an invariant property to variant and back + + CreateFrenchAndEnglishLangs(); + + var contentType = new ContentType(-1) + { + Alias = "contentType", + Name = "contentType", + Variations = ContentVariation.Culture + }; + + var properties = new PropertyTypeCollection(true) + { + new PropertyType("value1", ValueStorageType.Ntext) + { + Alias = "value1", + DataTypeId = -88, + Variations = ContentVariation.Culture + } + }; + + contentType.PropertyGroups.Add(new PropertyGroup(properties) { Name = "Content" }); + ServiceContext.ContentTypeService.Save(contentType); + + var document = (IContent)new Content("document", -1, contentType); + document.SetCultureName("doc1en", "en"); + document.SetCultureName("doc1fr", "fr"); + document.SetValue("value1", "v1en", "en"); + document.SetValue("value1", "v1fr", "fr"); + ServiceContext.ContentService.Save(document); + + //at this stage there will be 4 property values stored in the DB for "value1" against "en" and "fr" for both edited/published versions, + //for the published values, these will be null + + document = ServiceContext.ContentService.GetById(document.Id); + Assert.AreEqual("doc1en", document.Name); + Assert.AreEqual("doc1en", document.GetCultureName("en")); + Assert.AreEqual("doc1fr", document.GetCultureName("fr")); + Assert.AreEqual("v1en", document.GetValue("value1", "en")); + Assert.AreEqual("v1fr", document.GetValue("value1", "fr")); + 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 + contentType.PropertyTypes.First(x => x.Alias == "value1").Variations = ContentVariation.Nothing; + ServiceContext.ContentTypeService.Save(contentType); + + document = ServiceContext.ContentService.GetById(document.Id); + Assert.IsTrue(document.Edited); + + // publish the document + document.SetValue("value1", "v1inv"); //update the invariant value + ServiceContext.ContentService.SaveAndPublish(document); + + //at this stage there will be 6 property values stored in the DB for "value1" against "en", "fr" and null for both edited/published versions, + //for the published values for the cultures, these will still be null but the invariant edited/published values will both be "v1inv". + + document = ServiceContext.ContentService.GetById(document.Id); + Assert.AreEqual("doc1en", document.Name); + Assert.AreEqual("doc1en", document.GetCultureName("en")); + 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.AreEqual("v1inv", document.GetValue("value1")); + Assert.IsFalse(document.IsCultureEdited("en")); //This returns false, the culture shouldn't be marked as edited because the invariant property value is published + Assert.IsFalse(document.IsCultureEdited("fr")); //This returns false, the culture shouldn't be marked as edited because the invariant property value is published + Assert.IsFalse(document.Edited); + + // switch property back to Culture + contentType.PropertyTypes.First(x => x.Alias == "value1").Variations = ContentVariation.Culture; + ServiceContext.ContentTypeService.Save(contentType); + + 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.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); + + // publish again + document.SetValue("value1", "v1en2", "en"); //update the value now that it's variant again + document.SetValue("value1", "v1fr2", "fr"); //update the value now that it's variant again + ServiceContext.ContentService.SaveAndPublish(document); + + //at this stage there will be 4 property values stored in the DB for "value1" against "en", "fr" for both edited/published versions, + //with the change back to Culture, it deletes the invariant property values. + + document = ServiceContext.ContentService.GetById(document.Id); + Assert.AreEqual("doc1en", document.Name); + Assert.AreEqual("doc1en", document.GetCultureName("en")); + Assert.AreEqual("doc1fr", document.GetCultureName("fr")); + Assert.AreEqual("v1en2", document.GetValue("value1", "en")); + Assert.AreEqual("v1fr2", document.GetValue("value1", "fr")); + Assert.IsNull(document.GetValue("value1")); //The value is there but the business logic returns null + Assert.IsFalse(document.IsCultureEdited("en")); //This returns false, the variant property value has been published + Assert.IsFalse(document.IsCultureEdited("fr")); //This returns false, the variant property value has been published + Assert.IsFalse(document.Edited); + } + [Test] public void Change_Variations_ComposedContentType_1() { @@ -793,10 +895,7 @@ namespace Umbraco.Tests.Services // can change the composing content type to invariant and back // can change the composed content type to invariant and back - var languageEn = new Language("en") { IsDefault = true }; - ServiceContext.LocalizationService.Save(languageEn); - var languageFr = new Language("fr"); - ServiceContext.LocalizationService.Save(languageFr); + CreateFrenchAndEnglishLangs(); var composing = new ContentType(-1) { @@ -925,10 +1024,7 @@ namespace Umbraco.Tests.Services // can change the composing content type to invariant and back // can change the variant composed content type to invariant and back - var languageEn = new Language("en") { IsDefault = true }; - ServiceContext.LocalizationService.Save(languageEn); - var languageFr = new Language("fr"); - ServiceContext.LocalizationService.Save(languageFr); + CreateFrenchAndEnglishLangs(); var composing = new ContentType(-1) { @@ -1110,5 +1206,13 @@ namespace Umbraco.Tests.Services AssertJsonStartsWith(document2.Id, "{'properties':{'value11':[{'culture':'','seg':'','val':'v11'}],'value12':[{'culture':'','seg':'','val':'v12'}],'value31':[{'culture':'','seg':'','val':'v31'}],'value32':[{'culture':'','seg':'','val':'v32'}]},'cultureData':"); } + + private void CreateFrenchAndEnglishLangs() + { + var languageEn = new Language("en") { IsDefault = true }; + ServiceContext.LocalizationService.Save(languageEn); + var languageFr = new Language("fr"); + ServiceContext.LocalizationService.Save(languageFr); + } } } From 87e7cec02eb2d7eb062aaef2c37f226fa5c286b6 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 31 Jul 2019 18:30:34 +1000 Subject: [PATCH 04/12] WIP - commiting what i have. Solved part of the problem but there are others. --- .../Persistence/Factories/PropertyFactory.cs | 21 +- .../Implement/ContentTypeRepository.cs | 4 +- .../Implement/ContentTypeRepositoryBase.cs | 263 +++++++++++++++++- .../Implement/DocumentRepository.cs | 25 +- .../Implement/MediaTypeRepository.cs | 4 +- .../Implement/MemberTypeRepository.cs | 4 +- .../Repositories/ContentTypeRepositoryTest.cs | 9 +- .../Repositories/DocumentRepositoryTest.cs | 2 +- .../Repositories/DomainRepositoryTest.cs | 2 +- .../Repositories/MediaRepositoryTest.cs | 3 +- .../Repositories/MediaTypeRepositoryTest.cs | 3 +- .../Repositories/MemberRepositoryTest.cs | 3 +- .../Repositories/MemberTypeRepositoryTest.cs | 3 +- .../PublicAccessRepositoryTest.cs | 2 +- .../Repositories/TagRepositoryTest.cs | 5 +- .../Repositories/TemplateRepositoryTest.cs | 2 +- .../Repositories/UserRepositoryTest.cs | 5 +- .../Services/ContentServicePerformanceTest.cs | 12 +- .../Services/ContentServiceTests.cs | 2 +- .../ContentTypeServiceVariantsTests.cs | 31 ++- 20 files changed, 344 insertions(+), 61 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Factories/PropertyFactory.cs b/src/Umbraco.Core/Persistence/Factories/PropertyFactory.cs index f0937a3781..4e7dc1e982 100644 --- a/src/Umbraco.Core/Persistence/Factories/PropertyFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/PropertyFactory.cs @@ -105,9 +105,14 @@ namespace Umbraco.Core.Persistence.Factories /// /// out parameter indicating that one or more properties have been edited /// out parameter containing a collection of edited cultures when the contentVariation varies by culture + /// + /// 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) + /// /// public static IEnumerable BuildDtos(ContentVariation contentVariation, int currentVersionId, int publishedVersionId, IEnumerable properties, - ILanguageRepository languageRepository, out bool edited, out HashSet editedCultures) + ILanguageRepository languageRepository, out bool edited, + out HashSet editedCultures) { var propertyDataDtos = new List(); edited = false; @@ -141,14 +146,14 @@ 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 so here we need to only process edited cultures based on the + //// current value type and how the property varies. - if (property.PropertyType.VariesByCulture() && isInvariantValue) continue; - if (!property.PropertyType.VariesByCulture() && isCultureValue) continue; + //if (property.PropertyType.VariesByCulture() && isInvariantValue) continue; + //if (!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); diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepository.cs index 9d77eb0990..8b84145027 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepository.cs @@ -18,8 +18,8 @@ namespace Umbraco.Core.Persistence.Repositories.Implement /// internal class ContentTypeRepository : ContentTypeRepositoryBase, IContentTypeRepository { - public ContentTypeRepository(IScopeAccessor scopeAccessor, AppCaches cache, ILogger logger, IContentTypeCommonRepository commonRepository) - : base(scopeAccessor, cache, logger, commonRepository) + public ContentTypeRepository(IScopeAccessor scopeAccessor, AppCaches cache, ILogger logger, IContentTypeCommonRepository commonRepository, ILanguageRepository languageRepository) + : base(scopeAccessor, cache, logger, commonRepository, languageRepository) { } protected override bool SupportsPublishing => ContentType.SupportsPublishingConst; diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs index 22c9244d8f..756435692a 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs @@ -15,6 +15,7 @@ using Umbraco.Core.Persistence.Factories; using Umbraco.Core.Persistence.Querying; using Umbraco.Core.Scoping; using Umbraco.Core.Services; +using static Umbraco.Core.Persistence.NPocoSqlExtensions.Statics; namespace Umbraco.Core.Persistence.Repositories.Implement { @@ -26,14 +27,15 @@ namespace Umbraco.Core.Persistence.Repositories.Implement internal abstract class ContentTypeRepositoryBase : NPocoRepositoryBase, IReadRepository where TEntity : class, IContentTypeComposition { - protected ContentTypeRepositoryBase(IScopeAccessor scopeAccessor, AppCaches cache, ILogger logger, IContentTypeCommonRepository commonRepository) + protected ContentTypeRepositoryBase(IScopeAccessor scopeAccessor, AppCaches cache, ILogger logger, IContentTypeCommonRepository commonRepository, ILanguageRepository languageRepository) : base(scopeAccessor, cache, logger) { CommonRepository = commonRepository; + LanguageRepository = languageRepository; } protected IContentTypeCommonRepository CommonRepository { get; } - + protected ILanguageRepository LanguageRepository { get; } protected abstract bool SupportsPublishing { get; } public IEnumerable> Move(TEntity moving, EntityContainer container) @@ -646,10 +648,16 @@ AND umbracoNode.id <> @id", case ContentVariation.Culture: CopyPropertyData(null, defaultLanguageId, propertyTypeIds, impactedL); CopyTagData(null, defaultLanguageId, propertyTypeIds, impactedL); + RenormalizeDocumentCultureVariations(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); + //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.CultureAndSegment: case ContentVariation.Segment: @@ -659,6 +667,55 @@ AND umbracoNode.id <> @id", } } + //private HashSet GetEditedCultures(ContentVariation contentVariation, int currentVersionId, int publishedVersionId, IEnumerable properties) + //{ + // HashSet editedCultures = null; // don't allocate unless necessary + // string defaultCulture = null; //don't allocate unless necessary + + // var entityVariesByCulture = contentVariation.VariesByCulture(); + + // // create dtos for each property values, but only for values that do actually exist + // // ie have a non-null value, everything else is just ignored and won't have a db row + + // foreach (var property in properties) + // { + // if (property.PropertyType.SupportsPublishing) + // { + // //create the resulting hashset if it's not created and the entity varies by culture + // if (entityVariesByCulture && editedCultures == null) + // editedCultures = new HashSet(StringComparer.OrdinalIgnoreCase); + + // // publishing = deal with edit and published values + // foreach (var propertyValue in property.Values) + // { + // var isInvariantValue = propertyValue.Culture == null; + // var isCultureValue = propertyValue.Culture != null && propertyValue.Segment == null; + + // // 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); + + // if (entityVariesByCulture && !sameValues) + // { + // if (isCultureValue) + // { + // editedCultures.Add(propertyValue.Culture); // report culture as edited + // } + // else if (isInvariantValue) + // { + // // flag culture as edited if it contains an edited invariant property + // if (defaultCulture == null) + // defaultCulture = languageRepository.GetDefaultIsoCode(); + + // editedCultures.Add(defaultCulture); + // } + // } + // } + // } + // } + + // return editedCultures; + //} + /// /// Moves variant data for a content type variation change. /// @@ -963,6 +1020,208 @@ AND umbracoNode.id <> @id", Database.Execute(sqlDelete); } + + } + + /// + /// Re-normalizes the edited value in the umbracoDocumentCultureVariation table when property variations are changed + /// + /// + /// + /// + /// 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. + /// + private void RenormalizeDocumentCultureVariations(IReadOnlyCollection propertyTypeIds, IReadOnlyCollection 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 + //based on the current variance of each item to see if it's 'edited' value should be true/false. + + var whereInArgsCount = propertyTypeIds.Count + (contentTypeIds?.Count ?? 0); + if (whereInArgsCount > 2000) + throw new NotSupportedException("Too many property/content types."); + + var propertySql = Sql() + .Select() + .AndSelect(x => x.NodeId, x => x.Current) + .AndSelect(x => x.Published) + .AndSelect(x => x.Variations) + .From() + .InnerJoin().On((left, right) => left.Id == right.VersionId) + .InnerJoin().On((left, right) => left.Id == right.PropertyTypeId); + + if (contentTypeIds != null) + { + propertySql.InnerJoin().On((c, cversion) => c.NodeId == cversion.NodeId); + } + + propertySql.LeftJoin().On((docversion, cversion) => cversion.Id == docversion.Id) + .Where((docversion, cversion) => cversion.Current || docversion.Published) + .WhereIn(x => x.PropertyTypeId, propertyTypeIds); + + if (contentTypeIds != null) + { + propertySql.WhereIn(x => x.ContentTypeId, contentTypeIds); + } + + propertySql + .OrderBy(x => x.NodeId) + .OrderBy(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>(); + + 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. + //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(propertySql)) + { + //make sure to reset on each node/property change + if (nodeId != row.NodeId || propertyTypeId != row.PropertyTypeId) + { + nodeId = row.NodeId; + propertyTypeId = row.PropertyTypeId; + pubRow = null; + } + + if (row.Published) + pubRow = row; + + if (row.Current) + { + var propVariations = (ContentVariation)row.Variations; + + //if this prop doesn't vary but the row has a lang assigned or vice versa, flag this as not edited + if (!propVariations.VariesByCulture() && row.LanguageId.HasValue + || 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); + } + 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); + } + //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; + } + + //reset + pubRow = null; + } + } + + //lookup all matching rows in umbracoDocumentCultureVariation + var docCultureVariationsToUpdate = Database.Fetch( + Sql().Select().From() + .WhereIn(x => x.LanguageId, editedVersions.Keys.Select(x => x.langId).ToList()) + .WhereIn(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("cv1", x => x.NodeId, x => Alias(x.Id, "currentVersion")) + // .AndSelect("cvcv1", x => x.LanguageId, x => Alias(x.Name, "currentName")) + // .AndSelect("cvcv2", x => Alias(x.Name, "publishedName")) + // .AndSelect("dv", x => Alias(x.Id, "publishedVersion")) + // .AndSelect("dcv", x => x.Id, x => x.Edited) + // .From("cvcv1") + // .InnerJoin("cv1") + // .On((left, right) => left.Id == right.VersionId, "cv1", "cvcv1") + // .InnerJoin("dcv") + // .On((left, right, other) => left.NodeId == right.NodeId && left.LanguageId == other.LanguageId, "dcv", "cv1", "cvcv1") + // .LeftJoin(nested => + // nested.InnerJoin("dv") + // .On((left, right) => left.Id == right.Id && right.Published, "cv2", "dv"), "cv2") + // .On((left, right) => left.NodeId == right.NodeId, "cv1", "cv2") + // .LeftJoin("cvcv2") + // .On((left, right, other) => left.VersionId == right.Id && left.LanguageId == other.LanguageId, "cvcv2", "cv2", "cvcv1") + // .Where(x => x.Current, "cv1") + // .OrderBy("cv1.nodeId, cvcv1.versionId, cvcv1.languageId"); + + //var names = Database.Fetch(nameSql); + + } + + private static bool IsPropertyValueChanged(PropertyValueVersionDto pubRow, PropertyValueVersionDto row) + { + return !pubRow.TextValue.IsNullOrWhiteSpace() && pubRow.TextValue != row.TextValue + || !pubRow.VarcharValue.IsNullOrWhiteSpace() && pubRow.VarcharValue != row.VarcharValue + || pubRow.DateValue.HasValue && pubRow.DateValue != row.DateValue + || pubRow.DecimalValue.HasValue && pubRow.DecimalValue != row.DecimalValue + || pubRow.IntValue.HasValue && pubRow.IntValue != row.IntValue; + } + + private class NameCompareDto + { + public int NodeId { get; set; } + public int CurrentVersion { get; set; } + public int LanguageId { get; set; } + public string CurrentName { get; set; } + public string PublishedName { get; set; } + public int? PublishedVersion { get; set; } + public int Id { get; set; } // the Id of the DocumentCultureVariationDto + public bool Edited { get; set; } + } + + private class PropertyValueVersionDto + { + public int VersionId { get; set; } + public int PropertyTypeId { get; set; } + public int? LanguageId { get; set; } + public string Segment { get; set; } + public int? IntValue { get; set; } + + private decimal? _decimalValue; + [Column("decimalValue")] + public decimal? DecimalValue + { + get => _decimalValue; + set => _decimalValue = value?.Normalize(); + } + + public DateTime? DateValue { get; set; } + public string VarcharValue { get; set; } + public string TextValue { get; set; } + + public int NodeId { get; set; } + public bool Current { get; set; } + public bool Published { get; set; } + + public byte Variations { get; set; } } private void DeletePropertyType(int contentTypeId, int propertyTypeId) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs index 30a2927cc8..0dbfc61b8c 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs @@ -386,7 +386,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement Database.BulkInsertRecords(GetContentVariationDtos(entity, publishing)); // insert document variations - Database.BulkInsertRecords(GetDocumentVariationDtos(entity, publishing, editedCultures)); + Database.BulkInsertRecords(GetDocumentVariationDtos(entity, editedCultures)); } // refresh content @@ -571,7 +571,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement Database.BulkInsertRecords(GetContentVariationDtos(entity, publishing)); // insert document variations - Database.BulkInsertRecords(GetDocumentVariationDtos(entity, publishing, editedCultures)); + Database.BulkInsertRecords(GetDocumentVariationDtos(entity, editedCultures)); } // refresh content @@ -1297,25 +1297,30 @@ namespace Umbraco.Core.Persistence.Repositories.Implement }; } - private IEnumerable GetDocumentVariationDtos(IContent content, bool publishing, HashSet editedCultures) + private IEnumerable GetDocumentVariationDtos(IContent content, HashSet editedCultures) { var allCultures = content.AvailableCultures.Union(content.PublishedCultures); // union = distinct foreach (var culture in allCultures) - yield return new DocumentCultureVariationDto + { + var dto = new DocumentCultureVariationDto { NodeId = content.Id, LanguageId = LanguageRepository.GetIdByIsoCode(culture) ?? throw new InvalidOperationException("Not a valid culture."), Culture = culture, Name = content.GetCultureName(culture) ?? content.GetPublishName(culture), - - // note: can't use IsCultureEdited at that point - hasn't been updated yet - see PersistUpdatedItem - Available = content.IsCultureAvailable(culture), - Published = content.IsCulturePublished(culture), - Edited = content.IsCultureAvailable(culture) && - (!content.IsCulturePublished(culture) || (editedCultures != null && editedCultures.Contains(culture))) + Published = content.IsCulturePublished(culture) }; + + // note: can't use IsCultureEdited at that point - hasn't been updated yet - see PersistUpdatedItem + + dto.Edited = content.IsCultureAvailable(culture) && + (!content.IsCulturePublished(culture) || (editedCultures != null && editedCultures.Contains(culture))); + + yield return dto; + } + } private class ContentVariation diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/MediaTypeRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/MediaTypeRepository.cs index 1abc75cf3a..a2c7cc3f44 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/MediaTypeRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/MediaTypeRepository.cs @@ -17,8 +17,8 @@ namespace Umbraco.Core.Persistence.Repositories.Implement /// internal class MediaTypeRepository : ContentTypeRepositoryBase, IMediaTypeRepository { - public MediaTypeRepository(IScopeAccessor scopeAccessor, AppCaches cache, ILogger logger, IContentTypeCommonRepository commonRepository) - : base(scopeAccessor, cache, logger, commonRepository) + public MediaTypeRepository(IScopeAccessor scopeAccessor, AppCaches cache, ILogger logger, IContentTypeCommonRepository commonRepository, ILanguageRepository languageRepository) + : base(scopeAccessor, cache, logger, commonRepository, languageRepository) { } protected override bool SupportsPublishing => MediaType.SupportsPublishingConst; diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/MemberTypeRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/MemberTypeRepository.cs index d96854743e..b4d6033b9b 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/MemberTypeRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/MemberTypeRepository.cs @@ -18,8 +18,8 @@ namespace Umbraco.Core.Persistence.Repositories.Implement /// internal class MemberTypeRepository : ContentTypeRepositoryBase, IMemberTypeRepository { - public MemberTypeRepository(IScopeAccessor scopeAccessor, AppCaches cache, ILogger logger, IContentTypeCommonRepository commonRepository) - : base(scopeAccessor, cache, logger, commonRepository) + public MemberTypeRepository(IScopeAccessor scopeAccessor, AppCaches cache, ILogger logger, IContentTypeCommonRepository commonRepository, ILanguageRepository languageRepository) + : base(scopeAccessor, cache, logger, commonRepository, languageRepository) { } protected override bool SupportsPublishing => MemberType.SupportsPublishingConst; diff --git a/src/Umbraco.Tests/Persistence/Repositories/ContentTypeRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/ContentTypeRepositoryTest.cs index 53f150f140..f953b9cce6 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/ContentTypeRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/ContentTypeRepositoryTest.cs @@ -29,10 +29,11 @@ namespace Umbraco.Tests.Persistence.Repositories private DocumentRepository CreateRepository(IScopeAccessor scopeAccessor, out ContentTypeRepository contentTypeRepository) { + var langRepository = new LanguageRepository(scopeAccessor, AppCaches.Disabled, Logger); var templateRepository = new TemplateRepository(scopeAccessor, AppCaches.Disabled, Logger, TestObjects.GetFileSystemsMock()); var tagRepository = new TagRepository(scopeAccessor, AppCaches.Disabled, Logger); var commonRepository = new ContentTypeCommonRepository(scopeAccessor, templateRepository, AppCaches.Disabled); - contentTypeRepository = new ContentTypeRepository(scopeAccessor, AppCaches.Disabled, Logger, commonRepository); + contentTypeRepository = new ContentTypeRepository(scopeAccessor, AppCaches.Disabled, Logger, commonRepository, langRepository); var languageRepository = new LanguageRepository(scopeAccessor, AppCaches.Disabled, Logger); var repository = new DocumentRepository(scopeAccessor, AppCaches.Disabled, Logger, contentTypeRepository, templateRepository, tagRepository, languageRepository); return repository; @@ -40,9 +41,10 @@ namespace Umbraco.Tests.Persistence.Repositories private ContentTypeRepository CreateRepository(IScopeAccessor scopeAccessor) { + var langRepository = new LanguageRepository(scopeAccessor, AppCaches.Disabled, Logger); var templateRepository = new TemplateRepository(scopeAccessor, AppCaches.Disabled, Logger, TestObjects.GetFileSystemsMock()); var commonRepository = new ContentTypeCommonRepository(scopeAccessor, templateRepository, AppCaches.Disabled); - var contentTypeRepository = new ContentTypeRepository(scopeAccessor, AppCaches.Disabled, Logger, commonRepository); + var contentTypeRepository = new ContentTypeRepository(scopeAccessor, AppCaches.Disabled, Logger, commonRepository, langRepository); return contentTypeRepository; } @@ -50,7 +52,8 @@ namespace Umbraco.Tests.Persistence.Repositories { var templateRepository = new TemplateRepository(scopeAccessor, AppCaches.Disabled, Logger, TestObjects.GetFileSystemsMock()); var commonRepository = new ContentTypeCommonRepository(scopeAccessor, templateRepository, AppCaches.Disabled); - var contentTypeRepository = new MediaTypeRepository(scopeAccessor, AppCaches.Disabled, Logger, commonRepository); + var langRepository = new LanguageRepository(scopeAccessor, AppCaches.Disabled, Logger); + var contentTypeRepository = new MediaTypeRepository(scopeAccessor, AppCaches.Disabled, Logger, commonRepository, langRepository); return contentTypeRepository; } diff --git a/src/Umbraco.Tests/Persistence/Repositories/DocumentRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/DocumentRepositoryTest.cs index fd797662c0..4d62ec8301 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/DocumentRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/DocumentRepositoryTest.cs @@ -67,8 +67,8 @@ namespace Umbraco.Tests.Persistence.Repositories templateRepository = new TemplateRepository(scopeAccessor, appCaches, Logger, TestObjects.GetFileSystemsMock()); var tagRepository = new TagRepository(scopeAccessor, appCaches, Logger); var commonRepository = new ContentTypeCommonRepository(scopeAccessor, templateRepository, appCaches); - contentTypeRepository = new ContentTypeRepository(scopeAccessor, appCaches, Logger, commonRepository); var languageRepository = new LanguageRepository(scopeAccessor, appCaches, Logger); + contentTypeRepository = new ContentTypeRepository(scopeAccessor, appCaches, Logger, commonRepository, languageRepository); var repository = new DocumentRepository(scopeAccessor, appCaches, Logger, contentTypeRepository, templateRepository, tagRepository, languageRepository); return repository; } diff --git a/src/Umbraco.Tests/Persistence/Repositories/DomainRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/DomainRepositoryTest.cs index f00b2fd046..628f8d75a7 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/DomainRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/DomainRepositoryTest.cs @@ -23,8 +23,8 @@ namespace Umbraco.Tests.Persistence.Repositories var templateRepository = new TemplateRepository(accessor, Core.Cache.AppCaches.Disabled, Logger, TestObjects.GetFileSystemsMock()); var tagRepository = new TagRepository(accessor, Core.Cache.AppCaches.Disabled, Logger); var commonRepository = new ContentTypeCommonRepository(accessor, templateRepository, AppCaches); - contentTypeRepository = new ContentTypeRepository(accessor, Core.Cache.AppCaches.Disabled, Logger, commonRepository); languageRepository = new LanguageRepository(accessor, Core.Cache.AppCaches.Disabled, Logger); + contentTypeRepository = new ContentTypeRepository(accessor, Core.Cache.AppCaches.Disabled, Logger, commonRepository, languageRepository); documentRepository = new DocumentRepository(accessor, Core.Cache.AppCaches.Disabled, Logger, contentTypeRepository, templateRepository, tagRepository, languageRepository); var domainRepository = new DomainRepository(accessor, Core.Cache.AppCaches.Disabled, Logger); return domainRepository; diff --git a/src/Umbraco.Tests/Persistence/Repositories/MediaRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/MediaRepositoryTest.cs index 1d9cf6d022..e2123df9e3 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/MediaRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/MediaRepositoryTest.cs @@ -38,7 +38,8 @@ namespace Umbraco.Tests.Persistence.Repositories var templateRepository = new TemplateRepository(scopeAccessor, appCaches, Logger, TestObjects.GetFileSystemsMock()); var commonRepository = new ContentTypeCommonRepository(scopeAccessor, templateRepository, appCaches); - mediaTypeRepository = new MediaTypeRepository(scopeAccessor, appCaches, Logger, commonRepository); + var languageRepository = new LanguageRepository(scopeAccessor, appCaches, Logger); + mediaTypeRepository = new MediaTypeRepository(scopeAccessor, appCaches, Logger, commonRepository, languageRepository); var tagRepository = new TagRepository(scopeAccessor, appCaches, Logger); var repository = new MediaRepository(scopeAccessor, appCaches, Logger, mediaTypeRepository, tagRepository, Mock.Of()); return repository; diff --git a/src/Umbraco.Tests/Persistence/Repositories/MediaTypeRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/MediaTypeRepositoryTest.cs index f302d1d992..bb3286daed 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/MediaTypeRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/MediaTypeRepositoryTest.cs @@ -23,7 +23,8 @@ namespace Umbraco.Tests.Persistence.Repositories var cacheHelper = AppCaches.Disabled; var templateRepository = new TemplateRepository((IScopeAccessor)provider, cacheHelper, Logger, TestObjects.GetFileSystemsMock()); var commonRepository = new ContentTypeCommonRepository((IScopeAccessor)provider, templateRepository, AppCaches); - return new MediaTypeRepository((IScopeAccessor) provider, AppCaches.Disabled, Logger, commonRepository); + var languageRepository = new LanguageRepository((IScopeAccessor)provider, AppCaches, Logger); + return new MediaTypeRepository((IScopeAccessor) provider, AppCaches.Disabled, Logger, commonRepository, languageRepository); } private EntityContainerRepository CreateContainerRepository(IScopeProvider provider) diff --git a/src/Umbraco.Tests/Persistence/Repositories/MemberRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/MemberRepositoryTest.cs index a5f7f08f22..17b16ad7ab 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/MemberRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/MemberRepositoryTest.cs @@ -31,7 +31,8 @@ namespace Umbraco.Tests.Persistence.Repositories var accessor = (IScopeAccessor) provider; var templateRepository = Mock.Of(); var commonRepository = new ContentTypeCommonRepository(accessor, templateRepository, AppCaches); - memberTypeRepository = new MemberTypeRepository(accessor, AppCaches.Disabled, Logger, commonRepository); + var languageRepository = new LanguageRepository(accessor, AppCaches.Disabled, Logger); + memberTypeRepository = new MemberTypeRepository(accessor, AppCaches.Disabled, Logger, commonRepository, languageRepository); memberGroupRepository = new MemberGroupRepository(accessor, AppCaches.Disabled, Logger); var tagRepo = new TagRepository(accessor, AppCaches.Disabled, Logger); var repository = new MemberRepository(accessor, AppCaches.Disabled, Logger, memberTypeRepository, memberGroupRepository, tagRepo, Mock.Of()); diff --git a/src/Umbraco.Tests/Persistence/Repositories/MemberTypeRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/MemberTypeRepositoryTest.cs index 79e8e43804..4b9f3096ce 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/MemberTypeRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/MemberTypeRepositoryTest.cs @@ -24,7 +24,8 @@ namespace Umbraco.Tests.Persistence.Repositories { var templateRepository = Mock.Of(); var commonRepository = new ContentTypeCommonRepository((IScopeAccessor)provider, templateRepository, AppCaches); - return new MemberTypeRepository((IScopeAccessor) provider, AppCaches.Disabled, Mock.Of(), commonRepository); + var languageRepository = new LanguageRepository((IScopeAccessor)provider, AppCaches.Disabled, Mock.Of()); + return new MemberTypeRepository((IScopeAccessor) provider, AppCaches.Disabled, Mock.Of(), commonRepository, languageRepository); } [Test] diff --git a/src/Umbraco.Tests/Persistence/Repositories/PublicAccessRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/PublicAccessRepositoryTest.cs index 803eff25af..56041c24aa 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/PublicAccessRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/PublicAccessRepositoryTest.cs @@ -308,8 +308,8 @@ namespace Umbraco.Tests.Persistence.Repositories var templateRepository = new TemplateRepository(accessor, AppCaches, Logger, TestObjects.GetFileSystemsMock()); var tagRepository = new TagRepository(accessor, AppCaches, Logger); var commonRepository = new ContentTypeCommonRepository(accessor, templateRepository, AppCaches); - contentTypeRepository = new ContentTypeRepository(accessor, AppCaches, Logger, commonRepository); var languageRepository = new LanguageRepository(accessor, AppCaches, Logger); + contentTypeRepository = new ContentTypeRepository(accessor, AppCaches, Logger, commonRepository, languageRepository); var repository = new DocumentRepository(accessor, AppCaches, Logger, contentTypeRepository, templateRepository, tagRepository, languageRepository); return repository; } diff --git a/src/Umbraco.Tests/Persistence/Repositories/TagRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/TagRepositoryTest.cs index b6cc4dc50d..e3de2c2892 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/TagRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/TagRepositoryTest.cs @@ -956,8 +956,8 @@ namespace Umbraco.Tests.Persistence.Repositories var templateRepository = new TemplateRepository(accessor, AppCaches.Disabled, Logger, TestObjects.GetFileSystemsMock()); var tagRepository = new TagRepository(accessor, AppCaches.Disabled, Logger); var commonRepository = new ContentTypeCommonRepository(accessor, templateRepository, AppCaches.Disabled); - contentTypeRepository = new ContentTypeRepository(accessor, AppCaches.Disabled, Logger, commonRepository); var languageRepository = new LanguageRepository(accessor, AppCaches.Disabled, Logger); + contentTypeRepository = new ContentTypeRepository(accessor, AppCaches.Disabled, Logger, commonRepository, languageRepository); var repository = new DocumentRepository(accessor, AppCaches.Disabled, Logger, contentTypeRepository, templateRepository, tagRepository, languageRepository); return repository; } @@ -968,7 +968,8 @@ namespace Umbraco.Tests.Persistence.Repositories var templateRepository = new TemplateRepository(accessor, AppCaches.Disabled, Logger, TestObjects.GetFileSystemsMock()); var tagRepository = new TagRepository(accessor, AppCaches.Disabled, Logger); var commonRepository = new ContentTypeCommonRepository(accessor, templateRepository, AppCaches.Disabled); - mediaTypeRepository = new MediaTypeRepository(accessor, AppCaches.Disabled, Logger, commonRepository); + var languageRepository = new LanguageRepository(accessor, AppCaches.Disabled, Logger); + mediaTypeRepository = new MediaTypeRepository(accessor, AppCaches.Disabled, Logger, commonRepository, languageRepository); var repository = new MediaRepository(accessor, AppCaches.Disabled, Logger, mediaTypeRepository, tagRepository, Mock.Of()); return repository; } diff --git a/src/Umbraco.Tests/Persistence/Repositories/TemplateRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/TemplateRepositoryTest.cs index 13cbd463fb..b0f9a5335b 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/TemplateRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/TemplateRepositoryTest.cs @@ -239,8 +239,8 @@ namespace Umbraco.Tests.Persistence.Repositories var tagRepository = new TagRepository((IScopeAccessor) ScopeProvider, AppCaches.Disabled, Logger); var commonRepository = new ContentTypeCommonRepository(ScopeProvider, templateRepository, AppCaches); - var contentTypeRepository = new ContentTypeRepository((IScopeAccessor) ScopeProvider, AppCaches.Disabled, Logger, commonRepository); var languageRepository = new LanguageRepository((IScopeAccessor) ScopeProvider, AppCaches.Disabled, Logger); + var contentTypeRepository = new ContentTypeRepository((IScopeAccessor) ScopeProvider, AppCaches.Disabled, Logger, commonRepository, languageRepository); var contentRepo = new DocumentRepository((IScopeAccessor) ScopeProvider, AppCaches.Disabled, Logger, contentTypeRepository, templateRepository, tagRepository, languageRepository); var contentType = MockedContentTypes.CreateSimpleContentType("umbTextpage2", "Textpage"); diff --git a/src/Umbraco.Tests/Persistence/Repositories/UserRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/UserRepositoryTest.cs index b550091591..3e5919d7f3 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/UserRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/UserRepositoryTest.cs @@ -26,7 +26,8 @@ namespace Umbraco.Tests.Persistence.Repositories var accessor = (IScopeAccessor) provider; var templateRepository = new TemplateRepository(accessor, AppCaches.Disabled, Logger, TestObjects.GetFileSystemsMock()); var commonRepository = new ContentTypeCommonRepository(accessor, templateRepository, AppCaches); - mediaTypeRepository = new MediaTypeRepository(accessor, AppCaches, Mock.Of(), commonRepository); + var languageRepository = new LanguageRepository(accessor, AppCaches, Logger); + mediaTypeRepository = new MediaTypeRepository(accessor, AppCaches, Mock.Of(), commonRepository, languageRepository); var tagRepository = new TagRepository(accessor, AppCaches, Mock.Of()); var repository = new MediaRepository(accessor, AppCaches, Mock.Of(), mediaTypeRepository, tagRepository, Mock.Of()); return repository; @@ -44,8 +45,8 @@ namespace Umbraco.Tests.Persistence.Repositories templateRepository = new TemplateRepository(accessor, AppCaches, Logger, TestObjects.GetFileSystemsMock()); var tagRepository = new TagRepository(accessor, AppCaches, Logger); var commonRepository = new ContentTypeCommonRepository(accessor, templateRepository, AppCaches); - contentTypeRepository = new ContentTypeRepository(accessor, AppCaches, Logger, commonRepository); var languageRepository = new LanguageRepository(accessor, AppCaches, Logger); + contentTypeRepository = new ContentTypeRepository(accessor, AppCaches, Logger, commonRepository, languageRepository); var repository = new DocumentRepository(accessor, AppCaches, Logger, contentTypeRepository, templateRepository, tagRepository, languageRepository); return repository; } diff --git a/src/Umbraco.Tests/Services/ContentServicePerformanceTest.cs b/src/Umbraco.Tests/Services/ContentServicePerformanceTest.cs index ed5e6073ac..ef80672baf 100644 --- a/src/Umbraco.Tests/Services/ContentServicePerformanceTest.cs +++ b/src/Umbraco.Tests/Services/ContentServicePerformanceTest.cs @@ -166,8 +166,8 @@ namespace Umbraco.Tests.Services var tRepository = new TemplateRepository((IScopeAccessor) provider, AppCaches.Disabled, Logger, TestObjects.GetFileSystemsMock()); var tagRepo = new TagRepository((IScopeAccessor) provider, AppCaches.Disabled, Logger); var commonRepository = new ContentTypeCommonRepository((IScopeAccessor)provider, tRepository, AppCaches); - var ctRepository = new ContentTypeRepository((IScopeAccessor) provider, AppCaches.Disabled, Logger, commonRepository); var languageRepository = new LanguageRepository((IScopeAccessor) provider, AppCaches.Disabled, Logger); + var ctRepository = new ContentTypeRepository((IScopeAccessor) provider, AppCaches.Disabled, Logger, commonRepository, languageRepository); var repository = new DocumentRepository((IScopeAccessor) provider, AppCaches.Disabled, Logger, ctRepository, tRepository, tagRepo, languageRepository); // Act @@ -200,8 +200,8 @@ namespace Umbraco.Tests.Services var tRepository = new TemplateRepository((IScopeAccessor) provider, AppCaches.Disabled, Logger, TestObjects.GetFileSystemsMock()); var tagRepo = new TagRepository((IScopeAccessor) provider, AppCaches.Disabled, Logger); var commonRepository = new ContentTypeCommonRepository((IScopeAccessor)provider, tRepository, AppCaches); - var ctRepository = new ContentTypeRepository((IScopeAccessor) provider, AppCaches.Disabled, Logger, commonRepository); var languageRepository = new LanguageRepository((IScopeAccessor) provider, AppCaches.Disabled, Logger); + var ctRepository = new ContentTypeRepository((IScopeAccessor) provider, AppCaches.Disabled, Logger, commonRepository, languageRepository); var repository = new DocumentRepository((IScopeAccessor) provider, AppCaches.Disabled, Logger, ctRepository, tRepository, tagRepo, languageRepository); // Act @@ -232,8 +232,8 @@ namespace Umbraco.Tests.Services var tRepository = new TemplateRepository((IScopeAccessor) provider, AppCaches.Disabled, Logger, TestObjects.GetFileSystemsMock()); var tagRepo = new TagRepository((IScopeAccessor) provider, AppCaches.Disabled, Logger); var commonRepository = new ContentTypeCommonRepository((IScopeAccessor) provider, tRepository, AppCaches); - var ctRepository = new ContentTypeRepository((IScopeAccessor) provider, AppCaches.Disabled, Logger, commonRepository); - var languageRepository = new LanguageRepository((IScopeAccessor) provider, AppCaches.Disabled, Logger); + var languageRepository = new LanguageRepository((IScopeAccessor)provider, AppCaches.Disabled, Logger); + var ctRepository = new ContentTypeRepository((IScopeAccessor) provider, AppCaches.Disabled, Logger, commonRepository, languageRepository); var repository = new DocumentRepository((IScopeAccessor) provider, AppCaches.Disabled, Logger, ctRepository, tRepository, tagRepo, languageRepository); // Act @@ -267,8 +267,8 @@ namespace Umbraco.Tests.Services var tRepository = new TemplateRepository((IScopeAccessor) provider, AppCaches.Disabled, Logger, TestObjects.GetFileSystemsMock()); var tagRepo = new TagRepository((IScopeAccessor) provider, AppCaches.Disabled, Logger); var commonRepository = new ContentTypeCommonRepository((IScopeAccessor)provider, tRepository, AppCaches); - var ctRepository = new ContentTypeRepository((IScopeAccessor) provider, AppCaches.Disabled, Logger, commonRepository); - var languageRepository = new LanguageRepository((IScopeAccessor) provider, AppCaches.Disabled, Logger); + var languageRepository = new LanguageRepository((IScopeAccessor)provider, AppCaches.Disabled, Logger); + var ctRepository = new ContentTypeRepository((IScopeAccessor) provider, AppCaches.Disabled, Logger, commonRepository, languageRepository); var repository = new DocumentRepository((IScopeAccessor) provider, AppCaches.Disabled, Logger, ctRepository, tRepository, tagRepo, languageRepository); // Act diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index 222f40aeed..7acfd994d3 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -3008,8 +3008,8 @@ namespace Umbraco.Tests.Services var templateRepository = new TemplateRepository(accessor, AppCaches.Disabled, Logger, TestObjects.GetFileSystemsMock()); var tagRepository = new TagRepository(accessor, AppCaches.Disabled, Logger); var commonRepository = new ContentTypeCommonRepository(accessor, templateRepository, AppCaches); - contentTypeRepository = new ContentTypeRepository(accessor, AppCaches.Disabled, Logger, commonRepository); var languageRepository = new LanguageRepository(accessor, AppCaches.Disabled, Logger); + contentTypeRepository = new ContentTypeRepository(accessor, AppCaches.Disabled, Logger, commonRepository, languageRepository); var repository = new DocumentRepository(accessor, AppCaches.Disabled, Logger, contentTypeRepository, templateRepository, tagRepository, languageRepository); return repository; } diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs index f5fa4e8795..53a39ffe40 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs @@ -810,12 +810,18 @@ namespace Umbraco.Tests.Services var document = (IContent)new Content("document", -1, contentType); document.SetCultureName("doc1en", "en"); document.SetCultureName("doc1fr", "fr"); - document.SetValue("value1", "v1en", "en"); - document.SetValue("value1", "v1fr", "fr"); - ServiceContext.ContentService.Save(document); + document.SetValue("value1", "v1en-init", "en"); + document.SetValue("value1", "v1fr-init", "fr"); + ServiceContext.ContentService.SaveAndPublish(document); //all values are published which means the document is not 'edited' - //at this stage there will be 4 property values stored in the DB for "value1" against "en" and "fr" for both edited/published versions, - //for the published values, these will be null + document = ServiceContext.ContentService.GetById(document.Id); + Assert.IsFalse(document.IsCultureEdited("en")); + Assert.IsFalse(document.IsCultureEdited("fr")); + Assert.IsFalse(document.Edited); + + document.SetValue("value1", "v1en", "en"); //change the property culture value, so now this culture will be edited + document.SetValue("value1", "v1fr", "fr"); //change the property culture value, so now this culture will be edited + ServiceContext.ContentService.Save(document); document = ServiceContext.ContentService.GetById(document.Id); Assert.AreEqual("doc1en", document.Name); @@ -829,18 +835,17 @@ namespace Umbraco.Tests.Services // switch property type to Nothing contentType.PropertyTypes.First(x => x.Alias == "value1").Variations = ContentVariation.Nothing; - ServiceContext.ContentTypeService.Save(contentType); + ServiceContext.ContentTypeService.Save(contentType); //This is going to have to re-normalize the "Edited" flag document = ServiceContext.ContentService.GetById(document.Id); + Assert.IsTrue(document.IsCultureEdited("en")); //This will remain true because there is now a pending change for the invariant property data which is flagged under the default lang + 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.IsTrue(document.Edited); - // publish the document - document.SetValue("value1", "v1inv"); //update the invariant value + //update the invariant value and publish + document.SetValue("value1", "v1inv"); ServiceContext.ContentService.SaveAndPublish(document); - //at this stage there will be 6 property values stored in the DB for "value1" against "en", "fr" and null for both edited/published versions, - //for the published values for the cultures, these will still be null but the invariant edited/published values will both be "v1inv". - document = ServiceContext.ContentService.GetById(document.Id); Assert.AreEqual("doc1en", document.Name); Assert.AreEqual("doc1en", document.GetCultureName("en")); @@ -848,8 +853,8 @@ namespace Umbraco.Tests.Services 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.AreEqual("v1inv", document.GetValue("value1")); - Assert.IsFalse(document.IsCultureEdited("en")); //This returns false, the culture shouldn't be marked as edited because the invariant property value is published - Assert.IsFalse(document.IsCultureEdited("fr")); //This returns false, the culture shouldn't be marked as edited because the invariant property value is published + 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.IsFalse(document.Edited); // switch property back to Culture From ff952a6df169bbbe11d8dad29c653c7433fcbf05 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 1 Aug 2019 15:54:44 +1000 Subject: [PATCH 05/12] Passes test! Now to add more tests/assertions and then the logic to renormalize based on name changes. --- .../Persistence/Factories/PropertyFactory.cs | 23 ++-- .../Implement/ContentTypeRepositoryBase.cs | 112 +++++++++++------- .../Implement/DocumentRepository.cs | 2 +- .../ContentTypeServiceVariantsTests.cs | 21 ++-- 4 files changed, 95 insertions(+), 63 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Factories/PropertyFactory.cs b/src/Umbraco.Core/Persistence/Factories/PropertyFactory.cs index 4e7dc1e982..33dabe1b24 100644 --- a/src/Umbraco.Core/Persistence/Factories/PropertyFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/PropertyFactory.cs @@ -104,10 +104,9 @@ namespace Umbraco.Core.Persistence.Factories /// The properties to map /// /// out parameter indicating that one or more properties have been edited - /// out parameter containing a collection of edited cultures when the contentVariation varies by culture - /// - /// 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) + /// + /// 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. /// /// public static IEnumerable BuildDtos(ContentVariation contentVariation, int currentVersionId, int publishedVersionId, IEnumerable 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); diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs index 756435692a..b29565a35e 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs @@ -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", } /// - /// 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 /// /// /// /// /// 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. /// - private void RenormalizeDocumentCultureVariations(IReadOnlyCollection propertyTypeIds, IReadOnlyCollection contentTypeIds = null) + private void RenormalizeDocumentEditedFlags(IReadOnlyCollection propertyTypeIds, IReadOnlyCollection 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(x => x.NodeId) .OrderBy(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(); 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(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( - Sql().Select().From() - .WhereIn(x => x.LanguageId, editedVersions.Keys.Select(x => x.langId).ToList()) - .WhereIn(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("cv1", x => x.NodeId, x => Alias(x.Id, "currentVersion")) @@ -1172,8 +1151,57 @@ AND umbracoNode.id <> @id", // .Where(x => x.Current, "cv1") // .OrderBy("cv1.nodeId, cvcv1.versionId, cvcv1.languageId"); - //var names = Database.Fetch(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(nameSql)) + //{ + // if (name.CurrentName != name.PublishedName) + // { + // } + //} + + //lookup all matching rows in umbracoDocumentCultureVariation + var docCultureVariationsToUpdate = editedLanguageVersions.InGroupsOf(2000) + .SelectMany(_ => Database.Fetch( + Sql().Select().From() + .WhereIn(x => x.LanguageId, editedLanguageVersions.Keys.Select(x => x.langId).ToList()) + .WhereIn(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(); + 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(u => u.Set(x => x.Edited, editValue.Key)) + .WhereIn(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(u => u.Set(x => x.Edited, editValue.Key)) + .WhereIn(x => x.NodeId, editValue.Select(x => x.Key))); + } } private static bool IsPropertyValueChanged(PropertyValueVersionDto pubRow, PropertyValueVersionDto row) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs index 0dbfc61b8c..c18921b59e 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs @@ -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) diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs index 53a39ffe40..2fed37c389 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs @@ -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 From 10958158781737e970ceca2d6a845a8d9110333e Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 1 Aug 2019 16:09:52 +1000 Subject: [PATCH 06/12] removes notes --- src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs index 2fed37c389..0c54cf5975 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs @@ -880,9 +880,6 @@ namespace Umbraco.Tests.Services document.SetValue("value1", "v1fr2", "fr"); //update the value now that it's variant again ServiceContext.ContentService.SaveAndPublish(document); - //at this stage there will be 4 property values stored in the DB for "value1" against "en", "fr" for both edited/published versions, - //with the change back to Culture, it deletes the invariant property values. - document = ServiceContext.ContentService.GetById(document.Id); Assert.AreEqual("doc1en", document.Name); Assert.AreEqual("doc1en", document.GetCultureName("en")); From 9538981730d71698e90a00aba92fea700b587eef Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 1 Aug 2019 18:49:05 +1000 Subject: [PATCH 07/12] Adds more assertions and tests and validates variations --- src/Umbraco.Core/Models/IContentTypeBase.cs | 2 +- .../Implement/ContentTypeRepositoryBase.cs | 68 ++-- .../Implement/DocumentRepository.cs | 10 +- .../ContentTypeServiceVariantsTests.cs | 290 ++++++------------ 4 files changed, 133 insertions(+), 237 deletions(-) diff --git a/src/Umbraco.Core/Models/IContentTypeBase.cs b/src/Umbraco.Core/Models/IContentTypeBase.cs index 5f1fe6ed49..ed87c5f320 100644 --- a/src/Umbraco.Core/Models/IContentTypeBase.cs +++ b/src/Umbraco.Core/Models/IContentTypeBase.cs @@ -101,7 +101,7 @@ namespace Umbraco.Core.Models PropertyGroupCollection PropertyGroups { get; set; } /// - /// Gets all local property types belonging to a group, across all local property groups. + /// Gets all local property types all local property groups or ungrouped. /// IEnumerable PropertyTypes { get; } diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs index b29565a35e..8f88c71bf0 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs @@ -100,6 +100,8 @@ namespace Umbraco.Core.Persistence.Repositories.Implement protected void PersistNewBaseContentType(IContentTypeComposition entity) { + ValidateVariations(entity); + var dto = ContentTypeFactory.BuildContentTypeDto(entity); //Cannot add a duplicate content type @@ -165,11 +167,11 @@ AND umbracoNode.nodeObjectType = @objectType", foreach (var allowedContentType in entity.AllowedContentTypes) { Database.Insert(new ContentTypeAllowedContentTypeDto - { - Id = entity.Id, - AllowedId = allowedContentType.Id.Value, - SortOrder = allowedContentType.SortOrder - }); + { + Id = entity.Id, + AllowedId = allowedContentType.Id.Value, + SortOrder = allowedContentType.SortOrder + }); } @@ -216,6 +218,8 @@ AND umbracoNode.nodeObjectType = @objectType", protected void PersistUpdatedBaseContentType(IContentTypeComposition entity) { + ValidateVariations(entity); + var dto = ContentTypeFactory.BuildContentTypeDto(entity); // ensure the alias is not used already @@ -372,7 +376,7 @@ AND umbracoNode.id <> @id", foreach (var propertyGroup in entity.PropertyGroups) { // insert or update group - var groupDto = PropertyGroupFactory.BuildGroupDto(propertyGroup,entity.Id); + var groupDto = PropertyGroupFactory.BuildGroupDto(propertyGroup, entity.Id); var groupId = propertyGroup.HasIdentity ? Database.Update(groupDto) : Convert.ToInt32(Database.Insert(groupDto)); @@ -390,7 +394,7 @@ AND umbracoNode.id <> @id", //check if the content type variation has been changed var contentTypeVariationDirty = entity.IsPropertyDirty("Variations"); - var oldContentTypeVariation = (ContentVariation) dtoPk.Variations; + var oldContentTypeVariation = (ContentVariation)dtoPk.Variations; var newContentTypeVariation = entity.Variations; var contentTypeVariationChanging = contentTypeVariationDirty && oldContentTypeVariation != newContentTypeVariation; if (contentTypeVariationChanging) @@ -451,7 +455,7 @@ AND umbracoNode.id <> @id", // via composition, with their original variations (ie not filtered by this // content type variations - we need this true value to make decisions. - foreach (var propertyType in ((ContentTypeCompositionBase) entity).RawComposedPropertyTypes) + foreach (var propertyType in ((ContentTypeCompositionBase)entity).RawComposedPropertyTypes) { if (propertyType.VariesBySegment() || newContentTypeVariation.VariesBySegment()) throw new NotSupportedException(); // TODO: support this @@ -520,6 +524,19 @@ AND umbracoNode.id <> @id", CommonRepository.ClearCache(); // always } + /// + /// Ensures that no property types are flagged for a variance that is not supported by the content type itself + /// + /// + private void ValidateVariations(IContentTypeComposition entity) + { + //if the entity does not vary at all, then the property cannot have a variance value greater than it + if (entity.Variations == ContentVariation.Nothing) + foreach (var prop in entity.PropertyTypes) + if (prop.Variations > entity.Variations) + throw new InvalidOperationException($"The property {prop.Alias} cannot have variations of {prop.Variations} with the content type variations of {entity.Variations}"); + } + private IEnumerable GetImpactedContentTypes(IContentTypeComposition contentType, IEnumerable all) { var impact = new List(); @@ -527,12 +544,12 @@ AND umbracoNode.id <> @id", var tree = new Dictionary>(); foreach (var x in all) - foreach (var y in x.ContentTypeComposition) - { - if (!tree.TryGetValue(y.Id, out var list)) - list = tree[y.Id] = new List(); - list.Add(x); - } + foreach (var y in x.ContentTypeComposition) + { + if (!tree.TryGetValue(y.Id, out var list)) + list = tree[y.Id] = new List(); + list.Add(x); + } var nset = new List(); do @@ -574,7 +591,7 @@ AND umbracoNode.id <> @id", // new property type, ignore if (!oldVariations.TryGetValue(propertyType.Id, out var oldVariationB)) continue; - var oldVariation = (ContentVariation) oldVariationB; // NPoco cannot fetch directly + var oldVariation = (ContentVariation)oldVariationB; // NPoco cannot fetch directly // only those property types that *actually* changed var newVariation = propertyType.Variations; @@ -638,7 +655,7 @@ AND umbracoNode.id <> @id", var impactedL = impacted.Select(x => x.Id).ToList(); //Group by the "To" variation so we can bulk update in the correct batches - foreach(var grouping in propertyTypeChanges.GroupBy(x => x.Value.ToVariation)) + foreach (var grouping in propertyTypeChanges.GroupBy(x => x.Value.ToVariation)) { var propertyTypeIds = grouping.Select(x => x.Key).ToList(); var toVariation = grouping.Key; @@ -1037,8 +1054,8 @@ AND umbracoNode.id <> @id", { 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 - //based on the current variance of each item to see if it's 'edited' value should be true/false. + //This will build up a query to get the property values of both the current and the published version so that we can check + //based on the current variance of each item to see if it's 'edited' value should be true/false. var whereInArgsCount = propertyTypeIds.Count + (contentTypeIds?.Count ?? 0); if (whereInArgsCount > 2000) @@ -1077,7 +1094,7 @@ AND umbracoNode.id <> @id", var editedDocument = new Dictionary(); var nodeId = -1; var propertyTypeId = -1; - + PropertyValueVersionDto pubRow = null; //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. @@ -1087,7 +1104,7 @@ AND umbracoNode.id <> @id", { //make sure to reset on each node/property change if (nodeId != row.NodeId || propertyTypeId != row.PropertyTypeId) - { + { nodeId = row.NodeId; propertyTypeId = row.PropertyTypeId; pubRow = null; @@ -1114,7 +1131,7 @@ AND umbracoNode.id <> @id", else if (pubRow == null) { //this would mean that that this property is 'edited' since there is no published version - editedLanguageVersions.Add((row.NodeId, row.LanguageId), true); + editedLanguageVersions[(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 @@ -1181,10 +1198,9 @@ AND umbracoNode.id <> @id", toUpdate.Add(docVariations); } } - else + else if (ev.Key.langId.HasValue) { - //the row doesn't exist but needs creating - //TODO: Does this ever happen?? Need to see if we can test this + //This should never happen! If a property culture is flagged as edited then the culture must exist at the document level throw new PanicException($"The existing DocumentCultureVariationDto was not found for node {ev.Key.nodeId} and language {ev.Key.langId}"); } } @@ -1197,7 +1213,7 @@ AND umbracoNode.id <> @id", } //Now bulk update the umbracoDocument table - foreach(var editValue in editedDocument.GroupBy(x => x.Value)) + foreach (var editValue in editedDocument.GroupBy(x => x.Value)) { Database.Execute(Sql().Update(u => u.Set(x => x.Edited, editValue.Key)) .WhereIn(x => x.NodeId, editValue.Select(x => x.Key))); @@ -1226,7 +1242,7 @@ AND umbracoNode.id <> @id", } private class PropertyValueVersionDto - { + { public int VersionId { get; set; } public int PropertyTypeId { get; set; } public int? LanguageId { get; set; } diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs index c18921b59e..344557d815 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs @@ -1310,14 +1310,12 @@ namespace Umbraco.Core.Persistence.Repositories.Implement Name = content.GetCultureName(culture) ?? content.GetPublishName(culture), Available = content.IsCultureAvailable(culture), - Published = content.IsCulturePublished(culture) + Published = content.IsCulturePublished(culture), + // note: can't use IsCultureEdited at that point - hasn't been updated yet - see PersistUpdatedItem + Edited = content.IsCultureAvailable(culture) && + (!content.IsCulturePublished(culture) || (editedCultures != null && editedCultures.Contains(culture))) }; - // note: can't use IsCultureEdited at that point - hasn't been updated yet - see PersistUpdatedItem - - dto.Edited = content.IsCultureAvailable(culture) && - (!content.IsCulturePublished(culture) || (editedCultures != null && editedCultures.Contains(culture))); - yield return dto; } diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs index 0c54cf5975..4b8262d4a5 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs @@ -110,7 +110,6 @@ namespace Umbraco.Tests.Services [Test] public void Change_Content_Type_Variation_Clears_Redirects() { - //create content type with a property type that varies by culture var contentType = MockedContentTypes.CreateBasicContentType(); contentType.Variations = ContentVariation.Nothing; var contentCollection = new PropertyTypeCollection(true); @@ -154,8 +153,7 @@ namespace Umbraco.Tests.Services [Test] public void Change_Content_Type_From_Invariant_Variant() - { - //create content type with a property type that varies by culture + { var contentType = MockedContentTypes.CreateBasicContentType(); contentType.Variations = ContentVariation.Nothing; var contentCollection = new PropertyTypeCollection(true); @@ -205,7 +203,6 @@ namespace Umbraco.Tests.Services [Test] public void Change_Content_Type_From_Variant_Invariant() { - //create content type with a property type that varies by culture var contentType = MockedContentTypes.CreateBasicContentType(); contentType.Variations = ContentVariation.Culture; var contentCollection = new PropertyTypeCollection(true); @@ -264,39 +261,49 @@ namespace Umbraco.Tests.Services } [Test] - public void Change_Property_Type_From_Invariant_Variant() + public void Change_Property_Type_From_To_Variant_On_Invariant_Content_Type() { - //create content type with a property type that varies by culture var contentType = MockedContentTypes.CreateBasicContentType(); contentType.Variations = ContentVariation.Nothing; - var contentCollection = new PropertyTypeCollection(true); - contentCollection.Add(new PropertyType("test", ValueStorageType.Ntext) - { - Alias = "title", - Name = "Title", - Description = "", - Mandatory = false, - SortOrder = 1, - DataTypeId = -88, - Variations = ContentVariation.Nothing - }); - contentType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Content", SortOrder = 1 }); + var properties = CreatePropertyCollection(("title", ContentVariation.Nothing)); + contentType.PropertyGroups.Add(new PropertyGroup(properties) { Name = "Content" }); + ServiceContext.ContentTypeService.Save(contentType); + + //change the property type to be variant + contentType.PropertyTypes.First().Variations = ContentVariation.Culture; + + //Cannot change a property type to be variant if the content type itself is not variant + Assert.Throws(() => ServiceContext.ContentTypeService.Save(contentType)); + } + + [Test] + public void Change_Property_Type_From_Invariant_Variant() + { + var contentType = MockedContentTypes.CreateBasicContentType(); + contentType.Variations = ContentVariation.Culture; + var properties = CreatePropertyCollection(("title", ContentVariation.Nothing)); + contentType.PropertyGroups.Add(new PropertyGroup(properties) { Name = "Content" }); ServiceContext.ContentTypeService.Save(contentType); //create some content of this content type IContent doc = MockedContent.CreateBasicContent(contentType); - doc.Name = "Home"; + doc.SetCultureName("Home", "en-US"); doc.SetValue("title", "hello world"); ServiceContext.ContentService.Save(doc); + doc = ServiceContext.ContentService.GetById(doc.Id); //re-get Assert.AreEqual("hello world", doc.GetValue("title")); - + Assert.IsTrue(doc.IsCultureEdited("en-US")); //invariant prop changes show up on default lang + Assert.IsTrue(doc.Edited); + //change the property type to be variant contentType.PropertyTypes.First().Variations = ContentVariation.Culture; ServiceContext.ContentTypeService.Save(contentType); doc = ServiceContext.ContentService.GetById(doc.Id); //re-get Assert.AreEqual("hello world", doc.GetValue("title", "en-US")); + Assert.IsTrue(doc.IsCultureEdited("en-US")); + Assert.IsTrue(doc.Edited); //change back property type to be invariant contentType.PropertyTypes.First().Variations = ContentVariation.Nothing; @@ -304,6 +311,8 @@ namespace Umbraco.Tests.Services doc = ServiceContext.ContentService.GetById(doc.Id); //re-get Assert.AreEqual("hello world", doc.GetValue("title")); + Assert.IsTrue(doc.IsCultureEdited("en-US")); //invariant prop changes show up on default lang + Assert.IsTrue(doc.Edited); } [Test] @@ -470,28 +479,11 @@ namespace Umbraco.Tests.Services CreateFrenchAndEnglishLangs(); - var contentType = new ContentType(-1) - { - Alias = "contentType", - Name = "contentType", - Variations = ContentVariation.Culture - }; + var contentType = CreateContentType(ContentVariation.Culture); - var properties = new PropertyTypeCollection(true) - { - new PropertyType("value1", ValueStorageType.Ntext) - { - Alias = "value1", - DataTypeId = -88, - Variations = ContentVariation.Culture - }, - new PropertyType("value2", ValueStorageType.Ntext) - { - Alias = "value2", - DataTypeId = -88, - Variations = ContentVariation.Nothing - } - }; + var properties = CreatePropertyCollection( + ("value1", ContentVariation.Culture), + ("value2", ContentVariation.Nothing)); contentType.PropertyGroups.Add(new PropertyGroup(properties) { Name = "Content" }); ServiceContext.ContentTypeService.Save(contentType); @@ -579,28 +571,11 @@ namespace Umbraco.Tests.Services var languageFr = new Language("fr"); ServiceContext.LocalizationService.Save(languageFr); - var contentType = new ContentType(-1) - { - Alias = "contentType", - Name = "contentType", - Variations = ContentVariation.Nothing - }; + var contentType = CreateContentType(ContentVariation.Nothing); - var properties = new PropertyTypeCollection(true) - { - new PropertyType("value1", ValueStorageType.Ntext) - { - Alias = "value1", - DataTypeId = -88, - Variations = ContentVariation.Nothing - }, - new PropertyType("value2", ValueStorageType.Ntext) - { - Alias = "value2", - DataTypeId = -88, - Variations = ContentVariation.Nothing - } - }; + var properties = CreatePropertyCollection( + ("value1", ContentVariation.Nothing), + ("value2", ContentVariation.Nothing)); contentType.PropertyGroups.Add(new PropertyGroup(properties) { Name = "Content" }); ServiceContext.ContentTypeService.Save(contentType); @@ -681,28 +656,11 @@ namespace Umbraco.Tests.Services CreateFrenchAndEnglishLangs(); - var contentType = new ContentType(-1) - { - Alias = "contentType", - Name = "contentType", - Variations = ContentVariation.Culture - }; + var contentType = CreateContentType(ContentVariation.Culture); - var properties = new PropertyTypeCollection(true) - { - new PropertyType("value1", ValueStorageType.Ntext) - { - Alias = "value1", - DataTypeId = -88, - Variations = ContentVariation.Culture - }, - new PropertyType("value2", ValueStorageType.Ntext) - { - Alias = "value2", - DataTypeId = -88, - Variations = ContentVariation.Nothing - } - }; + var properties = CreatePropertyCollection( + ("value1", ContentVariation.Culture), + ("value2", ContentVariation.Nothing)); contentType.PropertyGroups.Add(new PropertyGroup(properties) { Name = "Content" }); ServiceContext.ContentTypeService.Save(contentType); @@ -780,29 +738,16 @@ namespace Umbraco.Tests.Services } [Test] - public void Change_Variations_SimpleContentType_VariantPropertyToInvariantAndBack_While_Publishing() + public void Change_Property_Variations_From_Variant_To_Invariant_And_Ensure_Edited_Values_Are_Renormalized() { // one simple content type, variant, with both variant and invariant properties // can change an invariant property to variant and back CreateFrenchAndEnglishLangs(); - var contentType = new ContentType(-1) - { - Alias = "contentType", - Name = "contentType", - Variations = ContentVariation.Culture - }; + var contentType = CreateContentType(ContentVariation.Culture); - var properties = new PropertyTypeCollection(true) - { - new PropertyType("value1", ValueStorageType.Ntext) - { - Alias = "value1", - DataTypeId = -88, - Variations = ContentVariation.Culture - } - }; + var properties = CreatePropertyCollection(("value1", ContentVariation.Culture)); contentType.PropertyGroups.Add(new PropertyGroup(properties) { Name = "Content" }); ServiceContext.ContentTypeService.Save(contentType); @@ -902,54 +847,20 @@ namespace Umbraco.Tests.Services CreateFrenchAndEnglishLangs(); - var composing = new ContentType(-1) - { - Alias = "composing", - Name = "composing", - Variations = ContentVariation.Culture - }; + var composing = CreateContentType(ContentVariation.Culture, "composing"); - var properties1 = new PropertyTypeCollection(true) - { - new PropertyType("value11", ValueStorageType.Ntext) - { - Alias = "value11", - DataTypeId = -88, - Variations = ContentVariation.Culture - }, - new PropertyType("value12", ValueStorageType.Ntext) - { - Alias = "value12", - DataTypeId = -88, - Variations = ContentVariation.Nothing - } - }; + var properties1 = CreatePropertyCollection( + ("value11", ContentVariation.Culture), + ("value12", ContentVariation.Nothing)); composing.PropertyGroups.Add(new PropertyGroup(properties1) { Name = "Content" }); ServiceContext.ContentTypeService.Save(composing); - var composed = new ContentType(-1) - { - Alias = "composed", - Name = "composed", - Variations = ContentVariation.Culture - }; + var composed = CreateContentType(ContentVariation.Culture, "composed"); - var properties2 = new PropertyTypeCollection(true) - { - new PropertyType("value21", ValueStorageType.Ntext) - { - Alias = "value21", - DataTypeId = -88, - Variations = ContentVariation.Culture - }, - new PropertyType("value22", ValueStorageType.Ntext) - { - Alias = "value22", - DataTypeId = -88, - Variations = ContentVariation.Nothing - } - }; + var properties2 = CreatePropertyCollection( + ("value21", ContentVariation.Culture), + ("value22", ContentVariation.Nothing)); composed.PropertyGroups.Add(new PropertyGroup(properties2) { Name = "Content" }); composed.AddContentType(composing); @@ -1031,81 +942,30 @@ namespace Umbraco.Tests.Services CreateFrenchAndEnglishLangs(); - var composing = new ContentType(-1) - { - Alias = "composing", - Name = "composing", - Variations = ContentVariation.Culture - }; + var composing = CreateContentType(ContentVariation.Culture, "composing"); - var properties1 = new PropertyTypeCollection(true) - { - new PropertyType("value11", ValueStorageType.Ntext) - { - Alias = "value11", - DataTypeId = -88, - Variations = ContentVariation.Culture - }, - new PropertyType("value12", ValueStorageType.Ntext) - { - Alias = "value12", - DataTypeId = -88, - Variations = ContentVariation.Nothing - } - }; + var properties1 = CreatePropertyCollection( + ("value11", ContentVariation.Culture), + ("value12", ContentVariation.Nothing)); composing.PropertyGroups.Add(new PropertyGroup(properties1) { Name = "Content" }); ServiceContext.ContentTypeService.Save(composing); - var composed1 = new ContentType(-1) - { - Alias = "composed1", - Name = "composed1", - Variations = ContentVariation.Culture - }; + var composed1 = CreateContentType(ContentVariation.Culture, "composed1"); - var properties2 = new PropertyTypeCollection(true) - { - new PropertyType("value21", ValueStorageType.Ntext) - { - Alias = "value21", - DataTypeId = -88, - Variations = ContentVariation.Culture - }, - new PropertyType("value22", ValueStorageType.Ntext) - { - Alias = "value22", - DataTypeId = -88, - Variations = ContentVariation.Nothing - } - }; + var properties2 = CreatePropertyCollection( + ("value21", ContentVariation.Culture), + ("value22", ContentVariation.Nothing)); composed1.PropertyGroups.Add(new PropertyGroup(properties2) { Name = "Content" }); composed1.AddContentType(composing); ServiceContext.ContentTypeService.Save(composed1); - var composed2 = new ContentType(-1) - { - Alias = "composed2", - Name = "composed2", - Variations = ContentVariation.Nothing - }; + var composed2 = CreateContentType(ContentVariation.Nothing, "composed2"); - var properties3 = new PropertyTypeCollection(true) - { - new PropertyType("value31", ValueStorageType.Ntext) - { - Alias = "value31", - DataTypeId = -88, - Variations = ContentVariation.Nothing - }, - new PropertyType("value32", ValueStorageType.Ntext) - { - Alias = "value32", - DataTypeId = -88, - Variations = ContentVariation.Nothing - } - }; + var properties3 = CreatePropertyCollection( + ("value31", ContentVariation.Nothing), + ("value32", ContentVariation.Nothing)); composed2.PropertyGroups.Add(new PropertyGroup(properties3) { Name = "Content" }); composed2.AddContentType(composing); @@ -1219,5 +1079,27 @@ namespace Umbraco.Tests.Services var languageFr = new Language("fr"); ServiceContext.LocalizationService.Save(languageFr); } + + private IContentType CreateContentType(ContentVariation variance, string alias = "contentType") => new ContentType(-1) + { + Alias = alias, + Name = alias, + Variations = variance + }; + + private PropertyTypeCollection CreatePropertyCollection(params (string alias, ContentVariation variance)[] props) + { + var propertyCollection = new PropertyTypeCollection(true); + + foreach (var (alias, variance) in props) + propertyCollection.Add(new PropertyType(alias, ValueStorageType.Ntext) + { + Alias = alias, + DataTypeId = -88, + Variations = variance + }); + + return propertyCollection; + } } } From 2e83ac9282384dbcd621582d8316b61a2af19880 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 1 Aug 2019 18:54:19 +1000 Subject: [PATCH 08/12] Cleans up some tests --- .../ContentTypeServiceVariantsTests.cs | 84 +++---------------- 1 file changed, 12 insertions(+), 72 deletions(-) diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs index 4b8262d4a5..39d71feee9 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs @@ -112,18 +112,8 @@ namespace Umbraco.Tests.Services { var contentType = MockedContentTypes.CreateBasicContentType(); contentType.Variations = ContentVariation.Nothing; - var contentCollection = new PropertyTypeCollection(true); - contentCollection.Add(new PropertyType("test", ValueStorageType.Ntext) - { - Alias = "title", - Name = "Title", - Description = "", - Mandatory = false, - SortOrder = 1, - DataTypeId = -88, - Variations = ContentVariation.Nothing - }); - contentType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Content", SortOrder = 1 }); + var properties = CreatePropertyCollection(("title", ContentVariation.Nothing)); + contentType.PropertyGroups.Add(new PropertyGroup(properties) { Name = "Content" }); ServiceContext.ContentTypeService.Save(contentType); var contentType2 = MockedContentTypes.CreateBasicContentType("test"); ServiceContext.ContentTypeService.Save(contentType2); @@ -156,18 +146,8 @@ namespace Umbraco.Tests.Services { var contentType = MockedContentTypes.CreateBasicContentType(); contentType.Variations = ContentVariation.Nothing; - var contentCollection = new PropertyTypeCollection(true); - contentCollection.Add(new PropertyType("test", ValueStorageType.Ntext) - { - Alias = "title", - Name = "Title", - Description = "", - Mandatory = false, - SortOrder = 1, - DataTypeId = -88, - Variations = ContentVariation.Nothing - }); - contentType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Content", SortOrder = 1 }); + var properties = CreatePropertyCollection(("title", ContentVariation.Nothing)); + contentType.PropertyGroups.Add(new PropertyGroup(properties) { Name = "Content" }); ServiceContext.ContentTypeService.Save(contentType); //create some content of this content type @@ -205,18 +185,8 @@ namespace Umbraco.Tests.Services { var contentType = MockedContentTypes.CreateBasicContentType(); contentType.Variations = ContentVariation.Culture; - var contentCollection = new PropertyTypeCollection(true); - contentCollection.Add(new PropertyType("test", ValueStorageType.Ntext) - { - Alias = "title", - Name = "Title", - Description = "", - Mandatory = false, - SortOrder = 1, - DataTypeId = -88, - Variations = ContentVariation.Culture - }); - contentType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Content", SortOrder = 1 }); + var properties = CreatePropertyCollection(("title", ContentVariation.Culture)); + contentType.PropertyGroups.Add(new PropertyGroup(properties) { Name = "Content" }); ServiceContext.ContentTypeService.Save(contentType); //create some content of this content type @@ -321,18 +291,8 @@ namespace Umbraco.Tests.Services //create content type with a property type that varies by culture var contentType = MockedContentTypes.CreateBasicContentType(); contentType.Variations = ContentVariation.Culture; - var contentCollection = new PropertyTypeCollection(true); - contentCollection.Add(new PropertyType("test", ValueStorageType.Ntext) - { - Alias = "title", - Name = "Title", - Description = "", - Mandatory = false, - SortOrder = 1, - DataTypeId = -88, - Variations = ContentVariation.Culture - }); - contentType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Content", SortOrder = 1 }); + var properties = CreatePropertyCollection(("title", ContentVariation.Culture)); + contentType.PropertyGroups.Add(new PropertyGroup(properties) { Name = "Content" }); ServiceContext.ContentTypeService.Save(contentType); //create some content of this content type @@ -364,18 +324,8 @@ namespace Umbraco.Tests.Services //create content type with a property type that varies by culture var contentType = MockedContentTypes.CreateBasicContentType(); contentType.Variations = ContentVariation.Culture; - var contentCollection = new PropertyTypeCollection(true); - contentCollection.Add(new PropertyType("test", ValueStorageType.Ntext) - { - Alias = "title", - Name = "Title", - Description = "", - Mandatory = false, - SortOrder = 1, - DataTypeId = -88, - Variations = ContentVariation.Culture - }); - contentType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Content", SortOrder = 1 }); + var properties = CreatePropertyCollection(("title", ContentVariation.Culture)); + contentType.PropertyGroups.Add(new PropertyGroup(properties) { Name = "Content" }); ServiceContext.ContentTypeService.Save(contentType); //compose this from the other one @@ -420,18 +370,8 @@ namespace Umbraco.Tests.Services //create content type with a property type that varies by culture var contentType = MockedContentTypes.CreateBasicContentType(); contentType.Variations = ContentVariation.Culture; - var contentCollection = new PropertyTypeCollection(true); - contentCollection.Add(new PropertyType("test", ValueStorageType.Ntext) - { - Alias = "title", - Name = "Title", - Description = "", - Mandatory = false, - SortOrder = 1, - DataTypeId = -88, - Variations = ContentVariation.Culture - }); - contentType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Content", SortOrder = 1 }); + var properties = CreatePropertyCollection(("title", ContentVariation.Culture)); + contentType.PropertyGroups.Add(new PropertyGroup(properties) { Name = "Content" }); ServiceContext.ContentTypeService.Save(contentType); //compose this from the other one From ded1a22e45e2d2e168f76f388358382d032bc3a6 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 1 Aug 2019 19:01:41 +1000 Subject: [PATCH 09/12] fixes validation check --- .../Repositories/Implement/ContentTypeRepositoryBase.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs index 8f88c71bf0..2705bab39b 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs @@ -532,9 +532,15 @@ AND umbracoNode.id <> @id", { //if the entity does not vary at all, then the property cannot have a variance value greater than it if (entity.Variations == ContentVariation.Nothing) + { foreach (var prop in entity.PropertyTypes) - if (prop.Variations > entity.Variations) + { + if (prop.IsPropertyDirty(nameof(prop.Variations)) && prop.Variations > entity.Variations) throw new InvalidOperationException($"The property {prop.Alias} cannot have variations of {prop.Variations} with the content type variations of {entity.Variations}"); + } + + } + } private IEnumerable GetImpactedContentTypes(IContentTypeComposition contentType, IEnumerable all) From 9efec9244b51250f85a9ae3eaeb7192356ea5707 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 1 Aug 2019 19:09:19 +1000 Subject: [PATCH 10/12] removes commented out code --- .../Implement/ContentTypeRepositoryBase.cs | 87 +------------------ 1 file changed, 2 insertions(+), 85 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs index 2705bab39b..f2efb03ba4 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs @@ -671,16 +671,12 @@ AND umbracoNode.id <> @id", case ContentVariation.Culture: CopyPropertyData(null, defaultLanguageId, propertyTypeIds, impactedL); CopyTagData(null, defaultLanguageId, 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 + RenormalizeDocumentEditedFlags(propertyTypeIds, impactedL); break; case ContentVariation.Nothing: CopyPropertyData(defaultLanguageId, null, propertyTypeIds, impactedL); CopyTagData(defaultLanguageId, null, 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 + RenormalizeDocumentEditedFlags(propertyTypeIds, impactedL); break; case ContentVariation.CultureAndSegment: case ContentVariation.Segment: @@ -690,55 +686,6 @@ AND umbracoNode.id <> @id", } } - //private HashSet GetEditedCultures(ContentVariation contentVariation, int currentVersionId, int publishedVersionId, IEnumerable properties) - //{ - // HashSet editedCultures = null; // don't allocate unless necessary - // string defaultCulture = null; //don't allocate unless necessary - - // var entityVariesByCulture = contentVariation.VariesByCulture(); - - // // create dtos for each property values, but only for values that do actually exist - // // ie have a non-null value, everything else is just ignored and won't have a db row - - // foreach (var property in properties) - // { - // if (property.PropertyType.SupportsPublishing) - // { - // //create the resulting hashset if it's not created and the entity varies by culture - // if (entityVariesByCulture && editedCultures == null) - // editedCultures = new HashSet(StringComparer.OrdinalIgnoreCase); - - // // publishing = deal with edit and published values - // foreach (var propertyValue in property.Values) - // { - // var isInvariantValue = propertyValue.Culture == null; - // var isCultureValue = propertyValue.Culture != null && propertyValue.Segment == null; - - // // 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); - - // if (entityVariesByCulture && !sameValues) - // { - // if (isCultureValue) - // { - // editedCultures.Add(propertyValue.Culture); // report culture as edited - // } - // else if (isInvariantValue) - // { - // // flag culture as edited if it contains an edited invariant property - // if (defaultCulture == null) - // defaultCulture = languageRepository.GetDefaultIsoCode(); - - // editedCultures.Add(defaultCulture); - // } - // } - // } - // } - // } - - // return editedCultures; - //} - /// /// Moves variant data for a content type variation change. /// @@ -1153,36 +1100,6 @@ AND umbracoNode.id <> @id", } } - ////Generate SQL to lookup the current name vs the publish name for each language - //var nameSql = Sql() - // .Select("cv1", x => x.NodeId, x => Alias(x.Id, "currentVersion")) - // .AndSelect("cvcv1", x => x.LanguageId, x => Alias(x.Name, "currentName")) - // .AndSelect("cvcv2", x => Alias(x.Name, "publishedName")) - // .AndSelect("dv", x => Alias(x.Id, "publishedVersion")) - // .AndSelect("dcv", x => x.Id, x => x.Edited) - // .From("cvcv1") - // .InnerJoin("cv1") - // .On((left, right) => left.Id == right.VersionId, "cv1", "cvcv1") - // .InnerJoin("dcv") - // .On((left, right, other) => left.NodeId == right.NodeId && left.LanguageId == other.LanguageId, "dcv", "cv1", "cvcv1") - // .LeftJoin(nested => - // nested.InnerJoin("dv") - // .On((left, right) => left.Id == right.Id && right.Published, "cv2", "dv"), "cv2") - // .On((left, right) => left.NodeId == right.NodeId, "cv1", "cv2") - // .LeftJoin("cvcv2") - // .On((left, right, other) => left.VersionId == right.Id && left.LanguageId == other.LanguageId, "cvcv2", "cv2", "cvcv1") - // .Where(x => x.Current, "cv1") - // .OrderBy("cv1.nodeId, cvcv1.versionId, cvcv1.languageId"); - - ////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(nameSql)) - //{ - // if (name.CurrentName != name.PublishedName) - // { - - // } - //} - //lookup all matching rows in umbracoDocumentCultureVariation var docCultureVariationsToUpdate = editedLanguageVersions.InGroupsOf(2000) .SelectMany(_ => Database.Fetch( From 600a29514f17d8917e751ec87f8540da1803d89a Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 1 Aug 2019 21:27:31 +1000 Subject: [PATCH 11/12] more asserts --- .../ContentTypeServiceVariantsTests.cs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs index 39d71feee9..381261173b 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs @@ -156,8 +156,12 @@ namespace Umbraco.Tests.Services doc.SetValue("title", "hello world"); ServiceContext.ContentService.Save(doc); + doc = ServiceContext.ContentService.GetById(doc.Id); //re-get + Assert.AreEqual("Hello1", doc.Name); Assert.AreEqual("hello world", doc.GetValue("title")); + Assert.IsTrue(doc.Edited); + Assert.IsFalse (doc.IsCultureEdited("en-US")); //change the content type to be variant, we will also update the name here to detect the copy changes doc.Name = "Hello2"; @@ -168,6 +172,8 @@ namespace Umbraco.Tests.Services Assert.AreEqual("Hello2", doc.GetCultureName("en-US")); Assert.AreEqual("hello world", doc.GetValue("title")); //We are not checking against en-US here because properties will remain invariant + Assert.IsTrue(doc.Edited); + Assert.IsTrue(doc.IsCultureEdited("en-US")); //change back property type to be invariant, we will also update the name here to detect the copy changes doc.SetCultureName("Hello3", "en-US"); @@ -178,6 +184,8 @@ namespace Umbraco.Tests.Services Assert.AreEqual("Hello3", doc.Name); Assert.AreEqual("hello world", doc.GetValue("title")); + Assert.IsTrue(doc.Edited); + Assert.IsFalse(doc.IsCultureEdited("en-US")); } [Test] @@ -195,8 +203,11 @@ namespace Umbraco.Tests.Services doc.SetValue("title", "hello world", "en-US"); ServiceContext.ContentService.Save(doc); + doc = ServiceContext.ContentService.GetById(doc.Id); //re-get Assert.AreEqual("Hello1", doc.GetCultureName("en-US")); Assert.AreEqual("hello world", doc.GetValue("title", "en-US")); + Assert.IsTrue(doc.Edited); + Assert.IsTrue(doc.IsCultureEdited("en-US")); //change the content type to be invariant, we will also update the name here to detect the copy changes doc.SetCultureName("Hello2", "en-US"); @@ -207,6 +218,8 @@ namespace Umbraco.Tests.Services Assert.AreEqual("Hello2", doc.Name); Assert.AreEqual("hello world", doc.GetValue("title")); + Assert.IsTrue(doc.Edited); + Assert.IsFalse(doc.IsCultureEdited("en-US")); //change back property type to be variant, we will also update the name here to detect the copy changes doc.Name = "Hello3"; @@ -219,6 +232,8 @@ namespace Umbraco.Tests.Services //exists it will not be returned because the property type is invariant, //so this check proves that null will be returned Assert.IsNull(doc.GetValue("title", "en-US")); + Assert.IsTrue(doc.Edited); + Assert.IsTrue(doc.IsCultureEdited("en-US")); // this is true because the name change is copied to the default language //we can now switch the property type to be variant and the value can be returned again contentType.PropertyTypes.First().Variations = ContentVariation.Culture; @@ -227,9 +242,12 @@ namespace Umbraco.Tests.Services Assert.AreEqual("Hello3", doc.GetCultureName("en-US")); Assert.AreEqual("hello world", doc.GetValue("title", "en-US")); + Assert.IsTrue(doc.Edited); + Assert.IsTrue(doc.IsCultureEdited("en-US")); } + [Test] public void Change_Property_Type_From_To_Variant_On_Invariant_Content_Type() { From be30bb0ad0706d9a146afd85498d22aa01b055a0 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 1 Aug 2019 21:41:43 +1000 Subject: [PATCH 12/12] adds test --- .../ContentTypeServiceVariantsTests.cs | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs index 381261173b..956de186be 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceVariantsTests.cs @@ -795,6 +795,85 @@ namespace Umbraco.Tests.Services Assert.IsFalse(document.Edited); } + [Test] + public void Change_Property_Variations_From_Invariant_To_Variant_And_Ensure_Edited_Values_Are_Renormalized() + { + // one simple content type, variant, with both variant and invariant properties + // can change an invariant property to variant and back + + CreateFrenchAndEnglishLangs(); + + var contentType = CreateContentType(ContentVariation.Culture); + + var properties = CreatePropertyCollection(("value1", ContentVariation.Nothing)); + + contentType.PropertyGroups.Add(new PropertyGroup(properties) { Name = "Content" }); + ServiceContext.ContentTypeService.Save(contentType); + + var document = (IContent)new Content("document", -1, contentType); + document.SetCultureName("doc1en", "en"); + document.SetCultureName("doc1fr", "fr"); + document.SetValue("value1", "v1en-init"); + ServiceContext.ContentService.SaveAndPublish(document); //all values are published which means the document is not 'edited' + + document = ServiceContext.ContentService.GetById(document.Id); + Assert.IsFalse(document.IsCultureEdited("en")); + Assert.IsFalse(document.IsCultureEdited("fr")); + Assert.IsFalse(document.Edited); + + document.SetValue("value1", "v1en"); //change the property value, so now the invariant (default) culture will be edited + ServiceContext.ContentService.Save(document); + + document = ServiceContext.ContentService.GetById(document.Id); + Assert.AreEqual("doc1en", document.Name); + Assert.AreEqual("doc1en", document.GetCultureName("en")); + Assert.AreEqual("doc1fr", document.GetCultureName("fr")); + Assert.AreEqual("v1en", document.GetValue("value1")); + Assert.AreEqual("v1en-init", document.GetValue("value1", published: true)); + Assert.IsTrue(document.IsCultureEdited("en")); //This is true because the invariant property reflects changes on the default lang + Assert.IsFalse(document.IsCultureEdited("fr")); + Assert.IsTrue(document.Edited); + + // switch property type to Culture + contentType.PropertyTypes.First(x => x.Alias == "value1").Variations = ContentVariation.Culture; + ServiceContext.ContentTypeService.Save(contentType); //This is going to have to re-normalize the "Edited" flag + + document = ServiceContext.ContentService.GetById(document.Id); + Assert.IsTrue(document.IsCultureEdited("en")); //Remains true + Assert.IsFalse(document.IsCultureEdited("fr")); //False because no french property has ever been edited + Assert.IsTrue(document.Edited); + + //update the culture value and publish + document.SetValue("value1", "v1en2", "en"); + ServiceContext.ContentService.SaveAndPublish(document); + + document = ServiceContext.ContentService.GetById(document.Id); + Assert.AreEqual("doc1en", document.Name); + Assert.AreEqual("doc1en", document.GetCultureName("en")); + Assert.AreEqual("doc1fr", document.GetCultureName("fr")); + Assert.IsNull(document.GetValue("value1")); //The values are there but the business logic returns null + Assert.IsNull(document.GetValue("value1", published: true)); //The values are there but the business logic returns null + Assert.AreEqual("v1en2", document.GetValue("value1", "en")); + Assert.AreEqual("v1en2", document.GetValue("value1", "en", published: true)); + Assert.IsFalse(document.IsCultureEdited("en")); //This returns false, everything is published + Assert.IsFalse(document.IsCultureEdited("fr")); //False because no french property has ever been edited + Assert.IsFalse(document.Edited); + + // switch property back to Invariant + contentType.PropertyTypes.First(x => x.Alias == "value1").Variations = ContentVariation.Nothing; + ServiceContext.ContentTypeService.Save(contentType); + + document = ServiceContext.ContentService.GetById(document.Id); + Assert.AreEqual("v1en2", document.GetValue("value1")); //The variant property value gets copied over to the invariant + Assert.AreEqual("v1en2", document.GetValue("value1", published: true)); + Assert.IsNull(document.GetValue("value1", "fr")); //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.IsFalse(document.IsCultureEdited("en")); //The variant published AND edited values are copied over to the invariant + Assert.IsFalse(document.IsCultureEdited("fr")); + Assert.IsFalse(document.Edited); + + } + [Test] public void Change_Variations_ComposedContentType_1() {