From 26759e97faabb065adfc998ad48e59ed0a10f4c4 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 3 Oct 2018 19:03:22 +0200 Subject: [PATCH 01/12] First iteration to populate property type changes when they are changed variant/invariant --- .../Components/RelateOnCopyComponent.cs | 2 + .../UpdateContentOnVariantChangesComponent.cs | 93 +++++++++++++++++++ src/Umbraco.Core/Models/ContentTypeBase.cs | 3 +- .../Models/ContentTypeBaseExtensions.cs | 49 +++++++++- .../Implement/ContentRepositoryBase.cs | 3 + .../Implement/ContentTypeRepositoryBase.cs | 4 +- .../Implement/LanguageRepository.cs | 23 +++-- .../ContentTypeServiceBaseOfTItemTService.cs | 6 +- ...peServiceBaseOfTRepositoryTItemTService.cs | 20 +++- src/Umbraco.Core/Umbraco.Core.csproj | 1 + .../Repositories/ContentTypeRepositoryTest.cs | 5 +- ...itoryTest.cs => DocumentRepositoryTest.cs} | 2 +- .../Services/ContentTypeServiceTests.cs | 49 ++++++++++ src/Umbraco.Tests/Umbraco.Tests.csproj | 2 +- .../NuCache/PublishedSnapshotService.cs | 15 +-- .../XmlPublishedCache/XmlStore.cs | 12 +-- 16 files changed, 253 insertions(+), 36 deletions(-) create mode 100644 src/Umbraco.Core/Components/UpdateContentOnVariantChangesComponent.cs rename src/Umbraco.Tests/Persistence/Repositories/{ContentRepositoryTest.cs => DocumentRepositoryTest.cs} (99%) diff --git a/src/Umbraco.Core/Components/RelateOnCopyComponent.cs b/src/Umbraco.Core/Components/RelateOnCopyComponent.cs index 4ebd309e9f..5356fa6e30 100644 --- a/src/Umbraco.Core/Components/RelateOnCopyComponent.cs +++ b/src/Umbraco.Core/Components/RelateOnCopyComponent.cs @@ -1,10 +1,12 @@ using Umbraco.Core.Composing; using Umbraco.Core.Models; +using Umbraco.Core.Persistence.Repositories.Implement; using Umbraco.Core.Services; using Umbraco.Core.Services.Implement; namespace Umbraco.Core.Components { + //TODO: This should just exist in the content service/repo! [RuntimeLevel(MinLevel = RuntimeLevel.Run)] public sealed class RelateOnCopyComponent : UmbracoComponentBase, IUmbracoCoreComponent diff --git a/src/Umbraco.Core/Components/UpdateContentOnVariantChangesComponent.cs b/src/Umbraco.Core/Components/UpdateContentOnVariantChangesComponent.cs new file mode 100644 index 0000000000..06d2b86166 --- /dev/null +++ b/src/Umbraco.Core/Components/UpdateContentOnVariantChangesComponent.cs @@ -0,0 +1,93 @@ +using System; +using System.Linq; +using Umbraco.Core.Models; +using Umbraco.Core.Models.Entities; +using Umbraco.Core.Services; +using Umbraco.Core.Services.Changes; +using Umbraco.Core.Services.Implement; + +namespace Umbraco.Core.Components +{ + /// + /// Manages data changes for when content/property types have variation changes + /// + [RuntimeLevel(MinLevel = RuntimeLevel.Run)] + public sealed class UpdateContentOnVariantChangesComponent : UmbracoComponentBase, IUmbracoCoreComponent + { + private IContentService _contentService; + private ILocalizationService _langService; + + public void Initialize(IContentService contentService, ILocalizationService langService) + { + _contentService = contentService; + _langService = langService; + ContentTypeService.ScopedRefreshedEntity += OnContentTypeRefreshedEntity; + } + + private void OnContentTypeRefreshedEntity(IContentTypeService sender, ContentTypeChange.EventArgs e) + { + var defaultLang = _langService.GetDefaultLanguageIsoCode(); //this will be cached + + foreach (var c in e.Changes) + { + // existing property alias change? + var hasAnyPropertyVariationChanged = c.Item.WasPropertyTypeVariationChanged(out var aliases); + if (hasAnyPropertyVariationChanged) + { + var contentOfType = _contentService.GetByType(c.Item.Id); + + foreach (var a in aliases) + { + var propType = c.Item.PropertyTypes.First(x => x.Alias == a); + + switch(propType.Variations) + { + case ContentVariation.Culture: + //if the current variation is culture it means that the previous was nothing + foreach (var content in contentOfType) + { + object invariantVal; + try + { + content.Properties[a].PropertyType.Variations = ContentVariation.Nothing; + //now get the culture val + invariantVal = content.GetValue(a); + } + finally + { + content.Properties[a].PropertyType.Variations = ContentVariation.Culture; + } + //set the invariant value + content.SetValue(a, invariantVal, defaultLang); + } + break; + case ContentVariation.Nothing: + //if the current variation is nothing it means that the previous was culture + foreach(var content in contentOfType) + { + object cultureVal; + try + { + content.Properties[a].PropertyType.Variations = ContentVariation.Culture; + //now get the culture val + cultureVal = content.GetValue(a, defaultLang); + } + finally + { + content.Properties[a].PropertyType.Variations = ContentVariation.Nothing; + } + //set the invariant value + content.SetValue(a, cultureVal); + } + break; + default: + throw new NotSupportedException("The variation change is not supported"); + } + } + + _contentService.Save(contentOfType); + } + } + } + } +} diff --git a/src/Umbraco.Core/Models/ContentTypeBase.cs b/src/Umbraco.Core/Models/ContentTypeBase.cs index 6f90b5201d..7467d83099 100644 --- a/src/Umbraco.Core/Models/ContentTypeBase.cs +++ b/src/Umbraco.Core/Models/ContentTypeBase.cs @@ -408,7 +408,8 @@ namespace Umbraco.Core.Models /// PropertyTypes that are not part of a PropertyGroup /// [IgnoreDataMember] - internal PropertyTypeCollection PropertyTypeCollection => _propertyTypes; + //fixme should we mark this as EditorBrowsable hidden since it really isn't ever used? + internal PropertyTypeCollection PropertyTypeCollection => _propertyTypes; /// /// Indicates whether a specific property on the current entity is dirty. diff --git a/src/Umbraco.Core/Models/ContentTypeBaseExtensions.cs b/src/Umbraco.Core/Models/ContentTypeBaseExtensions.cs index ef55f0d469..8af48bb881 100644 --- a/src/Umbraco.Core/Models/ContentTypeBaseExtensions.cs +++ b/src/Umbraco.Core/Models/ContentTypeBaseExtensions.cs @@ -1,4 +1,8 @@ -using Umbraco.Core.Models.PublishedContent; +using System; +using System.Collections.Generic; +using System.Linq; +using Umbraco.Core.Models.Entities; +using Umbraco.Core.Models.PublishedContent; namespace Umbraco.Core.Models { @@ -16,5 +20,48 @@ namespace Umbraco.Core.Models else if (typeof(IMemberType).IsAssignableFrom(type)) itemType = PublishedItemType.Member; return itemType; } + + /// + /// Used to check if any property type was changed between variant/invariant + /// + /// + /// + internal static bool WasPropertyTypeVariationChanged(this IContentTypeBase contentType) + { + return contentType.WasPropertyTypeVariationChanged(out var _); + } + + /// + /// Used to check if any property type was changed between variant/invariant + /// + /// + /// + internal static bool WasPropertyTypeVariationChanged(this IContentTypeBase contentType, out IReadOnlyCollection aliases) + { + var a = new List(); + + // property variation change? + var hasAnyPropertyVariationChanged = contentType.PropertyTypes.Any(propertyType => + { + if (!(propertyType is IRememberBeingDirty dirtyProperty)) + throw new Exception("oops"); + + // skip new properties + //TODO: This used to be WasPropertyDirty("HasIdentity") but i don't think that actually worked for detecting new entities this does seem to work properly + var isNewProperty = dirtyProperty.WasPropertyDirty("Id"); + if (isNewProperty) return false; + + // variation change? + var dirty = dirtyProperty.WasPropertyDirty("Variations"); + if (dirty) + a.Add(propertyType.Alias); + + return dirty; + + }); + + aliases = a; + return hasAnyPropertyVariationChanged; + } } } diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentRepositoryBase.cs index 6ace73fbc3..c258a76b30 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentRepositoryBase.cs @@ -608,6 +608,9 @@ namespace Umbraco.Core.Persistence.Repositories.Implement #region UnitOfWork Events + //fixme: The reason these events are in the repository is for legacy, the events should exist at the service + // level now since we can fire these events within the transaction... so move the events to service level + public class ScopedEntityEventArgs : EventArgs { public ScopedEntityEventArgs(IScope scope, TEntity entity) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs index 555fc56157..05b9bc8631 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs @@ -323,9 +323,7 @@ AND umbracoNode.id <> @id", }); } - // fixme below, manage the property type - - // delete ??? fixme wtf is this? + // delete property types // ... by excepting entries from db with entries from collections if (entity.IsPropertyDirty("PropertyTypes") || entity.PropertyTypes.Any(x => x.IsDirty())) { diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/LanguageRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/LanguageRepository.cs index ae5d9ae8b8..2026daba74 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/LanguageRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/LanguageRepository.cs @@ -30,7 +30,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement return new FullDataSetRepositoryCachePolicy(GlobalIsolatedCache, ScopeAccessor, GetEntityId, /*expires:*/ false); } - private FullDataSetRepositoryCachePolicy TypedCachePolicy => (FullDataSetRepositoryCachePolicy) CachePolicy; + private FullDataSetRepositoryCachePolicy TypedCachePolicy => CachePolicy as FullDataSetRepositoryCachePolicy; #region Overrides of RepositoryBase @@ -215,7 +215,10 @@ namespace Umbraco.Core.Persistence.Repositories.Implement public ILanguage GetByIsoCode(string isoCode) { - TypedCachePolicy.GetAllCached(PerformGetAll); // ensure cache is populated, in a non-expensive way + // ensure cache is populated, in a non-expensive way + if (TypedCachePolicy != null) + TypedCachePolicy.GetAllCached(PerformGetAll); + var id = GetIdByIsoCode(isoCode, throwOnNotFound: false); return id.HasValue ? Get(id.Value) : null; } @@ -228,7 +231,10 @@ namespace Umbraco.Core.Persistence.Repositories.Implement { if (isoCode == null) return null; - TypedCachePolicy.GetAllCached(PerformGetAll); // ensure cache is populated, in a non-expensive way + // ensure cache is populated, in a non-expensive way + if (TypedCachePolicy != null) + TypedCachePolicy.GetAllCached(PerformGetAll); + lock (_codeIdMap) { if (_codeIdMap.TryGetValue(isoCode, out var id)) return id; @@ -246,7 +252,10 @@ namespace Umbraco.Core.Persistence.Repositories.Implement { if (id == null) return null; - TypedCachePolicy.GetAllCached(PerformGetAll); // ensure cache is populated, in a non-expensive way + // ensure cache is populated, in a non-expensive way + if (TypedCachePolicy != null) + TypedCachePolicy.GetAllCached(PerformGetAll); + lock (_codeIdMap) // yes, we want to lock _codeIdMap { if (_idCodeMap.TryGetValue(id.Value, out var isoCode)) return isoCode; @@ -269,8 +278,10 @@ namespace Umbraco.Core.Persistence.Repositories.Implement // do NOT leak that language, it's not deep-cloned! private ILanguage GetDefault() { - // get all cached, non-cloned - var languages = TypedCachePolicy.GetAllCached(PerformGetAll).ToList(); + // get all cached + var languages = (TypedCachePolicy?.GetAllCached(PerformGetAll) //try to get all cached non-cloned if using the correct cache policy (not the case in unit tests) + ?? CachePolicy.GetAll(Array.Empty(), PerformGetAll)).ToList(); + var language = languages.FirstOrDefault(x => x.IsDefaultVariantLanguage); if (language != null) return language; diff --git a/src/Umbraco.Core/Services/Implement/ContentTypeServiceBaseOfTItemTService.cs b/src/Umbraco.Core/Services/Implement/ContentTypeServiceBaseOfTItemTService.cs index 9fa9a47003..33fb9a0894 100644 --- a/src/Umbraco.Core/Services/Implement/ContentTypeServiceBaseOfTItemTService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentTypeServiceBaseOfTItemTService.cs @@ -20,12 +20,12 @@ namespace Umbraco.Core.Services.Implement internal static event TypedEventHandler.EventArgs> Changed; // that one is always immediate (transactional) - public static event TypedEventHandler.EventArgs> UowRefreshedEntity; + public static event TypedEventHandler.EventArgs> ScopedRefreshedEntity; // used by tests to clear events internal static void ClearScopeEvents() { - UowRefreshedEntity = null; + ScopedRefreshedEntity = null; } // these must be dispatched @@ -48,7 +48,7 @@ namespace Umbraco.Core.Services.Implement protected void OnUowRefreshedEntity(ContentTypeChange.EventArgs args) { // that one is always immediate (not dispatched, transactional) - UowRefreshedEntity.RaiseEvent(args, This); + ScopedRefreshedEntity.RaiseEvent(args, This); } protected bool OnSavingCancelled(IScope scope, SaveEventArgs args) diff --git a/src/Umbraco.Core/Services/Implement/ContentTypeServiceBaseOfTRepositoryTItemTService.cs b/src/Umbraco.Core/Services/Implement/ContentTypeServiceBaseOfTRepositoryTItemTService.cs index 8ed0a0f645..a3bc87315b 100644 --- a/src/Umbraco.Core/Services/Implement/ContentTypeServiceBaseOfTRepositoryTItemTService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentTypeServiceBaseOfTRepositoryTItemTService.cs @@ -118,6 +118,8 @@ namespace Umbraco.Core.Services.Implement // - content type alias changed // - content type property removed, or alias changed // - content type composition removed (not testing if composition had properties...) + // - content type variation changed + // - property type variation changed // // because these are the changes that would impact the raw content data @@ -132,7 +134,8 @@ namespace Umbraco.Core.Services.Implement var dirty = (IRememberBeingDirty)contentType; // skip new content types - var isNewContentType = dirty.WasPropertyDirty("HasIdentity"); + //TODO: This used to be WasPropertyDirty("HasIdentity") but i don't think that actually worked for detecting new entities this does seem to work properly + var isNewContentType = dirty.WasPropertyDirty("Id"); if (isNewContentType) { AddChange(changes, contentType, ContentTypeChangeTypes.Create); @@ -149,12 +152,12 @@ namespace Umbraco.Core.Services.Implement throw new Exception("oops"); // skip new properties - var isNewProperty = dirtyProperty.WasPropertyDirty("HasIdentity"); + //TODO: This used to be WasPropertyDirty("HasIdentity") but i don't think that actually worked for detecting new entities this does seem to work properly + var isNewProperty = dirtyProperty.WasPropertyDirty("Id"); if (isNewProperty) return false; // alias change? - var hasPropertyAliasBeenChanged = dirtyProperty.WasPropertyDirty("Alias"); - return hasPropertyAliasBeenChanged; + return dirtyProperty.WasPropertyDirty("Alias"); }); // removed properties? @@ -163,8 +166,15 @@ namespace Umbraco.Core.Services.Implement // removed compositions? var hasAnyCompositionBeenRemoved = dirty.WasPropertyDirty("HasCompositionTypeBeenRemoved"); + // variation changed? + var hasContentTypeVariationChanged = dirty.WasPropertyDirty("Variations"); + + // property variation change? + var hasAnyPropertyVariationChanged = contentType.WasPropertyTypeVariationChanged(); + // main impact on properties? - var hasPropertyMainImpact = hasAnyCompositionBeenRemoved || hasAnyPropertyBeenRemoved || hasAnyPropertyChangedAlias; + var hasPropertyMainImpact = hasContentTypeVariationChanged || hasAnyPropertyVariationChanged + || hasAnyCompositionBeenRemoved || hasAnyPropertyBeenRemoved || hasAnyPropertyChangedAlias; if (hasAliasChanged || hasPropertyMainImpact) { diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 496a66137a..379d5ad48a 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -170,6 +170,7 @@ + diff --git a/src/Umbraco.Tests/Persistence/Repositories/ContentTypeRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/ContentTypeRepositoryTest.cs index 31105aa28e..6087279285 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/ContentTypeRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/ContentTypeRepositoryTest.cs @@ -63,8 +63,7 @@ namespace Umbraco.Tests.Persistence.Repositories } //TODO Add test to verify SetDefaultTemplates updates both AllowedTemplates and DefaultTemplate(id). - - + [Test] public void Maps_Templates_Correctly() { @@ -377,7 +376,7 @@ namespace Umbraco.Tests.Persistence.Repositories repository.Save(contentType); - var dirty = ((ICanBeDirty)contentType).IsDirty(); + var dirty = contentType.IsDirty(); // Assert Assert.That(contentType.HasIdentity, Is.True); diff --git a/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/DocumentRepositoryTest.cs similarity index 99% rename from src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs rename to src/Umbraco.Tests/Persistence/Repositories/DocumentRepositoryTest.cs index 7ff7c0a2e4..68e29c4efe 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/DocumentRepositoryTest.cs @@ -25,7 +25,7 @@ namespace Umbraco.Tests.Persistence.Repositories { [TestFixture] [UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)] - public class ContentRepositoryTest : TestWithDatabaseBase + public class DocumentRepositoryTest : TestWithDatabaseBase { public override void SetUp() { diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs index be07dbdd23..8ece9621ce 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs @@ -15,6 +15,7 @@ using Umbraco.Core.Services.Implement; using Umbraco.Tests.TestHelpers; using Umbraco.Tests.TestHelpers.Entities; using Umbraco.Tests.Testing; +using Umbraco.Core.Components; namespace Umbraco.Tests.Services { @@ -23,6 +24,54 @@ namespace Umbraco.Tests.Services [UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest, PublishedRepositoryEvents = true)] public class ContentTypeServiceTests : TestWithSomeContentBase { + [Test] + public void Change_Property_Type_From_Variant_Invariant() + { + //initialize the listener which is responsible for updating content based on variant/invariant changes + var listener = new UpdateContentOnVariantChangesComponent(); + listener.Initialize(ServiceContext.ContentService, ServiceContext.LocalizationService); + + //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 }); + contentType.ResetDirtyProperties(false); + 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 Deleting_Media_Type_With_Hierarchy_Of_Media_Items_Moves_Orphaned_Media_To_Recycle_Bin() { diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index a3f02b2fdc..aac1f0b865 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -123,6 +123,7 @@ + @@ -393,7 +394,6 @@ - diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs index 8786753e4f..a9903669b9 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs @@ -198,6 +198,9 @@ namespace Umbraco.Web.PublishedCache.NuCache private void InitializeRepositoryEvents() { + //fixme: The reason these events are in the repository is for legacy, the events should exist at the service + // level now since we can fire these events within the transaction... so move the events to service level + // plug repository event handlers // these trigger within the transaction to ensure consistency // and are used to maintain the central, database-level XML cache @@ -212,9 +215,9 @@ namespace Umbraco.Web.PublishedCache.NuCache MemberRepository.ScopedEntityRefresh += OnMemberRefreshedEntity; // plug - ContentTypeService.UowRefreshedEntity += OnContentTypeRefreshedEntity; - MediaTypeService.UowRefreshedEntity += OnMediaTypeRefreshedEntity; - MemberTypeService.UowRefreshedEntity += OnMemberTypeRefreshedEntity; + ContentTypeService.ScopedRefreshedEntity += OnContentTypeRefreshedEntity; + MediaTypeService.ScopedRefreshedEntity += OnMediaTypeRefreshedEntity; + MemberTypeService.ScopedRefreshedEntity += OnMemberTypeRefreshedEntity; } private void TearDownRepositoryEvents() @@ -229,9 +232,9 @@ namespace Umbraco.Web.PublishedCache.NuCache //MemberRepository.RemovedVersion -= OnMemberRemovedVersion; MemberRepository.ScopedEntityRefresh -= OnMemberRefreshedEntity; - ContentTypeService.UowRefreshedEntity -= OnContentTypeRefreshedEntity; - MediaTypeService.UowRefreshedEntity -= OnMediaTypeRefreshedEntity; - MemberTypeService.UowRefreshedEntity -= OnMemberTypeRefreshedEntity; + ContentTypeService.ScopedRefreshedEntity -= OnContentTypeRefreshedEntity; + MediaTypeService.ScopedRefreshedEntity -= OnMediaTypeRefreshedEntity; + MemberTypeService.ScopedRefreshedEntity -= OnMemberTypeRefreshedEntity; } public override void Dispose() diff --git a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlStore.cs b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlStore.cs index 5fa89e3f7b..5b816c2f26 100644 --- a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlStore.cs +++ b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlStore.cs @@ -194,9 +194,9 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache MemberRepository.ScopedEntityRefresh += OnMemberRefreshedEntity; // plug - ContentTypeService.UowRefreshedEntity += OnContentTypeRefreshedEntity; - MediaTypeService.UowRefreshedEntity += OnMediaTypeRefreshedEntity; - MemberTypeService.UowRefreshedEntity += OnMemberTypeRefreshedEntity; + ContentTypeService.ScopedRefreshedEntity += OnContentTypeRefreshedEntity; + MediaTypeService.ScopedRefreshedEntity += OnMediaTypeRefreshedEntity; + MemberTypeService.ScopedRefreshedEntity += OnMemberTypeRefreshedEntity; _withRepositoryEvents = true; } @@ -213,9 +213,9 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache MemberRepository.ScopeVersionRemove -= OnMemberRemovingVersion; MemberRepository.ScopedEntityRefresh -= OnMemberRefreshedEntity; - ContentTypeService.UowRefreshedEntity -= OnContentTypeRefreshedEntity; - MediaTypeService.UowRefreshedEntity -= OnMediaTypeRefreshedEntity; - MemberTypeService.UowRefreshedEntity -= OnMemberTypeRefreshedEntity; + ContentTypeService.ScopedRefreshedEntity -= OnContentTypeRefreshedEntity; + MediaTypeService.ScopedRefreshedEntity -= OnMediaTypeRefreshedEntity; + MemberTypeService.ScopedRefreshedEntity -= OnMemberTypeRefreshedEntity; _withRepositoryEvents = false; } From f76df08cd2df3c84453db4f16ba9b3d5ada697d0 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 3 Oct 2018 19:34:02 +0200 Subject: [PATCH 02/12] ensures that property type variations are set to nothing if the content type is updated to nothing --- .../UpdateContentOnVariantChangesComponent.cs | 7 +- .../Implement/ContentTypeRepositoryBase.cs | 9 ++ .../Services/ContentTypeServiceTests.cs | 99 +++++++++++++++++++ 3 files changed, 113 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Core/Components/UpdateContentOnVariantChangesComponent.cs b/src/Umbraco.Core/Components/UpdateContentOnVariantChangesComponent.cs index 06d2b86166..71fe4d46bd 100644 --- a/src/Umbraco.Core/Components/UpdateContentOnVariantChangesComponent.cs +++ b/src/Umbraco.Core/Components/UpdateContentOnVariantChangesComponent.cs @@ -12,6 +12,7 @@ namespace Umbraco.Core.Components /// Manages data changes for when content/property types have variation changes /// [RuntimeLevel(MinLevel = RuntimeLevel.Run)] + //fixme: this one MUST fire before any of the content cache ones public sealed class UpdateContentOnVariantChangesComponent : UmbracoComponentBase, IUmbracoCoreComponent { private IContentService _contentService; @@ -40,7 +41,9 @@ namespace Umbraco.Core.Components { var propType = c.Item.PropertyTypes.First(x => x.Alias == a); - switch(propType.Variations) + //fixme: this does not take into account segments, but how can we since we don't know what it changed from? + + switch (propType.Variations) { case ContentVariation.Culture: //if the current variation is culture it means that the previous was nothing @@ -50,7 +53,7 @@ namespace Umbraco.Core.Components try { content.Properties[a].PropertyType.Variations = ContentVariation.Nothing; - //now get the culture val + //now get the invariant val invariantVal = content.GetValue(a); } finally diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs index 05b9bc8631..743b7c858e 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs @@ -402,10 +402,19 @@ AND umbracoNode.id <> @id", propertyType.PropertyGroupId = new Lazy(() => groupId); } + //check if the content type variation has been changed to Nothing since we will need to update + //all property types to Nothing as well if they aren't already + //fixme: this does not take into account segments, but how can we since we don't know what it changed from? + var ctVariationChangedToNothing = entity.IsPropertyDirty("Variations") && entity.Variations == ContentVariation.Nothing; + // insert or update properties // all of them, no-group and in-groups foreach (var propertyType in entity.PropertyTypes) { + //fixme: this does not take into account segments, but how can we since we don't know what it changed from? + if (ctVariationChangedToNothing && propertyType.Variations != ContentVariation.Nothing) + propertyType.Variations = ContentVariation.Nothing; + var groupId = propertyType.PropertyGroupId?.Value ?? default(int); // if the Id of the DataType is not set, we resolve it from the db by its PropertyEditorAlias if (propertyType.DataTypeId == 0 || propertyType.DataTypeId == default(int)) diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs index 8ece9621ce..ad16f4228a 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs @@ -24,6 +24,105 @@ namespace Umbraco.Tests.Services [UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest, PublishedRepositoryEvents = true)] public class ContentTypeServiceTests : TestWithSomeContentBase { + + //TODO: Then write the ones for content type changes + + [Test] + public void Change_Content_Type_From_Variant_Invariant() + { + //initialize the listener which is responsible for updating content based on variant/invariant changes + var listener = new UpdateContentOnVariantChangesComponent(); + listener.Initialize(ServiceContext.ContentService, ServiceContext.LocalizationService); + + //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 }); + contentType.ResetDirtyProperties(false); + 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 content type to be invariant + contentType.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.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_Invariant_Variant() + { + //initialize the listener which is responsible for updating content based on variant/invariant changes + var listener = new UpdateContentOnVariantChangesComponent(); + listener.Initialize(ServiceContext.ContentService, ServiceContext.LocalizationService); + + //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.Nothing + }); + contentType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Content", SortOrder = 1 }); + contentType.ResetDirtyProperties(false); + 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"); + 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() { From 7b41730b6061227253e949a77da0720d9b8b4924 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 4 Oct 2018 14:15:54 +0200 Subject: [PATCH 03/12] Changes how variant property data is updated with bulk SQL and gets tests working --- .../UpdateContentOnVariantChangesComponent.cs | 96 --------- src/Umbraco.Core/Models/ContentTypeBase.cs | 16 +- .../Persistence/Dtos/PropertyDataDto.cs | 2 +- .../Persistence/NPocoSqlExtensions.cs | 8 +- .../Implement/ContentTypeRepositoryBase.cs | 185 +++++++++++++++++- src/Umbraco.Core/Umbraco.Core.csproj | 1 - .../Services/ContentTypeServiceTests.cs | 23 ++- 7 files changed, 207 insertions(+), 124 deletions(-) delete mode 100644 src/Umbraco.Core/Components/UpdateContentOnVariantChangesComponent.cs diff --git a/src/Umbraco.Core/Components/UpdateContentOnVariantChangesComponent.cs b/src/Umbraco.Core/Components/UpdateContentOnVariantChangesComponent.cs deleted file mode 100644 index 71fe4d46bd..0000000000 --- a/src/Umbraco.Core/Components/UpdateContentOnVariantChangesComponent.cs +++ /dev/null @@ -1,96 +0,0 @@ -using System; -using System.Linq; -using Umbraco.Core.Models; -using Umbraco.Core.Models.Entities; -using Umbraco.Core.Services; -using Umbraco.Core.Services.Changes; -using Umbraco.Core.Services.Implement; - -namespace Umbraco.Core.Components -{ - /// - /// Manages data changes for when content/property types have variation changes - /// - [RuntimeLevel(MinLevel = RuntimeLevel.Run)] - //fixme: this one MUST fire before any of the content cache ones - public sealed class UpdateContentOnVariantChangesComponent : UmbracoComponentBase, IUmbracoCoreComponent - { - private IContentService _contentService; - private ILocalizationService _langService; - - public void Initialize(IContentService contentService, ILocalizationService langService) - { - _contentService = contentService; - _langService = langService; - ContentTypeService.ScopedRefreshedEntity += OnContentTypeRefreshedEntity; - } - - private void OnContentTypeRefreshedEntity(IContentTypeService sender, ContentTypeChange.EventArgs e) - { - var defaultLang = _langService.GetDefaultLanguageIsoCode(); //this will be cached - - foreach (var c in e.Changes) - { - // existing property alias change? - var hasAnyPropertyVariationChanged = c.Item.WasPropertyTypeVariationChanged(out var aliases); - if (hasAnyPropertyVariationChanged) - { - var contentOfType = _contentService.GetByType(c.Item.Id); - - foreach (var a in aliases) - { - var propType = c.Item.PropertyTypes.First(x => x.Alias == a); - - //fixme: this does not take into account segments, but how can we since we don't know what it changed from? - - switch (propType.Variations) - { - case ContentVariation.Culture: - //if the current variation is culture it means that the previous was nothing - foreach (var content in contentOfType) - { - object invariantVal; - try - { - content.Properties[a].PropertyType.Variations = ContentVariation.Nothing; - //now get the invariant val - invariantVal = content.GetValue(a); - } - finally - { - content.Properties[a].PropertyType.Variations = ContentVariation.Culture; - } - //set the invariant value - content.SetValue(a, invariantVal, defaultLang); - } - break; - case ContentVariation.Nothing: - //if the current variation is nothing it means that the previous was culture - foreach(var content in contentOfType) - { - object cultureVal; - try - { - content.Properties[a].PropertyType.Variations = ContentVariation.Culture; - //now get the culture val - cultureVal = content.GetValue(a, defaultLang); - } - finally - { - content.Properties[a].PropertyType.Variations = ContentVariation.Nothing; - } - //set the invariant value - content.SetValue(a, cultureVal); - } - break; - default: - throw new NotSupportedException("The variation change is not supported"); - } - } - - _contentService.Save(contentOfType); - } - } - } - } -} diff --git a/src/Umbraco.Core/Models/ContentTypeBase.cs b/src/Umbraco.Core/Models/ContentTypeBase.cs index 7467d83099..41d3419f98 100644 --- a/src/Umbraco.Core/Models/ContentTypeBase.cs +++ b/src/Umbraco.Core/Models/ContentTypeBase.cs @@ -409,7 +409,7 @@ namespace Umbraco.Core.Models /// [IgnoreDataMember] //fixme should we mark this as EditorBrowsable hidden since it really isn't ever used? - internal PropertyTypeCollection PropertyTypeCollection => _propertyTypes; + internal PropertyTypeCollection PropertyTypeCollection => _propertyTypes; /// /// Indicates whether a specific property on the current entity is dirty. @@ -418,12 +418,18 @@ namespace Umbraco.Core.Models /// True if Property is dirty, otherwise False public override bool IsPropertyDirty(string propertyName) { - bool existsInEntity = base.IsPropertyDirty(propertyName); + var existsInEntity = base.IsPropertyDirty(propertyName); + if (existsInEntity) return true; - bool anyDirtyGroups = PropertyGroups.Any(x => x.IsPropertyDirty(propertyName)); - bool anyDirtyTypes = PropertyTypes.Any(x => x.IsPropertyDirty(propertyName)); + //check properties types for this alias if it exists + if (PropertyTypeExists(propertyName)) + return PropertyTypes.Any(x => x.IsPropertyDirty(propertyName)); - return existsInEntity || anyDirtyGroups || anyDirtyTypes; + //check property groups for this alias if it exists + if (PropertyGroups.Contains(propertyName)) + return PropertyGroups.Any(x => x.IsPropertyDirty(propertyName)); + + return false; } /// diff --git a/src/Umbraco.Core/Persistence/Dtos/PropertyDataDto.cs b/src/Umbraco.Core/Persistence/Dtos/PropertyDataDto.cs index cb47784d92..a3b28b5b54 100644 --- a/src/Umbraco.Core/Persistence/Dtos/PropertyDataDto.cs +++ b/src/Umbraco.Core/Persistence/Dtos/PropertyDataDto.cs @@ -9,7 +9,7 @@ namespace Umbraco.Core.Persistence.Dtos [ExplicitColumns] internal class PropertyDataDto { - private const string TableName = Constants.DatabaseSchema.Tables.PropertyData; + public const string TableName = Constants.DatabaseSchema.Tables.PropertyData; public const int VarcharLength = 512; public const int SegmentLength = 256; diff --git a/src/Umbraco.Core/Persistence/NPocoSqlExtensions.cs b/src/Umbraco.Core/Persistence/NPocoSqlExtensions.cs index d97c748b6f..4fdc72f52f 100644 --- a/src/Umbraco.Core/Persistence/NPocoSqlExtensions.cs +++ b/src/Umbraco.Core/Persistence/NPocoSqlExtensions.cs @@ -1036,7 +1036,7 @@ namespace Umbraco.Core.Persistence { var pd = sql.SqlContext.PocoDataFactory.ForType(typeof (TDto)); var tableName = tableAlias ?? pd.TableInfo.TableName; - var queryColumns = pd.QueryColumns; + var queryColumns = pd.QueryColumns.ToList(); Dictionary aliases = null; @@ -1056,7 +1056,11 @@ namespace Umbraco.Core.Persistence return fieldName; }).ToArray(); - queryColumns = queryColumns.Where(x => names.Contains(x.Key)).ToArray(); + //only get the columns that exist in the selected names + queryColumns = queryColumns.Where(x => names.Contains(x.Key)).ToList(); + + //ensure the order of the columns in the expressions is the order in the result + queryColumns.Sort((a, b) => names.IndexOf(a.Key).CompareTo(names.IndexOf(b.Key))); } string GetAlias(PocoColumn column) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs index 743b7c858e..13317e7b20 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs @@ -402,18 +402,44 @@ AND umbracoNode.id <> @id", propertyType.PropertyGroupId = new Lazy(() => groupId); } - //check if the content type variation has been changed to Nothing since we will need to update - //all property types to Nothing as well if they aren't already - //fixme: this does not take into account segments, but how can we since we don't know what it changed from? - var ctVariationChangedToNothing = entity.IsPropertyDirty("Variations") && entity.Variations == ContentVariation.Nothing; + //check if the content type variation has been changed + var ctVariationChanging = entity.IsPropertyDirty("Variations"); + if (ctVariationChanging) + { + //we've already looked up the previous version of the content type so we know it's previous variation state + MoveVariantData(entity, (ContentVariation)dtoPk.Variations, entity.Variations); + } + + //track any property types that are changing variation + var propertyTypeVariationChanges = new Dictionary(); // insert or update properties // all of them, no-group and in-groups foreach (var propertyType in entity.PropertyTypes) { - //fixme: this does not take into account segments, but how can we since we don't know what it changed from? - if (ctVariationChangedToNothing && propertyType.Variations != ContentVariation.Nothing) - propertyType.Variations = ContentVariation.Nothing; + //if the content type variation isn't changing track if any property type is changing + if (!ctVariationChanging) + { + if (propertyType.IsPropertyDirty("Variations")) + propertyTypeVariationChanges[propertyType.Id] = propertyType.Variations; + } + else + { + switch(entity.Variations) + { + case ContentVariation.Nothing: + //if the content type is changing to Nothing, then all property type's must change to nothing + propertyType.Variations = ContentVariation.Nothing; + break; + case ContentVariation.Culture: + //we don't need to modify the property type in this case + break; + case ContentVariation.CultureAndSegment: + case ContentVariation.Segment: + default: + throw new NotSupportedException(); //TODO: Support this + } + } var groupId = propertyType.PropertyGroupId?.Value ?? default(int); // if the Id of the DataType is not set, we resolve it from the db by its PropertyEditorAlias @@ -438,6 +464,27 @@ AND umbracoNode.id <> @id", orphanPropertyTypeIds.Remove(typeId); } + //check if any property types were changing variation + if (propertyTypeVariationChanges.Count > 0) + { + var changes = new Dictionary(); + + //now get the current property type variations for the changed ones so that we know which variation they + //are going from and to + var from = Database.Dictionary(Sql() + .Select(x => x.Id, x => x.Variations) + .From() + .WhereIn(x => x.Id, propertyTypeVariationChanges.Keys)); + + foreach(var f in from) + { + changes[f.Key] = (propertyTypeVariationChanges[f.Key], (ContentVariation)f.Value); + } + + //perform the move + MoveVariantData(changes); + } + // deal with orphan properties: those that were in a deleted tab, // and have not been re-mapped to another tab or to 'generic properties' if (orphanPropertyTypeIds != null) @@ -445,6 +492,130 @@ AND umbracoNode.id <> @id", DeletePropertyType(entity.Id, id); } + /// + /// Moves variant data for property type changes + /// + /// + private void MoveVariantData(IDictionary propertyTypeChanges) + { + var defaultLangId = Database.First(Sql().Select(x => x.Id).From().Where(x => x.IsDefaultVariantLanguage)); + + //Group by the "To" variation so we can bulk update in the correct batches + foreach(var g in propertyTypeChanges.GroupBy(x => x.Value.Item2)) + { + var propertyTypeIds = g.Select(s => s.Key).ToList(); + + //the ContentVariation that the data is moving "To" + var toVariantType = g.Key; + + switch(toVariantType) + { + case ContentVariation.Culture: + + MovePropertyDataToVariantCulture(defaultLangId, propertyTypeIds: propertyTypeIds); + + break; + case ContentVariation.Nothing: + + MovePropertyDataToVariantNothing(defaultLangId, propertyTypeIds: propertyTypeIds); + + break; + case ContentVariation.CultureAndSegment: + case ContentVariation.Segment: + default: + throw new NotSupportedException(); //TODO: Support this + } + } + } + + /// + /// Moves variant data for a content type variation change + /// + /// + /// + /// + private void MoveVariantData(IContentTypeComposition contentType, ContentVariation from, ContentVariation to) + { + var defaultLangId = Database.First(Sql().Select(x => x.Id).From().Where(x => x.IsDefaultVariantLanguage)); + + var sqlPropertyTypeIds = Sql().Select(x => x.Id).From().Where(x => x.ContentTypeId == contentType.Id); + switch (to) + { + case ContentVariation.Culture: + + MovePropertyDataToVariantCulture(defaultLangId, sqlPropertyTypeIds: sqlPropertyTypeIds); + + break; + case ContentVariation.Nothing: + + MovePropertyDataToVariantNothing(defaultLangId, sqlPropertyTypeIds: sqlPropertyTypeIds); + + break; + case ContentVariation.CultureAndSegment: + case ContentVariation.Segment: + default: + throw new NotSupportedException(); //TODO: Support this + } + } + + private void MovePropertyDataToVariantNothing(int defaultLangId, IReadOnlyCollection propertyTypeIds = null, Sql sqlPropertyTypeIds = null) + { + //first clear out any existing property data that might already exists under the default lang + var sqlDelete = Sql() + .Delete() + .Where(x => x.LanguageId == null); + if (sqlPropertyTypeIds != null) + sqlDelete.WhereIn(x => x.PropertyTypeId, sqlPropertyTypeIds); + if (propertyTypeIds != null) + sqlDelete.WhereIn(x => x.PropertyTypeId, propertyTypeIds); + + Database.Execute(sqlDelete); + + //now insert all property data into the default language that exists under the invariant lang + var cols = Sql().Columns(x => x.VersionId, x => x.PropertyTypeId, x => x.Segment, x => x.IntegerValue, x => x.DecimalValue, x => x.DateValue, x => x.VarcharValue, x => x.TextValue, x => x.LanguageId); + var sqlSelectData = Sql().Select(x => x.VersionId, x => x.PropertyTypeId, x => x.Segment, x => x.IntegerValue, x => x.DecimalValue, x => x.DateValue, x => x.VarcharValue, x => x.TextValue) + .Append(", NULL") //null language ID + .From() + .Where(x => x.LanguageId == defaultLangId); + if (sqlPropertyTypeIds != null) + sqlSelectData.WhereIn(x => x.PropertyTypeId, sqlPropertyTypeIds); + if (propertyTypeIds != null) + sqlSelectData.WhereIn(x => x.PropertyTypeId, propertyTypeIds); + + var sqlInsert = Sql($"INSERT INTO {PropertyDataDto.TableName} ({cols})").Append(sqlSelectData); + + Database.Execute(sqlInsert); + } + + private void MovePropertyDataToVariantCulture(int defaultLangId, IReadOnlyCollection propertyTypeIds = null, Sql sqlPropertyTypeIds = null) + { + //first clear out any existing property data that might already exists under the default lang + var sqlDelete = Sql() + .Delete() + .Where(x => x.LanguageId == defaultLangId); + if (sqlPropertyTypeIds != null) + sqlDelete.WhereIn(x => x.PropertyTypeId, sqlPropertyTypeIds); + if (propertyTypeIds != null) + sqlDelete.WhereIn(x => x.PropertyTypeId, propertyTypeIds); + + Database.Execute(sqlDelete); + + //now insert all property data into the default language that exists under the invariant lang + var cols = Sql().Columns(x => x.VersionId, x => x.PropertyTypeId, x => x.Segment, x => x.IntegerValue, x => x.DecimalValue, x => x.DateValue, x => x.VarcharValue, x => x.TextValue, x => x.LanguageId); + var sqlSelectData = Sql().Select(x => x.VersionId, x => x.PropertyTypeId, x => x.Segment, x => x.IntegerValue, x => x.DecimalValue, x => x.DateValue, x => x.VarcharValue, x => x.TextValue) + .Append($", {defaultLangId}") //default language ID + .From() + .Where(x => x.LanguageId == null); + if (sqlPropertyTypeIds != null) + sqlSelectData.WhereIn(x => x.PropertyTypeId, sqlPropertyTypeIds); + if (propertyTypeIds != null) + sqlSelectData.WhereIn(x => x.PropertyTypeId, propertyTypeIds); + + var sqlInsert = Sql($"INSERT INTO {PropertyDataDto.TableName} ({cols})").Append(sqlSelectData); + + Database.Execute(sqlInsert); + } + private void DeletePropertyType(int contentTypeId, int propertyTypeId) { // first clear dependencies diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 379d5ad48a..496a66137a 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -170,7 +170,6 @@ - diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs index ad16f4228a..5b4dcceb00 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs @@ -30,10 +30,6 @@ namespace Umbraco.Tests.Services [Test] public void Change_Content_Type_From_Variant_Invariant() { - //initialize the listener which is responsible for updating content based on variant/invariant changes - var listener = new UpdateContentOnVariantChangesComponent(); - listener.Initialize(ServiceContext.ContentService, ServiceContext.LocalizationService); - //create content type with a property type that varies by culture var contentType = MockedContentTypes.CreateBasicContentType(); contentType.Variations = ContentVariation.Culture; @@ -72,16 +68,23 @@ namespace Umbraco.Tests.Services 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("hello world", doc.GetValue("title", "en-US")); + } [Test] public void Change_Property_Type_From_Invariant_Variant() { - //initialize the listener which is responsible for updating content based on variant/invariant changes - var listener = new UpdateContentOnVariantChangesComponent(); - listener.Initialize(ServiceContext.ContentService, ServiceContext.LocalizationService); - //create content type with a property type that varies by culture var contentType = MockedContentTypes.CreateBasicContentType(); contentType.Variations = ContentVariation.Culture; @@ -126,10 +129,6 @@ namespace Umbraco.Tests.Services [Test] public void Change_Property_Type_From_Variant_Invariant() { - //initialize the listener which is responsible for updating content based on variant/invariant changes - var listener = new UpdateContentOnVariantChangesComponent(); - listener.Initialize(ServiceContext.ContentService, ServiceContext.LocalizationService); - //create content type with a property type that varies by culture var contentType = MockedContentTypes.CreateBasicContentType(); contentType.Variations = ContentVariation.Culture; From c5ae149d4054268d58971a80debfaf45ba878652 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 4 Oct 2018 16:27:41 +0200 Subject: [PATCH 04/12] Gets remaining tests in place along with moving name values --- .../Implement/ContentTypeRepositoryBase.cs | 91 ++++++++++++++++++- .../Implement/LanguageRepository.cs | 5 + .../Services/ContentTypeServiceTests.cs | 72 +++++++++++++-- 3 files changed, 154 insertions(+), 14 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs index 13317e7b20..eed7b5a071 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs @@ -511,14 +511,10 @@ AND umbracoNode.id <> @id", switch(toVariantType) { case ContentVariation.Culture: - MovePropertyDataToVariantCulture(defaultLangId, propertyTypeIds: propertyTypeIds); - break; case ContentVariation.Nothing: - MovePropertyDataToVariantNothing(defaultLangId, propertyTypeIds: propertyTypeIds); - break; case ContentVariation.CultureAndSegment: case ContentVariation.Segment: @@ -542,14 +538,89 @@ AND umbracoNode.id <> @id", switch (to) { case ContentVariation.Culture: + //move the property data MovePropertyDataToVariantCulture(defaultLangId, sqlPropertyTypeIds: sqlPropertyTypeIds); + //now we need to move the names + //first clear out any existing names that might already exists under the default lang + //there's 2x tables to update + + //clear out the versionCultureVariation table + var sqlSelect = Sql().Select(x => x.Id) + .From() + .InnerJoin().On(x => x.Id, x => x.VersionId) + .InnerJoin().On(x => x.NodeId, x => x.NodeId) + .Where(x => x.ContentTypeId == contentType.Id) + .Where(x => x.LanguageId == defaultLangId); + var sqlDelete = Sql() + .Delete() + .WhereIn(x => x.Id, sqlSelect); + Database.Execute(sqlDelete); + + //clear out the documentCultureVariation table + sqlSelect = Sql().Select(x => x.Id) + .From() + .InnerJoin().On(x => x.NodeId, x => x.NodeId) + .Where(x => x.ContentTypeId == contentType.Id) + .Where(x => x.LanguageId == defaultLangId); + sqlDelete = Sql() + .Delete() + .WhereIn(x => x.Id, sqlSelect); + Database.Execute(sqlDelete); + + //now we need to insert names into these 2 tables based on the invariant data + + //insert rows into the versionCultureVariationDto table based on the data from contentVersionDto for the default lang + var cols = Sql().Columns(x => x.VersionId, x => x.Name, x => x.UpdateUserId, x => x.UpdateDate, x => x.LanguageId); + sqlSelect = Sql().Select(x => x.Id, x => x.Text, x => x.UserId, x => x.VersionDate) + .Append($", {defaultLangId}") //default language ID + .From() + .InnerJoin().On(x => x.NodeId, x => x.NodeId) + .Where(x => x.ContentTypeId == contentType.Id); + var sqlInsert = Sql($"INSERT INTO {ContentVersionCultureVariationDto.TableName} ({cols})").Append(sqlSelect); + Database.Execute(sqlInsert); + + //insert rows into the documentCultureVariation table + cols = Sql().Columns(x => x.NodeId, x => x.Edited, x => x.Published, x => x.Name, x => x.Available, x => x.LanguageId); + sqlSelect = Sql().Select(x => x.NodeId, x => x.Edited, x => x.Published) + .AndSelect(x => x.Text) + .Append($", 1, {defaultLangId}") //make Available + default language ID + .From() + .InnerJoin().On(x => x.NodeId, x => x.NodeId) + .InnerJoin().On(x => x.NodeId, x => x.NodeId) + .Where(x => x.ContentTypeId == contentType.Id); + sqlInsert = Sql($"INSERT INTO {DocumentCultureVariationDto.TableName} ({cols})").Append(sqlSelect); + Database.Execute(sqlInsert); + break; case ContentVariation.Nothing: + //move the property data MovePropertyDataToVariantNothing(defaultLangId, sqlPropertyTypeIds: sqlPropertyTypeIds); + //we dont need to move the names! this is because we always keep the invariant names with the name of the default language. + + //however, if we were to move names, we could do this: BUT this doesn't work with SQLCE, for that we'd have to update row by row :( + + ////now we need to move the names + + ////first update umbracoNode from documentCultureVariation + //var sqlUpdate = Sql($"UPDATE {NodeDto.TableName} SET {Sql().Columns(x => x.Text)} = {Sql().Columns(x => x.Name)}") + // .From() + // .InnerJoin().On(x => x.NodeId, x => x.NodeId) + // .InnerJoin().On(x => x.NodeId, x => x.NodeId) + // .Where(x => x.ContentTypeId == contentType.Id); + //Database.Execute(sqlUpdate); + + ////next update umbracoContentVersion from contentVersionCultureVariation + //sqlUpdate = Sql($"UPDATE {ContentVersionDto.TableName} SET {Sql().Columns(x => x.Text)} = {Sql().Columns(x => x.Name)}") + // .From() + // .InnerJoin().On(x => x.VersionId, x => x.Id) + // .InnerJoin().On(x => x.NodeId, x => x.NodeId) + // .Where(x => x.ContentTypeId == contentType.Id); + //Database.Execute(sqlUpdate); + break; case ContentVariation.CultureAndSegment: case ContentVariation.Segment: @@ -558,6 +629,12 @@ AND umbracoNode.id <> @id", } } + /// + /// This will move all property data from variant to invariant + /// + /// + /// Optional list of property type ids of the properties to be updated + /// Optional SQL statement used for the sub-query to select the properties type ids for the properties to be updated private void MovePropertyDataToVariantNothing(int defaultLangId, IReadOnlyCollection propertyTypeIds = null, Sql sqlPropertyTypeIds = null) { //first clear out any existing property data that might already exists under the default lang @@ -587,6 +664,12 @@ AND umbracoNode.id <> @id", Database.Execute(sqlInsert); } + /// + /// This will move all property data from invariant to variant + /// + /// + /// Optional list of property type ids of the properties to be updated + /// Optional SQL statement used for the sub-query to select the properties type ids for the properties to be updated private void MovePropertyDataToVariantCulture(int defaultLangId, IReadOnlyCollection propertyTypeIds = null, Sql sqlPropertyTypeIds = null) { //first clear out any existing property data that might already exists under the default lang diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/LanguageRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/LanguageRepository.cs index 2026daba74..8a0ba73f85 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/LanguageRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/LanguageRepository.cs @@ -219,6 +219,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement if (TypedCachePolicy != null) TypedCachePolicy.GetAllCached(PerformGetAll); + var id = GetIdByIsoCode(isoCode, throwOnNotFound: false); return id.HasValue ? Get(id.Value) : null; } @@ -234,6 +235,8 @@ namespace Umbraco.Core.Persistence.Repositories.Implement // ensure cache is populated, in a non-expensive way if (TypedCachePolicy != null) TypedCachePolicy.GetAllCached(PerformGetAll); + else + PerformGetAll(); //we don't have a typed cache (i.e. unit tests) but need to populate the _codeIdMap lock (_codeIdMap) { @@ -255,6 +258,8 @@ namespace Umbraco.Core.Persistence.Repositories.Implement // ensure cache is populated, in a non-expensive way if (TypedCachePolicy != null) TypedCachePolicy.GetAllCached(PerformGetAll); + else + PerformGetAll(); lock (_codeIdMap) // yes, we want to lock _codeIdMap { diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs index 5b4dcceb00..892166e566 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs @@ -25,7 +25,55 @@ namespace Umbraco.Tests.Services public class ContentTypeServiceTests : TestWithSomeContentBase { - //TODO: Then write the ones for content type changes + [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() @@ -45,25 +93,30 @@ namespace Umbraco.Tests.Services Variations = ContentVariation.Culture }); contentType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Content", SortOrder = 1 }); - contentType.ResetDirtyProperties(false); ServiceContext.ContentTypeService.Save(contentType); //create some content of this content type IContent doc = MockedContent.CreateBasicContent(contentType); - doc.SetCultureName("Home", "en-US"); + 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 - contentType.Variations = ContentVariation.Nothing; + //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 + //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 @@ -78,6 +131,7 @@ namespace Umbraco.Tests.Services 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")); } @@ -87,7 +141,7 @@ namespace Umbraco.Tests.Services { //create content type with a property type that varies by culture var contentType = MockedContentTypes.CreateBasicContentType(); - contentType.Variations = ContentVariation.Culture; + contentType.Variations = ContentVariation.Nothing; var contentCollection = new PropertyTypeCollection(true); contentCollection.Add(new PropertyType("test", ValueStorageType.Ntext) { @@ -100,12 +154,11 @@ namespace Umbraco.Tests.Services Variations = ContentVariation.Nothing }); contentType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Content", SortOrder = 1 }); - contentType.ResetDirtyProperties(false); ServiceContext.ContentTypeService.Save(contentType); //create some content of this content type IContent doc = MockedContent.CreateBasicContent(contentType); - doc.SetCultureName("Home", "en-US"); + doc.Name = "Home"; doc.SetValue("title", "hello world"); ServiceContext.ContentService.Save(doc); @@ -144,7 +197,6 @@ namespace Umbraco.Tests.Services Variations = ContentVariation.Culture }); contentType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Content", SortOrder = 1 }); - contentType.ResetDirtyProperties(false); ServiceContext.ContentTypeService.Save(contentType); //create some content of this content type From 304c874006f35634101b81e024ee08372237411a Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 4 Oct 2018 16:36:19 +0200 Subject: [PATCH 05/12] removes comments --- .../Implement/ContentTypeRepositoryBase.cs | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs index eed7b5a071..93f8b123f7 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs @@ -602,24 +602,7 @@ AND umbracoNode.id <> @id", //we dont need to move the names! this is because we always keep the invariant names with the name of the default language. //however, if we were to move names, we could do this: BUT this doesn't work with SQLCE, for that we'd have to update row by row :( - - ////now we need to move the names - - ////first update umbracoNode from documentCultureVariation - //var sqlUpdate = Sql($"UPDATE {NodeDto.TableName} SET {Sql().Columns(x => x.Text)} = {Sql().Columns(x => x.Name)}") - // .From() - // .InnerJoin().On(x => x.NodeId, x => x.NodeId) - // .InnerJoin().On(x => x.NodeId, x => x.NodeId) - // .Where(x => x.ContentTypeId == contentType.Id); - //Database.Execute(sqlUpdate); - - ////next update umbracoContentVersion from contentVersionCultureVariation - //sqlUpdate = Sql($"UPDATE {ContentVersionDto.TableName} SET {Sql().Columns(x => x.Text)} = {Sql().Columns(x => x.Name)}") - // .From() - // .InnerJoin().On(x => x.VersionId, x => x.Id) - // .InnerJoin().On(x => x.NodeId, x => x.NodeId) - // .Where(x => x.ContentTypeId == contentType.Id); - //Database.Execute(sqlUpdate); + // if we want these SQL statements back, look into GIT history break; case ContentVariation.CultureAndSegment: From 0ae1d06c2d6212a9dde15b482bc3bd8f5a8b18d6 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 4 Oct 2018 16:39:31 +0200 Subject: [PATCH 06/12] fix merge --- .../Repositories/Implement/ContentTypeRepositoryBase.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs index 93f8b123f7..19f3623680 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs @@ -498,7 +498,7 @@ AND umbracoNode.id <> @id", /// private void MoveVariantData(IDictionary propertyTypeChanges) { - var defaultLangId = Database.First(Sql().Select(x => x.Id).From().Where(x => x.IsDefaultVariantLanguage)); + var defaultLangId = Database.First(Sql().Select(x => x.Id).From().Where(x => x.IsDefault)); //Group by the "To" variation so we can bulk update in the correct batches foreach(var g in propertyTypeChanges.GroupBy(x => x.Value.Item2)) @@ -532,7 +532,7 @@ AND umbracoNode.id <> @id", /// private void MoveVariantData(IContentTypeComposition contentType, ContentVariation from, ContentVariation to) { - var defaultLangId = Database.First(Sql().Select(x => x.Id).From().Where(x => x.IsDefaultVariantLanguage)); + var defaultLangId = Database.First(Sql().Select(x => x.Id).From().Where(x => x.IsDefault)); var sqlPropertyTypeIds = Sql().Select(x => x.Id).From().Where(x => x.ContentTypeId == contentType.Id); switch (to) From 2e5d0f85e81cfd3c7b07a4d53b1828e196f97589 Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 5 Oct 2018 11:53:35 +0200 Subject: [PATCH 07/12] clears redirects on variation change --- .../Implement/ContentTypeRepositoryBase.cs | 21 +++++++++ .../Services/ContentTypeServiceTests.cs | 44 +++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs index 19f3623680..7104c9f713 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs @@ -274,6 +274,8 @@ AND umbracoNode.id <> @id", if (compositionBase != null && compositionBase.RemovedContentTypeKeyTracker != null && compositionBase.RemovedContentTypeKeyTracker.Any()) { + //TODO: Could we do the below with bulk SQL statements instead of looking everything up and then manipulating? + // find Content based on the current ContentType var sql = Sql() .SelectAll() @@ -292,6 +294,7 @@ AND umbracoNode.id <> @id", // based on the PropertyTypes that belong to the removed ContentType. foreach (var contentDto in contentDtos) { + //TODO: This could be done with bulk SQL statements foreach (var propertyType in propertyTypes) { var nodeId = contentDto.NodeId; @@ -408,6 +411,7 @@ AND umbracoNode.id <> @id", { //we've already looked up the previous version of the content type so we know it's previous variation state MoveVariantData(entity, (ContentVariation)dtoPk.Variations, entity.Variations); + Clear301Redirects(entity); } //track any property types that are changing variation @@ -492,6 +496,23 @@ AND umbracoNode.id <> @id", DeletePropertyType(entity.Id, id); } + /// + /// Clear any redirects associated with content for a content type + /// + private void Clear301Redirects(IContentTypeComposition contentType) + { + //first clear out any existing property data that might already exists under the default lang + var sqlSelect = Sql().Select(x => x.UniqueId) + .From() + .InnerJoin().On(x => x.NodeId, x => x.NodeId) + .Where(x => x.ContentTypeId == contentType.Id); + var sqlDelete = Sql() + .Delete() + .WhereIn((System.Linq.Expressions.Expression>)(x => x.ContentKey), sqlSelect); + + Database.Execute(sqlDelete); + } + /// /// Moves variant data for property type changes /// diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs index 892166e566..2d0471b9bd 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs @@ -24,6 +24,50 @@ namespace Umbraco.Tests.Services [UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest, PublishedRepositoryEvents = true)] public class ContentTypeServiceTests : TestWithSomeContentBase { + [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() From 06d41086f310a009d5cf6485b2212cf433b2a07e Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 5 Oct 2018 11:56:47 +0200 Subject: [PATCH 08/12] adds stub code for clearing scheduled publishing --- .../Repositories/Implement/ContentTypeRepositoryBase.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs index 7104c9f713..d1737d495a 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs @@ -412,6 +412,7 @@ AND umbracoNode.id <> @id", //we've already looked up the previous version of the content type so we know it's previous variation state MoveVariantData(entity, (ContentVariation)dtoPk.Variations, entity.Variations); Clear301Redirects(entity); + ClearScheduledPublishing(entity); } //track any property types that are changing variation @@ -513,6 +514,14 @@ AND umbracoNode.id <> @id", Database.Execute(sqlDelete); } + /// + /// Clear any scheduled publishing associated with content for a content type + /// + private void ClearScheduledPublishing(IContentTypeComposition contentType) + { + //TODO: Fill this in when scheduled publishing is enabled for variants + } + /// /// Moves variant data for property type changes /// From fdc94b9d1b423b54982e33fb576e17dd5b7c5bfe Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 5 Oct 2018 15:07:19 +0200 Subject: [PATCH 09/12] Writes tests to ensure that property values are updated in docs that are of a composed type of the one that variants are being changed in. --- .../Models/ContentTypeBaseExtensions.cs | 15 +++ .../Models/IContentTypeComposition.cs | 1 + .../IContentTypeRepositoryBase.cs | 6 + .../Implement/ContentTypeRepositoryBase.cs | 10 +- .../Services/ContentTypeServiceExtensions.cs | 18 +-- ...peServiceBaseOfTRepositoryTItemTService.cs | 3 + .../Services/ContentTypeServiceTests.cs | 113 ++++++++++++++++++ .../Editors/ContentTypeControllerBase.cs | 2 +- 8 files changed, 148 insertions(+), 20 deletions(-) diff --git a/src/Umbraco.Core/Models/ContentTypeBaseExtensions.cs b/src/Umbraco.Core/Models/ContentTypeBaseExtensions.cs index 8af48bb881..0d2f817660 100644 --- a/src/Umbraco.Core/Models/ContentTypeBaseExtensions.cs +++ b/src/Umbraco.Core/Models/ContentTypeBaseExtensions.cs @@ -63,5 +63,20 @@ namespace Umbraco.Core.Models aliases = a; return hasAnyPropertyVariationChanged; } + + /// + /// Returns the list of content types the composition is used in + /// + /// + /// + /// + internal static IEnumerable GetWhereCompositionIsUsedInContentTypes(this IContentTypeComposition source, + IContentTypeComposition[] allContentTypes) + { + var sourceId = source != null ? source.Id : 0; + + // find which content types are using this composition + return allContentTypes.Where(x => x.ContentTypeComposition.Any(y => y.Id == sourceId)).ToArray(); + } } } diff --git a/src/Umbraco.Core/Models/IContentTypeComposition.cs b/src/Umbraco.Core/Models/IContentTypeComposition.cs index b5277a23be..36ace19f0f 100644 --- a/src/Umbraco.Core/Models/IContentTypeComposition.cs +++ b/src/Umbraco.Core/Models/IContentTypeComposition.cs @@ -10,6 +10,7 @@ namespace Umbraco.Core.Models /// /// Gets or sets the content types that compose this content type. /// + //fixme: we should be storing key references, not the object else we are caching way too much IEnumerable ContentTypeComposition { get; set; } /// diff --git a/src/Umbraco.Core/Persistence/Repositories/IContentTypeRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/IContentTypeRepositoryBase.cs index f5e9013082..3bb1ac38ca 100644 --- a/src/Umbraco.Core/Persistence/Repositories/IContentTypeRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/IContentTypeRepositoryBase.cs @@ -10,6 +10,12 @@ namespace Umbraco.Core.Persistence.Repositories { TItem Get(string alias); IEnumerable> Move(TItem moving, EntityContainer container); + + /// + /// Returns the content types that are direct compositions of the content type + /// + /// The content type id + /// IEnumerable GetTypesDirectlyComposedOf(int id); /// diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs index d1737d495a..3f1ea3116e 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs @@ -415,7 +415,7 @@ AND umbracoNode.id <> @id", ClearScheduledPublishing(entity); } - //track any property types that are changing variation + //track any content type/property types that are changing variation which will require content updates var propertyTypeVariationChanges = new Dictionary(); // insert or update properties @@ -426,7 +426,9 @@ AND umbracoNode.id <> @id", if (!ctVariationChanging) { if (propertyType.IsPropertyDirty("Variations")) + { propertyTypeVariationChanges[propertyType.Id] = propertyType.Variations; + } } else { @@ -481,7 +483,7 @@ AND umbracoNode.id <> @id", .From() .WhereIn(x => x.Id, propertyTypeVariationChanges.Keys)); - foreach(var f in from) + foreach (var f in from) { changes[f.Key] = (propertyTypeVariationChanges[f.Key], (ContentVariation)f.Value); } @@ -490,6 +492,7 @@ AND umbracoNode.id <> @id", MoveVariantData(changes); } + // deal with orphan properties: those that were in a deleted tab, // and have not been re-mapped to another tab or to 'generic properties' if (orphanPropertyTypeIds != null) @@ -845,8 +848,11 @@ AND umbracoNode.id <> @id", } } + /// public IEnumerable GetTypesDirectlyComposedOf(int id) { + //fixme - this will probably be more efficient to simply load all content types and do the calculation, see GetWhereCompositionIsUsedInContentTypes + var sql = Sql() .SelectAll() .From() diff --git a/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs b/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs index 9e82213aa5..66b3982b49 100644 --- a/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs +++ b/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs @@ -109,23 +109,7 @@ namespace Umbraco.Core.Services return new ContentTypeAvailableCompositionsResults(ancestors, result); } - /// - /// Returns the list of content types the composition is used in - /// - /// - /// - /// - /// - internal static IEnumerable GetWhereCompositionIsUsedInContentTypes(this IContentTypeService ctService, - IContentTypeComposition source, - IContentTypeComposition[] allContentTypes) - { - - var sourceId = source != null ? source.Id : 0; - - // find which content types are using this composition - return allContentTypes.Where(x => x.ContentTypeComposition.Any(y => y.Id == sourceId)).ToArray(); - } + private static IContentTypeComposition[] GetAncestors(IContentTypeComposition ctype, IContentTypeComposition[] allContentTypes) { diff --git a/src/Umbraco.Core/Services/Implement/ContentTypeServiceBaseOfTRepositoryTItemTService.cs b/src/Umbraco.Core/Services/Implement/ContentTypeServiceBaseOfTRepositoryTItemTService.cs index a3bc87315b..a114f415cc 100644 --- a/src/Umbraco.Core/Services/Implement/ContentTypeServiceBaseOfTRepositoryTItemTService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentTypeServiceBaseOfTRepositoryTItemTService.cs @@ -346,6 +346,9 @@ namespace Umbraco.Core.Services.Implement public IEnumerable GetComposedOf(int id) { + //fixme: this is essentially the same as ContentTypeServiceExtensions.GetWhereCompositionIsUsedInContentTypes which loads + // all content types to figure this out, this instead makes quite a few queries so should be replaced + using (var scope = ScopeProvider.CreateScope(autoComplete: true)) { scope.ReadLock(ReadLockIds); diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs index 2d0471b9bd..f25382d557 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs @@ -266,6 +266,119 @@ namespace Umbraco.Tests.Services 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_Media_Type_With_Hierarchy_Of_Media_Items_Moves_Orphaned_Media_To_Recycle_Bin() { diff --git a/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs b/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs index fa1aaf7345..22d86631ca 100644 --- a/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs +++ b/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs @@ -161,7 +161,7 @@ namespace Umbraco.Web.Editors throw new ArgumentOutOfRangeException("The entity type was not a content type"); } - var contentTypesWhereCompositionIsUsed = Services.ContentTypeService.GetWhereCompositionIsUsedInContentTypes(source, allContentTypes); + var contentTypesWhereCompositionIsUsed = source.GetWhereCompositionIsUsedInContentTypes(allContentTypes); return contentTypesWhereCompositionIsUsed .Select(x => Mapper.Map(x)) .Select(x => From b2d1a48c7ad157953ef91250f34a6f78f86aa02f Mon Sep 17 00:00:00 2001 From: Robert Date: Tue, 9 Oct 2018 08:19:36 +0200 Subject: [PATCH 10/12] Added null check for EmailValidator value, fixed js error. --- src/Umbraco.Core/PropertyEditors/Validators/EmailValidator.cs | 2 +- .../src/common/directives/components/content/edit.controller.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Core/PropertyEditors/Validators/EmailValidator.cs b/src/Umbraco.Core/PropertyEditors/Validators/EmailValidator.cs index 762b6dd7dd..4df11e4f60 100644 --- a/src/Umbraco.Core/PropertyEditors/Validators/EmailValidator.cs +++ b/src/Umbraco.Core/PropertyEditors/Validators/EmailValidator.cs @@ -14,7 +14,7 @@ namespace Umbraco.Core.PropertyEditors.Validators /// public IEnumerable Validate(object value, string valueType, object dataTypeConfiguration) { - var asString = value.ToString(); + var asString = value == null ? "" : value.ToString(); var emailVal = new EmailAddressAttribute(); diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/content/edit.controller.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/content/edit.controller.js index 69c871f5ce..084b72ed97 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/content/edit.controller.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/content/edit.controller.js @@ -107,7 +107,7 @@ evts.push(eventsService.on("editors.documentType.saved", function (name, args) { // if this content item uses the updated doc type we need to reload the content item - if (args && args.documentType && args.documentType.key === content.documentType.key) { + if (args && args.documentType && args.documentType.key === $scope.content.documentType.key) { loadContent(); } })); From 931fcd565a92568259b2038b616c99a2b481d5e7 Mon Sep 17 00:00:00 2001 From: Robert Date: Wed, 10 Oct 2018 08:26:10 +0200 Subject: [PATCH 11/12] Removed the override of IsPropertyDirty as is not needed. --- src/Umbraco.Core/Models/ContentTypeBase.cs | 49 ++++++++-------------- 1 file changed, 17 insertions(+), 32 deletions(-) diff --git a/src/Umbraco.Core/Models/ContentTypeBase.cs b/src/Umbraco.Core/Models/ContentTypeBase.cs index 41d3419f98..9e73205c36 100644 --- a/src/Umbraco.Core/Models/ContentTypeBase.cs +++ b/src/Umbraco.Core/Models/ContentTypeBase.cs @@ -363,22 +363,28 @@ namespace Umbraco.Core.Models /// Alias of the to remove public void RemovePropertyType(string propertyTypeAlias) { - //check if the property exist in one of our collections - if (PropertyGroups.Any(group => group.PropertyTypes.Any(pt => pt.Alias == propertyTypeAlias)) - || _propertyTypes.Any(x => x.Alias == propertyTypeAlias)) - { - //set the flag that a property has been removed - HasPropertyTypeBeenRemoved = true; - } - + //check through each property group to see if we can remove the property type by alias from it foreach (var propertyGroup in PropertyGroups) { - propertyGroup.PropertyTypes.RemoveItem(propertyTypeAlias); + if (propertyGroup.PropertyTypes.RemoveItem(propertyTypeAlias)) + { + if (!HasPropertyTypeBeenRemoved) + { + HasPropertyTypeBeenRemoved = true; + OnPropertyChanged(Ps.Value.PropertyTypeCollectionSelector); + } + break; + } } - if (_propertyTypes.Any(x => x.Alias == propertyTypeAlias)) + //check through each local property type collection (not assigned to a tab) + if (_propertyTypes.RemoveItem(propertyTypeAlias)) { - _propertyTypes.RemoveItem(propertyTypeAlias); + if (!HasPropertyTypeBeenRemoved) + { + HasPropertyTypeBeenRemoved = true; + OnPropertyChanged(Ps.Value.PropertyTypeCollectionSelector); + } } } @@ -411,27 +417,6 @@ namespace Umbraco.Core.Models //fixme should we mark this as EditorBrowsable hidden since it really isn't ever used? internal PropertyTypeCollection PropertyTypeCollection => _propertyTypes; - /// - /// Indicates whether a specific property on the current entity is dirty. - /// - /// Name of the property to check - /// True if Property is dirty, otherwise False - public override bool IsPropertyDirty(string propertyName) - { - var existsInEntity = base.IsPropertyDirty(propertyName); - if (existsInEntity) return true; - - //check properties types for this alias if it exists - if (PropertyTypeExists(propertyName)) - return PropertyTypes.Any(x => x.IsPropertyDirty(propertyName)); - - //check property groups for this alias if it exists - if (PropertyGroups.Contains(propertyName)) - return PropertyGroups.Any(x => x.IsPropertyDirty(propertyName)); - - return false; - } - /// /// Indicates whether the current entity is dirty. /// From d41251819f0c4a52cd3206b8587e9b3df95ccdb6 Mon Sep 17 00:00:00 2001 From: Robert Date: Wed, 10 Oct 2018 08:27:20 +0200 Subject: [PATCH 12/12] RemoveItem now returns a bool --- src/Umbraco.Core/Models/PropertyTypeCollection.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Core/Models/PropertyTypeCollection.cs b/src/Umbraco.Core/Models/PropertyTypeCollection.cs index 53b75f7e48..47710e04cb 100644 --- a/src/Umbraco.Core/Models/PropertyTypeCollection.cs +++ b/src/Umbraco.Core/Models/PropertyTypeCollection.cs @@ -124,10 +124,11 @@ namespace Umbraco.Core.Models return this.Any(x => x.Alias == propertyAlias); } - public void RemoveItem(string propertyTypeAlias) + public bool RemoveItem(string propertyTypeAlias) { var key = IndexOfKey(propertyTypeAlias); if (key != -1) RemoveItem(key); + return key != -1; } public int IndexOfKey(string key)