From eb5589e0645d645eaf771797c3956ee7952eeb28 Mon Sep 17 00:00:00 2001 From: Stephan Date: Tue, 17 Nov 2015 12:30:37 +0100 Subject: [PATCH] Remove PropertyGroup.ParentId, sanitize Property groups/types --- .../Models/ContentTypeCompositionBase.cs | 81 ++++---- src/Umbraco.Core/Models/PropertyGroup.cs | 34 ---- ...ntTypeDto.cs => ContentTypeTemplateDto.cs} | 54 ++--- .../Models/Rdbms/PropertyTypeGroupDto.cs | 6 - .../Rdbms/PropertyTypeGroupReadOnlyDto.cs | 3 - .../Factories/ContentTypeFactory.cs | 178 ++++++++++------- .../Persistence/Factories/MediaTypeFactory.cs | 82 -------- .../Factories/MemberTypeFactory.cs | 94 --------- .../Factories/MemberTypeReadOnlyFactory.cs | 21 +- .../Factories/PropertyGroupFactory.cs | 39 ++-- .../Mappers/PropertyGroupMapper.cs | 1 - .../Initial/DatabaseSchemaCreation.cs | 2 +- .../AddUniqueIdPropertyTypeGroupColumn.cs | 9 +- .../RemoveParentIdPropertyTypeGroupColumn.cs | 29 +++ .../UpdatePropertyTypesAndGroups.cs | 69 +++---- .../Repositories/ContentTypeBaseRepository.cs | 151 ++++++-------- .../Repositories/ContentTypeRepository.cs | 71 ++++--- .../Repositories/MediaTypeRepository.cs | 10 +- .../Repositories/MemberTypeRepository.cs | 18 +- src/Umbraco.Core/Umbraco.Core.csproj | 5 +- .../Models/PropertyGroupTests.cs | 7 +- .../Persistence/BaseTableByTableTest.cs | 2 +- .../Mappers/PropertyGroupMapperTest.cs | 13 -- .../ContentTypeRepositorySqlClausesTest.cs | 12 +- .../Querying/ContentTypeSqlMappingTests.cs | 4 +- .../Services/ContentTypeServiceTests.cs | 3 - .../Entities/MockedContentTypes.cs | 10 - .../config/ClientDependency.config | 2 +- .../ContentEditing/PropertyGroupBasic.cs | 17 +- .../ContentEditing/PropertyGroupDisplay.cs | 22 +-- .../ContentEditing/PropertyTypeBasic.cs | 13 +- .../Models/Mapping/ContentTypeModelMapper.cs | 25 ++- .../ContentTypeModelMapperExtensions.cs | 184 ++++++++++++----- .../Mapping/PropertyTypeGroupResolver.cs | 186 +++++++++--------- .../Mapping/TabsAndPropertiesResolver.cs | 86 ++++---- .../umbraco/controls/ContentControl.cs | 10 +- .../controls/ContentTypeControlNew.ascx.cs | 36 ++-- src/umbraco.cms/businesslogic/ContentType.cs | 53 ++--- .../propertytype/PropertyTypeGroup.cs | 54 +++-- 39 files changed, 777 insertions(+), 919 deletions(-) rename src/Umbraco.Core/Models/Rdbms/{DocumentTypeDto.cs => ContentTypeTemplateDto.cs} (93%) delete mode 100644 src/Umbraco.Core/Persistence/Factories/MediaTypeFactory.cs delete mode 100644 src/Umbraco.Core/Persistence/Factories/MemberTypeFactory.cs create mode 100644 src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenFourZero/RemoveParentIdPropertyTypeGroupColumn.cs diff --git a/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs b/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs index ea5a8fbcbf..a1b9cb9f1a 100644 --- a/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs +++ b/src/Umbraco.Core/Models/ContentTypeCompositionBase.cs @@ -157,31 +157,44 @@ namespace Umbraco.Core.Models /// /// Adds a PropertyGroup. - /// This method will also check if a group already exists with the same name and link it to the parent. /// /// Name of the PropertyGroup to add /// Returns True if a PropertyGroup with the passed in name was added, otherwise False public override bool AddPropertyGroup(string groupName) { - if (PropertyGroups.Any(x => x.Name == groupName)) - return false; + return AddAndReturnPropertyGroup(groupName) != null; + } - var propertyGroup = new PropertyGroup {Name = groupName, SortOrder = 0}; + private PropertyGroup AddAndReturnPropertyGroup(string name) + { + // ensure we don't have it already + if (PropertyGroups.Any(x => x.Name == name)) + return null; - if (CompositionPropertyGroups.Any(x => x.Name == groupName)) + // create the new group + var group = new PropertyGroup { Name = name, SortOrder = 0 }; + + // check if it is inherited - there might be more than 1 but we want the 1st, to + // reuse its sort order - if there are more than 1 and they have different sort + // orders... there isn't much we can do anyways + var inheritGroup = CompositionPropertyGroups.FirstOrDefault(x => x.Name == name); + if (inheritGroup == null) { - var firstGroup = CompositionPropertyGroups.First(x => x.Name == groupName && x.ParentId.HasValue == false); - propertyGroup.SetLazyParentId(new Lazy(() => firstGroup.Id)); + // no, just local, set sort order + var lastGroup = PropertyGroups.LastOrDefault(); + if (lastGroup != null) + group.SortOrder = lastGroup.SortOrder + 1; + } + else + { + // yes, inherited, re-use sort order + group.SortOrder = inheritGroup.SortOrder; } - if (PropertyGroups.Any()) - { - var last = PropertyGroups.Last(); - propertyGroup.SortOrder = last.SortOrder + 1; - } + // add + PropertyGroups.Add(group); - PropertyGroups.Add(propertyGroup); - return true; + return group; } /// @@ -193,38 +206,24 @@ namespace Umbraco.Core.Models public override bool AddPropertyType(PropertyType propertyType, string propertyGroupName) { if (propertyType.HasIdentity == false) - { propertyType.Key = Guid.NewGuid(); - } - if (PropertyTypeExists(propertyType.Alias) == false) - { - if (PropertyGroups.Contains(propertyGroupName)) - { - propertyType.PropertyGroupId = new Lazy(() => PropertyGroups[propertyGroupName].Id); - PropertyGroups[propertyGroupName].PropertyTypes.Add(propertyType); - } - else - { - //If the PropertyGroup doesn't already exist we create a new one - var propertyTypes = new List { propertyType }; - var propertyGroup = new PropertyGroup(new PropertyTypeCollection(propertyTypes)) { Name = propertyGroupName, SortOrder = 1 }; - //and check if its an inherited PropertyGroup, which exists in the composition - if (CompositionPropertyGroups.Any(x => x.Name == propertyGroupName)) - { - var parentPropertyGroup = CompositionPropertyGroups.First(x => x.Name == propertyGroupName && x.ParentId.HasValue == false); - propertyGroup.SortOrder = parentPropertyGroup.SortOrder; - //propertyGroup.ParentId = parentPropertyGroup.Id; - propertyGroup.SetLazyParentId(new Lazy(() => parentPropertyGroup.Id)); - } + // ensure no duplicate alias - over all composition properties + if (PropertyTypeExists(propertyType.Alias)) + return false; - PropertyGroups.Add(propertyGroup); - } + // get and ensure a group local to this content type + var group = PropertyGroups.Contains(propertyGroupName) + ? PropertyGroups[propertyGroupName] + : AddAndReturnPropertyGroup(propertyGroupName); + if (group == null) + return false; - return true; - } + // add property to group + propertyType.PropertyGroupId = new Lazy(() => group.Id); + group.PropertyTypes.Add(propertyType); - return false; + return true; } /// diff --git a/src/Umbraco.Core/Models/PropertyGroup.cs b/src/Umbraco.Core/Models/PropertyGroup.cs index 7976a7eac6..de88012c0e 100644 --- a/src/Umbraco.Core/Models/PropertyGroup.cs +++ b/src/Umbraco.Core/Models/PropertyGroup.cs @@ -17,7 +17,6 @@ namespace Umbraco.Core.Models public class PropertyGroup : Entity, IEquatable { private string _name; - private Lazy _parentId; private int _sortOrder; private PropertyTypeCollection _propertyTypes; @@ -31,7 +30,6 @@ namespace Umbraco.Core.Models } private static readonly PropertyInfo NameSelector = ExpressionHelper.GetPropertyInfo(x => x.Name); - private static readonly PropertyInfo ParentIdSelector = ExpressionHelper.GetPropertyInfo(x => x.ParentId); private static readonly PropertyInfo SortOrderSelector = ExpressionHelper.GetPropertyInfo(x => x.SortOrder); private readonly static PropertyInfo PropertyTypeCollectionSelector = ExpressionHelper.GetPropertyInfo(x => x.PropertyTypes); void PropertyTypesChanged(object sender, NotifyCollectionChangedEventArgs e) @@ -56,29 +54,6 @@ namespace Umbraco.Core.Models } } - /// - /// Gets or sets the Id of the Parent PropertyGroup. - /// - /// - /// A Parent PropertyGroup corresponds to an inherited PropertyGroup from a composition. - /// If a PropertyType is inserted into an inherited group then a new group will be created with an Id reference to the parent. - /// - [DataMember] - public int? ParentId - { - get - { - if (_parentId == null) - return default(int?); - return _parentId.Value; - } - set - { - _parentId = new Lazy(() => value); - OnPropertyChanged(ParentIdSelector); - } - } - /// /// Gets or sets the Sort Order of the Group /// @@ -117,15 +92,6 @@ namespace Umbraco.Core.Models } } - /// - /// Sets the ParentId from the lazy integer id - /// - /// Id of the Parent - internal void SetLazyParentId(Lazy id) - { - _parentId = id; - } - public bool Equals(PropertyGroup other) { if (base.Equals(other)) return true; diff --git a/src/Umbraco.Core/Models/Rdbms/DocumentTypeDto.cs b/src/Umbraco.Core/Models/Rdbms/ContentTypeTemplateDto.cs similarity index 93% rename from src/Umbraco.Core/Models/Rdbms/DocumentTypeDto.cs rename to src/Umbraco.Core/Models/Rdbms/ContentTypeTemplateDto.cs index e3fdfb9810..88ef02ea90 100644 --- a/src/Umbraco.Core/Models/Rdbms/DocumentTypeDto.cs +++ b/src/Umbraco.Core/Models/Rdbms/ContentTypeTemplateDto.cs @@ -1,28 +1,28 @@ -using Umbraco.Core.Persistence; -using Umbraco.Core.Persistence.DatabaseAnnotations; - -namespace Umbraco.Core.Models.Rdbms -{ - [TableName("cmsDocumentType")] - [PrimaryKey("contentTypeNodeId", autoIncrement = false)] - [ExplicitColumns] - internal class DocumentTypeDto - { - [Column("contentTypeNodeId")] - [PrimaryKeyColumn(AutoIncrement = false, Name = "PK_cmsDocumentType", OnColumns = "contentTypeNodeId, templateNodeId")] - [ForeignKey(typeof(ContentTypeDto), Column = "nodeId")] - [ForeignKey(typeof(NodeDto))] - public int ContentTypeNodeId { get; set; } - - [Column("templateNodeId")] - [ForeignKey(typeof(TemplateDto), Column = "nodeId")] - public int TemplateNodeId { get; set; } - - [Column("IsDefault")] - [Constraint(Default = "0")] - public bool IsDefault { get; set; } - - [ResultColumn] - public ContentTypeDto ContentTypeDto { get; set; } - } +using Umbraco.Core.Persistence; +using Umbraco.Core.Persistence.DatabaseAnnotations; + +namespace Umbraco.Core.Models.Rdbms +{ + [TableName("cmsDocumentType")] + [PrimaryKey("contentTypeNodeId", autoIncrement = false)] + [ExplicitColumns] + internal class ContentTypeTemplateDto + { + [Column("contentTypeNodeId")] + [PrimaryKeyColumn(AutoIncrement = false, Name = "PK_cmsDocumentType", OnColumns = "contentTypeNodeId, templateNodeId")] + [ForeignKey(typeof(ContentTypeDto), Column = "nodeId")] + [ForeignKey(typeof(NodeDto))] + public int ContentTypeNodeId { get; set; } + + [Column("templateNodeId")] + [ForeignKey(typeof(TemplateDto), Column = "nodeId")] + public int TemplateNodeId { get; set; } + + [Column("IsDefault")] + [Constraint(Default = "0")] + public bool IsDefault { get; set; } + + [ResultColumn] + public ContentTypeDto ContentTypeDto { get; set; } + } } \ No newline at end of file diff --git a/src/Umbraco.Core/Models/Rdbms/PropertyTypeGroupDto.cs b/src/Umbraco.Core/Models/Rdbms/PropertyTypeGroupDto.cs index fcbbc94db1..42abd9ed49 100644 --- a/src/Umbraco.Core/Models/Rdbms/PropertyTypeGroupDto.cs +++ b/src/Umbraco.Core/Models/Rdbms/PropertyTypeGroupDto.cs @@ -21,12 +21,6 @@ namespace Umbraco.Core.Models.Rdbms [PrimaryKeyColumn(IdentitySeed = 12)] public int Id { get; set; } - [Column("parentGroupId")] - [NullSetting(NullSetting = NullSettings.Null)] - //[Constraint(Default = "NULL")] - [ForeignKey(typeof(PropertyTypeGroupDto))] - public int? ParentGroupId { get; set; } - [Column("contenttypeNodeId")] [ForeignKey(typeof(ContentTypeDto), Column = "nodeId")] public int ContentTypeNodeId { get; set; } diff --git a/src/Umbraco.Core/Models/Rdbms/PropertyTypeGroupReadOnlyDto.cs b/src/Umbraco.Core/Models/Rdbms/PropertyTypeGroupReadOnlyDto.cs index 8dcc4af29c..beebef9eeb 100644 --- a/src/Umbraco.Core/Models/Rdbms/PropertyTypeGroupReadOnlyDto.cs +++ b/src/Umbraco.Core/Models/Rdbms/PropertyTypeGroupReadOnlyDto.cs @@ -10,9 +10,6 @@ namespace Umbraco.Core.Models.Rdbms [Column("PropertyTypeGroupId")] public int? Id { get; set; } - [Column("parentGroupId")] - public int? ParentGroupId { get; set; } - [Column("PropertyGroupName")] public string Text { get; set; } diff --git a/src/Umbraco.Core/Persistence/Factories/ContentTypeFactory.cs b/src/Umbraco.Core/Persistence/Factories/ContentTypeFactory.cs index fdd2759d76..54c7d8d2c9 100644 --- a/src/Umbraco.Core/Persistence/Factories/ContentTypeFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/ContentTypeFactory.cs @@ -1,98 +1,144 @@ using System; +using System.Collections.Generic; using System.Globalization; +using System.Linq; using Umbraco.Core.Models; +using Umbraco.Core.Models.EntityBase; using Umbraco.Core.Models.Membership; using Umbraco.Core.Models.Rdbms; namespace Umbraco.Core.Persistence.Factories { + // factory for + // IContentType (document types) + // IMediaType (media types) + // IMemberType (member types) + // internal class ContentTypeFactory { - private readonly Guid _nodeObjectType; + #region IContentType - public ContentTypeFactory(Guid nodeObjectType) + public IContentType BuildContentTypeEntity(ContentTypeDto dto) { - _nodeObjectType = nodeObjectType; - } + var contentType = new ContentType(dto.NodeDto.ParentId); + BuildCommonEntity(contentType, dto); - #region Implementation of IEntityFactory - - public IContentType BuildEntity(DocumentTypeDto dto) - { - var contentType = new ContentType(dto.ContentTypeDto.NodeDto.ParentId) - { - Id = dto.ContentTypeDto.NodeDto.NodeId, - Key = dto.ContentTypeDto.NodeDto.UniqueId, - Alias = dto.ContentTypeDto.Alias, - Name = dto.ContentTypeDto.NodeDto.Text, - Icon = dto.ContentTypeDto.Icon, - Thumbnail = dto.ContentTypeDto.Thumbnail, - SortOrder = dto.ContentTypeDto.NodeDto.SortOrder, - Description = dto.ContentTypeDto.Description, - CreateDate = dto.ContentTypeDto.NodeDto.CreateDate, - Path = dto.ContentTypeDto.NodeDto.Path, - Level = dto.ContentTypeDto.NodeDto.Level, - CreatorId = dto.ContentTypeDto.NodeDto.UserId.Value, - AllowedAsRoot = dto.ContentTypeDto.AllowAtRoot, - IsContainer = dto.ContentTypeDto.IsContainer, - Trashed = dto.ContentTypeDto.NodeDto.Trashed, - DefaultTemplateId = dto.TemplateNodeId - }; //on initial construction we don't want to have dirty properties tracked // http://issues.umbraco.org/issue/U4-1946 contentType.ResetDirtyProperties(false); - return contentType; - } - public DocumentTypeDto BuildDto(IContentType entity) - { - var documentTypeDto = new DocumentTypeDto - {ContentTypeDto = BuildContentTypeDto(entity), ContentTypeNodeId = entity.Id}; - - var contentType = entity as ContentType; - if(contentType != null) - { - documentTypeDto.TemplateNodeId = contentType.DefaultTemplateId; - documentTypeDto.IsDefault = true; - } - return documentTypeDto; + return contentType; } #endregion - private ContentTypeDto BuildContentTypeDto(IContentType entity) + #region IMediaType + + public IMediaType BuildMediaTypeEntity(ContentTypeDto dto) { + var contentType = new MediaType(dto.NodeDto.ParentId); + BuildCommonEntity(contentType, dto); + + //on initial construction we don't want to have dirty properties tracked + // http://issues.umbraco.org/issue/U4-1946 + contentType.ResetDirtyProperties(false); + + return contentType; + } + + #endregion + + #region IMemberType + + public IMemberType BuildMemberTypeEntity(ContentTypeDto dto) + { + throw new NotImplementedException(); + } + + public IEnumerable BuildMemberTypeDtos(IMemberType entity) + { + var memberType = entity as MemberType; + if (memberType == null || memberType.PropertyTypes.Any() == false) + return Enumerable.Empty(); + + var dtos = memberType.PropertyTypes.Select(x => new MemberTypeDto + { + NodeId = entity.Id, + PropertyTypeId = x.Id, + CanEdit = memberType.MemberCanEditProperty(x.Alias), + ViewOnProfile = memberType.MemberCanViewProperty(x.Alias) + }).ToList(); + return dtos; + } + + #endregion + + #region Common + + private static void BuildCommonEntity(ContentTypeBase entity, ContentTypeDto dto) + { + entity.Id = dto.NodeDto.NodeId; + entity.Key = dto.NodeDto.UniqueId; + entity.Alias = dto.Alias; + entity.Name = dto.NodeDto.Text; + entity.Icon = dto.Icon; + entity.Thumbnail = dto.Thumbnail; + entity.SortOrder = dto.NodeDto.SortOrder; + entity.Description = dto.Description; + entity.CreateDate = dto.NodeDto.CreateDate; + entity.Path = dto.NodeDto.Path; + entity.Level = dto.NodeDto.Level; + entity.CreatorId = dto.NodeDto.UserId.Value; + entity.AllowedAsRoot = dto.AllowAtRoot; + entity.IsContainer = dto.IsContainer; + entity.Trashed = dto.NodeDto.Trashed; + } + + public ContentTypeDto BuildContentTypeDto(IContentTypeBase entity) + { + Guid nodeObjectType; + if (entity is IContentType) + nodeObjectType = Constants.ObjectTypes.DocumentTypeGuid; + else if (entity is IMediaType) + nodeObjectType = Constants.ObjectTypes.MediaTypeGuid; + else if (entity is IMemberType) + nodeObjectType = Constants.ObjectTypes.MemberTypeGuid; + else + throw new Exception("oops: invalid entity."); + var contentTypeDto = new ContentTypeDto - { - Alias = entity.Alias, - Description = entity.Description, - Icon = entity.Icon, - Thumbnail = entity.Thumbnail, - NodeId = entity.Id, - AllowAtRoot = entity.AllowedAsRoot, - IsContainer = entity.IsContainer, - NodeDto = BuildNodeDto(entity) - }; + { + Alias = entity.Alias, + Description = entity.Description, + Icon = entity.Icon, + Thumbnail = entity.Thumbnail, + NodeId = entity.Id, + AllowAtRoot = entity.AllowedAsRoot, + IsContainer = entity.IsContainer, + NodeDto = BuildNodeDto(entity, nodeObjectType) + }; return contentTypeDto; } - private NodeDto BuildNodeDto(IContentType entity) + private static NodeDto BuildNodeDto(IUmbracoEntity entity, Guid nodeObjectType) { var nodeDto = new NodeDto - { - CreateDate = entity.CreateDate, - NodeId = entity.Id, - Level = short.Parse(entity.Level.ToString(CultureInfo.InvariantCulture)), - NodeObjectType = _nodeObjectType, - ParentId = entity.ParentId, - Path = entity.Path, - SortOrder = entity.SortOrder, - Text = entity.Name, - Trashed = false, - UniqueId = entity.Key, - UserId = entity.CreatorId - }; + { + CreateDate = entity.CreateDate, + NodeId = entity.Id, + Level = short.Parse(entity.Level.ToString(CultureInfo.InvariantCulture)), + NodeObjectType = nodeObjectType, + ParentId = entity.ParentId, + Path = entity.Path, + SortOrder = entity.SortOrder, + Text = entity.Name, + Trashed = false, + UniqueId = entity.Key, + UserId = entity.CreatorId + }; return nodeDto; } + + #endregion } } \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Factories/MediaTypeFactory.cs b/src/Umbraco.Core/Persistence/Factories/MediaTypeFactory.cs deleted file mode 100644 index 98048cd3a7..0000000000 --- a/src/Umbraco.Core/Persistence/Factories/MediaTypeFactory.cs +++ /dev/null @@ -1,82 +0,0 @@ -using System; -using System.Globalization; -using Umbraco.Core.Models; -using Umbraco.Core.Models.Rdbms; - -namespace Umbraco.Core.Persistence.Factories -{ - internal class MediaTypeFactory - { - private readonly Guid _nodeObjectType; - - public MediaTypeFactory(Guid nodeObjectType) - { - _nodeObjectType = nodeObjectType; - } - - #region Implementation of IEntityFactory - - public IMediaType BuildEntity(ContentTypeDto dto) - { - var contentType = new MediaType(dto.NodeDto.ParentId) - { - Id = dto.NodeDto.NodeId, - Key = dto.NodeDto.UniqueId, - Alias = dto.Alias, - Name = dto.NodeDto.Text, - Icon = dto.Icon, - Thumbnail = dto.Thumbnail, - SortOrder = dto.NodeDto.SortOrder, - Description = dto.Description, - CreateDate = dto.NodeDto.CreateDate, - Path = dto.NodeDto.Path, - Level = dto.NodeDto.Level, - CreatorId = dto.NodeDto.UserId.Value, - AllowedAsRoot = dto.AllowAtRoot, - IsContainer = dto.IsContainer, - Trashed = dto.NodeDto.Trashed - }; - //on initial construction we don't want to have dirty properties tracked - // http://issues.umbraco.org/issue/U4-1946 - contentType.ResetDirtyProperties(false); - return contentType; - } - - public ContentTypeDto BuildDto(IMediaType entity) - { - var contentTypeDto = new ContentTypeDto - { - Alias = entity.Alias, - Description = entity.Description, - Icon = entity.Icon, - Thumbnail = entity.Thumbnail, - NodeId = entity.Id, - AllowAtRoot = entity.AllowedAsRoot, - IsContainer = entity.IsContainer, - NodeDto = BuildNodeDto(entity) - }; - return contentTypeDto; - } - - #endregion - - private NodeDto BuildNodeDto(IMediaType entity) - { - var nodeDto = new NodeDto - { - CreateDate = entity.CreateDate, - NodeId = entity.Id, - Level = short.Parse(entity.Level.ToString(CultureInfo.InvariantCulture)), - NodeObjectType = _nodeObjectType, - ParentId = entity.ParentId, - Path = entity.Path, - SortOrder = entity.SortOrder, - Text = entity.Name, - Trashed = false, - UniqueId = entity.Key, - UserId = entity.CreatorId - }; - return nodeDto; - } - } -} \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Factories/MemberTypeFactory.cs b/src/Umbraco.Core/Persistence/Factories/MemberTypeFactory.cs deleted file mode 100644 index 5879016fa6..0000000000 --- a/src/Umbraco.Core/Persistence/Factories/MemberTypeFactory.cs +++ /dev/null @@ -1,94 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Globalization; -using System.Linq; -using Umbraco.Core.Models; -using Umbraco.Core.Models.Rdbms; - -namespace Umbraco.Core.Persistence.Factories -{ - internal class MemberTypeFactory - { - private readonly Guid _nodeObjectType; - - public MemberTypeFactory(Guid nodeObjectType) - { - _nodeObjectType = nodeObjectType; - } - - public IMemberType BuildEntity(ContentTypeDto dto) - { - throw new System.NotImplementedException(); - } - - public ContentTypeDto BuildDto(IMemberType entity) - { - var contentTypeDto = new ContentTypeDto - { - Alias = entity.Alias, - Description = entity.Description, - Icon = entity.Icon, - Thumbnail = entity.Thumbnail, - NodeId = entity.Id, - AllowAtRoot = entity.AllowedAsRoot, - IsContainer = entity.IsContainer, - NodeDto = BuildNodeDto(entity) - }; - return contentTypeDto; - } - - public IEnumerable BuildMemberTypeDtos(IMemberType entity) - { - var memberType = entity as MemberType; - if (memberType == null || memberType.PropertyTypes.Any() == false) - return Enumerable.Empty(); - - var memberTypes = new List(); - foreach (var propertyType in memberType.PropertyTypes) - { - memberTypes.Add(new MemberTypeDto - { - NodeId = entity.Id, - PropertyTypeId = propertyType.Id, - CanEdit = memberType.MemberCanEditProperty(propertyType.Alias), - ViewOnProfile = memberType.MemberCanViewProperty(propertyType.Alias) - }); - } - - return memberTypes; - } - - private NodeDto BuildNodeDto(IMemberType entity) - { - var nodeDto = new NodeDto - { - CreateDate = entity.CreateDate, - NodeId = entity.Id, - Level = short.Parse(entity.Level.ToString(CultureInfo.InvariantCulture)), - NodeObjectType = _nodeObjectType, - ParentId = entity.ParentId, - Path = entity.Path, - SortOrder = entity.SortOrder, - Text = entity.Name, - Trashed = false, - UniqueId = entity.Key, - UserId = entity.CreatorId - }; - return nodeDto; - } - - private int DeterminePropertyTypeId(int initialId, string alias, IEnumerable propertyTypes) - { - if (initialId == 0 || initialId == default(int)) - { - var propertyType = propertyTypes.SingleOrDefault(x => x.Alias.Equals(alias)); - if (propertyType == null) - return default(int); - - return propertyType.Id; - } - - return initialId; - } - } -} \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Factories/MemberTypeReadOnlyFactory.cs b/src/Umbraco.Core/Persistence/Factories/MemberTypeReadOnlyFactory.cs index 38ef4542f4..604e52eb72 100644 --- a/src/Umbraco.Core/Persistence/Factories/MemberTypeReadOnlyFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/MemberTypeReadOnlyFactory.cs @@ -58,24 +58,21 @@ namespace Umbraco.Core.Persistence.Factories private PropertyGroupCollection GetPropertyTypeGroupCollection(MemberTypeReadOnlyDto dto, MemberType memberType, Dictionary standardProps) { - var propertyGroups = new PropertyGroupCollection(); - + // see PropertyGroupFactory, repeating code here... + + var propertyGroups = new PropertyGroupCollection(); foreach (var groupDto in dto.PropertyTypeGroups.Where(x => x.Id.HasValue)) { var group = new PropertyGroup(); - - //Only assign an Id if the PropertyGroup belongs to this ContentType + + // if the group is defined on the current member type, + // assign its identifier, else it will be zero if (groupDto.ContentTypeNodeId == memberType.Id) { + // note: no idea why Id is nullable here, but better check + if (groupDto.Id.HasValue == false) + throw new Exception("oops: groupDto.Id has no value."); group.Id = groupDto.Id.Value; - - if (groupDto.ParentGroupId.HasValue) - group.ParentId = groupDto.ParentGroupId.Value; - } - else - { - //If the PropertyGroup is inherited, we add a reference to the group as a Parent. - group.ParentId = groupDto.Id; } group.Name = groupDto.Text; diff --git a/src/Umbraco.Core/Persistence/Factories/PropertyGroupFactory.cs b/src/Umbraco.Core/Persistence/Factories/PropertyGroupFactory.cs index 799b667732..11a6741a00 100644 --- a/src/Umbraco.Core/Persistence/Factories/PropertyGroupFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/PropertyGroupFactory.cs @@ -8,21 +8,21 @@ namespace Umbraco.Core.Persistence.Factories { internal class PropertyGroupFactory { - private readonly int _id; + private readonly int _contentTypeId; private readonly DateTime _createDate; private readonly DateTime _updateDate; //a callback to create a property type which can be injected via a contructor private readonly Func _propertyTypeCtor; - public PropertyGroupFactory(int id) + public PropertyGroupFactory(int contentTypeId) { - _id = id; + _contentTypeId = contentTypeId; _propertyTypeCtor = (propertyEditorAlias, dbType, alias) => new PropertyType(propertyEditorAlias, dbType); } - public PropertyGroupFactory(int id, DateTime createDate, DateTime updateDate, Func propertyTypeCtor) + public PropertyGroupFactory(int contentTypeId, DateTime createDate, DateTime updateDate, Func propertyTypeCtor) { - _id = id; + _contentTypeId = contentTypeId; _createDate = createDate; _updateDate = updateDate; _propertyTypeCtor = propertyTypeCtor; @@ -30,25 +30,19 @@ namespace Umbraco.Core.Persistence.Factories #region Implementation of IEntityFactory,IEnumerable> - public IEnumerable BuildEntity(IEnumerable dto) + public IEnumerable BuildEntity(IEnumerable groupDtos) { + // groupDtos contains all the groups, those that are defined on the current + // content type, and those that are inherited from composition content types var propertyGroups = new PropertyGroupCollection(); - foreach (var groupDto in dto) + foreach (var groupDto in groupDtos) { var group = new PropertyGroup(); - //Only assign an Id if the PropertyGroup belongs to this ContentType - if (groupDto.ContentTypeNodeId == _id) - { - group.Id = groupDto.Id; - if (groupDto.ParentGroupId.HasValue) - group.ParentId = groupDto.ParentGroupId.Value; - } - else - { - //If the PropertyGroup is inherited, we add a reference to the group as a Parent. - group.ParentId = groupDto.Id; - } + // if the group is defined on the current content type, + // assign its identifier, else it will be zero + if (groupDto.ContentTypeNodeId == _contentTypeId) + group.Id = groupDto.Id; group.Name = groupDto.Text; group.SortOrder = groupDto.SortOrder; @@ -102,7 +96,7 @@ namespace Umbraco.Core.Persistence.Factories { var dto = new PropertyTypeGroupDto { - ContentTypeNodeId = _id, + ContentTypeNodeId = _contentTypeId, SortOrder = propertyGroup.SortOrder, Text = propertyGroup.Name, UniqueId = propertyGroup.HasIdentity @@ -112,9 +106,6 @@ namespace Umbraco.Core.Persistence.Factories : Guid.NewGuid() }; - if (propertyGroup.ParentId.HasValue) - dto.ParentGroupId = propertyGroup.ParentId.Value; - if (propertyGroup.HasIdentity) dto.Id = propertyGroup.Id; @@ -128,7 +119,7 @@ namespace Umbraco.Core.Persistence.Factories var propertyTypeDto = new PropertyTypeDto { Alias = propertyType.Alias, - ContentTypeId = _id, + ContentTypeId = _contentTypeId, DataTypeId = propertyType.DataTypeDefinitionId, Description = propertyType.Description, Mandatory = propertyType.Mandatory, diff --git a/src/Umbraco.Core/Persistence/Mappers/PropertyGroupMapper.cs b/src/Umbraco.Core/Persistence/Mappers/PropertyGroupMapper.cs index 044892821e..655c40fc61 100644 --- a/src/Umbraco.Core/Persistence/Mappers/PropertyGroupMapper.cs +++ b/src/Umbraco.Core/Persistence/Mappers/PropertyGroupMapper.cs @@ -33,7 +33,6 @@ namespace Umbraco.Core.Persistence.Mappers { CacheMap(src => src.Id, dto => dto.Id); CacheMap(src => src.Key, dto => dto.UniqueId); - CacheMap(src => src.ParentId, dto => dto.ParentGroupId); CacheMap(src => src.SortOrder, dto => dto.SortOrder); CacheMap(src => src.Name, dto => dto.Text); } diff --git a/src/Umbraco.Core/Persistence/Migrations/Initial/DatabaseSchemaCreation.cs b/src/Umbraco.Core/Persistence/Migrations/Initial/DatabaseSchemaCreation.cs index 0ba269a3ba..9ffce4b8de 100644 --- a/src/Umbraco.Core/Persistence/Migrations/Initial/DatabaseSchemaCreation.cs +++ b/src/Umbraco.Core/Persistence/Migrations/Initial/DatabaseSchemaCreation.cs @@ -44,7 +44,7 @@ namespace Umbraco.Core.Persistence.Migrations.Initial {4, typeof (ContentVersionDto)}, {5, typeof (DocumentDto)}, - {6, typeof (DocumentTypeDto)}, + {6, typeof (ContentTypeTemplateDto)}, {7, typeof (DataTypeDto)}, {8, typeof (DataTypePreValueDto)}, {9, typeof (DictionaryDto)}, diff --git a/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenFourZero/AddUniqueIdPropertyTypeGroupColumn.cs b/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenFourZero/AddUniqueIdPropertyTypeGroupColumn.cs index e320c72e11..bb09faaa7f 100644 --- a/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenFourZero/AddUniqueIdPropertyTypeGroupColumn.cs +++ b/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenFourZero/AddUniqueIdPropertyTypeGroupColumn.cs @@ -7,18 +7,16 @@ using Umbraco.Core.Persistence.SqlSyntax; namespace Umbraco.Core.Persistence.Migrations.Upgrades.TargetVersionSevenFourZero { - [Migration("7.4.0", 2, GlobalSettings.UmbracoMigrationName)] public class AddUniqueIdPropertyTypeGroupColumn : MigrationBase { public AddUniqueIdPropertyTypeGroupColumn(ISqlSyntaxProvider sqlSyntax, ILogger logger) : base(sqlSyntax, logger) - { - } + { } public override void Up() { - //Don't exeucte if the column is already there + // don't execute if the column is already there var columns = SqlSyntax.GetColumnsInSchema(Context.Database).ToArray(); if (columns.Any(x => x.TableName.InvariantEquals("cmsPropertyTypeGroup") && x.ColumnName.InvariantEquals("uniqueID")) == false) @@ -77,7 +75,6 @@ ON cmsContentType.nodeId = umbracoNode.id")) } public override void Down() - { - } + { } } } \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenFourZero/RemoveParentIdPropertyTypeGroupColumn.cs b/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenFourZero/RemoveParentIdPropertyTypeGroupColumn.cs new file mode 100644 index 0000000000..46f082f74c --- /dev/null +++ b/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenFourZero/RemoveParentIdPropertyTypeGroupColumn.cs @@ -0,0 +1,29 @@ +using System.Linq; +using Umbraco.Core.Configuration; +using Umbraco.Core.Logging; +using Umbraco.Core.Persistence.SqlSyntax; + +namespace Umbraco.Core.Persistence.Migrations.Upgrades.TargetVersionSevenFourZero +{ + [Migration("7.4.0", 3, GlobalSettings.UmbracoMigrationName)] + public class RemoveParentIdPropertyTypeGroupColumn : MigrationBase + { + public RemoveParentIdPropertyTypeGroupColumn(ISqlSyntaxProvider sqlSyntax, ILogger logger) + : base(sqlSyntax, logger) + { } + + public override void Up() + { + // don't execute if the column is not there anymore + var columns = SqlSyntax.GetColumnsInSchema(Context.Database).ToArray(); + + if (columns.Any(x => x.TableName.InvariantEquals("cmsPropertyTypeGroup") && x.ColumnName.InvariantEquals("parentGroupId")) == false) + return; + + Delete.Column("parentGroupId").FromTable("cmsPropertyTypeGroup"); + } + + public override void Down() + { } + } +} diff --git a/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSixZeroOne/UpdatePropertyTypesAndGroups.cs b/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSixZeroOne/UpdatePropertyTypesAndGroups.cs index 6598719454..eebad824ff 100644 --- a/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSixZeroOne/UpdatePropertyTypesAndGroups.cs +++ b/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSixZeroOne/UpdatePropertyTypesAndGroups.cs @@ -37,48 +37,43 @@ namespace Umbraco.Core.Persistence.Migrations.Upgrades.TargetVersionSixZeroOne foreach (var propertyType in propertyTypes) { - //Get the PropertyTypeGroup that the current PropertyType references - var parentPropertyTypeGroup = propertyGroups.FirstOrDefault(x => x.Id == propertyType.PropertyTypeGroupId); - if (parentPropertyTypeGroup != null) + // get the PropertyTypeGroup of the current PropertyType, skip if not found + var propertyTypeGroup = propertyGroups.FirstOrDefault(x => x.Id == propertyType.PropertyTypeGroupId); + if (propertyTypeGroup == null) continue; + + // if the PropretyTypeGroup belongs to the same content type as the PropertyType, then fine + if (propertyTypeGroup.ContentTypeNodeId == propertyType.ContentTypeId) continue; + + // else we want to assign the PropertyType to a proper PropertyTypeGroup + // ie one that does belong to the same content - look for it + var okPropertyTypeGroup = propertyGroups.FirstOrDefault(x => + x.Text == propertyTypeGroup.Text && // same name + x.ContentTypeNodeId == propertyType.ContentTypeId); // but for proper content type + + if (okPropertyTypeGroup == null) { - //If the ContentType is the same on the PropertyType and the PropertyTypeGroup the group is valid and we skip to the next - if (parentPropertyTypeGroup.ContentTypeNodeId == propertyType.ContentTypeId) continue; - - //Check if the 'new' PropertyTypeGroup has already been created - var existingPropertyTypeGroup = - propertyGroups.FirstOrDefault( - x => - x.ParentGroupId == parentPropertyTypeGroup.Id && x.Text == parentPropertyTypeGroup.Text && - x.ContentTypeNodeId == propertyType.ContentTypeId); - - //This should ensure that we don't create duplicate groups for a single ContentType - if (existingPropertyTypeGroup == null) + // does not exist, create a new PropertyTypeGroup, + var propertyGroup = new PropertyTypeGroupDto { + ContentTypeNodeId = propertyType.ContentTypeId, + Text = propertyTypeGroup.Text, + SortOrder = propertyTypeGroup.SortOrder + }; - //Create a new PropertyTypeGroup that references the parent group that the PropertyType was referencing pre-6.0.1 - var propertyGroup = new PropertyTypeGroupDto - { - ContentTypeNodeId = propertyType.ContentTypeId, - ParentGroupId = parentPropertyTypeGroup.Id, - Text = parentPropertyTypeGroup.Text, - SortOrder = parentPropertyTypeGroup.SortOrder - }; + // save + add to list of groups + int id = Convert.ToInt16(database.Insert(propertyGroup)); + propertyGroup.Id = id; + propertyGroups.Add(propertyGroup); - //Save the PropertyTypeGroup in the database and update the list of groups with this new group - int id = Convert.ToInt16(database.Insert(propertyGroup)); - propertyGroup.Id = id; - propertyGroups.Add(propertyGroup); - //Update the reference to the new PropertyTypeGroup on the current PropertyType - propertyType.PropertyTypeGroupId = id; - database.Update(propertyType); - } - else - { - //Update the reference to the existing PropertyTypeGroup on the current PropertyType - propertyType.PropertyTypeGroupId = existingPropertyTypeGroup.Id; - database.Update(propertyType); - } + // update the PropertyType to use the new PropertyTypeGroup + propertyType.PropertyTypeGroupId = id; } + else + { + // exists, update PropertyType to use the PropertyTypeGroup + propertyType.PropertyTypeGroupId = okPropertyTypeGroup.Id; + } + database.Update(propertyType); } } diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentTypeBaseRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentTypeBaseRepository.cs index 17af2ca56f..beb88d4ccf 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentTypeBaseRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentTypeBaseRepository.cs @@ -149,8 +149,11 @@ namespace Umbraco.Core.Persistence.Repositories return new PropertyType(propertyEditorAlias, dbType, propertyTypeAlias); } - protected void PersistNewBaseContentType(ContentTypeDto dto, IContentTypeComposition entity) + protected void PersistNewBaseContentType(IContentTypeComposition entity) { + var factory = new ContentTypeFactory(); + var dto = factory.BuildContentTypeDto(entity); + //Cannot add a duplicate content type type var exists = Database.ExecuteScalar(@"SELECT COUNT(*) FROM cmsContentType INNER JOIN umbracoNode ON cmsContentType.nodeId = umbracoNode.id @@ -262,38 +265,35 @@ AND umbracoNode.nodeObjectType = @objectType", } } - protected void PersistUpdatedBaseContentType(ContentTypeDto dto, IContentTypeComposition entity) + protected void PersistUpdatedBaseContentType(IContentTypeComposition entity) { + var factory = new ContentTypeFactory(); + var dto = factory.BuildContentTypeDto(entity); - //Cannot update to a duplicate alias + // ensure the alias is not used already var exists = Database.ExecuteScalar(@"SELECT COUNT(*) FROM cmsContentType INNER JOIN umbracoNode ON cmsContentType.nodeId = umbracoNode.id WHERE cmsContentType." + SqlSyntax.GetQuotedColumnName("alias") + @"= @alias AND umbracoNode.nodeObjectType = @objectType AND umbracoNode.id <> @id", - new { id = dto.NodeId, alias = entity.Alias, objectType = NodeObjectTypeId }); + new { id = dto.NodeId, alias = dto.Alias, objectType = NodeObjectTypeId }); if (exists > 0) - { - throw new DuplicateNameException("An item with the alias " + entity.Alias + " already exists"); - } - - var propertyGroupFactory = new PropertyGroupFactory(entity.Id); + throw new DuplicateNameException("An item with the alias " + dto.Alias + " already exists"); + // handle (update) the node var nodeDto = dto.NodeDto; - var o = Database.Update(nodeDto); + Database.Update(nodeDto); + // fixme - why? we are UPDATING so we should ALREADY have a PK! //Look up ContentType entry to get PrimaryKey for updating the DTO var dtoPk = Database.First("WHERE nodeId = @Id", new { Id = entity.Id }); dto.PrimaryKey = dtoPk.PrimaryKey; Database.Update(dto); - //Delete the ContentType composition entries before adding the updated collection + // handle (delete then recreate) compositions Database.Delete("WHERE childContentTypeId = @Id", new { Id = entity.Id }); - //Update ContentType composition in new table foreach (var composition in entity.ContentTypeComposition) - { Database.Insert(new ContentType2ContentTypeDto { ParentId = composition.Id, ChildId = entity.Id }); - } //Removing a ContentType from a composition (U4-1690) //1. Find content based on the current ContentType: entity.Id @@ -372,54 +372,43 @@ AND umbracoNode.id <> @id", } } - if (entity.IsPropertyDirty("PropertyGroups") || - entity.PropertyGroups.Any(x => x.IsDirty())) + if (entity.IsPropertyDirty("PropertyGroups") || entity.PropertyGroups.Any(x => x.IsDirty())) { - //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)) - .ToList(); - var entityPropertyGroups = entity.PropertyGroups.Select(x => new Tuple(x.Id, x.Name)).ToList(); - 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) + // todo + // we used to try to propagate tabs renaming downstream, relying on ParentId, but + // 1) ParentId makes no sense (if a tab can be inherited from multiple composition + // types) so we would need to figure things out differently, visiting downstream + // content types and looking for tabs with the same name... + // 2) It was not deployable as changing a content type changes other content types + // that was not deterministic, because it would depend on the order of the changes. + // That last point could be fixed if (1) is fixed, but then it still is an issue with + // deploy because changing a content type changes other content types that are not + // dependencies but dependents, and then what? + // + // So... for the time being, all renaming propagation is disabled. We just don't do it. + + // (all gone) + + // delete tabs that do not exist anymore + // get the tabs that are currently existing (in the db) + // get the tabs that we want, now + // and derive the tabs that we want to delete + var existingPropertyGroups = Database.Fetch("WHERE contentTypeNodeId = @id", new { id = entity.Id }) + .Select(x => x.Id) + .ToList(); + var newPropertyGroups = entity.PropertyGroups.Select(x => x.Id).ToList(); + var tabsToDelete = existingPropertyGroups + .Except(newPropertyGroups) + .ToArray(); + + // move properties to generic properties, and delete the tabs + if (tabsToDelete.Length > 0) { - Database.Update("SET Text = @TabName WHERE parentGroupId = @TabId", - new { TabName = propertyGroup.Item2, TabId = propertyGroup.Item1 }); - - var childGroups = Database.Fetch("WHERE parentGroupId = @TabId", new { TabId = propertyGroup.Item1 }); - foreach (var childGroup in childGroups) - { - var sibling = Database.Fetch("WHERE contenttypeNodeId = @Id AND text = @Name", - new { Id = childGroup.ContentTypeNodeId, Name = propertyGroup.Item2 }) - .FirstOrDefault(x => x.ParentGroupId.HasValue == false || x.ParentGroupId.Value.Equals(propertyGroup.Item1) == false); - //If the child group doesn't have a sibling there is no chance of duplicates and we continue - if (sibling == null || (sibling.ParentGroupId.HasValue && sibling.ParentGroupId.Value.Equals(propertyGroup.Item1))) continue; - - //Since the child group has a sibling with the same name we need to point any PropertyTypes to the sibling - //as this child group is about to leave the party. - Database.Update( - "SET propertyTypeGroupId = @PropertyTypeGroupId WHERE propertyTypeGroupId = @PropertyGroupId AND ContentTypeId = @ContentTypeId", - new { PropertyTypeGroupId = sibling.Id, PropertyGroupId = childGroup.Id, ContentTypeId = childGroup.ContentTypeNodeId }); - - //Since the parent group has been renamed and we have duplicates we remove this group - //and leave our sibling in charge of the part. - Database.Delete(childGroup); - } - } - //Do Tab updates - foreach (var tab in tabs) - { - Database.Update("SET propertyTypeGroupId = NULL WHERE propertyTypeGroupId = @PropertyGroupId", - new { PropertyGroupId = tab.Item1 }); - Database.Update("SET parentGroupId = NULL WHERE parentGroupId = @TabId", - new { TabId = tab.Item1 }); - Database.Delete("WHERE contenttypeNodeId = @Id AND text = @Name", - new { Id = entity.Id, Name = tab.Item2 }); + Database.Update("SET propertyTypeGroupId=NULL WHERE propertyTypeGroupId IN (@ids)", new { ids = tabsToDelete }); + Database.Delete("WHERE id IN (@ids)", new { ids = tabsToDelete }); } } + var propertyGroupFactory = new PropertyGroupFactory(entity.Id); //Run through all groups to insert or update entries foreach (var propertyGroup in entity.PropertyGroups) @@ -463,25 +452,6 @@ AND umbracoNode.id <> @id", if (propertyType.HasIdentity == false) propertyType.Id = typePrimaryKey; //Set Id on new PropertyType } - - //If a Composition is removed we need to update/reset references to the PropertyGroups on that ContentType - if (entity.IsPropertyDirty("ContentTypeComposition") && - compositionBase != null && - compositionBase.RemovedContentTypeKeyTracker != null && - compositionBase.RemovedContentTypeKeyTracker.Any()) - { - foreach (var compositionId in compositionBase.RemovedContentTypeKeyTracker) - { - var dbPropertyGroups = - Database.Fetch("WHERE contenttypeNodeId = @Id", new { Id = compositionId }) - .Select(x => x.Id); - foreach (var propertyGroup in dbPropertyGroups) - { - Database.Update("SET parentGroupId = NULL WHERE parentGroupId = @TabId AND contenttypeNodeId = @ContentTypeNodeId", - new { TabId = propertyGroup, ContentTypeNodeId = entity.Id }); - } - } - } } protected IEnumerable GetAllowedContentTypeIds(int id) @@ -901,8 +871,8 @@ AND umbracoNode.id <> @id", //now create the media type object - var factory = new MediaTypeFactory(new Guid(Constants.ObjectTypes.MediaType)); - var mediaType = factory.BuildEntity(contentTypeDto); + var factory = new ContentTypeFactory(); + var mediaType = factory.BuildMediaTypeEntity(contentTypeDto); //map the allowed content types //map the child content type ids @@ -1038,7 +1008,7 @@ AND umbracoNode.id <> @id", var defaultTemplate = defaultTemplates.FirstOrDefault(x => x.Item1.Value) ?? defaultTemplates.FirstOrDefault(); - var dtDto = new DocumentTypeDto + var dtDto = new ContentTypeTemplateDto { //create the content type dto ContentTypeDto = new ContentTypeDto @@ -1090,8 +1060,15 @@ AND umbracoNode.id <> @id", //now create the content type object - var factory = new ContentTypeFactory(new Guid(Constants.ObjectTypes.DocumentType)); - var contentType = factory.BuildEntity(dtDto); + var factory = new ContentTypeFactory(); + var contentType = factory.BuildContentTypeEntity(dtDto.ContentTypeDto); + + // NOTE + // that was done by the factory but makes little sense, moved here, so + // now we have to reset dirty props again (as the factory does it) and yet, + // we are not managing allowed templates... the whole thing is weird. + ((ContentType) contentType).DefaultTemplateId = dtDto.TemplateNodeId; + contentType.ResetDirtyProperties(false); //map the allowed content types //map the child content type ids @@ -1165,7 +1142,7 @@ AND umbracoNode.id <> @id", var sqlBuilder = new StringBuilder(@"SELECT PG.contenttypeNodeId as contentTypeId, PT.ptUniqueId as ptUniqueID, PT.ptId, PT.ptAlias, PT.ptDesc,PT.ptMandatory,PT.ptName,PT.ptSortOrder,PT.ptRegExp, PT.dtId,PT.dtDbType,PT.dtPropEdAlias, - PG.id as pgId, PG.uniqueID as pgKey, PG.parentGroupId as pgParentGroupId, PG.sortorder as pgSortOrder, PG." + sqlSyntax.GetQuotedColumnName("text") + @" as pgText + PG.id as pgId, PG.uniqueID as pgKey, PG.sortorder as pgSortOrder, PG." + sqlSyntax.GetQuotedColumnName("text") + @" as pgText FROM cmsPropertyTypeGroup as PG LEFT JOIN ( @@ -1186,7 +1163,7 @@ AND umbracoNode.id <> @id", PT.uniqueID as ptUniqueID, PT.id as ptId, PT.Alias as ptAlias, PT." + sqlSyntax.GetQuotedColumnName("Description") + @" as ptDesc, PT.mandatory as ptMandatory, PT.Name as ptName, PT.sortOrder as ptSortOrder, PT.validationRegExp as ptRegExp, DT.nodeId as dtId, DT.dbType as dtDbType, DT.propertyEditorAlias as dtPropEdAlias, - PG.id as pgId, PG.uniqueID as pgKey, PG.parentGroupId as pgParentGroupId, PG.sortorder as pgSortOrder, PG." + sqlSyntax.GetQuotedColumnName("text") + @" as pgText + PG.id as pgId, PG.uniqueID as pgKey, PG.sortorder as pgSortOrder, PG." + sqlSyntax.GetQuotedColumnName("text") + @" as pgText FROM cmsPropertyType as PT INNER JOIN cmsDataType as DT ON PT.dataTypeId = DT.nodeId @@ -1219,7 +1196,7 @@ AND umbracoNode.id <> @id", //filter based on the current content type .Where(x => x.contentTypeId == currId) //turn that into a custom object containing only the group info - .Select(x => new { GroupId = x.pgId, ParentGroupId = x.pgParentGroupId, SortOrder = x.pgSortOrder, Text = x.pgText }) + .Select(x => new { GroupId = x.pgId, SortOrder = x.pgSortOrder, Text = x.pgText, Key = x.pgKey }) //get distinct data by id .DistinctBy(x => (int)x.GroupId) //for each of these groups, create a group object with it's associated properties @@ -1243,8 +1220,8 @@ AND umbracoNode.id <> @id", //fill in the rest of the group properties Id = group.GroupId, Name = group.Text, - ParentId = group.ParentGroupId, - SortOrder = group.SortOrder + SortOrder = group.SortOrder, + Key = group.Key }).ToArray()); allPropertyGroupCollection[currId] = propertyGroupCollection; diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentTypeRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentTypeRepository.cs index 04b3e46e3a..9899a765dc 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentTypeRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentTypeRepository.cs @@ -60,7 +60,7 @@ namespace Umbraco.Core.Persistence.Repositories var sql = translator.Translate() .OrderBy(x => x.Text, SqlSyntax); - var dtos = Database.Fetch(sql); + var dtos = Database.Fetch(sql); return dtos.Any() ? GetAll(dtos.DistinctBy(x => x.ContentTypeDto.NodeId).Select(x => x.ContentTypeDto.NodeId).ToArray()) : Enumerable.Empty(); @@ -96,8 +96,8 @@ namespace Umbraco.Core.Persistence.Repositories .From(SqlSyntax) .InnerJoin(SqlSyntax) .On(SqlSyntax, left => left.NodeId, right => right.NodeId) - .LeftJoin(SqlSyntax) - .On(SqlSyntax ,left => left.ContentTypeNodeId, right => right.NodeId) + .LeftJoin(SqlSyntax) + .On(SqlSyntax, left => left.ContentTypeNodeId, right => right.NodeId) .Where(x => x.NodeObjectType == NodeObjectTypeId); return sql; @@ -171,26 +171,39 @@ namespace Umbraco.Core.Persistence.Repositories ((ContentType)entity).AddingEntity(); - var factory = new ContentTypeFactory(NodeObjectTypeId); - var dto = factory.BuildDto(entity); - - PersistNewBaseContentType(dto.ContentTypeDto, entity); - //Inserts data into the cmsDocumentType table if a template exists - if (dto.TemplateNodeId > 0) - { - dto.ContentTypeNodeId = entity.Id; - Database.Insert(dto); - } - - //Insert allowed Templates not including the default one, as that has already been inserted - foreach (var template in entity.AllowedTemplates.Where(x => x != null && x.Id != dto.TemplateNodeId)) - { - Database.Insert(new DocumentTypeDto { ContentTypeNodeId = entity.Id, TemplateNodeId = template.Id, IsDefault = false }); - } + PersistNewBaseContentType(entity); + PersisteTemplates(entity, false); entity.ResetDirtyProperties(); } + protected void PersisteTemplates(IContentType entity, bool clearAll) + { + // remove and insert, if required + Database.Delete("WHERE contentTypeNodeId = @Id", new { Id = entity.Id }); + + // we could do it all in foreach if we assume that the default template is an allowed template?? + var defaultTemplateId = ((ContentType) entity).DefaultTemplateId; + if (defaultTemplateId > 0) + { + Database.Insert(new ContentTypeTemplateDto + { + ContentTypeNodeId = entity.Id, + TemplateNodeId = defaultTemplateId, + IsDefault = true + }); + } + foreach (var template in entity.AllowedTemplates.Where(x => x != null && x.Id != defaultTemplateId)) + { + Database.Insert(new ContentTypeTemplateDto + { + ContentTypeNodeId = entity.Id, + TemplateNodeId = template.Id, + IsDefault = false + }); + } + } + protected override void PersistUpdatedItem(IContentType entity) { ValidateAlias(entity); @@ -211,24 +224,8 @@ namespace Umbraco.Core.Persistence.Repositories entity.SortOrder = maxSortOrder + 1; } - var factory = new ContentTypeFactory(NodeObjectTypeId); - var dto = factory.BuildDto(entity); - - PersistUpdatedBaseContentType(dto.ContentTypeDto, entity); - - //Look up DocumentType entries for updating - this could possibly be a "remove all, insert all"-approach - Database.Delete("WHERE contentTypeNodeId = @Id", new { Id = entity.Id }); - //Insert the updated DocumentTypeDto if a template exists - if (dto.TemplateNodeId > 0) - { - Database.Insert(dto); - } - - //Insert allowed Templates not including the default one, as that has already been inserted - foreach (var template in entity.AllowedTemplates.Where(x => x != null && x.Id != dto.TemplateNodeId)) - { - Database.Insert(new DocumentTypeDto { ContentTypeNodeId = entity.Id, TemplateNodeId = template.Id, IsDefault = false }); - } + PersistUpdatedBaseContentType(entity); + PersisteTemplates(entity, true); entity.ResetDirtyProperties(); } diff --git a/src/Umbraco.Core/Persistence/Repositories/MediaTypeRepository.cs b/src/Umbraco.Core/Persistence/Repositories/MediaTypeRepository.cs index c4fa80a7d5..fcdfaf456d 100644 --- a/src/Umbraco.Core/Persistence/Repositories/MediaTypeRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/MediaTypeRepository.cs @@ -120,10 +120,7 @@ namespace Umbraco.Core.Persistence.Repositories { ((MediaType)entity).AddingEntity(); - var factory = new MediaTypeFactory(NodeObjectTypeId); - var dto = factory.BuildDto(entity); - - PersistNewBaseContentType(dto, entity); + PersistNewBaseContentType(entity); entity.ResetDirtyProperties(); } @@ -148,10 +145,7 @@ namespace Umbraco.Core.Persistence.Repositories entity.SortOrder = maxSortOrder + 1; } - var factory = new MediaTypeFactory(NodeObjectTypeId); - var dto = factory.BuildDto(entity); - - PersistUpdatedBaseContentType(dto, entity); + PersistUpdatedBaseContentType(entity); entity.ResetDirtyProperties(); } diff --git a/src/Umbraco.Core/Persistence/Repositories/MemberTypeRepository.cs b/src/Umbraco.Core/Persistence/Repositories/MemberTypeRepository.cs index 5b4c1ff877..91b6e67cf9 100644 --- a/src/Umbraco.Core/Persistence/Repositories/MemberTypeRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/MemberTypeRepository.cs @@ -102,7 +102,7 @@ namespace Umbraco.Core.Persistence.Repositories "cmsPropertyType.validationRegExp", "cmsPropertyType.dataTypeId", "cmsPropertyType.sortOrder AS PropertyTypeSortOrder", "cmsPropertyType.propertyTypeGroupId AS PropertyTypesGroupId", "cmsMemberType.memberCanEdit", "cmsMemberType.viewOnProfile", "cmsDataType.propertyEditorAlias", "cmsDataType.dbType", "cmsPropertyTypeGroup.id AS PropertyTypeGroupId", - "cmsPropertyTypeGroup.text AS PropertyGroupName", "cmsPropertyTypeGroup.parentGroupId", + "cmsPropertyTypeGroup.text AS PropertyGroupName", "cmsPropertyTypeGroup.sortorder AS PropertyGroupSortOrder", "cmsPropertyTypeGroup.contenttypeNodeId") .From() .InnerJoin().On(left => left.NodeId, right => right.NodeId) @@ -182,12 +182,10 @@ namespace Umbraco.Core.Persistence.Repositories entity.AddPropertyType(standardPropertyType.Value, Constants.Conventions.Member.StandardPropertiesGroupName); } - var factory = new MemberTypeFactory(NodeObjectTypeId); - var dto = factory.BuildDto(entity); + var factory = new ContentTypeFactory(); EnsureExplicitDataTypeForBuiltInProperties(entity); - - PersistNewBaseContentType(dto, entity); + PersistNewBaseContentType(entity); //Handles the MemberTypeDto (cmsMemberType table) var memberTypeDtos = factory.BuildMemberTypeDtos(entity); @@ -219,17 +217,13 @@ namespace Umbraco.Core.Persistence.Repositories entity.SortOrder = maxSortOrder + 1; } - var factory = new MemberTypeFactory(NodeObjectTypeId); - var dto = factory.BuildDto(entity); + var factory = new ContentTypeFactory(); EnsureExplicitDataTypeForBuiltInProperties(entity); + PersistUpdatedBaseContentType(entity); - PersistUpdatedBaseContentType(dto, entity); - - //Remove existing entries before inserting new ones + // remove and insert - handle cmsMemberType table Database.Delete("WHERE NodeId = @Id", new { Id = entity.Id }); - - //Handles the MemberTypeDto (cmsMemberType table) var memberTypeDtos = factory.BuildMemberTypeDtos(entity); foreach (var memberTypeDto in memberTypeDtos) { diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index b9e3c803e0..ce7fe9f5a2 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -410,6 +410,7 @@ + @@ -728,7 +729,6 @@ - @@ -804,7 +804,6 @@ - @@ -1130,7 +1129,7 @@ - + diff --git a/src/Umbraco.Tests/Models/PropertyGroupTests.cs b/src/Umbraco.Tests/Models/PropertyGroupTests.cs index c5ae39be07..ec345051ec 100644 --- a/src/Umbraco.Tests/Models/PropertyGroupTests.cs +++ b/src/Umbraco.Tests/Models/PropertyGroupTests.cs @@ -54,8 +54,7 @@ namespace Umbraco.Tests.Models Key = Guid.NewGuid(), Name = "Group1", SortOrder = 555, - UpdateDate = DateTime.Now, - ParentId = 9 + UpdateDate = DateTime.Now }; var clone = (PropertyGroup)pg.DeepClone(); @@ -68,7 +67,6 @@ namespace Umbraco.Tests.Models Assert.AreEqual(clone.Name, pg.Name); Assert.AreEqual(clone.SortOrder, pg.SortOrder); Assert.AreEqual(clone.UpdateDate, pg.UpdateDate); - Assert.AreEqual(clone.ParentId, pg.ParentId); Assert.AreNotSame(clone.PropertyTypes, pg.PropertyTypes); Assert.AreEqual(clone.PropertyTypes, pg.PropertyTypes); Assert.AreEqual(clone.PropertyTypes.Count, pg.PropertyTypes.Count); @@ -133,8 +131,7 @@ namespace Umbraco.Tests.Models Key = Guid.NewGuid(), Name = "Group1", SortOrder = 555, - UpdateDate = DateTime.Now, - ParentId = 9 + UpdateDate = DateTime.Now }; var result = ss.ToStream(pg); diff --git a/src/Umbraco.Tests/Persistence/BaseTableByTableTest.cs b/src/Umbraco.Tests/Persistence/BaseTableByTableTest.cs index 1da8237582..5aa2649caf 100644 --- a/src/Umbraco.Tests/Persistence/BaseTableByTableTest.cs +++ b/src/Umbraco.Tests/Persistence/BaseTableByTableTest.cs @@ -305,7 +305,7 @@ namespace Umbraco.Tests.Persistence DatabaseSchemaHelper.CreateTable(); DatabaseSchemaHelper.CreateTable(); DatabaseSchemaHelper.CreateTable(); - DatabaseSchemaHelper.CreateTable(); + DatabaseSchemaHelper.CreateTable(); //transaction.Complete(); } diff --git a/src/Umbraco.Tests/Persistence/Mappers/PropertyGroupMapperTest.cs b/src/Umbraco.Tests/Persistence/Mappers/PropertyGroupMapperTest.cs index a8101cad79..490cc8db4a 100644 --- a/src/Umbraco.Tests/Persistence/Mappers/PropertyGroupMapperTest.cs +++ b/src/Umbraco.Tests/Persistence/Mappers/PropertyGroupMapperTest.cs @@ -20,19 +20,6 @@ namespace Umbraco.Tests.Persistence.Mappers Assert.That(column, Is.EqualTo("[cmsPropertyTypeGroup].[id]")); } - [Test] - public void Can_Map_ParentId_Property() - { - // Arrange - SqlSyntaxContext.SqlSyntaxProvider = new SqlCeSyntaxProvider(); - - // Act - string column = new PropertyGroupMapper().Map("ParentId"); - - // Assert - Assert.That(column, Is.EqualTo("[cmsPropertyTypeGroup].[parentGroupId]")); - } - [Test] public void Can_Map_SortOrder_Property() { diff --git a/src/Umbraco.Tests/Persistence/Querying/ContentTypeRepositorySqlClausesTest.cs b/src/Umbraco.Tests/Persistence/Querying/ContentTypeRepositorySqlClausesTest.cs index e25b95383e..1edda026fc 100644 --- a/src/Umbraco.Tests/Persistence/Querying/ContentTypeRepositorySqlClausesTest.cs +++ b/src/Umbraco.Tests/Persistence/Querying/ContentTypeRepositorySqlClausesTest.cs @@ -27,13 +27,13 @@ namespace Umbraco.Tests.Persistence.Querying var sql = new Sql(); sql.Select("*") - .From() + .From() .RightJoin() - .On(left => left.NodeId, right => right.ContentTypeNodeId) + .On(left => left.NodeId, right => right.ContentTypeNodeId) .InnerJoin() .On(left => left.NodeId, right => right.NodeId) .Where(x => x.NodeObjectType == NodeObjectType) - .Where(x => x.IsDefault == true); + .Where(x => x.IsDefault == true); Assert.That(sql.SQL, Is.EqualTo(expected.SQL)); @@ -64,13 +64,13 @@ namespace Umbraco.Tests.Persistence.Querying var sql = new Sql(); sql.Select("*") - .From() + .From() .RightJoin() - .On(left => left.NodeId, right => right.ContentTypeNodeId) + .On(left => left.NodeId, right => right.ContentTypeNodeId) .InnerJoin() .On(left => left.NodeId, right => right.NodeId) .Where(x => x.NodeObjectType == NodeObjectType) - .Where(x => x.IsDefault) + .Where(x => x.IsDefault) .Where(x => x.NodeId == 1050); Assert.That(sql.SQL, Is.EqualTo(expected.SQL)); diff --git a/src/Umbraco.Tests/Persistence/Querying/ContentTypeSqlMappingTests.cs b/src/Umbraco.Tests/Persistence/Querying/ContentTypeSqlMappingTests.cs index f63eea7c88..916b6d3333 100644 --- a/src/Umbraco.Tests/Persistence/Querying/ContentTypeSqlMappingTests.cs +++ b/src/Umbraco.Tests/Persistence/Querying/ContentTypeSqlMappingTests.cs @@ -43,8 +43,8 @@ namespace Umbraco.Tests.Persistence.Querying DatabaseContext.Database.Insert("cmsContentType", "pk", false, new ContentTypeDto { PrimaryKey = 88889, NodeId = 99999, Alias = "TestContentType3", Icon = "icon-folder", Thumbnail = "folder.png", IsContainer = false, AllowAtRoot = true }); DatabaseContext.Database.Execute(new Sql(string.Format("SET IDENTITY_INSERT {0} OFF ", SqlSyntaxContext.SqlSyntaxProvider.GetQuotedTableName("cmsContentType")))); - DatabaseContext.Database.Insert(new DocumentTypeDto { ContentTypeNodeId = 99997, IsDefault = true, TemplateNodeId = 55555 }); - DatabaseContext.Database.Insert(new DocumentTypeDto { ContentTypeNodeId = 99997, IsDefault = false, TemplateNodeId = 55554 }); + DatabaseContext.Database.Insert(new ContentTypeTemplateDto { ContentTypeNodeId = 99997, IsDefault = true, TemplateNodeId = 55555 }); + DatabaseContext.Database.Insert(new ContentTypeTemplateDto { ContentTypeNodeId = 99997, IsDefault = false, TemplateNodeId = 55554 }); DatabaseContext.Database.Insert(new ContentTypeAllowedContentTypeDto { AllowedId = 99998, Id = 99997, SortOrder = 1 }); DatabaseContext.Database.Insert(new ContentTypeAllowedContentTypeDto { AllowedId = 99999, Id = 99997, SortOrder = 2}); diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs index 4b3b300dd5..c70aac47a2 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs @@ -1162,7 +1162,6 @@ namespace Umbraco.Tests.Services var contentType = service.GetContentType("contentPage"); var propertyGroup = contentType.PropertyGroups["Content"]; - Assert.That(propertyGroup.ParentId.HasValue, Is.False); } [Test] @@ -1247,7 +1246,6 @@ namespace Umbraco.Tests.Services var contentType = service.GetContentType("contentPage"); var propertyGroup = contentType.PropertyGroups["Content"]; - Assert.That(propertyGroup.ParentId.HasValue, Is.False); var numberOfContentTabs = contentType.CompositionPropertyGroups.Count(x => x.Name.Equals("Content")); Assert.That(numberOfContentTabs, Is.EqualTo(3)); @@ -1266,7 +1264,6 @@ namespace Umbraco.Tests.Services var propertyGroupReloaded = contentPageReloaded.PropertyGroups["Content"]; var hasDescriptionPropertyType = propertyGroupReloaded.PropertyTypes.Contains("description"); Assert.That(hasDescriptionPropertyType, Is.True); - Assert.That(propertyGroupReloaded.ParentId.HasValue, Is.False); var descriptionPropertyTypeReloaded = propertyGroupReloaded.PropertyTypes["description"]; Assert.That(descriptionPropertyTypeReloaded.PropertyGroupId.IsValueCreated, Is.False); diff --git a/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs b/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs index b97f069b12..09839bf251 100644 --- a/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs +++ b/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs @@ -185,16 +185,6 @@ namespace Umbraco.Tests.TestHelpers.Entities var pg = new PropertyGroup(contentCollection) {Name = propertyGroupName, SortOrder = 1}; contentType.PropertyGroups.Add(pg); - if (parent != null) - { - var foundPg = parent.PropertyGroups.FirstOrDefault(x => x.Name == propertyGroupName); - if (foundPg != null) - { - //this exists on the parent, so set the parent id - pg.SetLazyParentId(new Lazy(() => foundPg.Id)); - } - } - //ensure that nothing is marked as dirty contentType.ResetDirtyProperties(false); diff --git a/src/Umbraco.Web.UI/config/ClientDependency.config b/src/Umbraco.Web.UI/config/ClientDependency.config index 2f0bc30fad..a0c0655dbe 100644 --- a/src/Umbraco.Web.UI/config/ClientDependency.config +++ b/src/Umbraco.Web.UI/config/ClientDependency.config @@ -10,7 +10,7 @@ NOTES: * Compression/Combination/Minification is not enabled unless debug="false" is specified on the 'compiliation' element in the web.config * A new version will invalidate both client and server cache and create new persisted files --> - +