From a3f171e7b304f117b10105ba061fd74c46223962 Mon Sep 17 00:00:00 2001 From: Stephan Date: Tue, 9 Dec 2014 17:58:54 +0100 Subject: [PATCH 01/29] U4-5931 - fix DocType inheritance issue --- .../controls/ContentTypeControlNew.ascx.cs | 111 +++++++++--------- 1 file changed, 56 insertions(+), 55 deletions(-) diff --git a/src/Umbraco.Web/umbraco.presentation/umbraco/controls/ContentTypeControlNew.ascx.cs b/src/Umbraco.Web/umbraco.presentation/umbraco/controls/ContentTypeControlNew.ascx.cs index b0fb5bacb0..3d4cf04cce 100644 --- a/src/Umbraco.Web/umbraco.presentation/umbraco/controls/ContentTypeControlNew.ascx.cs +++ b/src/Umbraco.Web/umbraco.presentation/umbraco/controls/ContentTypeControlNew.ascx.cs @@ -322,56 +322,63 @@ namespace umbraco.controls var ids = SaveAllowedChildTypes(); _contentType.ContentTypeItem.AllowedContentTypes = ids.Select(x => new ContentTypeSort {Id = new Lazy(() => x), SortOrder = i++}); - //Saving ContentType Compositions - var compositionIds = SaveCompositionContentTypes(); - var existingCompsitionIds = _contentType.ContentTypeItem.CompositionIds().ToList(); - if (compositionIds.Any()) + // figure out whether compositions are locked + var allContentTypes = Request.Path.ToLowerInvariant().Contains("editmediatype.aspx") + ? ApplicationContext.Services.ContentTypeService.GetAllMediaTypes().Cast().ToArray() + : ApplicationContext.Services.ContentTypeService.GetAllContentTypes().Cast().ToArray(); + var isUsing = allContentTypes.Where(x => x.ContentTypeComposition.Any(y => y.Id == _contentType.Id)).ToArray(); + + // if compositions are locked, do nothing (leave them as they are) + // else process the checkbox list and add/remove compositions accordingly + + if (isUsing.Length == 0) { - //Iterate ContentType Ids from the save-collection - foreach (var compositionId in compositionIds) + //Saving ContentType Compositions + var compositionIds = SaveCompositionContentTypes(); + var existingCompsitionIds = _contentType.ContentTypeItem.CompositionIds().ToList(); + if (compositionIds.Any()) { - //If the compositionId is the Id of the current ContentType we skip it - if(_contentType.Id.Equals(compositionId)) continue; + // if some compositions were checked in the list, iterate over them + foreach (var compositionId in compositionIds) + { + // ignore if it is the current content type + if (_contentType.Id.Equals(compositionId)) continue; - //If the Id already exists we'll just skip it - if (existingCompsitionIds.Any(x => x.Equals(compositionId))) continue; + // ignore if it is already a composition of the content type + if (existingCompsitionIds.Any(x => x.Equals(compositionId))) continue; - //New Ids will get added to the collection - var compositionType = isMediaType - ? Services.ContentTypeService.GetMediaType(compositionId) - .SafeCast() - : Services.ContentTypeService.GetContentType(compositionId) - .SafeCast(); - var added = _contentType.ContentTypeItem.AddContentType(compositionType); - //TODO if added=false then return error message + // add to the content type compositions + var compositionType = isMediaType + ? Services.ContentTypeService.GetMediaType(compositionId).SafeCast() + : Services.ContentTypeService.GetContentType(compositionId).SafeCast(); + var added = _contentType.ContentTypeItem.AddContentType(compositionType); + //TODO if added=false then return error message + } + + // then iterate over removed = existing except checked + var removeIds = existingCompsitionIds.Except(compositionIds); + foreach (var removeId in removeIds) + { + // and remove from the content type composition + var compositionType = isMediaType + ? Services.ContentTypeService.GetMediaType(removeId).SafeCast() + : Services.ContentTypeService.GetContentType(removeId).SafeCast(); + var removed = _contentType.ContentTypeItem.RemoveContentType(compositionType.Alias); + } } - - //Iterate the set except of existing and new Ids - var removeIds = existingCompsitionIds.Except(compositionIds); - foreach (var removeId in removeIds) + else if (existingCompsitionIds.Any()) { - //Remove ContentTypes that was deselected in the list - var compositionType = isMediaType - ? Services.ContentTypeService.GetMediaType(removeId) - .SafeCast() - : Services.ContentTypeService.GetContentType(removeId) - .SafeCast(); - var removed = _contentType.ContentTypeItem.RemoveContentType(compositionType.Alias); - } - } - else if (existingCompsitionIds.Any()) - { - //Iterate the set except of existing and new Ids - var removeIds = existingCompsitionIds.Except(compositionIds); - foreach (var removeId in removeIds) - { - //Remove ContentTypes that was deselected in the list - var compositionType = isMediaType - ? Services.ContentTypeService.GetMediaType(removeId) - .SafeCast() - : Services.ContentTypeService.GetContentType(removeId) - .SafeCast(); - var removed = _contentType.ContentTypeItem.RemoveContentType(compositionType.Alias); + // else none were checked - if the content type had compositions, + // iterate over them all and remove them + var removeIds = existingCompsitionIds.Except(compositionIds); // except here makes no sense? + foreach (var removeId in removeIds) + { + // remove existing + var compositionType = isMediaType + ? Services.ContentTypeService.GetMediaType(removeId).SafeCast() + : Services.ContentTypeService.GetContentType(removeId).SafeCast(); + var removed = _contentType.ContentTypeItem.RemoveContentType(compositionType.Alias); + } } } @@ -710,7 +717,7 @@ jQuery(document).ready(function() {{ refreshDropDowns(); }}); DualContentTypeCompositions.Value = ""; PlaceHolderContentTypeCompositions.Controls.Add(new Literal { Text = "This content type is used as a parent and/or in " - + "a composition, and therefore cannot be composed itself.

" + + "a composition, and therefore cannot be composed itself.

