From 3fe235fa35a40fd32c57b026bc39b233bd6db46c Mon Sep 17 00:00:00 2001 From: Stephan Date: Tue, 17 Nov 2015 19:01:19 +0100 Subject: [PATCH] Sanitize content types .ContentTypeComposition and .PropertyTypes --- src/Umbraco.Core/Models/ContentTypeBase.cs | 32 +++++++------------ .../Models/ContentTypeCompositionBase.cs | 23 +++++++------ src/Umbraco.Core/Models/IContentTypeBase.cs | 7 +++- .../Models/IContentTypeComposition.cs | 8 ++--- .../Factories/MemberTypeReadOnlyFactory.cs | 2 +- .../Repositories/ContentTypeBaseRepository.cs | 2 +- src/Umbraco.Tests/Models/ContentTypeTests.cs | 2 +- .../ContentTypeModelMapperExtensions.cs | 11 ++----- 8 files changed, 41 insertions(+), 46 deletions(-) diff --git a/src/Umbraco.Core/Models/ContentTypeBase.cs b/src/Umbraco.Core/Models/ContentTypeBase.cs index 4346b84982..46127f8131 100644 --- a/src/Umbraco.Core/Models/ContentTypeBase.cs +++ b/src/Umbraco.Core/Models/ContentTypeBase.cs @@ -378,27 +378,25 @@ namespace Umbraco.Core.Models } /// - /// List of PropertyTypes available on this ContentType. - /// This list aggregates PropertyTypes across the PropertyGroups. + /// Gets all property types, across all property groups. /// - /// - /// - /// The setter is used purely to set the property types that DO NOT belong to a group! - /// - /// Marked as DoNotClone because the result of this property is not the natural result of the data, it is - /// a union of data so when auto-cloning if the setter is used it will be setting the unnatural result of the - /// data. We manually clone this instead. - /// [IgnoreDataMember] [DoNotClone] public virtual IEnumerable PropertyTypes { get { - var types = _propertyTypes.Union(PropertyGroups.SelectMany(x => x.PropertyTypes)); - return types; + return _propertyTypes.Union(PropertyGroups.SelectMany(x => x.PropertyTypes)); } - internal set + } + + /// + /// Gets or sets the property types that are not in a group. + /// + public IEnumerable NoGroupPropertyTypes + { + get { return _propertyTypes; } + set { _propertyTypes = new PropertyTypeCollection(value); _propertyTypes.CollectionChanged += PropertyTypesChanged; @@ -406,14 +404,6 @@ namespace Umbraco.Core.Models } } - /// - /// Returns the property type collection containing types that are non-groups - used for tests - /// - internal IEnumerable NonGroupedPropertyTypes - { - get { return _propertyTypes; } - } - /// /// A boolean flag indicating if a property type has been removed from this instance. /// diff --git a/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs b/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs index a1b9cb9f1a..5ac21885d7 100644 --- a/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs +++ b/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs @@ -37,16 +37,21 @@ namespace Umbraco.Core.Models x => x.ContentTypeComposition); /// - /// List of ContentTypes that make up a composition of PropertyGroups and PropertyTypes for the current ContentType + /// Gets or sets the content types that compose this content type. /// [DataMember] public IEnumerable ContentTypeComposition { get { return _contentTypeComposition; } + set + { + _contentTypeComposition = value.ToList(); + OnPropertyChanged(ContentTypeCompositionSelector); + } } /// - /// Returns a list of objects from the composition + /// Gets the property groups for the entire composition. /// [IgnoreDataMember] public IEnumerable CompositionPropertyGroups @@ -59,7 +64,7 @@ namespace Umbraco.Core.Models } /// - /// Returns a list of objects from the composition + /// Gets the property types for the entire composition. /// [IgnoreDataMember] public IEnumerable CompositionPropertyTypes @@ -72,10 +77,10 @@ namespace Umbraco.Core.Models } /// - /// Adds a new ContentType to the list of composite ContentTypes + /// Adds a content type to the composition. /// - /// to add - /// True if ContentType was added, otherwise returns False + /// The content type to add. + /// True if the content type was added, otherwise false. public bool AddContentType(IContentTypeComposition contentType) { if (contentType.ContentTypeComposition.Any(x => x.CompositionAliases().Any(ContentTypeCompositionExists))) @@ -104,10 +109,10 @@ namespace Umbraco.Core.Models } /// - /// Removes a ContentType with the supplied alias from the the list of composite ContentTypes + /// Removes a content type with a specified alias from the composition. /// - /// Alias of a - /// True if ContentType was removed, otherwise returns False + /// The alias of the content type to remove. + /// True if the content type was removed, otherwise false. public bool RemoveContentType(string alias) { if (ContentTypeCompositionExists(alias)) diff --git a/src/Umbraco.Core/Models/IContentTypeBase.cs b/src/Umbraco.Core/Models/IContentTypeBase.cs index 89fca20b8e..80e62f50cf 100644 --- a/src/Umbraco.Core/Models/IContentTypeBase.cs +++ b/src/Umbraco.Core/Models/IContentTypeBase.cs @@ -54,10 +54,15 @@ namespace Umbraco.Core.Models PropertyGroupCollection PropertyGroups { get; set; } /// - /// Gets an enumerable list of Property Types aggregated for all groups + /// Gets all property types, across all property groups. /// IEnumerable PropertyTypes { get; } + /// + /// Gets or sets the property types that are not in a group. + /// + IEnumerable NoGroupPropertyTypes { get; set; } + /// /// Removes a PropertyType from the current ContentType /// diff --git a/src/Umbraco.Core/Models/IContentTypeComposition.cs b/src/Umbraco.Core/Models/IContentTypeComposition.cs index 8b5049f236..ab7e068fdd 100644 --- a/src/Umbraco.Core/Models/IContentTypeComposition.cs +++ b/src/Umbraco.Core/Models/IContentTypeComposition.cs @@ -8,17 +8,17 @@ namespace Umbraco.Core.Models public interface IContentTypeComposition : IContentTypeBase { /// - /// Gets a list of ContentTypes that make up a composition of PropertyGroups and PropertyTypes for the current ContentType + /// Gets or sets the content types that compose this content type. /// - IEnumerable ContentTypeComposition { get; } + IEnumerable ContentTypeComposition { get; set; } /// - /// Gets a list of objects from the composition + /// Gets the property groups for the entire composition. /// IEnumerable CompositionPropertyGroups { get; } /// - /// Gets a list of objects from the composition + /// Gets the property types for the entire composition. /// IEnumerable CompositionPropertyTypes { get; } diff --git a/src/Umbraco.Core/Persistence/Factories/MemberTypeReadOnlyFactory.cs b/src/Umbraco.Core/Persistence/Factories/MemberTypeReadOnlyFactory.cs index 604e52eb72..93ad99a5c1 100644 --- a/src/Umbraco.Core/Persistence/Factories/MemberTypeReadOnlyFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/MemberTypeReadOnlyFactory.cs @@ -51,7 +51,7 @@ namespace Umbraco.Core.Persistence.Factories memberType.MemberTypePropertyTypes.Add(standardPropertyType.Key, new MemberTypePropertyProfileAccess(false, false)); } - memberType.PropertyTypes = propertyTypes; + memberType.NoGroupPropertyTypes = propertyTypes; return memberType; } diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentTypeBaseRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentTypeBaseRepository.cs index beb88d4ccf..986a24b9db 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentTypeBaseRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentTypeBaseRepository.cs @@ -671,7 +671,7 @@ AND umbracoNode.id <> @id", foreach (var contentType in contentTypes) { contentType.PropertyGroups = allPropGroups[contentType.Id]; - ((ContentTypeBase)contentType).PropertyTypes = allPropTypes[contentType.Id]; + contentType.NoGroupPropertyTypes = allPropTypes[contentType.Id]; } //NOTE: SQL call #3++ diff --git a/src/Umbraco.Tests/Models/ContentTypeTests.cs b/src/Umbraco.Tests/Models/ContentTypeTests.cs index 325c8aefc1..0589d7d173 100644 --- a/src/Umbraco.Tests/Models/ContentTypeTests.cs +++ b/src/Umbraco.Tests/Models/ContentTypeTests.cs @@ -205,7 +205,7 @@ namespace Umbraco.Tests.Models } Assert.AreNotSame(clone.PropertyTypes, contentType.PropertyTypes); Assert.AreEqual(clone.PropertyTypes.Count(), contentType.PropertyTypes.Count()); - Assert.AreEqual(0, ((ContentTypeBase)clone).NonGroupedPropertyTypes.Count()); + Assert.AreEqual(0, clone.NoGroupPropertyTypes.Count()); for (var index = 0; index < contentType.PropertyTypes.Count(); index++) { Assert.AreNotSame(clone.PropertyTypes.ElementAt(index), contentType.PropertyTypes.ElementAt(index)); diff --git a/src/Umbraco.Web/Models/Mapping/ContentTypeModelMapperExtensions.cs b/src/Umbraco.Web/Models/Mapping/ContentTypeModelMapperExtensions.cs index 10033ebd65..2641216394 100644 --- a/src/Umbraco.Web/Models/Mapping/ContentTypeModelMapperExtensions.cs +++ b/src/Umbraco.Web/Models/Mapping/ContentTypeModelMapperExtensions.cs @@ -120,6 +120,7 @@ namespace Umbraco.Web.Models.Mapping .ForMember(dto => dto.SortOrder, expression => expression.Ignore()) //ignore, we'll do this in after map .ForMember(dto => dto.PropertyGroups, expression => expression.Ignore()) + .ForMember(dto => dto.NoGroupPropertyTypes, expression => expression.Ignore()) .ForMember( dto => dto.AllowedContentTypes, @@ -145,7 +146,7 @@ namespace Umbraco.Web.Models.Mapping // handle actual groups (non-generic-properties) var destOrigGroups = dest.PropertyGroups.ToArray(); // local groups - var destOrigProperties = dest.PropertyTypes.ToArray(); // get_PropertyTypes returns grouped props too + var destOrigProperties = dest.PropertyTypes.ToArray(); // all properties, in groups or not var destGroups = new List(); var sourceGroups = source.Groups.Where(x => x.IsGenericProperties == false).ToArray(); foreach (var sourceGroup in sourceGroups) @@ -189,13 +190,7 @@ namespace Umbraco.Web.Models.Mapping // ensure no duplicate alias, then assign the generic properties collection EnsureUniqueAliases(destProperties); - - // set_PropertyTypes sets generic properties *only* - // this is all very awkward - var dest2 = dest as ContentTypeBase; - if (dest2 == null) - throw new InvalidOperationException("Cannot map if dest is not ContentTypeBase."); - dest2.PropertyTypes = new PropertyTypeCollection(destProperties); + dest.NoGroupPropertyTypes = new PropertyTypeCollection(destProperties); } // because all property collections were rebuilt, there is no need to remove