diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentTypeBaseRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentTypeBaseRepository.cs index 81a7321cbe..541f9c5c88 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentTypeBaseRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentTypeBaseRepository.cs @@ -297,9 +297,11 @@ AND umbracoNode.id <> @id", //Delete Tabs/Groups by excepting entries from db with entries from collections var dbPropertyGroups = Database.Fetch("WHERE contenttypeNodeId = @Id", new {Id = entity.Id}) - .Select(x => new Tuple(x.Id, x.Text)); + .Select(x => new Tuple(x.Id, x.Text)) + .ToList(); var entityPropertyGroups = entity.PropertyGroups.Select(x => new Tuple(x.Id, x.Name)).ToList(); - var tabs = dbPropertyGroups.Except(entityPropertyGroups); + var tabsToDelete = dbPropertyGroups.Select(x => x.Item1).Except(entityPropertyGroups.Select(x => x.Item1)); + var tabs = dbPropertyGroups.Where(x => tabsToDelete.Any(y => y == x.Item1)); //Update Tab name downstream to ensure renaming is done properly foreach (var propertyGroup in entityPropertyGroups) { diff --git a/src/Umbraco.Core/Services/ContentTypeService.cs b/src/Umbraco.Core/Services/ContentTypeService.cs index 176b7fc3fe..7252c2d3ba 100644 --- a/src/Umbraco.Core/Services/ContentTypeService.cs +++ b/src/Umbraco.Core/Services/ContentTypeService.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Data; +using System.Diagnostics; using System.Linq; using System.Text; using System.Xml.Linq; @@ -302,52 +303,30 @@ namespace Umbraco.Core.Services else throw new Exception("Composition is neither IContentType nor IMediaType?"); - // recursively find all descendants + var compositions = compositionContentType.ContentTypeComposition; + var propertyTypeAliases = compositionContentType.PropertyTypes.Select(x => x.Alias.ToLowerInvariant()).ToArray(); + var indirectReferences = allContentTypes.Where(x => x.ContentTypeComposition.Any(y => y.Id == compositionContentType.Id)); var comparer = new DelegateEqualityComparer((x, y) => x.Id == y.Id, x => x.Id); - var descendants = new HashSet(comparer); - var stack = new Stack(); - foreach (var composition in allContentTypes.Where(x => x.ContentTypeComposition.Any(y => y.Id == compositionContentType.Id))) - stack.Push(composition); - while (stack.Count > 0) + var dependencies = new HashSet(compositions, comparer); + foreach (var indirectReference in indirectReferences) { - var item = stack.Pop(); - descendants.Add(item); - foreach (var composition in allContentTypes.Where(x => x.ContentTypeComposition.Any(y => y.Id == item.Id))) - stack.Push(composition); + dependencies.Add(indirectReference); + var directReferences = indirectReference.ContentTypeComposition; + foreach (var directReference in directReferences) + { + if(directReference.Id == compositionContentType.Id || directReference.Alias.Equals(compositionContentType.Alias)) continue; + dependencies.Add(directReference); + } } - // ensure that no descendant has a property with an alias that is used by content type - var aliases = compositionContentType.PropertyTypes.Select(x => x.Alias.ToLowerInvariant()).ToArray(); - foreach (var descendant in descendants) + foreach (var dependency in dependencies) { - var intersect = descendant.CompositionPropertyTypes.Select(x => x.Alias.ToLowerInvariant()).Intersect(aliases).ToArray(); + var contentTypeDependency = allContentTypes.FirstOrDefault(x => x.Alias.Equals(dependency.Alias)); + if (contentTypeDependency == null) continue; + var intersect = contentTypeDependency.PropertyTypes.Select(x => x.Alias.ToLowerInvariant()).Intersect(propertyTypeAliases).ToArray(); if (intersect.Length == 0) continue; - var message = string.Format("The following property aliases conflict with descendants : {0}.", - string.Join(", ", intersect)); - throw new Exception(message); - } - - // find all ancestors - var ancestors = new HashSet(comparer); - stack.Clear(); - foreach (var composition in compositionContentType.ContentTypeComposition) - stack.Push(composition); - while (stack.Count > 0) - { - var item = stack.Pop(); - ancestors.Add(item); - foreach (var composition in item.ContentTypeComposition) - stack.Push(composition); - } - - // ensure that no ancestor has a property with an alias that is used by content type - foreach (var ancestor in ancestors) - { - var intersect = ancestor.PropertyTypes.Select(x => x.Alias.ToLowerInvariant()).Intersect(aliases).ToArray(); - if (intersect.Length == 0) continue; - - var message = string.Format("The following property aliases conflict with ancestors : {0}.", + var message = string.Format("The following PropertyType aliases from the current ContentType conflict with existing PropertyType aliases: {0}.", string.Join(", ", intersect)); throw new Exception(message); } diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs index 8185170638..b6a9792cfa 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs @@ -5,6 +5,7 @@ using System.Linq; using Umbraco.Core; using Umbraco.Core.Models; using Umbraco.Core.Models.Rdbms; +using Umbraco.Tests.CodeFirst.TestModels.Composition; using Umbraco.Tests.TestHelpers; using Umbraco.Tests.TestHelpers.Entities; @@ -466,7 +467,7 @@ namespace Umbraco.Tests.Services } [Test] - public void Cannot_Add_Duplicate_PropertyType_Alias_To__Referenced_Composition() + public void Cannot_Add_Duplicate_PropertyType_Alias_To_Referenced_Composition() { //Related the second issue in screencast from this post http://issues.umbraco.org/issue/U4-5986 @@ -497,6 +498,98 @@ namespace Umbraco.Tests.Services Assert.DoesNotThrow(() => service.GetContentType("simpleChildPage")); } + [Test] + public void Cannot_Add_Duplicate_PropertyType_Alias_In_Composition_Graph() + { + // Arrange + var service = ServiceContext.ContentTypeService; + + var basePage = MockedContentTypes.CreateSimpleContentType("basePage", "Base Page", null, true); + service.Save(basePage); + var contentPage = MockedContentTypes.CreateSimpleContentType("contentPage", "Content Page", basePage); + service.Save(contentPage); + var advancedPage = MockedContentTypes.CreateSimpleContentType("advancedPage", "Advanced Page", contentPage, true); + service.Save(advancedPage); + + var metaComposition = MockedContentTypes.CreateMetaContentType(); + service.Save(metaComposition); + var seoComposition = MockedContentTypes.CreateSeoContentType(); + service.Save(seoComposition); + + var metaAdded = contentPage.AddContentType(metaComposition); + service.Save(contentPage); + var seoAdded = advancedPage.AddContentType(seoComposition); + service.Save(advancedPage); + + // Act + var duplicatePropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "title", Name = "Title", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var addedToBasePage = basePage.AddPropertyType(duplicatePropertyType, "Content"); + var addedToAdvancedPage = advancedPage.AddPropertyType(duplicatePropertyType, "Content"); + var addedToMeta = metaComposition.AddPropertyType(duplicatePropertyType, "Meta"); + var addedToSeo = seoComposition.AddPropertyType(duplicatePropertyType, "Seo"); + + // Assert + Assert.That(metaAdded, Is.True); + Assert.That(seoAdded, Is.True); + + Assert.That(addedToBasePage, Is.True); + Assert.That(addedToAdvancedPage, Is.False); + Assert.That(addedToMeta, Is.True); + Assert.That(addedToSeo, Is.True); + + Assert.Throws(() => service.Save(basePage)); + Assert.Throws(() => service.Save(metaComposition)); + Assert.Throws(() => service.Save(seoComposition)); + + Assert.DoesNotThrow(() => service.GetContentType("contentPage")); + Assert.DoesNotThrow(() => service.GetContentType("advancedPage")); + Assert.DoesNotThrow(() => service.GetContentType("meta")); + Assert.DoesNotThrow(() => service.GetContentType("seo")); + } + + [Test] + public void Can_Add_PropertyType_Alias_Which_Exists_In_Composition_Outside_Graph() + { + // Arrange + var service = ServiceContext.ContentTypeService; + + var basePage = MockedContentTypes.CreateSimpleContentType("basePage", "Base Page", null, true); + service.Save(basePage); + var contentPage = MockedContentTypes.CreateSimpleContentType("contentPage", "Content Page", basePage, true); + service.Save(contentPage); + var advancedPage = MockedContentTypes.CreateSimpleContentType("advancedPage", "Advanced Page", contentPage, true); + service.Save(advancedPage); + + var metaComposition = MockedContentTypes.CreateMetaContentType(); + service.Save(metaComposition); + + var contentMetaComposition = MockedContentTypes.CreateContentMetaContentType(); + service.Save(contentMetaComposition); + + var metaAdded = contentPage.AddContentType(metaComposition); + service.Save(contentPage); + + var metaAddedToComposition = contentMetaComposition.AddContentType(metaComposition); + service.Save(contentMetaComposition); + + // Act + var propertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "title", Name = "Title", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var addedToContentPage = contentPage.AddPropertyType(propertyType, "Content"); + + // Assert + Assert.That(metaAdded, Is.True); + Assert.That(metaAddedToComposition, Is.True); + + Assert.That(addedToContentPage, Is.True); + Assert.DoesNotThrow(() => service.Save(contentPage)); + } + [Test] public void Can_Rename_PropertyGroup_With_Inherited_PropertyGroups() { diff --git a/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs b/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs index c2ecc1e3e2..9f13017580 100644 --- a/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs +++ b/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs @@ -65,6 +65,57 @@ namespace Umbraco.Tests.TestHelpers.Entities return contentType; } + public static ContentType CreateContentMetaContentType() + { + var contentType = new ContentType(-1) + { + Alias = "contentMeta", + Name = "Content Meta", + Description = "ContentType used for Content Meta", + Icon = ".sprTreeDoc3", + Thumbnail = "doc.png", + SortOrder = 1, + CreatorId = 0, + Trashed = false + }; + + var metaCollection = new PropertyTypeCollection(); + metaCollection.Add(new PropertyType("test", DataTypeDatabaseType.Ntext) { Alias = "title", Name = "Title", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 }); + + contentType.PropertyGroups.Add(new PropertyGroup(metaCollection) { Name = "Content", SortOrder = 2 }); + + //ensure that nothing is marked as dirty + contentType.ResetDirtyProperties(false); + + return contentType; + } + + public static ContentType CreateSeoContentType() + { + var contentType = new ContentType(-1) + { + Alias = "seo", + Name = "Seo", + Description = "ContentType used for Seo", + Icon = ".sprTreeDoc3", + Thumbnail = "doc.png", + SortOrder = 1, + CreatorId = 0, + Trashed = false + }; + + var metaCollection = new PropertyTypeCollection(); + metaCollection.Add(new PropertyType("seotest", DataTypeDatabaseType.Ntext) { Alias = "seokeywords", Name = "Seo Keywords", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 }); + metaCollection.Add(new PropertyType("seotest", DataTypeDatabaseType.Ntext) { Alias = "seodescription", Name = "Seo Description", Description = "", HelpText = "", Mandatory = false, SortOrder = 2, DataTypeDefinitionId = -89 }); + + contentType.PropertyGroups.Add(new PropertyGroup(metaCollection) { Name = "Seo", SortOrder = 5 }); + + //ensure that nothing is marked as dirty + contentType.ResetDirtyProperties(false); + + return contentType; + } + public static ContentType CreateSimpleContentType() { var contentType = new ContentType(-1)