From b3f6f4883472971178c6c753fc572ceba7a20b09 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 9 Mar 2015 16:42:30 +1100 Subject: [PATCH] fixes ugly null check on ContentTypeSort comparison --- src/Umbraco.Core/Models/ContentTypeSort.cs | 25 +++++++++++++++++-- .../Repositories/ContentTypeBaseRepository.cs | 2 +- .../ContentTypeDefinitionFactory.cs | 6 ++--- .../Repositories/ContentRepositoryTest.cs | 7 +----- .../Repositories/ContentTypeRepositoryTest.cs | 14 ++--------- .../Services/PerformanceTests.cs | 7 +----- .../controls/ContentTypeControlNew.ascx.cs | 2 +- src/umbraco.cms/businesslogic/ContentType.cs | 2 +- 8 files changed, 32 insertions(+), 33 deletions(-) diff --git a/src/Umbraco.Core/Models/ContentTypeSort.cs b/src/Umbraco.Core/Models/ContentTypeSort.cs index 5aa81d9db0..40beb70939 100644 --- a/src/Umbraco.Core/Models/ContentTypeSort.cs +++ b/src/Umbraco.Core/Models/ContentTypeSort.cs @@ -8,8 +8,16 @@ namespace Umbraco.Core.Models /// public class ContentTypeSort : IValueObject, IDeepCloneable { + [Obsolete("This parameterless constructor should never be used")] public ContentTypeSort() { + + } + + public ContentTypeSort(Lazy id, int sortOrder) + { + Id = id; + SortOrder = sortOrder; } public ContentTypeSort(Lazy id, int sortOrder, string @alias) @@ -45,9 +53,16 @@ namespace Umbraco.Core.Models protected bool Equals(ContentTypeSort other) { - return Id.Value.Equals(other.Id.Value) && string.Equals(Alias, other.Alias); + return Id.Equals(other.Id) && string.Equals(Alias, other.Alias); } + /// + /// Determines whether the specified is equal to the current . + /// + /// + /// true if the specified object is equal to the current object; otherwise, false. + /// + /// The object to compare with the current object. public override bool Equals(object obj) { if (ReferenceEquals(null, obj)) return false; @@ -56,11 +71,17 @@ namespace Umbraco.Core.Models return Equals((ContentTypeSort) obj); } + /// + /// Serves as a hash function for a particular type. + /// + /// + /// A hash code for the current . + /// public override int GetHashCode() { unchecked { - return (Id.GetHashCode()*397) ^ Alias.GetHashCode(); + return (Id.Value.GetHashCode()*397) ^ (Alias != null ? Alias.GetHashCode() : 0); } } diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentTypeBaseRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentTypeBaseRepository.cs index 863f9ebeb7..5477e5dd96 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentTypeBaseRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentTypeBaseRepository.cs @@ -414,7 +414,7 @@ AND umbracoNode.id <> @id", .Where(x => x.Id == id); var allowedContentTypeDtos = Database.Fetch(sql); - return allowedContentTypeDtos.Select(x => new ContentTypeSort { Id = new Lazy(() => x.AllowedId), Alias = x.ContentTypeDto.Alias, SortOrder = x.SortOrder }).ToList(); + return allowedContentTypeDtos.Select(x => new ContentTypeSort(new Lazy(() => x.AllowedId), x.SortOrder, x.ContentTypeDto.Alias)).ToList(); } protected PropertyGroupCollection GetPropertyGroupCollection(int id, DateTime createDate, DateTime updateDate) diff --git a/src/Umbraco.Tests/CodeFirst/Definitions/ContentTypeDefinitionFactory.cs b/src/Umbraco.Tests/CodeFirst/Definitions/ContentTypeDefinitionFactory.cs index f1b98c53e6..43672de279 100644 --- a/src/Umbraco.Tests/CodeFirst/Definitions/ContentTypeDefinitionFactory.cs +++ b/src/Umbraco.Tests/CodeFirst/Definitions/ContentTypeDefinitionFactory.cs @@ -347,12 +347,10 @@ namespace Umbraco.Tests.CodeFirst.Definitions { if(type == currentType) continue;//If the referenced type is equal to the current type we skip it to avoid a circular dependency - var contentTypeSort = new ContentTypeSort(); + var isResolved = _contentTypeCache.ContainsKey(type.FullName); var lazy = isResolved ? _contentTypeCache[type.FullName].ContentType : GetContentTypeDefinition(type); - contentTypeSort.Id = new Lazy(() => lazy.Value.Id); - contentTypeSort.Alias = lazy.Value.Alias; - contentTypeSort.SortOrder = order; + var contentTypeSort = new ContentTypeSort(new Lazy(() => lazy.Value.Id), order, lazy.Value.Alias); contentTypeSorts.Add(contentTypeSort); order++; } diff --git a/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs index 86c26fd0e5..f55b185d8b 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs @@ -172,12 +172,7 @@ namespace Umbraco.Tests.Persistence.Repositories var contentType = MockedContentTypes.CreateSimpleContentType("umbTextpage1", "Textpage"); contentType.AllowedContentTypes = new List { - new ContentTypeSort - { - Alias = contentType.Alias, - Id = new Lazy(() => contentType.Id), - SortOrder = 0 - } + new ContentTypeSort(new Lazy(() => contentType.Id), 0, contentType.Alias) }; var parentPage = MockedContent.CreateSimpleContent(contentType); contentTypeRepository.AddOrUpdate(contentType); diff --git a/src/Umbraco.Tests/Persistence/Repositories/ContentTypeRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/ContentTypeRepositoryTest.cs index 5d94e604a4..81fae8137c 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/ContentTypeRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/ContentTypeRepositoryTest.cs @@ -386,18 +386,8 @@ namespace Umbraco.Tests.Persistence.Repositories var contentType = repository.Get(NodeDto.NodeIdSeed); contentType.AllowedContentTypes = new List { - new ContentTypeSort - { - Alias = subpageContentType.Alias, - Id = new Lazy(() => subpageContentType.Id), - SortOrder = 0 - }, - new ContentTypeSort - { - Alias = simpleSubpageContentType.Alias, - Id = new Lazy(() => simpleSubpageContentType.Id), - SortOrder = 1 - } + new ContentTypeSort(new Lazy(() => subpageContentType.Id), 0, subpageContentType.Alias), + new ContentTypeSort(new Lazy(() => simpleSubpageContentType.Id), 1, simpleSubpageContentType.Alias) }; repository.AddOrUpdate(contentType); unitOfWork.Commit(); diff --git a/src/Umbraco.Tests/Services/PerformanceTests.cs b/src/Umbraco.Tests/Services/PerformanceTests.cs index 7e754b6635..d128107d37 100644 --- a/src/Umbraco.Tests/Services/PerformanceTests.cs +++ b/src/Umbraco.Tests/Services/PerformanceTests.cs @@ -239,12 +239,7 @@ namespace Umbraco.Tests.Services ServiceContext.ContentTypeService.Save(contentType1); contentType1.AllowedContentTypes = new List { - new ContentTypeSort - { - Alias = contentType1.Alias, - Id = new Lazy(() => contentType1.Id), - SortOrder = 0 - } + new ContentTypeSort(new Lazy(() => contentType1.Id), 0, contentType1.Alias) }; var result = new List(); ServiceContext.ContentTypeService.Save(contentType1); 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 6ad092c2b6..3bd510690e 100644 --- a/src/Umbraco.Web/umbraco.presentation/umbraco/controls/ContentTypeControlNew.ascx.cs +++ b/src/Umbraco.Web/umbraco.presentation/umbraco/controls/ContentTypeControlNew.ascx.cs @@ -321,7 +321,7 @@ namespace umbraco.controls int i = 0; var ids = SaveAllowedChildTypes(); - _contentType.ContentTypeItem.AllowedContentTypes = ids.Select(x => new ContentTypeSort {Id = new Lazy(() => x), SortOrder = i++}); + _contentType.ContentTypeItem.AllowedContentTypes = ids.Select(x => new ContentTypeSort(new Lazy(() => x), i++)); // figure out whether compositions are locked var allContentTypes = Request.Path.ToLowerInvariant().Contains("editmediatype.aspx") diff --git a/src/umbraco.cms/businesslogic/ContentType.cs b/src/umbraco.cms/businesslogic/ContentType.cs index 01a63f0625..a9f11321fe 100644 --- a/src/umbraco.cms/businesslogic/ContentType.cs +++ b/src/umbraco.cms/businesslogic/ContentType.cs @@ -880,7 +880,7 @@ namespace umbraco.cms.businesslogic foreach (var i in value) { int id = i; - list.Add(new ContentTypeSort { Id = new Lazy(() => id), SortOrder = sort }); + list.Add(new ContentTypeSort(new Lazy(() => id), sort)); sort++; }