Used by: " + string.Join(", ", isUsing.Select(x => x.Name)) + "" }); } @@ -795,16 +802,10 @@ jQuery(document).ready(function() {{ refreshDropDowns(); }}); private int[] SaveCompositionContentTypes() { - var tmp = new ArrayList(); - foreach (ListItem li in lstContentTypeCompositions.Items) - { - if (li.Selected) - tmp.Add(int.Parse(li.Value)); - } - var ids = new int[tmp.Count]; - for (int i = 0; i < tmp.Count; i++) ids[i] = (int)tmp[i]; - - return ids; + return lstContentTypeCompositions.Items.Cast() + .Where(x => x.Selected) + .Select(x => int.Parse(x.Value)) + .ToArray(); } #endregion From f1107b7a91f44873802aab827be760280a77bd4a Mon Sep 17 00:00:00 2001 From: Sebastiaan Janssen Date: Tue, 9 Dec 2014 19:30:48 +0100 Subject: [PATCH 02/29] Fixes U4-5986 Compositions: Possible to get stuck in document type ysod's #U4-5986 Fixed Due in version 7.2.1 --- .../umbraco/create/nodetypeTasks.cs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/Umbraco.Web/umbraco.presentation/umbraco/create/nodetypeTasks.cs b/src/Umbraco.Web/umbraco.presentation/umbraco/create/nodetypeTasks.cs index 07c33f3dc4..04e0f45851 100644 --- a/src/Umbraco.Web/umbraco.presentation/umbraco/create/nodetypeTasks.cs +++ b/src/Umbraco.Web/umbraco.presentation/umbraco/create/nodetypeTasks.cs @@ -24,13 +24,18 @@ namespace umbraco //NOTE: TypeID is the parent id! //NOTE: ParentID is aparently a flag to determine if we are to create a template! Hack much ?! :P var parentId = TypeID != 0 ? TypeID : -1; - var contentType = parentId == -1 - ? new ContentType(-1) - : new ContentType(ApplicationContext.Current.Services.ContentTypeService.GetContentType(parentId)); - contentType.CreatorId = User.Id; + // when creating a content type, enforce PascalCase // preserve separator because contentType.Alias will re-alias it - contentType.Alias = Alias.ToCleanString(CleanStringType.Alias | CleanStringType.PascalCase, ' '); + var cleanAlias = Alias.ToCleanString(CleanStringType.Alias | CleanStringType.PascalCase, ' '); + + var contentType = parentId == -1 + ? new ContentType(-1) + : new ContentType(ApplicationContext.Current.Services.ContentTypeService.GetContentType(parentId), cleanAlias); + + contentType.CreatorId = User.Id; + + contentType.Alias = cleanAlias; contentType.Name = Alias.Replace("'", "''"); contentType.Icon = ".sprTreeFolder"; From 899d325ca16763f4e3184a49258a895e616a4e77 Mon Sep 17 00:00:00 2001 From: Sebastiaan Janssen Date: Wed, 10 Dec 2014 11:42:45 +0100 Subject: [PATCH 03/29] Updates unit tests to use the newer new ContentType method and obsoletes the old one --- src/Umbraco.Core/Models/ContentType.cs | 17 +++++++++-------- .../PublishedContentExtensionTests.cs | 2 +- .../Services/ContentTypeServiceTests.cs | 6 +++--- .../TestHelpers/Entities/MockedContentTypes.cs | 2 +- 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/Umbraco.Core/Models/ContentType.cs b/src/Umbraco.Core/Models/ContentType.cs index f8f19ef8d1..fc53a21c3f 100644 --- a/src/Umbraco.Core/Models/ContentType.cs +++ b/src/Umbraco.Core/Models/ContentType.cs @@ -15,7 +15,7 @@ namespace Umbraco.Core.Models { private int _defaultTemplate; private IEnumerable _allowedTemplates; - + /// /// Constuctor for creating a ContentType with the parent's id. /// @@ -31,9 +31,10 @@ namespace Umbraco.Core.Models /// /// Use this to ensure inheritance from parent. /// - public ContentType(IContentType parent) : this(parent, null) - { - } + [Obsolete("This method is obsolete, use ContentType(IContentType parent, string alias) instead.", false)] + public ContentType(IContentType parent) : this(parent, null) + { + } /// /// Constuctor for creating a ContentType with the parent as an inherited type. @@ -49,7 +50,7 @@ namespace Umbraco.Core.Models private static readonly PropertyInfo DefaultTemplateSelector = ExpressionHelper.GetPropertyInfo(x => x.DefaultTemplateId); private static readonly PropertyInfo AllowedTemplatesSelector = ExpressionHelper.GetPropertyInfo>(x => x.AllowedTemplates); - + /// /// Gets or sets the alias of the default Template. /// @@ -106,7 +107,7 @@ namespace Umbraco.Core.Models } DefaultTemplateId = template.Id; - if(_allowedTemplates.Any(x => x != null && x.Id == template.Id) == false) + if (_allowedTemplates.Any(x => x != null && x.Id == template.Id) == false) { var templates = AllowedTemplates.ToList(); templates.Add(template); @@ -140,7 +141,7 @@ namespace Umbraco.Core.Models { base.AddingEntity(); - if(Key == Guid.Empty) + if (Key == Guid.Empty) Key = Guid.NewGuid(); } @@ -151,7 +152,7 @@ namespace Umbraco.Core.Models /// [Obsolete("Use DeepCloneWithResetIdentities instead")] public IContentType Clone(string alias) - { + { return DeepCloneWithResetIdentities(alias); } diff --git a/src/Umbraco.Tests/PublishedContent/PublishedContentExtensionTests.cs b/src/Umbraco.Tests/PublishedContent/PublishedContentExtensionTests.cs index 5a9067a095..b5f1b255c6 100644 --- a/src/Umbraco.Tests/PublishedContent/PublishedContentExtensionTests.cs +++ b/src/Umbraco.Tests/PublishedContent/PublishedContentExtensionTests.cs @@ -74,7 +74,7 @@ namespace Umbraco.Tests.PublishedContent { var contentTypeService = ctx.Application.Services.ContentTypeService; var baseType = new ContentType(-1) {Alias = "base", Name = "Base"}; - var inheritedType = new ContentType(baseType) {Alias = "inherited", Name = "Inherited"}; + var inheritedType = new ContentType(baseType, baseType.Alias) {Alias = "inherited", Name = "Inherited"}; contentTypeService.Save(baseType); contentTypeService.Save(inheritedType); createContentTypes = false; diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs index 5898c27853..7799d46474 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs @@ -178,7 +178,7 @@ namespace Umbraco.Tests.Services /*,"Navigation"*/); cts.Save(ctBase); - var ctHomePage = new ContentType(ctBase) + var ctHomePage = new ContentType(ctBase, ctBase.Alias) { Name = "Home Page", Alias = "HomePage", @@ -487,7 +487,7 @@ namespace Umbraco.Tests.Services private ContentType CreateBannerComponent(ContentType parent) { - var banner = new ContentType(parent) + var banner = new ContentType(parent, parent.Alias) { Alias = "banner", Name = "Banner Component", @@ -535,7 +535,7 @@ namespace Umbraco.Tests.Services private ContentType CreateHomepage(ContentType parent) { - var contentType = new ContentType(parent) + var contentType = new ContentType(parent, parent.Alias) { Alias = "homepage", Name = "Homepage", diff --git a/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs b/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs index 812495d3da..4227a8303b 100644 --- a/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs +++ b/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs @@ -94,7 +94,7 @@ namespace Umbraco.Tests.TestHelpers.Entities public static ContentType CreateSimpleContentType(string alias, string name, IContentType parent = null, bool randomizeAliases = false) { - var contentType = parent == null ? new ContentType(-1) : new ContentType(parent); + var contentType = parent == null ? new ContentType(-1) : new ContentType(parent, parent.Alias); contentType.Alias = alias; contentType.Name = name; From d445f7cb0d8a9e21b2247ffd130bcfaf73214ecb Mon Sep 17 00:00:00 2001 From: Stephan Date: Wed, 10 Dec 2014 12:02:54 +0100 Subject: [PATCH 04/29] U4-5986 - reject ppty alias used by descendant --- .../Services/ContentTypeService.cs | 69 ++++++++++++++++++- .../controls/ContentTypeControlNew.ascx.cs | 5 ++ 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Core/Services/ContentTypeService.cs b/src/Umbraco.Core/Services/ContentTypeService.cs index b87dfa1ac1..ca3261f868 100644 --- a/src/Umbraco.Core/Services/ContentTypeService.cs +++ b/src/Umbraco.Core/Services/ContentTypeService.cs @@ -8,11 +8,13 @@ using System.Threading; using Umbraco.Core.Auditing; using Umbraco.Core.Configuration; using Umbraco.Core.Events; +using Umbraco.Core.Exceptions; using Umbraco.Core.Logging; using Umbraco.Core.Models; using Umbraco.Core.Models.Rdbms; using Umbraco.Core.Persistence; using Umbraco.Core.Persistence.Querying; +using Umbraco.Core.Persistence.Repositories; using Umbraco.Core.Persistence.UnitOfWork; namespace Umbraco.Core.Services @@ -273,6 +275,60 @@ namespace Umbraco.Core.Services } + public void Validate(IContentTypeComposition compo) + { + using (new WriteLock(Locker)) + { + ValidateLocked(compo); + } + } + + private void ValidateLocked(IContentTypeComposition compo) + { + // performs business-level validation of the composition + // should ensure that it is absolutely safe to save the composition + + // eg maybe a property has been added, with an alias that's OK (no conflict with ancestors) + // but that cannot be used (conflict with descendants) + + var contentType = compo as IContentType; + var mediaType = compo as IMediaType; + + IContentTypeComposition[] allContentTypes; + if (contentType != null) + allContentTypes = GetAllContentTypes().Cast().ToArray(); + else if (mediaType != null) + allContentTypes = GetAllMediaTypes().Cast().ToArray(); + else + throw new Exception("Composition is neither IContentType nor IMediaType?"); + + // recursively find all descendants + var comparer = new DelegateEqualityComparer((x, y) => x.Id == y.Id, x => x.Id); + var descendants = new HashSet(comparer); + var stack = new Stack(); + foreach (var z in allContentTypes.Where(x => x.ContentTypeComposition.Any(y => y.Id == compo.Id))) + stack.Push(z); + while (stack.Count > 0) + { + var c = stack.Pop(); + descendants.Add(c); + foreach (var z in allContentTypes.Where(x => x.ContentTypeComposition.Any(y => y.Id == c.Id))) + stack.Push(z); + } + + // ensure that no descendant has a property with an alias that is used by content type + var aliases = compo.PropertyTypes.Select(x => x.Alias.ToLowerInvariant()).ToArray(); + foreach (var d in descendants) + { + var intersect = d.PropertyTypes.Select(x => x.Alias.ToLowerInvariant()).Intersect(aliases).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); + } + } + /// /// Saves a single object /// @@ -288,6 +344,7 @@ namespace Umbraco.Core.Services var uow = _uowProvider.GetUnitOfWork(); using (var repository = _repositoryFactory.CreateContentTypeRepository(uow)) { + ValidateLocked(contentType); // throws if invalid contentType.CreatorId = userId; repository.AddOrUpdate(contentType); @@ -317,6 +374,11 @@ namespace Umbraco.Core.Services var uow = _uowProvider.GetUnitOfWork(); using (var repository = _repositoryFactory.CreateContentTypeRepository(uow)) { + // all-or-nothing, validate them all first + foreach (var contentType in asArray) + { + ValidateLocked(contentType); // throws if invalid + } foreach (var contentType in asArray) { contentType.CreatorId = userId; @@ -487,6 +549,7 @@ namespace Umbraco.Core.Services var uow = _uowProvider.GetUnitOfWork(); using (var repository = _repositoryFactory.CreateMediaTypeRepository(uow)) { + ValidateLocked(mediaType); // throws if invalid mediaType.CreatorId = userId; repository.AddOrUpdate(mediaType); uow.Commit(); @@ -517,7 +580,11 @@ namespace Umbraco.Core.Services var uow = _uowProvider.GetUnitOfWork(); using (var repository = _repositoryFactory.CreateMediaTypeRepository(uow)) { - + // all-or-nothing, validate them all first + foreach (var mediaType in asArray) + { + ValidateLocked(mediaType); // throws if invalid + } foreach (var mediaType in asArray) { mediaType.CreatorId = userId; diff --git a/src/Umbraco.Web/umbraco.presentation/umbraco/controls/ContentTypeControlNew.ascx.cs b/src/Umbraco.Web/umbraco.presentation/umbraco/controls/ContentTypeControlNew.ascx.cs index 3d4cf04cce..a1f4762aa6 100644 --- a/src/Umbraco.Web/umbraco.presentation/umbraco/controls/ContentTypeControlNew.ascx.cs +++ b/src/Umbraco.Web/umbraco.presentation/umbraco/controls/ContentTypeControlNew.ascx.cs @@ -426,6 +426,11 @@ namespace umbraco.controls state.SaveArgs.Message = ex.Message; return; } + catch (Exception ex) + { + state.SaveArgs.IconType = BasePage.speechBubbleIcon.error; + state.SaveArgs.Message = ex.Message; + } Trace.Write("ContentTypeControlNew", "task completing"); }; From d11a377f0fa487f438ecbbcc1c75a4b47dee2e78 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 9 Dec 2014 18:55:54 +1100 Subject: [PATCH 05/29] bump version --- build/UmbracoVersion.txt | 2 +- src/Umbraco.Core/Configuration/UmbracoVersion.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/build/UmbracoVersion.txt b/build/UmbracoVersion.txt index 450cb62597..85a02bb894 100644 --- a/build/UmbracoVersion.txt +++ b/build/UmbracoVersion.txt @@ -1,2 +1,2 @@ # Usage: on line 2 put the release version, on line 3 put the version comment (example: beta) -7.2.0 \ No newline at end of file +7.2.1 \ No newline at end of file diff --git a/src/Umbraco.Core/Configuration/UmbracoVersion.cs b/src/Umbraco.Core/Configuration/UmbracoVersion.cs index 1012ce793f..f7163bfe7f 100644 --- a/src/Umbraco.Core/Configuration/UmbracoVersion.cs +++ b/src/Umbraco.Core/Configuration/UmbracoVersion.cs @@ -5,7 +5,7 @@ namespace Umbraco.Core.Configuration { public class UmbracoVersion { - private static readonly Version Version = new Version("7.2.0"); + private static readonly Version Version = new Version("7.2.1"); /// /// Gets the current version of Umbraco. From da2bc6764b45b449effc1fd89bc13fe89d80c1b7 Mon Sep 17 00:00:00 2001 From: Sebastiaan Janssen Date: Wed, 10 Dec 2014 12:59:08 +0100 Subject: [PATCH 06/29] Version also updates in the csproj --- src/Umbraco.Web.UI/Umbraco.Web.UI.csproj | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj index fa41a70892..e5eb27e012 100644 --- a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj +++ b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj @@ -2542,9 +2542,9 @@ xcopy "$(ProjectDir)"..\packages\SqlServerCE.4.0.0.0\x86\*.* "$(TargetDir)x86\" True True - 7200 + 7210 / - http://localhost:7200 + http://localhost:7210 False False From 99c96bce86478cd4c5b656a8820db3b15bd294cb Mon Sep 17 00:00:00 2001 From: Stephan Date: Wed, 10 Dec 2014 13:00:56 +0100 Subject: [PATCH 07/29] U4-5986 - reject ppty alias used by ancestor --- .../Services/ContentTypeService.cs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/Umbraco.Core/Services/ContentTypeService.cs b/src/Umbraco.Core/Services/ContentTypeService.cs index ca3261f868..0acbf8b693 100644 --- a/src/Umbraco.Core/Services/ContentTypeService.cs +++ b/src/Umbraco.Core/Services/ContentTypeService.cs @@ -327,6 +327,30 @@ namespace Umbraco.Core.Services string.Join(", ", intersect)); throw new Exception(message); } + + // find all ancestors + var ancestors = new HashSet(comparer); + stack.Clear(); + foreach (var z in compo.ContentTypeComposition) + stack.Push(z); + while (stack.Count > 0) + { + var c = stack.Pop(); + ancestors.Add(c); + foreach (var z in c.ContentTypeComposition) + stack.Push(z); + } + + // ensure that no ancestor has a property with an alias that is used by content type + foreach (var a in ancestors) + { + var intersect = a.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}.", + string.Join(", ", intersect)); + throw new Exception(message); + } } /// From 01670e08329f1b228b088bc6b0b307f9c8630a24 Mon Sep 17 00:00:00 2001 From: Peter Gregory Date: Thu, 11 Dec 2014 16:27:06 +1000 Subject: [PATCH 08/29] Fixes issue U4-5925 GetMedia returning nothing Removed the use of FromXElement that was only used by GetMedia. The change introduced in commit f3f65043987ab73bc3b29c64e3453091853dea06 caused GetMedia to return nothing due to the introduction of the FromXElement method. --- .../umbraco.presentation/library.cs | 32 +++++++++++++------ 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/src/Umbraco.Web/umbraco.presentation/library.cs b/src/Umbraco.Web/umbraco.presentation/library.cs index 8b74c54929..ae76941688 100644 --- a/src/Umbraco.Web/umbraco.presentation/library.cs +++ b/src/Umbraco.Web/umbraco.presentation/library.cs @@ -501,14 +501,23 @@ namespace umbraco if (xml != null) { - return FromXElement(xml, xml.Attribute("nodeTypeAlias").Value); + //removed the use of FromXElement as it was causing GetMedia to return nothing + //return FromXElement(xml, xml.Attribute("nodeTypeAlias").Value); + + //returning the root element of the Media item fixes the problem + return xml.CreateNavigator().Select("/"); } } else { var xml = GetMediaDo(MediaId, Deep); - return FromXElement(xml, xml.Attribute("nodeTypeAlias").Value); + + //removed the use of FromXElement as it was causing GetMedia to return nothing + //return FromXElement(xml, xml.Attribute("nodeTypeAlias").Value); + + //returning the root element of the Media item fixes the problem + return xml.CreateNavigator().Select("/"); } } catch(Exception ex) @@ -536,14 +545,17 @@ namespace umbraco return serialized; } - private static XPathNodeIterator FromXElement(XNode xml, string mediaContentType) - { - var xp = xml.CreateNavigator(); - var xpath = UmbracoConfig.For.UmbracoSettings().Content.UseLegacyXmlSchema - ? "/node" - : String.Format("/{0}", Casing.SafeAliasWithForcingCheck(mediaContentType)); - return xp.Select(xpath); - } + /*This was the offending method that I dont think is really necessary + it only returns the innerXML which causes the GetMedia to break */ + + //private static XPathNodeIterator FromXElement(XNode xml, string mediaContentType) + //{ + // var xp = xml.CreateNavigator(); + // var xpath = UmbracoConfig.For.UmbracoSettings().Content.UseLegacyXmlSchema + // ? "/node" + // : String.Format("/{0}", Casing.SafeAliasWithForcingCheck(mediaContentType)); + // return xp.Select(xpath); + //} /// /// Get a member as an xml object From 52ad36a6c27b69da2913723d0ef5670214beb180 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 11 Dec 2014 18:42:51 +1100 Subject: [PATCH 09/29] removes commented out code form PR --- .../umbraco.presentation/library.cs | 20 +------------------ 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/src/Umbraco.Web/umbraco.presentation/library.cs b/src/Umbraco.Web/umbraco.presentation/library.cs index ae76941688..3a557b3873 100644 --- a/src/Umbraco.Web/umbraco.presentation/library.cs +++ b/src/Umbraco.Web/umbraco.presentation/library.cs @@ -500,10 +500,7 @@ namespace umbraco () => GetMediaDo(MediaId, Deep)); if (xml != null) - { - //removed the use of FromXElement as it was causing GetMedia to return nothing - //return FromXElement(xml, xml.Attribute("nodeTypeAlias").Value); - + { //returning the root element of the Media item fixes the problem return xml.CreateNavigator().Select("/"); } @@ -513,9 +510,6 @@ namespace umbraco { var xml = GetMediaDo(MediaId, Deep); - //removed the use of FromXElement as it was causing GetMedia to return nothing - //return FromXElement(xml, xml.Attribute("nodeTypeAlias").Value); - //returning the root element of the Media item fixes the problem return xml.CreateNavigator().Select("/"); } @@ -545,18 +539,6 @@ namespace umbraco return serialized; } - /*This was the offending method that I dont think is really necessary - it only returns the innerXML which causes the GetMedia to break */ - - //private static XPathNodeIterator FromXElement(XNode xml, string mediaContentType) - //{ - // var xp = xml.CreateNavigator(); - // var xpath = UmbracoConfig.For.UmbracoSettings().Content.UseLegacyXmlSchema - // ? "/node" - // : String.Format("/{0}", Casing.SafeAliasWithForcingCheck(mediaContentType)); - // return xp.Select(xpath); - //} - /// /// Get a member as an xml object /// From 504c1f72549443fff69b8c9ddca106c70d077225 Mon Sep 17 00:00:00 2001 From: Morten Christensen Date: Wed, 10 Dec 2014 22:19:17 +0100 Subject: [PATCH 10/29] Updating tests related to #U4-5986 Conflicts: src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs --- .../Services/ContentTypeServiceTests.cs | 85 +++++++++++++++++++ .../Entities/MockedContentTypes.cs | 27 +++++- 2 files changed, 109 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs index 7799d46474..a986bb678e 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs @@ -464,6 +464,91 @@ namespace Umbraco.Tests.Services Assert.AreNotEqual(clonedContentType.PropertyGroups.First(x => x.Name.StartsWith("Content")).Id, originalContentType.PropertyGroups.First(x => x.Name.StartsWith("Content")).Id); } + [Test] + 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 + + // Arrange + var service = ServiceContext.ContentTypeService; + + var parent = MockedContentTypes.CreateSimpleContentType(); + service.Save(parent); + var child = MockedContentTypes.CreateSimpleContentType("simpleChildPage", "Simple Child Page", parent, true); + service.Save(child); + var composition = MockedContentTypes.CreateMetaContentType(); + service.Save(composition); + + //Adding Meta-composition to child doc type + child.AddContentType(composition); + service.Save(child); + + // Act + var duplicatePropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "title", Name = "Title", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var added = composition.AddPropertyType(duplicatePropertyType, "Meta"); + service.Save(composition); + + // Assert + Assert.That(added, Is.True); + Assert.DoesNotThrow(() => service.GetContentType("simpleChildPage")); + } + + [Test] + public void Can_Rename_PropertyGroup_With_Inherited_PropertyGroups() + { + //Related the first issue in screencast from this post http://issues.umbraco.org/issue/U4-5986 + + // Arrange + var service = ServiceContext.ContentTypeService; + + var page = MockedContentTypes.CreateSimpleContentType("page", "Page", null, false, "Content_"); + service.Save(page); + var contentPage = MockedContentTypes.CreateSimpleContentType("contentPage", "Content Page", page, true); + service.Save(contentPage); + var composition = MockedContentTypes.CreateMetaContentType(); + composition.AddPropertyGroup("Content"); + service.Save(composition); + //Adding Meta-composition to child doc type + contentPage.AddContentType(composition); + service.Save(contentPage); + + // Act + var propertyTypeOne = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "testTextbox", Name = "Test Textbox", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var firstOneAdded = contentPage.AddPropertyType(propertyTypeOne, "Content_"); + var propertyTypeTwo = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "anotherTextbox", Name = "Another Test Textbox", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var secondOneAdded = contentPage.AddPropertyType(propertyTypeTwo, "Content"); + service.Save(contentPage); + + Assert.That(page.PropertyGroups.Contains("Content_"), Is.True); + var propertyGroup = page.PropertyGroups["Content_"]; + page.PropertyGroups.Add(new PropertyGroup{ Id = propertyGroup.Id, Name = "ContentTab", SortOrder = 0}); + service.Save(page); + + // Assert + Assert.That(firstOneAdded, Is.True); + Assert.That(secondOneAdded, Is.True); + + var contentType = service.GetContentType("contentPage"); + Assert.That(contentType, Is.Not.Null); + + var compositionPropertyGroups = contentType.CompositionPropertyGroups; + Assert.That(compositionPropertyGroups.Count(x => x.Name.Equals("Content_")), Is.EqualTo(0)); + + var propertyTypeCount = contentType.PropertyTypes.Count(); + var compPropertyTypeCount = contentType.CompositionPropertyTypes.Count(); + Assert.That(propertyTypeCount, Is.EqualTo(5)); + Assert.That(compPropertyTypeCount, Is.EqualTo(10)); + } + private ContentType CreateComponent() { var component = new ContentType(-1) diff --git a/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs b/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs index 4227a8303b..c2ecc1e3e2 100644 --- a/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs +++ b/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs @@ -92,9 +92,9 @@ namespace Umbraco.Tests.TestHelpers.Entities return contentType; } - public static ContentType CreateSimpleContentType(string alias, string name, IContentType parent = null, bool randomizeAliases = false) + public static ContentType CreateSimpleContentType(string alias, string name, IContentType parent = null, bool randomizeAliases = false, string propertyGroupName = "Content") { - var contentType = parent == null ? new ContentType(-1) : new ContentType(parent, parent.Alias); + var contentType = parent == null ? new ContentType(-1) : new ContentType(parent, alias); contentType.Alias = alias; contentType.Name = name; @@ -110,7 +110,7 @@ namespace Umbraco.Tests.TestHelpers.Entities contentCollection.Add(new PropertyType(Constants.PropertyEditors.TinyMCEAlias, DataTypeDatabaseType.Ntext) { Alias = RandomAlias("bodyText", randomizeAliases), Name = "Body Text", Description = "", HelpText = "", Mandatory = false, SortOrder = 2, DataTypeDefinitionId = -87 }); contentCollection.Add(new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) { Alias = RandomAlias("author", randomizeAliases) , Name = "Author", Description = "Name of the author", HelpText = "", Mandatory = false, SortOrder = 3, DataTypeDefinitionId = -88 }); - contentType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = "Content", SortOrder = 1 }); + contentType.PropertyGroups.Add(new PropertyGroup(contentCollection) { Name = propertyGroupName, SortOrder = 1 }); //ensure that nothing is marked as dirty contentType.ResetDirtyProperties(false); @@ -167,6 +167,27 @@ namespace Umbraco.Tests.TestHelpers.Entities return contentType; } + public static ContentType CreateSimpleContentType(string alias, string name, PropertyTypeCollection collection, string propertyGroupName, IContentType parent = null) + { + var contentType = parent == null ? new ContentType(-1) : new ContentType(parent, alias); + + contentType.Alias = alias; + contentType.Name = name; + contentType.Description = "ContentType used for simple text pages"; + contentType.Icon = ".sprTreeDoc3"; + contentType.Thumbnail = "doc2.png"; + contentType.SortOrder = 1; + contentType.CreatorId = 0; + contentType.Trashed = false; + + contentType.PropertyGroups.Add(new PropertyGroup(collection) { Name = propertyGroupName, SortOrder = 1 }); + + //ensure that nothing is marked as dirty + contentType.ResetDirtyProperties(false); + + return contentType; + } + public static ContentType CreateSimpleContentType(string alias, string name, PropertyTypeCollection groupedCollection, PropertyTypeCollection nonGroupedCollection) { var contentType = CreateSimpleContentType(alias, name, groupedCollection); From 57985c26a2669a6f6be38c7acc841f3884697f54 Mon Sep 17 00:00:00 2001 From: Morten Christensen Date: Wed, 10 Dec 2014 22:20:00 +0100 Subject: [PATCH 11/29] Updating the name of PropertyGroups in DB to ensure renaming is done properly --- .../Models/PropertyGroupCollection.cs | 39 +++++++++++++++++-- .../Repositories/ContentTypeBaseRepository.cs | 9 ++++- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/src/Umbraco.Core/Models/PropertyGroupCollection.cs b/src/Umbraco.Core/Models/PropertyGroupCollection.cs index 6e1847856b..3162634ceb 100644 --- a/src/Umbraco.Core/Models/PropertyGroupCollection.cs +++ b/src/Umbraco.Core/Models/PropertyGroupCollection.cs @@ -71,16 +71,30 @@ namespace Umbraco.Core.Models { using (new WriteLock(_addLocker)) { - var key = GetKeyForItem(item); - if (key != null) + //Note this is done to ensure existig groups can be renamed + if (item.HasIdentity && item.Id > 0) { - var exists = this.Contains(key); + var exists = this.Contains(item.Id); if (exists) { - SetItem(IndexOfKey(key), item); + SetItem(IndexOfKey(item.Id), item); return; } } + else + { + var key = GetKeyForItem(item); + if (key != null) + { + var exists = this.Contains(key); + if (exists) + { + SetItem(IndexOfKey(key), item); + return; + } + } + } + base.Add(item); OnAdd.IfNotNull(x => x.Invoke());//Could this not be replaced by a Mandate/Contract for ensuring item is not null @@ -99,6 +113,11 @@ namespace Umbraco.Core.Models return this.Any(x => x.Name == groupName); } + public bool Contains(int id) + { + return this.Any(x => x.Id == id); + } + public void RemoveItem(string propertyGroupName) { var key = IndexOfKey(propertyGroupName); @@ -119,6 +138,18 @@ namespace Umbraco.Core.Models return -1; } + public int IndexOfKey(int id) + { + for (var i = 0; i < this.Count; i++) + { + if (this[i].Id == id) + { + return i; + } + } + return -1; + } + protected override string GetKeyForItem(PropertyGroup item) { return item.Name; diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentTypeBaseRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentTypeBaseRepository.cs index 2533fe62e8..81a7321cbe 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentTypeBaseRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentTypeBaseRepository.cs @@ -298,8 +298,15 @@ AND umbracoNode.id <> @id", var dbPropertyGroups = Database.Fetch("WHERE contenttypeNodeId = @Id", new {Id = entity.Id}) .Select(x => new Tuple(x.Id, x.Text)); - var entityPropertyGroups = entity.PropertyGroups.Select(x => new Tuple(x.Id, x.Name)); + var entityPropertyGroups = entity.PropertyGroups.Select(x => new Tuple(x.Id, x.Name)).ToList(); var tabs = dbPropertyGroups.Except(entityPropertyGroups); + //Update Tab name downstream to ensure renaming is done properly + foreach (var propertyGroup in entityPropertyGroups) + { + Database.Update("SET Text = @TabName WHERE parentGroupId = @TabId", + new { TabId = propertyGroup.Item1, TabName = propertyGroup.Item2 }); + } + //Do Tab updates foreach (var tab in tabs) { Database.Update("SET propertyTypeGroupId = NULL WHERE propertyTypeGroupId = @PropertyGroupId", From 7e7acd00495e45b0f609c95083ef66451749e302 Mon Sep 17 00:00:00 2001 From: Sebastiaan Janssen Date: Thu, 11 Dec 2014 11:02:51 +0100 Subject: [PATCH 12/29] Correcting my mistakes to the unit test, using the wrong alias --- .../PublishedContentExtensionTests.cs | 3 ++- .../Services/ContentTypeServiceTests.cs | 17 ++++++++++------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/Umbraco.Tests/PublishedContent/PublishedContentExtensionTests.cs b/src/Umbraco.Tests/PublishedContent/PublishedContentExtensionTests.cs index b5f1b255c6..e2e1b91087 100644 --- a/src/Umbraco.Tests/PublishedContent/PublishedContentExtensionTests.cs +++ b/src/Umbraco.Tests/PublishedContent/PublishedContentExtensionTests.cs @@ -74,7 +74,8 @@ namespace Umbraco.Tests.PublishedContent { var contentTypeService = ctx.Application.Services.ContentTypeService; var baseType = new ContentType(-1) {Alias = "base", Name = "Base"}; - var inheritedType = new ContentType(baseType, baseType.Alias) {Alias = "inherited", Name = "Inherited"}; + const string contentTypeAlias = "inherited"; + var inheritedType = new ContentType(baseType, contentTypeAlias) {Alias = contentTypeAlias, Name = "Inherited"}; contentTypeService.Save(baseType); contentTypeService.Save(inheritedType); createContentTypes = false; diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs index a986bb678e..3aea421ad7 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs @@ -178,10 +178,11 @@ namespace Umbraco.Tests.Services /*,"Navigation"*/); cts.Save(ctBase); - var ctHomePage = new ContentType(ctBase, ctBase.Alias) + const string contentTypeAlias = "HomePage"; + var ctHomePage = new ContentType(ctBase, contentTypeAlias) { Name = "Home Page", - Alias = "HomePage", + Alias = contentTypeAlias, Icon = "settingDomain.gif", Thumbnail = "folder.png", AllowedAsRoot = true @@ -191,7 +192,7 @@ namespace Umbraco.Tests.Services cts.Save(ctHomePage); // Act - var homeDoc = cs.CreateContent("Home Page", -1, "HomePage"); + var homeDoc = cs.CreateContent("Home Page", -1, contentTypeAlias); cs.SaveAndPublishWithStatus(homeDoc); // Assert @@ -572,9 +573,10 @@ namespace Umbraco.Tests.Services private ContentType CreateBannerComponent(ContentType parent) { - var banner = new ContentType(parent, parent.Alias) + const string contentTypeAlias = "banner"; + var banner = new ContentType(parent, contentTypeAlias) { - Alias = "banner", + Alias = contentTypeAlias, Name = "Banner Component", Description = "ContentType used for Banner Component", Icon = ".sprTreeDoc3", @@ -620,9 +622,10 @@ namespace Umbraco.Tests.Services private ContentType CreateHomepage(ContentType parent) { - var contentType = new ContentType(parent, parent.Alias) + const string contentTypeAlias = "homepage"; + var contentType = new ContentType(parent, contentTypeAlias) { - Alias = "homepage", + Alias = contentTypeAlias, Name = "Homepage", Description = "ContentType used for the Homepage", Icon = ".sprTreeDoc3", From 07f9a9700d974e2c886ed24d1c2880c2c0a79a9f Mon Sep 17 00:00:00 2001 From: Morten Christensen Date: Thu, 11 Dec 2014 12:22:49 +0100 Subject: [PATCH 13/29] Ensures that inherited PropertyType aliases are also checked for indirect references --- .../Services/ContentTypeService.cs | 40 +++++++++---------- .../Services/ContentTypeServiceTests.cs | 8 ++-- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/Umbraco.Core/Services/ContentTypeService.cs b/src/Umbraco.Core/Services/ContentTypeService.cs index 0acbf8b693..176b7fc3fe 100644 --- a/src/Umbraco.Core/Services/ContentTypeService.cs +++ b/src/Umbraco.Core/Services/ContentTypeService.cs @@ -283,7 +283,7 @@ namespace Umbraco.Core.Services } } - private void ValidateLocked(IContentTypeComposition compo) + private void ValidateLocked(IContentTypeComposition compositionContentType) { // performs business-level validation of the composition // should ensure that it is absolutely safe to save the composition @@ -291,8 +291,8 @@ namespace Umbraco.Core.Services // eg maybe a property has been added, with an alias that's OK (no conflict with ancestors) // but that cannot be used (conflict with descendants) - var contentType = compo as IContentType; - var mediaType = compo as IMediaType; + var contentType = compositionContentType as IContentType; + var mediaType = compositionContentType as IMediaType; IContentTypeComposition[] allContentTypes; if (contentType != null) @@ -306,21 +306,21 @@ namespace Umbraco.Core.Services var comparer = new DelegateEqualityComparer((x, y) => x.Id == y.Id, x => x.Id); var descendants = new HashSet(comparer); var stack = new Stack(); - foreach (var z in allContentTypes.Where(x => x.ContentTypeComposition.Any(y => y.Id == compo.Id))) - stack.Push(z); + foreach (var composition in allContentTypes.Where(x => x.ContentTypeComposition.Any(y => y.Id == compositionContentType.Id))) + stack.Push(composition); while (stack.Count > 0) { - var c = stack.Pop(); - descendants.Add(c); - foreach (var z in allContentTypes.Where(x => x.ContentTypeComposition.Any(y => y.Id == c.Id))) - stack.Push(z); + 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); } // ensure that no descendant has a property with an alias that is used by content type - var aliases = compo.PropertyTypes.Select(x => x.Alias.ToLowerInvariant()).ToArray(); - foreach (var d in descendants) + var aliases = compositionContentType.PropertyTypes.Select(x => x.Alias.ToLowerInvariant()).ToArray(); + foreach (var descendant in descendants) { - var intersect = d.PropertyTypes.Select(x => x.Alias.ToLowerInvariant()).Intersect(aliases).ToArray(); + var intersect = descendant.CompositionPropertyTypes.Select(x => x.Alias.ToLowerInvariant()).Intersect(aliases).ToArray(); if (intersect.Length == 0) continue; var message = string.Format("The following property aliases conflict with descendants : {0}.", @@ -331,20 +331,20 @@ namespace Umbraco.Core.Services // find all ancestors var ancestors = new HashSet(comparer); stack.Clear(); - foreach (var z in compo.ContentTypeComposition) - stack.Push(z); + foreach (var composition in compositionContentType.ContentTypeComposition) + stack.Push(composition); while (stack.Count > 0) { - var c = stack.Pop(); - ancestors.Add(c); - foreach (var z in c.ContentTypeComposition) - stack.Push(z); + 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 a in ancestors) + foreach (var ancestor in ancestors) { - var intersect = a.PropertyTypes.Select(x => x.Alias.ToLowerInvariant()).Intersect(aliases).ToArray(); + 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}.", diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs index 3aea421ad7..8185170638 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs @@ -244,13 +244,13 @@ namespace Umbraco.Tests.Services var global = MockedContentTypes.CreateSimpleContentType("global", "Global"); service.Save(global); - var components = MockedContentTypes.CreateSimpleContentType("components", "Components", global); + var components = MockedContentTypes.CreateSimpleContentType("components", "Components", global, true); service.Save(components); - var component = MockedContentTypes.CreateSimpleContentType("component", "Component", components); + var component = MockedContentTypes.CreateSimpleContentType("component", "Component", components, true); service.Save(component); - var category = MockedContentTypes.CreateSimpleContentType("category", "Category", global); + var category = MockedContentTypes.CreateSimpleContentType("category", "Category", global, true); service.Save(category); var success = category.AddContentType(component); @@ -490,10 +490,10 @@ namespace Umbraco.Tests.Services Alias = "title", Name = "Title", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 }; var added = composition.AddPropertyType(duplicatePropertyType, "Meta"); - service.Save(composition); // Assert Assert.That(added, Is.True); + Assert.Throws(() => service.Save(composition)); Assert.DoesNotThrow(() => service.GetContentType("simpleChildPage")); } From a446cc224d8c0b845747a59fbf75b1973d83dde2 Mon Sep 17 00:00:00 2001 From: Morten Christensen Date: Thu, 11 Dec 2014 17:14:01 +0100 Subject: [PATCH 14/29] Refactoring the logic in the validation method for when ContentTypes are saved --- .../Repositories/ContentTypeBaseRepository.cs | 6 +- .../Services/ContentTypeService.cs | 57 ++++------- .../Services/ContentTypeServiceTests.cs | 95 ++++++++++++++++++- .../Entities/MockedContentTypes.cs | 51 ++++++++++ 4 files changed, 167 insertions(+), 42 deletions(-) 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) From 8f26f9385db0158be9d6ad165c8855021d9c70d8 Mon Sep 17 00:00:00 2001 From: Morten Christensen Date: Fri, 12 Dec 2014 17:02:03 +0100 Subject: [PATCH 15/29] Refactoring the logic around saving updates to PropertyTypes and PropertyGroups --- src/Umbraco.Core/Models/ContentTypeBase.cs | 4 +++- .../Models/PropertyGroupCollection.cs | 4 ++++ src/Umbraco.Core/Models/PropertyType.cs | 3 ++- .../Repositories/ContentTypeBaseRepository.cs | 23 ++++++++++++++++++- 4 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Core/Models/ContentTypeBase.cs b/src/Umbraco.Core/Models/ContentTypeBase.cs index 618b5bd3bc..30ac526f4a 100644 --- a/src/Umbraco.Core/Models/ContentTypeBase.cs +++ b/src/Umbraco.Core/Models/ContentTypeBase.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Collections.Specialized; using System.ComponentModel; +using System.Diagnostics; using System.Linq; using System.Reflection; using System.Runtime.Serialization; @@ -15,6 +16,7 @@ namespace Umbraco.Core.Models /// [Serializable] [DataContract(IsReference = true)] + [DebuggerDisplay("Id: {Id}, Name: {Name}, Alias: {Alias}")] public abstract class ContentTypeBase : Entity, IContentTypeBase { private Lazy _parentId; @@ -497,7 +499,6 @@ 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)) @@ -524,6 +525,7 @@ namespace Umbraco.Core.Models public void RemovePropertyGroup(string propertyGroupName) { PropertyGroups.RemoveItem(propertyGroupName); + OnPropertyChanged(PropertyGroupCollectionSelector); } /// diff --git a/src/Umbraco.Core/Models/PropertyGroupCollection.cs b/src/Umbraco.Core/Models/PropertyGroupCollection.cs index 3162634ceb..399fa22be6 100644 --- a/src/Umbraco.Core/Models/PropertyGroupCollection.cs +++ b/src/Umbraco.Core/Models/PropertyGroupCollection.cs @@ -77,6 +77,10 @@ namespace Umbraco.Core.Models var exists = this.Contains(item.Id); if (exists) { + var keyExists = this.Contains(item.Name); + if(keyExists) + throw new Exception(string.Format("Naming conflict: Changing the name of PropertyGroup '{0}' would result in duplicates", item.Name)); + SetItem(IndexOfKey(item.Id), item); return; } diff --git a/src/Umbraco.Core/Models/PropertyType.cs b/src/Umbraco.Core/Models/PropertyType.cs index fb7e82b27e..a19b7d98c2 100644 --- a/src/Umbraco.Core/Models/PropertyType.cs +++ b/src/Umbraco.Core/Models/PropertyType.cs @@ -1,9 +1,9 @@ using System; +using System.Diagnostics; using System.Reflection; using System.Runtime.Serialization; using System.Text.RegularExpressions; using Umbraco.Core.Models.EntityBase; -using Umbraco.Core.Persistence; using Umbraco.Core.PropertyEditors; using Umbraco.Core.Strings; @@ -14,6 +14,7 @@ namespace Umbraco.Core.Models /// [Serializable] [DataContract(IsReference = true)] + [DebuggerDisplay("Id: {Id}, Name: {Name}, Alias: {Alias}")] public class PropertyType : Entity, IEquatable { private readonly bool _isExplicitDbType; diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentTypeBaseRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentTypeBaseRepository.cs index 541f9c5c88..8c1b24329c 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentTypeBaseRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentTypeBaseRepository.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Data; +using System.Diagnostics; using System.Globalization; using System.Linq; using System.Text; @@ -306,7 +307,27 @@ AND umbracoNode.id <> @id", foreach (var propertyGroup in entityPropertyGroups) { Database.Update("SET Text = @TabName WHERE parentGroupId = @TabId", - new { TabId = propertyGroup.Item1, TabName = propertyGroup.Item2 }); + 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) From ed899ec73dafc66818710691d43b0edf2835759b Mon Sep 17 00:00:00 2001 From: Morten Christensen Date: Fri, 12 Dec 2014 17:04:11 +0100 Subject: [PATCH 16/29] Adding additional test coverage for Compositions and updating PropertyTypes and PropertyGroups --- .../Services/ContentTypeServiceTests.cs | 311 ++++++++++++++++++ .../Entities/MockedContentTypes.cs | 20 +- 2 files changed, 330 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs index b6a9792cfa..7fe1ba4eeb 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs @@ -1,3 +1,4 @@ +using System.Runtime.Remoting; using NUnit.Framework; using System; using System.Collections.Generic; @@ -5,6 +6,7 @@ using System.Linq; using Umbraco.Core; using Umbraco.Core.Models; using Umbraco.Core.Models.Rdbms; +using Umbraco.Core.Persistence.Caching; using Umbraco.Tests.CodeFirst.TestModels.Composition; using Umbraco.Tests.TestHelpers; using Umbraco.Tests.TestHelpers.Entities; @@ -550,6 +552,67 @@ namespace Umbraco.Tests.Services Assert.DoesNotThrow(() => service.GetContentType("seo")); } + [Test] + public void Cannot_Rename_PropertyGroup_On_Child_Avoiding_Conflict_With_Parent_PropertyGroup() + { + // Arrange + var service = ServiceContext.ContentTypeService; + var page = MockedContentTypes.CreateSimpleContentType("page", "Page", null, true, "Content"); + service.Save(page); + var contentPage = MockedContentTypes.CreateSimpleContentType("contentPage", "Content Page", page, true, "Content_"); + service.Save(contentPage); + var advancedPage = MockedContentTypes.CreateSimpleContentType("advancedPage", "Advanced Page", contentPage, true, "Details"); + service.Save(advancedPage); + + var contentMetaComposition = MockedContentTypes.CreateContentMetaContentType(); + service.Save(contentMetaComposition); + + // Act + var subtitlePropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "subtitle", + Name = "Subtitle", + Description = "", + HelpText = "", + Mandatory = false, + SortOrder = 1, + DataTypeDefinitionId = -88 + }; + var authorPropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "author", + Name = "Author", + Description = "", + HelpText = "", + Mandatory = false, + SortOrder = 1, + DataTypeDefinitionId = -88 + }; + var subtitleAdded = contentPage.AddPropertyType(subtitlePropertyType, "Content"); + var authorAdded = contentPage.AddPropertyType(authorPropertyType, "Content"); + service.Save(contentPage); + + var compositionAdded = contentPage.AddContentType(contentMetaComposition); + service.Save(contentPage); + + //Change the name of the tab on the "root" content type 'page'. + var propertyGroup = contentPage.PropertyGroups["Content_"]; + Assert.Throws(() => contentPage.PropertyGroups.Add(new PropertyGroup + { + Id = propertyGroup.Id, + Name = "Content", + SortOrder = 0 + })); + + // Assert + Assert.That(compositionAdded, Is.True); + Assert.That(subtitleAdded, Is.True); + Assert.That(authorAdded, Is.True); + + Assert.DoesNotThrow(() => service.GetContentType("contentPage")); + Assert.DoesNotThrow(() => service.GetContentType("advancedPage")); + } + [Test] public void Can_Add_PropertyType_Alias_Which_Exists_In_Composition_Outside_Graph() { @@ -643,6 +706,254 @@ namespace Umbraco.Tests.Services Assert.That(compPropertyTypeCount, Is.EqualTo(10)); } + [Test] + public void Can_Rename_PropertyGroup_On_Parent_Without_Causing_Duplicate_PropertyGroups() + { + // Arrange + var service = ServiceContext.ContentTypeService; + var page = MockedContentTypes.CreateSimpleContentType("page", "Page", null, true, "Content_"); + service.Save(page); + var contentPage = MockedContentTypes.CreateSimpleContentType("contentPage", "Content Page", page, true, "Contentx"); + service.Save(contentPage); + var advancedPage = MockedContentTypes.CreateSimpleContentType("advancedPage", "Advanced Page", contentPage, true, "Contenty"); + service.Save(advancedPage); + + var contentMetaComposition = MockedContentTypes.CreateContentMetaContentType(); + service.Save(contentMetaComposition); + var compositionAdded = contentPage.AddContentType(contentMetaComposition); + service.Save(contentPage); + + // Act + var bodyTextPropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "bodyText", Name = "Body Text", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var subtitlePropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "subtitle", Name = "Subtitle", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var bodyTextAdded = contentPage.AddPropertyType(bodyTextPropertyType, "Content_");//Will be added to the parent tab + var subtitleAdded = contentPage.AddPropertyType(subtitlePropertyType, "Content");//Will be added to the "Content Meta" composition + service.Save(contentPage); + + var authorPropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "author", Name = "Author", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var descriptionPropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "description", Name = "Description", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var keywordsPropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "keywords", Name = "Keywords", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var authorAdded = advancedPage.AddPropertyType(authorPropertyType, "Content_");//Will be added to an ancestor tab + var descriptionAdded = advancedPage.AddPropertyType(descriptionPropertyType, "Contentx");//Will be added to a parent tab + var keywordsAdded = advancedPage.AddPropertyType(keywordsPropertyType, "Content");//Will be added to the "Content Meta" composition + service.Save(advancedPage); + + //Change the name of the tab on the "root" content type 'page'. + var propertyGroup = page.PropertyGroups["Content_"]; + page.PropertyGroups.Add(new PropertyGroup { Id = propertyGroup.Id, Name = "Content", SortOrder = 0 }); + service.Save(page); + + // Assert + Assert.That(compositionAdded, Is.True); + Assert.That(bodyTextAdded, Is.True); + Assert.That(subtitleAdded, Is.True); + Assert.That(authorAdded, Is.True); + Assert.That(descriptionAdded, Is.True); + Assert.That(keywordsAdded, Is.True); + + Assert.DoesNotThrow(() => service.GetContentType("contentPage")); + Assert.DoesNotThrow(() => service.GetContentType("advancedPage")); + + var advancedPageReloaded = service.GetContentType("advancedPage"); + var contentUnderscoreTabExists = advancedPageReloaded.CompositionPropertyGroups.Any(x => x.Name.Equals("Content_")); + Assert.That(contentUnderscoreTabExists, Is.False); + + var numberOfContentTabs = advancedPageReloaded.CompositionPropertyGroups.Count(x => x.Name.Equals("Content")); + Assert.That(numberOfContentTabs, Is.EqualTo(4)); + } + + [Test] + public void Can_Rename_PropertyGroup_On_Parent_Without_Causing_Duplicate_PropertyGroups_v2() + { + // Arrange + var service = ServiceContext.ContentTypeService; + var page = MockedContentTypes.CreateSimpleContentType("page", "Page", null, true, "Content_"); + service.Save(page); + var contentPage = MockedContentTypes.CreateSimpleContentType("contentPage", "Content Page", page, true, "Content"); + service.Save(contentPage); + + var contentMetaComposition = MockedContentTypes.CreateContentMetaContentType(); + service.Save(contentMetaComposition); + + // Act + var bodyTextPropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "bodyText", Name = "Body Text", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var subtitlePropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "subtitle", Name = "Subtitle", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var authorPropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "author", Name = "Author", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var bodyTextAdded = page.AddPropertyType(bodyTextPropertyType, "Content_"); + var subtitleAdded = contentPage.AddPropertyType(subtitlePropertyType, "Content"); + var authorAdded = contentPage.AddPropertyType(authorPropertyType, "Content_"); + service.Save(page); + service.Save(contentPage); + + var compositionAdded = contentPage.AddContentType(contentMetaComposition); + service.Save(contentPage); + + //Change the name of the tab on the "root" content type 'page'. + var propertyGroup = page.PropertyGroups["Content_"]; + page.PropertyGroups.Add(new PropertyGroup { Id = propertyGroup.Id, Name = "Content", SortOrder = 0 }); + service.Save(page); + + // Assert + Assert.That(compositionAdded, Is.True); + Assert.That(bodyTextAdded, Is.True); + Assert.That(subtitleAdded, Is.True); + Assert.That(authorAdded, Is.True); + + Assert.DoesNotThrow(() => service.GetContentType("contentPage")); + } + + [Test] + public void Can_Remove_PropertyGroup_On_Parent_Without_Causing_Duplicate_PropertyGroups() + { + // Arrange + var service = ServiceContext.ContentTypeService; + var basePage = MockedContentTypes.CreateBasicContentType(); + service.Save(basePage); + + var contentPage = MockedContentTypes.CreateBasicContentType("contentPage", "Content Page", basePage); + service.Save(contentPage); + + var advancedPage = MockedContentTypes.CreateBasicContentType("advancedPage", "Advanced Page", contentPage); + service.Save(advancedPage); + + var contentMetaComposition = MockedContentTypes.CreateContentMetaContentType(); + service.Save(contentMetaComposition); + + // Act + var bodyTextPropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "bodyText", Name = "Body Text", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var bodyTextAdded = basePage.AddPropertyType(bodyTextPropertyType, "Content"); + service.Save(basePage); + + var authorPropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "author", Name = "Author", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var authorAdded = contentPage.AddPropertyType(authorPropertyType, "Content"); + service.Save(contentPage); + + var compositionAdded = contentPage.AddContentType(contentMetaComposition); + service.Save(contentPage); + + basePage.RemovePropertyGroup("Content"); + service.Save(basePage); + + // Assert + Assert.That(bodyTextAdded, Is.True); + Assert.That(authorAdded, Is.True); + Assert.That(compositionAdded, Is.True); + + Assert.DoesNotThrow(() => service.GetContentType("contentPage")); + Assert.DoesNotThrow(() => service.GetContentType("advancedPage")); + + var contentType = service.GetContentType("contentPage"); + var propertyGroup = contentType.PropertyGroups["Content"]; + Assert.That(propertyGroup.ParentId.HasValue, Is.False); + } + + [Test] + public void Can_Add_PropertyGroup_With_Same_Name_On_Parent_and_Child() + { + /* + * BasePage + * - Content Page + * -- Advanced Page + * Content Meta :: Composition + */ + + // Arrange + var service = ServiceContext.ContentTypeService; + var basePage = MockedContentTypes.CreateBasicContentType(); + service.Save(basePage); + + var contentPage = MockedContentTypes.CreateBasicContentType("contentPage", "Content Page", basePage); + service.Save(contentPage); + + var advancedPage = MockedContentTypes.CreateBasicContentType("advancedPage", "Advanced Page", contentPage); + service.Save(advancedPage); + + var contentMetaComposition = MockedContentTypes.CreateContentMetaContentType(); + service.Save(contentMetaComposition); + + // Act + var authorPropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "author", Name = "Author", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var authorAdded = contentPage.AddPropertyType(authorPropertyType, "Content"); + service.Save(contentPage); + + var bodyTextPropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "bodyText", Name = "Body Text", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var bodyTextAdded = basePage.AddPropertyType(bodyTextPropertyType, "Content"); + service.Save(basePage); + + var compositionAdded = contentPage.AddContentType(contentMetaComposition); + service.Save(contentPage); + + // Assert + Assert.That(bodyTextAdded, Is.True); + Assert.That(authorAdded, Is.True); + Assert.That(compositionAdded, Is.True); + + Assert.DoesNotThrow(() => service.GetContentType("contentPage")); + Assert.DoesNotThrow(() => service.GetContentType("advancedPage")); + + 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)); + + //Ensure that adding a new PropertyType to the "Content"-tab also adds it to the right group + + var descriptionPropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "description", Name = "Description", Description = "", HelpText = "", Mandatory = false, SortOrder = 1,DataTypeDefinitionId = -88 + }; + var descriptionAdded = contentType.AddPropertyType(descriptionPropertyType, "Content"); + service.Save(contentType); + Assert.That(descriptionAdded, Is.True); + + var contentPageReloaded = service.GetContentType("contentPage"); + 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); + } + private ContentType CreateComponent() { var component = new ContentType(-1) diff --git a/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs b/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs index 9f13017580..39916453dc 100644 --- a/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs +++ b/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs @@ -6,7 +6,25 @@ namespace Umbraco.Tests.TestHelpers.Entities { public class MockedContentTypes { - + public static ContentType CreateBasicContentType(string alias = "basePage", string name = "Base Page", + ContentType parent = null) + { + var contentType = parent == null ? new ContentType(-1) : new ContentType(parent, alias); + + contentType.Alias = alias; + contentType.Name = name; + contentType.Description = "ContentType used for basic pages"; + contentType.Icon = ".sprTreeDoc3"; + contentType.Thumbnail = "doc2.png"; + contentType.SortOrder = 1; + contentType.CreatorId = 0; + contentType.Trashed = false; + + //ensure that nothing is marked as dirty + contentType.ResetDirtyProperties(false); + + return contentType; + } public static ContentType CreateTextpageContentType(string alias = "textPage", string name = "Text Page") { From 2a9960afb37bbf20e3bd602b5ee9d7202e3ae282 Mon Sep 17 00:00:00 2001 From: Morten Christensen Date: Fri, 12 Dec 2014 17:04:53 +0100 Subject: [PATCH 17/29] Adding UI feedback for invalid compositions --- .../umbraco/controls/ContentTypeControlNew.ascx.cs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Web/umbraco.presentation/umbraco/controls/ContentTypeControlNew.ascx.cs b/src/Umbraco.Web/umbraco.presentation/umbraco/controls/ContentTypeControlNew.ascx.cs index a1f4762aa6..34462105a0 100644 --- a/src/Umbraco.Web/umbraco.presentation/umbraco/controls/ContentTypeControlNew.ascx.cs +++ b/src/Umbraco.Web/umbraco.presentation/umbraco/controls/ContentTypeControlNew.ascx.cs @@ -16,6 +16,7 @@ using System.Web.UI.WebControls; using ClientDependency.Core; using Umbraco.Core; using Umbraco.Core.Configuration; +using Umbraco.Core.Exceptions; using Umbraco.Core.Logging; using Umbraco.Core.Models; using Umbraco.Core.Strings; @@ -351,8 +352,16 @@ namespace umbraco.controls var compositionType = isMediaType ? Services.ContentTypeService.GetMediaType(compositionId).SafeCast() : Services.ContentTypeService.GetContentType(compositionId).SafeCast(); - var added = _contentType.ContentTypeItem.AddContentType(compositionType); - //TODO if added=false then return error message + try + { + //TODO if added=false then return error message + var added = _contentType.ContentTypeItem.AddContentType(compositionType); + } + catch (InvalidCompositionException ex) + { + state.SaveArgs.IconType = BasePage.speechBubbleIcon.error; + state.SaveArgs.Message = ex.Message; + } } // then iterate over removed = existing except checked From 7316c5462a34bbeea50769b4149725e2c1fbd1b0 Mon Sep 17 00:00:00 2001 From: Sebastiaan Janssen Date: Sun, 14 Dec 2014 16:50:11 +0100 Subject: [PATCH 18/29] U4-5928 Umbraco 7.2 RC GetCurrentMemberProfileModel Null Ref #U4-5928 Fixed Due in version 7.2.1 --- src/Umbraco.Web/Security/MembershipHelper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Web/Security/MembershipHelper.cs b/src/Umbraco.Web/Security/MembershipHelper.cs index 0faf9a61a3..f066476948 100644 --- a/src/Umbraco.Web/Security/MembershipHelper.cs +++ b/src/Umbraco.Web/Security/MembershipHelper.cs @@ -415,7 +415,7 @@ namespace Umbraco.Web.Security if (member != null) { var propValue = member.Properties[prop.Alias]; - if (propValue != null) + if (propValue != null && propValue.Value != null) { value = propValue.Value.ToString(); } From a480d7bbe6417e6ce29939ee034b51ae40aba29e Mon Sep 17 00:00:00 2001 From: Sebastiaan Janssen Date: Sun, 14 Dec 2014 17:22:10 +0100 Subject: [PATCH 19/29] U4-5959 Getting YSOD when wanting to create partial view in a subfolder in the backoffie #U4-5959 Fixed Due in version 7.2.1 --- .../umbraco/config/create/UI.Release.xml | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Web.UI/umbraco/config/create/UI.Release.xml b/src/Umbraco.Web.UI/umbraco/config/create/UI.Release.xml index d05908e2cf..464fc05154 100644 --- a/src/Umbraco.Web.UI/umbraco/config/create/UI.Release.xml +++ b/src/Umbraco.Web.UI/umbraco/config/create/UI.Release.xml @@ -130,7 +130,7 @@ - +
member
/create/member.ascx @@ -323,6 +323,14 @@ +
Macro
+ /Create/PartialView.ascx + + + + + +
Macro
/Create/PartialView.ascx @@ -338,4 +346,12 @@ + +
Macro
+ /Create/PartialViewMacro.ascx + + + + + From 60a1116acd116d0d83423ceb007577835833daf8 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 15 Dec 2014 11:45:55 +1100 Subject: [PATCH 20/29] Fixes: U4-6009 Distributed calls not working in 7.2 --- .../umbraco/controls/ContentTypeControlNew.ascx.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Web/umbraco.presentation/umbraco/controls/ContentTypeControlNew.ascx.cs b/src/Umbraco.Web/umbraco.presentation/umbraco/controls/ContentTypeControlNew.ascx.cs index 34462105a0..9db83ae975 100644 --- a/src/Umbraco.Web/umbraco.presentation/umbraco/controls/ContentTypeControlNew.ascx.cs +++ b/src/Umbraco.Web/umbraco.presentation/umbraco/controls/ContentTypeControlNew.ascx.cs @@ -309,7 +309,7 @@ namespace umbraco.controls Trace.Write("ContentTypeControlNew", "executing task"); //we need to re-set the UmbracoContext since it will be nulled and our cache handlers need it - //global::Umbraco.Web.UmbracoContext.Current = asyncState.UmbracoContext; + global::Umbraco.Web.UmbracoContext.Current = asyncState.UmbracoContext; _contentType.ContentTypeItem.Name = txtName.Text; _contentType.ContentTypeItem.Alias = txtAlias.Text; // raw, contentType.Alias takes care of it From 84752babc8459930c97d0a473f43e4dbc0a7402f Mon Sep 17 00:00:00 2001 From: Sebastiaan Janssen Date: Mon, 15 Dec 2014 09:04:25 +0100 Subject: [PATCH 21/29] Moved both new sections to the bottom so it's easier for upgraders to merge them --- .../umbraco/config/create/UI.Release.xml | 16 ++++++++-------- src/Umbraco.Web.UI/umbraco/config/create/UI.xml | 16 ++++++++-------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/Umbraco.Web.UI/umbraco/config/create/UI.Release.xml b/src/Umbraco.Web.UI/umbraco/config/create/UI.Release.xml index 464fc05154..0e13eba345 100644 --- a/src/Umbraco.Web.UI/umbraco/config/create/UI.Release.xml +++ b/src/Umbraco.Web.UI/umbraco/config/create/UI.Release.xml @@ -330,14 +330,6 @@ - -
Macro
- /Create/PartialView.ascx - - - - -
Macro
/Create/PartialViewMacro.ascx @@ -346,6 +338,14 @@ + +
Macro
+ /Create/PartialView.ascx + + + + +
Macro
/Create/PartialViewMacro.ascx diff --git a/src/Umbraco.Web.UI/umbraco/config/create/UI.xml b/src/Umbraco.Web.UI/umbraco/config/create/UI.xml index c2a4c90d0a..1ab0f4180c 100644 --- a/src/Umbraco.Web.UI/umbraco/config/create/UI.xml +++ b/src/Umbraco.Web.UI/umbraco/config/create/UI.xml @@ -330,14 +330,6 @@ - -
Macro
- /Create/PartialView.ascx - - - - -
Macro
/Create/PartialViewMacro.ascx @@ -346,6 +338,14 @@ + +
Macro
+ /Create/PartialView.ascx + + + + +
Macro
/Create/PartialViewMacro.ascx From 8e4e575276002ce738568870149b9a32efc7b0e9 Mon Sep 17 00:00:00 2001 From: Sebastiaan Janssen Date: Mon, 15 Dec 2014 09:30:04 +0100 Subject: [PATCH 22/29] Makes sure messages are shown in the user's configured language --- .../umbraco/controls/ContentTypeControlNew.ascx.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Umbraco.Web/umbraco.presentation/umbraco/controls/ContentTypeControlNew.ascx.cs b/src/Umbraco.Web/umbraco.presentation/umbraco/controls/ContentTypeControlNew.ascx.cs index 9db83ae975..3a051939a5 100644 --- a/src/Umbraco.Web/umbraco.presentation/umbraco/controls/ContentTypeControlNew.ascx.cs +++ b/src/Umbraco.Web/umbraco.presentation/umbraco/controls/ContentTypeControlNew.ascx.cs @@ -1082,7 +1082,7 @@ jQuery(document).ready(function() {{ refreshDropDowns(); }}); private string GetHtmlForNoPropertiesMessageListItem() { - return @"
  • " + ui.Text("settings", "noPropertiesDefinedOnTab") + "
    • "; + return @"
  • " + ui.Text("settings", "noPropertiesDefinedOnTab", Security.CurrentUser) + "
    • "; } private void SavePropertyType(SaveClickEventArgs e, IContentTypeComposition contentTypeItem) @@ -1136,7 +1136,7 @@ jQuery(document).ready(function() {{ refreshDropDowns(); }}); } else { - e.Message = ui.Text("contentTypeDublicatePropertyType"); + e.Message = ui.Text("contentTypeDublicatePropertyType", Security.CurrentUser); e.IconType = BasePage.speechBubbleIcon.warning; } } @@ -1439,7 +1439,7 @@ jQuery(document).ready(function() {{ refreshDropDowns(); }}); LoadContentType(); - var ea = new SaveClickEventArgs(ui.Text("contentTypeTabCreated")); + var ea = new SaveClickEventArgs(ui.Text("contentTypeTabCreated", Security.CurrentUser)); ea.IconType = BasePage.speechBubbleIcon.success; RaiseBubbleEvent(new object(), ea); @@ -1478,7 +1478,7 @@ jQuery(document).ready(function() {{ refreshDropDowns(); }}); LoadContentType(); - var ea = new SaveClickEventArgs(ui.Text("contentTypeTabDeleted")); + var ea = new SaveClickEventArgs(ui.Text("contentTypeTabDeleted", Security.CurrentUser)); ea.IconType = BasePage.speechBubbleIcon.success; RaiseBubbleEvent(new object(), ea); From 2936ff0bf9cc6553d04fdb8ad18d21e5bdb36ffa Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 15 Dec 2014 20:44:00 +1100 Subject: [PATCH 23/29] Makes provider user key type for new membership provider configurable, by default it is int --- src/Umbraco.Core/Models/Member.cs | 35 +------------------ .../Membership/MembershipUserExtensions.cs | 4 +-- .../Membership/UmbracoMembershipMember.cs | 5 +-- src/Umbraco.Core/Models/Membership/User.cs | 33 ----------------- .../Providers/MembersMembershipProvider.cs | 12 ++++++- .../Providers/UsersMembershipProvider.cs | 2 +- 6 files changed, 18 insertions(+), 73 deletions(-) diff --git a/src/Umbraco.Core/Models/Member.cs b/src/Umbraco.Core/Models/Member.cs index 1706ba5ab1..c4c0362acc 100644 --- a/src/Umbraco.Core/Models/Member.cs +++ b/src/Umbraco.Core/Models/Member.cs @@ -20,7 +20,6 @@ namespace Umbraco.Core.Models private string _email; private string _rawPasswordValue; private object _providerUserKey; - private Type _userTypeKey; /// /// Constructor for creating an empty Member object @@ -114,7 +113,6 @@ namespace Umbraco.Core.Models private static readonly PropertyInfo EmailSelector = ExpressionHelper.GetPropertyInfo(x => x.Email); private static readonly PropertyInfo PasswordSelector = ExpressionHelper.GetPropertyInfo(x => x.RawPasswordValue); private static readonly PropertyInfo ProviderUserKeySelector = ExpressionHelper.GetPropertyInfo(x => x.ProviderUserKey); - private static readonly PropertyInfo UserTypeKeySelector = ExpressionHelper.GetPropertyInfo(x => x.ProviderUserKeyType); /// /// Gets or sets the Username @@ -502,38 +500,7 @@ namespace Umbraco.Core.Models } } - /// - /// Gets or sets the type of the provider user key. - /// - /// - /// The type of the provider user key. - /// - [IgnoreDataMember] - internal Type ProviderUserKeyType - { - get - { - return _userTypeKey; - } - private set - { - SetPropertyValueAndDetectChanges(o => - { - _userTypeKey = value; - return _userTypeKey; - }, _userTypeKey, UserTypeKeySelector); - } - } - - /// - /// Sets the type of the provider user key. - /// - /// The type. - internal void SetProviderUserKeyType(Type type) - { - ProviderUserKeyType = type; - } - + /// /// Method to call when Entity is being saved /// diff --git a/src/Umbraco.Core/Models/Membership/MembershipUserExtensions.cs b/src/Umbraco.Core/Models/Membership/MembershipUserExtensions.cs index 6008c0ae02..2289983033 100644 --- a/src/Umbraco.Core/Models/Membership/MembershipUserExtensions.cs +++ b/src/Umbraco.Core/Models/Membership/MembershipUserExtensions.cs @@ -7,9 +7,9 @@ namespace Umbraco.Core.Models.Membership { internal static class MembershipUserExtensions { - internal static UmbracoMembershipMember AsConcreteMembershipUser(this IMembershipUser member, string providerName) + internal static UmbracoMembershipMember AsConcreteMembershipUser(this IMembershipUser member, string providerName, bool providerKeyAsGuid = false) { - var membershipMember = new UmbracoMembershipMember(member, providerName); + var membershipMember = new UmbracoMembershipMember(member, providerName, providerKeyAsGuid); return membershipMember; } diff --git a/src/Umbraco.Core/Models/Membership/UmbracoMembershipMember.cs b/src/Umbraco.Core/Models/Membership/UmbracoMembershipMember.cs index 1b8c7f5393..0f02f4f73b 100644 --- a/src/Umbraco.Core/Models/Membership/UmbracoMembershipMember.cs +++ b/src/Umbraco.Core/Models/Membership/UmbracoMembershipMember.cs @@ -1,5 +1,6 @@ using System; using System.Web.Security; +using Umbraco.Core.Configuration; namespace Umbraco.Core.Models.Membership { @@ -25,7 +26,7 @@ namespace Umbraco.Core.Models.Membership //NOTE: We are not calling the base constructor which will validate that a provider with the specified name exists which causes issues with unit tests. The ctor // validation for that doesn't need to be there anyways (have checked the source). - public UmbracoMembershipMember(IMembershipUser member, string providerName) + public UmbracoMembershipMember(IMembershipUser member, string providerName, bool providerKeyAsGuid = false) { _member = member; //NOTE: We are copying the values here so that everything is consistent with how the underlying built-in ASP.Net membership user @@ -37,7 +38,7 @@ namespace Umbraco.Core.Models.Membership if (member.PasswordQuestion != null) _passwordQuestion = member.PasswordQuestion.Trim(); _providerName = providerName; - _providerUserKey = member.ProviderUserKey; + _providerUserKey = providerKeyAsGuid ? member.ProviderUserKey : member.Id; _comment = member.Comments; _isApproved = member.IsApproved; _isLockedOut = member.IsLockedOut; diff --git a/src/Umbraco.Core/Models/Membership/User.cs b/src/Umbraco.Core/Models/Membership/User.cs index ba9b89d779..c9b55bd937 100644 --- a/src/Umbraco.Core/Models/Membership/User.cs +++ b/src/Umbraco.Core/Models/Membership/User.cs @@ -58,7 +58,6 @@ namespace Umbraco.Core.Models.Membership private IUserType _userType; private string _name; - private Type _userTypeKey; private List _addedSections; private List _removedSections; private ObservableCollection _sectionCollection; @@ -80,7 +79,6 @@ namespace Umbraco.Core.Models.Membership private static readonly PropertyInfo StartMediaIdSelector = ExpressionHelper.GetPropertyInfo(x => x.StartMediaId); private static readonly PropertyInfo AllowedSectionsSelector = ExpressionHelper.GetPropertyInfo>(x => x.AllowedSections); private static readonly PropertyInfo NameSelector = ExpressionHelper.GetPropertyInfo(x => x.Name); - private static readonly PropertyInfo UserTypeKeySelector = ExpressionHelper.GetPropertyInfo(x => x.ProviderUserKeyType); private static readonly PropertyInfo UsernameSelector = ExpressionHelper.GetPropertyInfo(x => x.Username); private static readonly PropertyInfo EmailSelector = ExpressionHelper.GetPropertyInfo(x => x.Email); @@ -101,37 +99,6 @@ namespace Umbraco.Core.Models.Membership set { throw new NotSupportedException("Cannot set the provider user key for a user"); } } - /// - /// Gets or sets the type of the provider user key. - /// - /// - /// The type of the provider user key. - /// - [IgnoreDataMember] - internal Type ProviderUserKeyType - { - get - { - return _userTypeKey; - } - private set - { - SetPropertyValueAndDetectChanges(o => - { - _userTypeKey = value; - return _userTypeKey; - }, _userTypeKey, UserTypeKeySelector); - } - } - - /// - /// Sets the type of the provider user key. - /// - /// The type. - internal void SetProviderUserKeyType(Type type) - { - ProviderUserKeyType = type; - } [DataMember] public string Username diff --git a/src/Umbraco.Web/Security/Providers/MembersMembershipProvider.cs b/src/Umbraco.Web/Security/Providers/MembersMembershipProvider.cs index 4ad0836a12..916194b5cc 100644 --- a/src/Umbraco.Web/Security/Providers/MembersMembershipProvider.cs +++ b/src/Umbraco.Web/Security/Providers/MembersMembershipProvider.cs @@ -42,6 +42,7 @@ namespace Umbraco.Web.Security.Providers private string _defaultMemberTypeAlias = "Member"; private volatile bool _hasDefaultMember = false; private static readonly object Locker = new object(); + private bool _providerKeyAsGuid = false; public override string ProviderName { @@ -58,7 +59,7 @@ namespace Umbraco.Web.Security.Providers protected override MembershipUser ConvertToMembershipUser(IMember entity) { - return entity.AsConcreteMembershipUser(Name); + return entity.AsConcreteMembershipUser(Name, _providerKeyAsGuid); } public string LockPropertyTypeAlias { get; private set; } @@ -85,6 +86,15 @@ namespace Umbraco.Web.Security.Providers } _hasDefaultMember = true; } + + //devs can configure the provider user key to be a guid if they want, by default it is int + if (config["providerKeyType"] != null) + { + if (config["providerKeyType"] == "guid") + { + _providerKeyAsGuid = true; + } + } } public override string DefaultMemberTypeAlias diff --git a/src/Umbraco.Web/Security/Providers/UsersMembershipProvider.cs b/src/Umbraco.Web/Security/Providers/UsersMembershipProvider.cs index 16ce523645..5d18039fb6 100644 --- a/src/Umbraco.Web/Security/Providers/UsersMembershipProvider.cs +++ b/src/Umbraco.Web/Security/Providers/UsersMembershipProvider.cs @@ -47,7 +47,7 @@ namespace Umbraco.Web.Security.Providers protected override MembershipUser ConvertToMembershipUser(IUser entity) { //the provider user key is always the int id - return entity.AsConcreteMembershipUser(Name); + return entity.AsConcreteMembershipUser(Name, true); } public override void Initialize(string name, System.Collections.Specialized.NameValueCollection config) From e202f21934a9d071c7e5879b53473121264f3227 Mon Sep 17 00:00:00 2001 From: Morten Christensen Date: Mon, 15 Dec 2014 14:29:48 +0100 Subject: [PATCH 24/29] Adds new test, fix, refactoring for adding duplicate PropertyType aliases --- .../Services/ContentTypeService.cs | 10 ++- .../Services/ContentTypeServiceTests.cs | 69 +++++++++++++++++++ 2 files changed, 77 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Core/Services/ContentTypeService.cs b/src/Umbraco.Core/Services/ContentTypeService.cs index 7252c2d3ba..d4c29e7585 100644 --- a/src/Umbraco.Core/Services/ContentTypeService.cs +++ b/src/Umbraco.Core/Services/ContentTypeService.cs @@ -308,15 +308,21 @@ namespace Umbraco.Core.Services 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 dependencies = new HashSet(compositions, comparer); - foreach (var indirectReference in indirectReferences) + var stack = new Stack(); + indirectReferences.ForEach(stack.Push);//Push indirect references to a stack, so we can add recursively + while (stack.Count > 0) { + var indirectReference = stack.Pop(); dependencies.Add(indirectReference); + //Get all compositions for the current indirect reference var directReferences = indirectReference.ContentTypeComposition; foreach (var directReference in directReferences) { - if(directReference.Id == compositionContentType.Id || directReference.Alias.Equals(compositionContentType.Alias)) continue; + if (directReference.Id == compositionContentType.Id || directReference.Alias.Equals(compositionContentType.Alias)) continue; dependencies.Add(directReference); } + //Recursive lookup of indirect references + allContentTypes.Where(x => x.ContentTypeComposition.Any(y => y.Id == indirectReference.Id)).ForEach(stack.Push); } foreach (var dependency in dependencies) diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs index 7fe1ba4eeb..c6186d43a5 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs @@ -552,6 +552,68 @@ namespace Umbraco.Tests.Services Assert.DoesNotThrow(() => service.GetContentType("seo")); } + [Test] + public void Cannot_Add_Duplicate_PropertyType_Alias_At_Root_Which_Conflicts_With_Third_Levels_Composition() + { + /* + * BasePage, gets 'Title' added but should not be allowed + * -- Content Page + * ---- Advanced Page -> Content Meta + * Content Meta :: Composition, has 'Title' + * + * Content Meta has 'Title' PropertyType + * Adding 'Title' to BasePage should fail + */ + + // Arrange + var service = ServiceContext.ContentTypeService; + var basePage = MockedContentTypes.CreateBasicContentType(); + service.Save(basePage); + var contentPage = MockedContentTypes.CreateBasicContentType("contentPage", "Content Page", basePage); + service.Save(contentPage); + var advancedPage = MockedContentTypes.CreateBasicContentType("advancedPage", "Advanced Page", contentPage); + service.Save(advancedPage); + + var contentMetaComposition = MockedContentTypes.CreateContentMetaContentType(); + service.Save(contentMetaComposition); + + // Act + var bodyTextPropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "bodyText", Name = "Body Text", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var bodyTextAdded = basePage.AddPropertyType(bodyTextPropertyType, "Content"); + service.Save(basePage); + + var authorPropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "author", Name = "Author", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var authorAdded = contentPage.AddPropertyType(authorPropertyType, "Content"); + service.Save(contentPage); + + var compositionAdded = advancedPage.AddContentType(contentMetaComposition); + service.Save(advancedPage); + + //NOTE: It should not be possible to Save 'BasePage' with the Title PropertyType added + var titlePropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "title", Name = "Title", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var titleAdded = basePage.AddPropertyType(titlePropertyType, "Content"); + + // Assert + Assert.That(bodyTextAdded, Is.True); + Assert.That(authorAdded, Is.True); + Assert.That(titleAdded, Is.True); + Assert.That(compositionAdded, Is.True); + + Assert.Throws(() => service.Save(basePage)); + + Assert.DoesNotThrow(() => service.GetContentType("contentPage")); + Assert.DoesNotThrow(() => service.GetContentType("advancedPage")); + } + [Test] public void Cannot_Rename_PropertyGroup_On_Child_Avoiding_Conflict_With_Parent_PropertyGroup() { @@ -616,6 +678,13 @@ namespace Umbraco.Tests.Services [Test] public void Can_Add_PropertyType_Alias_Which_Exists_In_Composition_Outside_Graph() { + /* + * Meta (Composition) + * Content Meta (Composition) has 'Title' -> Meta + * BasePage + * -- ContentPage gets 'Title' added -> Meta + * ---- Advanced Page + */ // Arrange var service = ServiceContext.ContentTypeService; From 21f08ca491a65c291b16b5ad82e00c5dac352165 Mon Sep 17 00:00:00 2001 From: Morten Christensen Date: Mon, 15 Dec 2014 15:33:05 +0100 Subject: [PATCH 25/29] Refactoring validation and adding new test case for renaming PropertyTypes --- .../Services/ContentTypeService.cs | 5 +- .../Services/ContentTypeServiceTests.cs | 53 +++++++++++++++++++ 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Core/Services/ContentTypeService.cs b/src/Umbraco.Core/Services/ContentTypeService.cs index d4c29e7585..96bb025cba 100644 --- a/src/Umbraco.Core/Services/ContentTypeService.cs +++ b/src/Umbraco.Core/Services/ContentTypeService.cs @@ -303,7 +303,8 @@ namespace Umbraco.Core.Services else throw new Exception("Composition is neither IContentType nor IMediaType?"); - var compositions = compositionContentType.ContentTypeComposition; + var compositionAliases = compositionContentType.CompositionAliases(); + var compositions = allContentTypes.Where(x => compositionAliases.Any(y => x.Alias.Equals(y))); 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); @@ -327,7 +328,7 @@ namespace Umbraco.Core.Services foreach (var dependency in dependencies) { - var contentTypeDependency = allContentTypes.FirstOrDefault(x => x.Alias.Equals(dependency.Alias)); + var contentTypeDependency = allContentTypes.FirstOrDefault(x => x.Alias.Equals(dependency.Alias, StringComparison.InvariantCultureIgnoreCase)); if (contentTypeDependency == null) continue; var intersect = contentTypeDependency.PropertyTypes.Select(x => x.Alias.ToLowerInvariant()).Intersect(propertyTypeAliases).ToArray(); if (intersect.Length == 0) continue; diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs index c6186d43a5..ab6783327a 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs @@ -675,6 +675,59 @@ namespace Umbraco.Tests.Services Assert.DoesNotThrow(() => service.GetContentType("advancedPage")); } + [Test] + public void Cannot_Rename_PropertyType_Alias_Causing_Conflicts_With_Parents() + { + // Arrange + var service = ServiceContext.ContentTypeService; + var basePage = MockedContentTypes.CreateBasicContentType(); + service.Save(basePage); + var contentPage = MockedContentTypes.CreateBasicContentType("contentPage", "Content Page", basePage); + service.Save(contentPage); + var advancedPage = MockedContentTypes.CreateBasicContentType("advancedPage", "Advanced Page", contentPage); + service.Save(advancedPage); + + // Act + var titlePropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "title", Name = "Title", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var titleAdded = basePage.AddPropertyType(titlePropertyType, "Content"); + var bodyTextPropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "bodyText", Name = "Body Text", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var bodyTextAdded = contentPage.AddPropertyType(bodyTextPropertyType, "Content"); + var subtitlePropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "subtitle", Name = "Subtitle", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var subtitleAdded = contentPage.AddPropertyType(subtitlePropertyType, "Content"); + var authorPropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "author", Name = "Author", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var authorAdded = advancedPage.AddPropertyType(authorPropertyType, "Content"); + service.Save(basePage); + service.Save(contentPage); + service.Save(advancedPage); + + //Rename the PropertyType to something that already exists in the Composition - NOTE this should not be allowed and Saving should throw an exception + var authorPropertyTypeToRename = advancedPage.PropertyTypes.First(x => x.Alias.Equals("author")); + authorPropertyTypeToRename.Alias = "title"; + + // Assert + Assert.That(bodyTextAdded, Is.True); + Assert.That(authorAdded, Is.True); + Assert.That(titleAdded, Is.True); + Assert.That(subtitleAdded, Is.True); + + Assert.Throws(() => service.Save(advancedPage)); + + Assert.DoesNotThrow(() => service.GetContentType("contentPage")); + Assert.DoesNotThrow(() => service.GetContentType("advancedPage")); + } + [Test] public void Can_Add_PropertyType_Alias_Which_Exists_In_Composition_Outside_Graph() { From 2d8169140750d9a406305b14f42eaad5171c2742 Mon Sep 17 00:00:00 2001 From: Morten Christensen Date: Mon, 15 Dec 2014 16:34:41 +0100 Subject: [PATCH 26/29] Adding new test case for renaming PropertyType aliases on indirect references --- .../Services/ContentTypeService.cs | 3 + .../Services/ContentTypeServiceTests.cs | 96 ++++++++++++++++--- 2 files changed, 85 insertions(+), 14 deletions(-) diff --git a/src/Umbraco.Core/Services/ContentTypeService.cs b/src/Umbraco.Core/Services/ContentTypeService.cs index 96bb025cba..6ccdf5611e 100644 --- a/src/Umbraco.Core/Services/ContentTypeService.cs +++ b/src/Umbraco.Core/Services/ContentTypeService.cs @@ -321,6 +321,9 @@ namespace Umbraco.Core.Services { if (directReference.Id == compositionContentType.Id || directReference.Alias.Equals(compositionContentType.Alias)) continue; dependencies.Add(directReference); + //A direct reference has compositions of its own - these also need to be taken into account + var directReferenceGraph = directReference.CompositionAliases(); + allContentTypes.Where(x => directReferenceGraph.Any(y => x.Alias.Equals(y))).ForEach(c => dependencies.Add(c)); } //Recursive lookup of indirect references allContentTypes.Where(x => x.ContentTypeComposition.Any(y => y.Id == indirectReference.Id)).ForEach(stack.Push); diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs index ab6783327a..4bc0d17d83 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs @@ -614,6 +614,86 @@ namespace Umbraco.Tests.Services Assert.DoesNotThrow(() => service.GetContentType("advancedPage")); } + [Test] + public void Cannot_Rename_PropertyType_Alias_On_Composition_Which_Would_Cause_Conflict_In_Other_Composition() + { + /* + * Meta renames alias to 'title' + * Seo has 'Title' + * BasePage + * -- ContentPage + * ---- AdvancedPage -> Seo + * ------ MoreAdvanedPage -> Meta + */ + + // Arrange + var service = ServiceContext.ContentTypeService; + var basePage = MockedContentTypes.CreateBasicContentType(); + service.Save(basePage); + var contentPage = MockedContentTypes.CreateBasicContentType("contentPage", "Content Page", basePage); + service.Save(contentPage); + var advancedPage = MockedContentTypes.CreateBasicContentType("advancedPage", "Advanced Page", contentPage); + service.Save(advancedPage); + var moreAdvancedPage = MockedContentTypes.CreateBasicContentType("moreAdvancedPage", "More Advanced Page", advancedPage); + service.Save(moreAdvancedPage); + + var seoComposition = MockedContentTypes.CreateSeoContentType(); + service.Save(seoComposition); + var metaComposition = MockedContentTypes.CreateMetaContentType(); + service.Save(metaComposition); + + // Act + var bodyTextPropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "bodyText", Name = "Body Text", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var bodyTextAdded = basePage.AddPropertyType(bodyTextPropertyType, "Content"); + service.Save(basePage); + + var authorPropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "author", Name = "Author", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var authorAdded = contentPage.AddPropertyType(authorPropertyType, "Content"); + service.Save(contentPage); + + var subtitlePropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "subtitle", Name = "Subtitle", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var subtitleAdded = advancedPage.AddPropertyType(subtitlePropertyType, "Content"); + service.Save(advancedPage); + + var titlePropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "title", Name = "Title", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var titleAdded = seoComposition.AddPropertyType(titlePropertyType, "Content"); + service.Save(seoComposition); + + var seoCompositionAdded = advancedPage.AddContentType(seoComposition); + var metaCompositionAdded = moreAdvancedPage.AddContentType(metaComposition); + service.Save(advancedPage); + service.Save(moreAdvancedPage); + + var keywordsPropertyType = metaComposition.PropertyTypes.First(x => x.Alias.Equals("metakeywords")); + keywordsPropertyType.Alias = "title"; + + // Assert + Assert.That(bodyTextAdded, Is.True); + Assert.That(subtitleAdded, Is.True); + Assert.That(authorAdded, Is.True); + Assert.That(titleAdded, Is.True); + Assert.That(seoCompositionAdded, Is.True); + Assert.That(metaCompositionAdded, Is.True); + + Assert.Throws(() => service.Save(metaComposition)); + + Assert.DoesNotThrow(() => service.GetContentType("contentPage")); + Assert.DoesNotThrow(() => service.GetContentType("advancedPage")); + Assert.DoesNotThrow(() => service.GetContentType("moreAdvancedPage")); + } + [Test] public void Cannot_Rename_PropertyGroup_On_Child_Avoiding_Conflict_With_Parent_PropertyGroup() { @@ -632,23 +712,11 @@ namespace Umbraco.Tests.Services // Act var subtitlePropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) { - Alias = "subtitle", - Name = "Subtitle", - Description = "", - HelpText = "", - Mandatory = false, - SortOrder = 1, - DataTypeDefinitionId = -88 + Alias = "subtitle", Name = "Subtitle", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 }; var authorPropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) { - Alias = "author", - Name = "Author", - Description = "", - HelpText = "", - Mandatory = false, - SortOrder = 1, - DataTypeDefinitionId = -88 + Alias = "author", Name = "Author", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 }; var subtitleAdded = contentPage.AddPropertyType(subtitlePropertyType, "Content"); var authorAdded = contentPage.AddPropertyType(authorPropertyType, "Content"); From fa9132129976573ea667e6b9bd12c202baa0a394 Mon Sep 17 00:00:00 2001 From: Sebastiaan Janssen Date: Mon, 15 Dec 2014 18:56:58 +0100 Subject: [PATCH 27/29] Adds test for adding properties to a composition that has been saved and now doesn't allow a new property to be added --- .../Services/ContentTypeServiceTests.cs | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs index 4bc0d17d83..e486eaaf5a 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs @@ -694,6 +694,96 @@ namespace Umbraco.Tests.Services Assert.DoesNotThrow(() => service.GetContentType("moreAdvancedPage")); } + [Test] + public void Can_Add_Additional_Properties_On_Composition_Once_Composition_Has_Been_Saved() + { + /* + * Meta renames alias to 'title' + * Seo has 'Title' + * BasePage + * -- ContentPage + * ---- AdvancedPage -> Seo + * ------ MoreAdvancedPage -> Meta + */ + + // Arrange + var service = ServiceContext.ContentTypeService; + var basePage = MockedContentTypes.CreateBasicContentType(); + service.Save(basePage); + var contentPage = MockedContentTypes.CreateBasicContentType("contentPage", "Content Page", basePage); + service.Save(contentPage); + var advancedPage = MockedContentTypes.CreateBasicContentType("advancedPage", "Advanced Page", contentPage); + service.Save(advancedPage); + var moreAdvancedPage = MockedContentTypes.CreateBasicContentType("moreAdvancedPage", "More Advanced Page", advancedPage); + service.Save(moreAdvancedPage); + + var seoComposition = MockedContentTypes.CreateSeoContentType(); + service.Save(seoComposition); + var metaComposition = MockedContentTypes.CreateMetaContentType(); + service.Save(metaComposition); + + // Act + var bodyTextPropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "bodyText", Name = "Body Text", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var bodyTextAdded = basePage.AddPropertyType(bodyTextPropertyType, "Content"); + service.Save(basePage); + + var authorPropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "author", Name = "Author", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var authorAdded = contentPage.AddPropertyType(authorPropertyType, "Content"); + service.Save(contentPage); + + var subtitlePropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "subtitle", Name = "Subtitle", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var subtitleAdded = advancedPage.AddPropertyType(subtitlePropertyType, "Content"); + service.Save(advancedPage); + + var titlePropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "title", Name = "Title", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var titleAdded = seoComposition.AddPropertyType(titlePropertyType, "Content"); + service.Save(seoComposition); + + + var seoCompositionAdded = advancedPage.AddContentType(seoComposition); + var metaCompositionAdded = moreAdvancedPage.AddContentType(metaComposition); + service.Save(advancedPage); + service.Save(moreAdvancedPage); + + var keywordsPropertyType = metaComposition.PropertyTypes.First(x => x.Alias.Equals("metakeywords")); + keywordsPropertyType.Alias = "title"; + + // Assert + Assert.That(bodyTextAdded, Is.True); + Assert.That(subtitleAdded, Is.True); + Assert.That(authorAdded, Is.True); + Assert.That(titleAdded, Is.True); + Assert.That(seoCompositionAdded, Is.True); + Assert.That(metaCompositionAdded, Is.True); + + Assert.Throws(() => service.Save(metaComposition)); + + var testPropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) + { + Alias = "test", Name = "Test", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 + }; + var testAdded = seoComposition.AddPropertyType(testPropertyType, "Content"); + service.Save(seoComposition); + + Assert.That(testAdded, Is.True); + + Assert.DoesNotThrow(() => service.GetContentType("contentPage")); + Assert.DoesNotThrow(() => service.GetContentType("advancedPage")); + Assert.DoesNotThrow(() => service.GetContentType("moreAdvancedPage")); + } + [Test] public void Cannot_Rename_PropertyGroup_On_Child_Avoiding_Conflict_With_Parent_PropertyGroup() { From e62597521e3777940cdcea5ea48f335f92fc3d8b Mon Sep 17 00:00:00 2001 From: Sebastiaan Janssen Date: Mon, 15 Dec 2014 19:51:37 +0100 Subject: [PATCH 28/29] Remove the renaming to 'title' which would cause a duplicate and make the next test fail --- src/Umbraco.Tests/Services/ContentTypeServiceTests.cs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs index e486eaaf5a..f430a93e23 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs @@ -750,16 +750,12 @@ namespace Umbraco.Tests.Services }; var titleAdded = seoComposition.AddPropertyType(titlePropertyType, "Content"); service.Save(seoComposition); - - + var seoCompositionAdded = advancedPage.AddContentType(seoComposition); var metaCompositionAdded = moreAdvancedPage.AddContentType(metaComposition); service.Save(advancedPage); service.Save(moreAdvancedPage); - var keywordsPropertyType = metaComposition.PropertyTypes.First(x => x.Alias.Equals("metakeywords")); - keywordsPropertyType.Alias = "title"; - // Assert Assert.That(bodyTextAdded, Is.True); Assert.That(subtitleAdded, Is.True); @@ -767,9 +763,7 @@ namespace Umbraco.Tests.Services Assert.That(titleAdded, Is.True); Assert.That(seoCompositionAdded, Is.True); Assert.That(metaCompositionAdded, Is.True); - - Assert.Throws(() => service.Save(metaComposition)); - + var testPropertyType = new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) { Alias = "test", Name = "Test", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 From a8d7a1d01de07a723b1febd965428949e75a82c8 Mon Sep 17 00:00:00 2001 From: Sebastiaan Janssen Date: Mon, 15 Dec 2014 19:52:28 +0100 Subject: [PATCH 29/29] Makes the test green: don't add the passed in composition as a dependency to itself.. also added another invariant check --- src/Umbraco.Core/Services/ContentTypeService.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Core/Services/ContentTypeService.cs b/src/Umbraco.Core/Services/ContentTypeService.cs index 6ccdf5611e..4c1f9cd038 100644 --- a/src/Umbraco.Core/Services/ContentTypeService.cs +++ b/src/Umbraco.Core/Services/ContentTypeService.cs @@ -317,13 +317,14 @@ namespace Umbraco.Core.Services dependencies.Add(indirectReference); //Get all compositions for the current indirect reference var directReferences = indirectReference.ContentTypeComposition; + foreach (var directReference in directReferences) { if (directReference.Id == compositionContentType.Id || directReference.Alias.Equals(compositionContentType.Alias)) continue; dependencies.Add(directReference); //A direct reference has compositions of its own - these also need to be taken into account var directReferenceGraph = directReference.CompositionAliases(); - allContentTypes.Where(x => directReferenceGraph.Any(y => x.Alias.Equals(y))).ForEach(c => dependencies.Add(c)); + allContentTypes.Where(x => directReferenceGraph.Any(y => x.Alias.Equals(y, StringComparison.InvariantCultureIgnoreCase))).ForEach(c => dependencies.Add(c)); } //Recursive lookup of indirect references allContentTypes.Where(x => x.ContentTypeComposition.Any(y => y.Id == indirectReference.Id)).ForEach(stack.Push); @@ -331,6 +332,7 @@ namespace Umbraco.Core.Services foreach (var dependency in dependencies) { + if (dependency.Id == compositionContentType.Id) continue; var contentTypeDependency = allContentTypes.FirstOrDefault(x => x.Alias.Equals(dependency.Alias, StringComparison.InvariantCultureIgnoreCase)); if (contentTypeDependency == null) continue; var intersect = contentTypeDependency.PropertyTypes.Select(x => x.Alias.ToLowerInvariant()).Intersect(propertyTypeAliases).ToArray();