From e8df5f9eb4a55b91a374723f010e4617a5fe7e20 Mon Sep 17 00:00:00 2001 From: Claus Date: Wed, 29 Mar 2017 11:08:57 +0200 Subject: [PATCH] U4-9689 Content type description isn't always saved the same way also fixed for member types (and added test for media types also using the ContentTypeService) --- .../Services/ContentTypeService.cs | 118 +++++++++--------- .../Services/MemberTypeService.cs | 6 +- .../Services/ContentTypeServiceTests.cs | 34 ++++- .../Services/MemberTypeServiceTests.cs | 18 ++- .../Entities/MockedContentTypes.cs | 1 - 5 files changed, 112 insertions(+), 65 deletions(-) diff --git a/src/Umbraco.Core/Services/ContentTypeService.cs b/src/Umbraco.Core/Services/ContentTypeService.cs index 9f625c27b8..dc7d9dfa1e 100644 --- a/src/Umbraco.Core/Services/ContentTypeService.cs +++ b/src/Umbraco.Core/Services/ContentTypeService.cs @@ -1,22 +1,15 @@ using System; using System.Collections.Generic; -using System.Data; -using System.Diagnostics; using System.Linq; using System.Text; -using System.Xml.Linq; using System.Threading; -using AutoMapper; -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 @@ -26,14 +19,15 @@ namespace Umbraco.Core.Services /// public class ContentTypeService : ContentTypeServiceBase, IContentTypeService { - private readonly IContentService _contentService; + private readonly IContentService _contentService; private readonly IMediaService _mediaService; //Support recursive locks because some of the methods that require locking call other methods that require locking. //for example, the Move method needs to be locked but this calls the Save method which also needs to be locked. private static readonly ReaderWriterLockSlim Locker = new ReaderWriterLockSlim(LockRecursionPolicy.SupportsRecursion); - public ContentTypeService(IDatabaseUnitOfWorkProvider provider, RepositoryFactory repositoryFactory, ILogger logger, IEventMessagesFactory eventMessagesFactory, IContentService contentService, IMediaService mediaService) + public ContentTypeService(IDatabaseUnitOfWorkProvider provider, RepositoryFactory repositoryFactory, ILogger logger, IEventMessagesFactory eventMessagesFactory, IContentService contentService, + IMediaService mediaService) : base(provider, repositoryFactory, logger, eventMessagesFactory) { if (contentService == null) throw new ArgumentNullException("contentService"); @@ -154,8 +148,8 @@ namespace Umbraco.Core.Services } if (savingEvent.IsRaisedEventCancelled( - new SaveEventArgs(container, evtMsgs), - this)) + new SaveEventArgs(container, evtMsgs), + this)) { return OperationStatus.Cancelled(evtMsgs); } @@ -214,7 +208,7 @@ namespace Umbraco.Core.Services public IEnumerable GetMediaTypeContainers(IMediaType mediaType) { - var ancestorIds = mediaType.Path.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries) + var ancestorIds = mediaType.Path.Split(new[] {','}, StringSplitOptions.RemoveEmptyEntries) .Select(x => { var asInt = x.TryConvertTo(); @@ -290,8 +284,8 @@ namespace Umbraco.Core.Services if (container == null) return OperationStatus.NoOperation(evtMsgs); if (DeletingContentTypeContainer.IsRaisedEventCancelled( - new DeleteEventArgs(container, evtMsgs), - this)) + new DeleteEventArgs(container, evtMsgs), + this)) { return Attempt.Fail(new OperationStatus(OperationStatusType.FailedCancelledByEvent, evtMsgs)); } @@ -316,8 +310,8 @@ namespace Umbraco.Core.Services if (container == null) return OperationStatus.NoOperation(evtMsgs); if (DeletingMediaTypeContainer.IsRaisedEventCancelled( - new DeleteEventArgs(container, evtMsgs), - this)) + new DeleteEventArgs(container, evtMsgs), + this)) { return Attempt.Fail(new OperationStatus(OperationStatusType.FailedCancelledByEvent, evtMsgs)); } @@ -421,7 +415,7 @@ namespace Umbraco.Core.Services clone.Name = name; - var compositionAliases = clone.CompositionAliases().Except(new[] { alias }).ToList(); + var compositionAliases = clone.CompositionAliases().Except(new[] {alias}).ToList(); //remove all composition that is not it's current alias foreach (var a in compositionAliases) { @@ -680,7 +674,7 @@ namespace Umbraco.Core.Services var comparer = new DelegateEqualityComparer((x, y) => x.Id == y.Id, x => x.Id); var dependencies = new HashSet(compositions, comparer); var stack = new Stack(); - indirectReferences.ForEach(stack.Push);//Push indirect references to a stack, so we can add recursively + indirectReferences.ForEach(stack.Push); //Push indirect references to a stack, so we can add recursively while (stack.Count > 0) { var indirectReference = stack.Pop(); @@ -734,6 +728,8 @@ namespace Umbraco.Core.Services { ValidateLocked(contentType); // throws if invalid contentType.CreatorId = userId; + if (contentType.Description == string.Empty) + contentType.Description = null; repository.AddOrUpdate(contentType); uow.Commit(); @@ -742,7 +738,7 @@ namespace Umbraco.Core.Services UpdateContentXmlStructure(contentType); } SavedContentType.RaiseEvent(new SaveEventArgs(contentType, false), this); - Audit(AuditType.Save, string.Format("Save ContentType performed by user"), userId, contentType.Id); + Audit(AuditType.Save, string.Format("Save ContentType performed by user"), userId, contentType.Id); } /// @@ -755,7 +751,7 @@ namespace Umbraco.Core.Services var asArray = contentTypes.ToArray(); if (SavingContentType.IsRaisedEventCancelled(new SaveEventArgs(asArray), this)) - return; + return; using (new WriteLock(Locker)) { @@ -770,6 +766,8 @@ namespace Umbraco.Core.Services foreach (var contentType in asArray) { contentType.CreatorId = userId; + if (contentType.Description == string.Empty) + contentType.Description = null; repository.AddOrUpdate(contentType); } @@ -780,7 +778,7 @@ namespace Umbraco.Core.Services UpdateContentXmlStructure(asArray.Cast().ToArray()); } SavedContentType.RaiseEvent(new SaveEventArgs(asArray, false), this); - Audit(AuditType.Save, string.Format("Save ContentTypes performed by user"), userId, -1); + Audit(AuditType.Save, string.Format("Save ContentTypes performed by user"), userId, -1); } /// @@ -791,8 +789,8 @@ namespace Umbraco.Core.Services /// Deleting a will delete all the objects based on this public void Delete(IContentType contentType, int userId = 0) { - if (DeletingContentType.IsRaisedEventCancelled(new DeleteEventArgs(contentType), this)) - return; + if (DeletingContentType.IsRaisedEventCancelled(new DeleteEventArgs(contentType), this)) + return; using (new WriteLock(Locker)) { @@ -836,7 +834,7 @@ namespace Umbraco.Core.Services var asArray = contentTypes.ToArray(); if (DeletingContentType.IsRaisedEventCancelled(new DeleteEventArgs(asArray), this)) - return; + return; using (new WriteLock(Locker)) { @@ -1004,8 +1002,8 @@ namespace Umbraco.Core.Services var evtMsgs = EventMessagesFactory.Get(); if (MovingMediaType.IsRaisedEventCancelled( - new MoveEventArgs(evtMsgs, new MoveEventInfo(toMove, toMove.Path, containerId)), - this)) + new MoveEventArgs(evtMsgs, new MoveEventInfo(toMove, toMove.Path, containerId)), + this)) { return Attempt.Fail( new OperationStatus( @@ -1047,8 +1045,8 @@ namespace Umbraco.Core.Services var evtMsgs = EventMessagesFactory.Get(); if (MovingContentType.IsRaisedEventCancelled( - new MoveEventArgs(evtMsgs, new MoveEventInfo(toMove, toMove.Path, containerId)), - this)) + new MoveEventArgs(evtMsgs, new MoveEventInfo(toMove, toMove.Path, containerId)), + this)) { return Attempt.Fail( new OperationStatus( @@ -1178,8 +1176,8 @@ namespace Umbraco.Core.Services /// Optional Id of the user saving the MediaType public void Save(IMediaType mediaType, int userId = 0) { - if (SavingMediaType.IsRaisedEventCancelled(new SaveEventArgs(mediaType), this)) - return; + if (SavingMediaType.IsRaisedEventCancelled(new SaveEventArgs(mediaType), this)) + return; using (new WriteLock(Locker)) { @@ -1188,6 +1186,8 @@ namespace Umbraco.Core.Services { ValidateLocked(mediaType); // throws if invalid mediaType.CreatorId = userId; + if (mediaType.Description == string.Empty) + mediaType.Description = null; repository.AddOrUpdate(mediaType); uow.Commit(); @@ -1197,7 +1197,7 @@ namespace Umbraco.Core.Services } SavedMediaType.RaiseEvent(new SaveEventArgs(mediaType, false), this); - Audit(AuditType.Save, string.Format("Save MediaType performed by user"), userId, mediaType.Id); + Audit(AuditType.Save, string.Format("Save MediaType performed by user"), userId, mediaType.Id); } /// @@ -1210,7 +1210,7 @@ namespace Umbraco.Core.Services var asArray = mediaTypes.ToArray(); if (SavingMediaType.IsRaisedEventCancelled(new SaveEventArgs(asArray), this)) - return; + return; using (new WriteLock(Locker)) { @@ -1225,6 +1225,8 @@ namespace Umbraco.Core.Services foreach (var mediaType in asArray) { mediaType.CreatorId = userId; + if (mediaType.Description == string.Empty) + mediaType.Description = null; repository.AddOrUpdate(mediaType); } @@ -1236,7 +1238,7 @@ namespace Umbraco.Core.Services } SavedMediaType.RaiseEvent(new SaveEventArgs(asArray, false), this); - Audit(AuditType.Save, string.Format("Save MediaTypes performed by user"), userId, -1); + Audit(AuditType.Save, string.Format("Save MediaTypes performed by user"), userId, -1); } /// @@ -1247,8 +1249,8 @@ namespace Umbraco.Core.Services /// Deleting a will delete all the objects based on this public void Delete(IMediaType mediaType, int userId = 0) { - if (DeletingMediaType.IsRaisedEventCancelled(new DeleteEventArgs(mediaType), this)) - return; + if (DeletingMediaType.IsRaisedEventCancelled(new DeleteEventArgs(mediaType), this)) + return; using (new WriteLock(Locker)) { _mediaService.DeleteMediaOfType(mediaType.Id, userId); @@ -1280,7 +1282,7 @@ namespace Umbraco.Core.Services var asArray = mediaTypes.ToArray(); if (DeletingMediaType.IsRaisedEventCancelled(new DeleteEventArgs(asArray), this)) - return; + return; using (new WriteLock(Locker)) { foreach (var mediaType in asArray) @@ -1390,40 +1392,40 @@ namespace Umbraco.Core.Services /// public static event TypedEventHandler> DeletingContentType; - /// - /// Occurs after Delete - /// - public static event TypedEventHandler> DeletedContentType; + /// + /// Occurs after Delete + /// + public static event TypedEventHandler> DeletedContentType; - /// - /// Occurs before Delete - /// - public static event TypedEventHandler> DeletingMediaType; + /// + /// Occurs before Delete + /// + public static event TypedEventHandler> DeletingMediaType; - /// - /// Occurs after Delete - /// - public static event TypedEventHandler> DeletedMediaType; + /// + /// Occurs after Delete + /// + public static event TypedEventHandler> DeletedMediaType; /// /// Occurs before Save /// - public static event TypedEventHandler> SavingContentType; + public static event TypedEventHandler> SavingContentType; /// /// Occurs after Save /// - public static event TypedEventHandler> SavedContentType; + public static event TypedEventHandler> SavedContentType; - /// - /// Occurs before Save - /// - public static event TypedEventHandler> SavingMediaType; + /// + /// Occurs before Save + /// + public static event TypedEventHandler> SavingMediaType; - /// - /// Occurs after Save - /// - public static event TypedEventHandler> SavedMediaType; + /// + /// Occurs after Save + /// + public static event TypedEventHandler> SavedMediaType; /// /// Occurs before Move diff --git a/src/Umbraco.Core/Services/MemberTypeService.cs b/src/Umbraco.Core/Services/MemberTypeService.cs index 164bbc8243..0621cc7432 100644 --- a/src/Umbraco.Core/Services/MemberTypeService.cs +++ b/src/Umbraco.Core/Services/MemberTypeService.cs @@ -2,12 +2,10 @@ using System; using System.Collections.Generic; using System.Linq; using System.Threading; -using Umbraco.Core.Auditing; using Umbraco.Core.Events; using Umbraco.Core.Logging; using Umbraco.Core.Models; using Umbraco.Core.Persistence; -using Umbraco.Core.Persistence.Querying; using Umbraco.Core.Persistence.UnitOfWork; namespace Umbraco.Core.Services @@ -89,6 +87,8 @@ namespace Umbraco.Core.Services using (var repository = RepositoryFactory.CreateMemberTypeRepository(uow)) { memberType.CreatorId = userId; + if (memberType.Description == string.Empty) + memberType.Description = null; repository.AddOrUpdate(memberType); uow.Commit(); @@ -114,6 +114,8 @@ namespace Umbraco.Core.Services foreach (var memberType in asArray) { memberType.CreatorId = userId; + if (memberType.Description == string.Empty) + memberType.Description = null; repository.AddOrUpdate(memberType); } diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs index 78f4a5dad9..ffd27bedb6 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceTests.cs @@ -1,4 +1,3 @@ -using System.Runtime.Remoting; using NUnit.Framework; using System; using System.Collections.Generic; @@ -8,7 +7,6 @@ using Umbraco.Core.Exceptions; using Umbraco.Core.Models; using Umbraco.Core.Models.Rdbms; using Umbraco.Core.Services; -using Umbraco.Tests.CodeFirst.TestModels.Composition; using Umbraco.Tests.TestHelpers; using Umbraco.Tests.TestHelpers.Entities; @@ -1420,6 +1418,38 @@ namespace Umbraco.Tests.Services Assert.That(descriptionPropertyTypeReloaded.PropertyGroupId.IsValueCreated, Is.False); } + [Test] + public void Empty_Description_Is_Always_Null_After_Saving_Content_Type() + { + var service = ServiceContext.ContentTypeService; + var contentType = MockedContentTypes.CreateBasicContentType(); + contentType.Description = null; + service.Save(contentType); + + var contentType2 = MockedContentTypes.CreateBasicContentType("basePage2", "Base Page 2"); + contentType2.Description = string.Empty; + service.Save(contentType2); + + Assert.IsNull(contentType.Description); + Assert.IsNull(contentType2.Description); + } + + [Test] + public void Empty_Description_Is_Always_Null_After_Saving_Media_Type() + { + var service = ServiceContext.ContentTypeService; + var mediaType = MockedContentTypes.CreateSimpleMediaType("mediaType", "Media Type"); + mediaType.Description = null; + service.Save(mediaType); + + var mediaType2 = MockedContentTypes.CreateSimpleMediaType("mediaType2", "Media Type 2"); + mediaType2.Description = string.Empty; + service.Save(mediaType2); + + Assert.IsNull(mediaType.Description); + Assert.IsNull(mediaType2.Description); + } + private ContentType CreateComponent() { var component = new ContentType(-1) diff --git a/src/Umbraco.Tests/Services/MemberTypeServiceTests.cs b/src/Umbraco.Tests/Services/MemberTypeServiceTests.cs index 8dd18b8b50..373551a524 100644 --- a/src/Umbraco.Tests/Services/MemberTypeServiceTests.cs +++ b/src/Umbraco.Tests/Services/MemberTypeServiceTests.cs @@ -1,8 +1,6 @@ using System; -using System.Collections.Generic; using System.Linq; using NUnit.Framework; -using umbraco.cms.presentation.create.controls; using Umbraco.Core; using Umbraco.Core.Models; using Umbraco.Core.Models.Rdbms; @@ -183,6 +181,22 @@ namespace Umbraco.Tests.Services Assert.Throws(() => ServiceContext.MemberTypeService.Save(memberType)); } + [Test] + public void Empty_Description_Is_Always_Null_After_Saving_Member_Type() + { + var service = ServiceContext.MemberTypeService; + var memberType = MockedContentTypes.CreateSimpleMemberType(); + memberType.Description = null; + service.Save(memberType); + + var memberType2 = MockedContentTypes.CreateSimpleMemberType("memberType2", "Member Type 2"); + memberType2.Description = string.Empty; + service.Save(memberType2); + + Assert.IsNull(memberType.Description); + Assert.IsNull(memberType2.Description); + } + //[Test] //public void Can_Save_MemberType_Structure_And_Create_A_Member_Based_On_It() //{ diff --git a/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs b/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs index 85dd604203..a51fa39133 100644 --- a/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs +++ b/src/Umbraco.Tests/TestHelpers/Entities/MockedContentTypes.cs @@ -1,5 +1,4 @@ using System; -using System.Linq; using Umbraco.Core; using Umbraco.Core.Models;