From 3199aa6693655c5dc1ccc7f27230073f84bf3c08 Mon Sep 17 00:00:00 2001 From: Stephan Date: Thu, 31 Jan 2019 14:03:09 +0100 Subject: [PATCH 01/26] Support culture in content Saving and Publishing events --- src/Umbraco.Core/Events/EventNameExtractor.cs | 29 ++- src/Umbraco.Core/Models/Content.cs | 47 ++-- src/Umbraco.Core/Models/ContentBase.cs | 14 +- .../Models/ContentCultureInfosCollection.cs | 4 +- ...ContentCultureInfosCollectionExtensions.cs | 12 + src/Umbraco.Core/Models/IContent.cs | 24 ++ src/Umbraco.Core/Models/IContentBase.cs | 17 ++ .../Implement/DocumentRepository.cs | 3 + .../Services/Implement/ContentService.cs | 5 +- src/Umbraco.Core/Umbraco.Core.csproj | 1 + .../Services/AmbiguousEventTests.cs | 78 ++++++ .../Services/ContentServiceEventTests.cs | 224 ++++++++++++++++++ .../Services/ContentServiceTests.cs | 32 +-- src/Umbraco.Tests/TestHelpers/TestObjects.cs | 6 +- src/Umbraco.Tests/Umbraco.Tests.csproj | 2 + 15 files changed, 431 insertions(+), 67 deletions(-) create mode 100644 src/Umbraco.Core/Models/ContentCultureInfosCollectionExtensions.cs create mode 100644 src/Umbraco.Tests/Services/AmbiguousEventTests.cs create mode 100644 src/Umbraco.Tests/Services/ContentServiceEventTests.cs diff --git a/src/Umbraco.Core/Events/EventNameExtractor.cs b/src/Umbraco.Core/Events/EventNameExtractor.cs index 5cb9ca64ef..627426f2ee 100644 --- a/src/Umbraco.Core/Events/EventNameExtractor.cs +++ b/src/Umbraco.Core/Events/EventNameExtractor.cs @@ -35,6 +35,24 @@ namespace Umbraco.Core.Events /// null if not found or an ambiguous match /// public static Attempt FindEvent(Type senderType, Type argsType, Func exclude) + { + var events = FindEvents(senderType, argsType, exclude); + + switch (events.Length) + { + case 0: + return Attempt.Fail(new EventNameExtractorResult(EventNameExtractorError.NoneFound)); + + case 1: + return Attempt.Succeed(new EventNameExtractorResult(events[0])); + + default: + //there's more than one left so it's ambiguous! + return Attempt.Fail(new EventNameExtractorResult(EventNameExtractorError.Ambiguous)); + } + } + + internal static string[] FindEvents(Type senderType, Type argsType, Func exclude) { var found = MatchedEventNames.GetOrAdd(new Tuple(senderType, argsType), tuple => { @@ -78,16 +96,7 @@ namespace Umbraco.Core.Events }).Select(x => x.EventInfo.Name).ToArray(); }); - var filtered = found.Where(x => exclude(x) == false).ToArray(); - - if (filtered.Length == 0) - return Attempt.Fail(new EventNameExtractorResult(EventNameExtractorError.NoneFound)); - - if (filtered.Length == 1) - return Attempt.Succeed(new EventNameExtractorResult(filtered[0])); - - //there's more than one left so it's ambiguous! - return Attempt.Fail(new EventNameExtractorResult(EventNameExtractorError.Ambiguous)); + return found.Where(x => exclude(x) == false).ToArray(); } /// diff --git a/src/Umbraco.Core/Models/Content.cs b/src/Umbraco.Core/Models/Content.cs index 0c679e5e70..1093ef3a0c 100644 --- a/src/Umbraco.Core/Models/Content.cs +++ b/src/Umbraco.Core/Models/Content.cs @@ -15,14 +15,12 @@ namespace Umbraco.Core.Models [DataContract(IsReference = true)] public class Content : ContentBase, IContent { - private IContentType _contentType; private int? _templateId; private ContentScheduleCollection _schedule; private bool _published; private PublishedState _publishedState; - private ContentCultureInfosCollection _publishInfos; - private ContentCultureInfosCollection _publishInfosOrig; private HashSet _editedCultures; + private ContentCultureInfosCollection _publishInfos, _publishInfos1, _publishInfos2; private static readonly Lazy Ps = new Lazy(); @@ -48,7 +46,7 @@ namespace Umbraco.Core.Models public Content(string name, IContent parent, IContentType contentType, PropertyCollection properties, string culture = null) : base(name, parent, contentType, properties, culture) { - _contentType = contentType ?? throw new ArgumentNullException(nameof(contentType)); + ContentType = contentType ?? throw new ArgumentNullException(nameof(contentType)); _publishedState = PublishedState.Unpublished; PublishedVersionId = 0; } @@ -75,7 +73,7 @@ namespace Umbraco.Core.Models public Content(string name, int parentId, IContentType contentType, PropertyCollection properties, string culture = null) : base(name, parentId, contentType, properties, culture) { - _contentType = contentType ?? throw new ArgumentNullException(nameof(contentType)); + ContentType = contentType ?? throw new ArgumentNullException(nameof(contentType)); _publishedState = PublishedState.Unpublished; PublishedVersionId = 0; } @@ -137,7 +135,6 @@ namespace Umbraco.Core.Models set => SetPropertyValueAndDetectChanges(value, ref _templateId, Ps.Value.TemplateSelector); } - /// /// Gets or sets a value indicating whether this content item is published or not. /// @@ -181,7 +178,7 @@ namespace Umbraco.Core.Models /// Gets the ContentType used by this content object /// [IgnoreDataMember] - public IContentType ContentType => _contentType; + public IContentType ContentType { get; private set; } /// [IgnoreDataMember] @@ -217,7 +214,7 @@ namespace Umbraco.Core.Models public bool WasCulturePublished(string culture) // just check _publishInfosOrig - a copy of _publishInfos // a non-available culture could not become published anyways - => _publishInfosOrig != null && _publishInfosOrig.ContainsKey(culture); + => _publishInfos1 != null && _publishInfos1.ContainsKey(culture); // adjust dates to sync between version, cultures etc // used by the repo when persisting @@ -228,7 +225,7 @@ namespace Umbraco.Core.Models if (_publishInfos == null || !_publishInfos.TryGetValue(culture, out var publishInfos)) continue; - if (_publishInfosOrig != null && _publishInfosOrig.TryGetValue(culture, out var publishInfosOrig) + if (_publishInfos1 != null && _publishInfos1.TryGetValue(culture, out var publishInfosOrig) && publishInfosOrig.Date == publishInfos.Date) continue; @@ -285,6 +282,24 @@ namespace Umbraco.Core.Models _publishInfos.AddOrUpdate(culture, name, date); } + // internal for repository + internal void AknPublishInfo() + { + _publishInfos1 = _publishInfos2 = new ContentCultureInfosCollection(_publishInfos); + } + + /// + public bool IsPublishingCulture(string culture) => _publishInfos.IsCultureUpdated(_publishInfos1, culture); + + /// + public bool IsUnpublishingCulture(string culture) => _publishInfos.IsCultureRemoved(_publishInfos1, culture); + + /// + public bool HasPublishedCulture(string culture) => _publishInfos1.IsCultureUpdated(_publishInfos2, culture); + + /// + public bool HasUnpublishedCulture(string culture) => _publishInfos1.IsCultureRemoved(_publishInfos2, culture); + private void ClearPublishInfos() { _publishInfos = null; @@ -300,7 +315,7 @@ namespace Umbraco.Core.Models if (_publishInfos.Count == 0) _publishInfos = null; // set the culture to be dirty - it's been modified - TouchCultureInfo(culture); + TouchCulture(culture); } // sets a publish edited @@ -423,7 +438,7 @@ namespace Umbraco.Core.Models public void ChangeContentType(IContentType contentType) { ContentTypeId = contentType.Id; - _contentType = contentType; + ContentType = contentType; ContentTypeBase = contentType; Properties.EnsurePropertyTypes(PropertyTypes); @@ -442,7 +457,7 @@ namespace Umbraco.Core.Models if(clearProperties) { ContentTypeId = contentType.Id; - _contentType = contentType; + ContentType = contentType; ContentTypeBase = contentType; Properties.EnsureCleanPropertyTypes(PropertyTypes); @@ -457,16 +472,18 @@ namespace Umbraco.Core.Models public override void ResetDirtyProperties(bool rememberDirty) { base.ResetDirtyProperties(rememberDirty); - + if (ContentType != null) ContentType.ResetDirtyProperties(rememberDirty); // take care of the published state _publishedState = _published ? PublishedState.Published : PublishedState.Unpublished; + _publishInfos2 = _publishInfos1; + // Make a copy of the _publishInfos, this is purely so that we can detect // if this entity's previous culture publish state (regardless of the rememberDirty flag) - _publishInfosOrig = _publishInfos == null + _publishInfos1 = _publishInfos == null ? null : new ContentCultureInfosCollection(_publishInfos); @@ -500,7 +517,7 @@ namespace Umbraco.Core.Models var clonedContent = (Content)clone; //need to manually clone this since it's not settable - clonedContent._contentType = (IContentType) ContentType.DeepClone(); + clonedContent.ContentType = (IContentType) ContentType.DeepClone(); //if culture infos exist then deal with event bindings if (clonedContent._publishInfos != null) diff --git a/src/Umbraco.Core/Models/ContentBase.cs b/src/Umbraco.Core/Models/ContentBase.cs index ca1152a9a4..e540866c3e 100644 --- a/src/Umbraco.Core/Models/ContentBase.cs +++ b/src/Umbraco.Core/Models/ContentBase.cs @@ -25,7 +25,7 @@ namespace Umbraco.Core.Models protected IContentTypeComposition ContentTypeBase; private int _writerId; private PropertyCollection _properties; - private ContentCultureInfosCollection _cultureInfos; + private ContentCultureInfosCollection _cultureInfos, _cultureInfos1, _cultureInfos2; /// /// Initializes a new instance of the class. @@ -222,7 +222,8 @@ namespace Umbraco.Core.Models _cultureInfos = null; } - protected void TouchCultureInfo(string culture) + /// + public void TouchCulture(string culture) { if (_cultureInfos == null || !_cultureInfos.TryGetValue(culture, out var infos)) return; _cultureInfos.AddOrUpdate(culture, infos.Name, DateTime.Now); @@ -400,6 +401,9 @@ namespace Umbraco.Core.Models foreach (var prop in Properties) prop.ResetDirtyProperties(rememberDirty); + _cultureInfos2 = _cultureInfos1; + _cultureInfos1 = _cultureInfos == null ? null : new ContentCultureInfosCollection(_cultureInfos); + // take care of culture infos if (_cultureInfos == null) return; @@ -475,6 +479,12 @@ namespace Umbraco.Core.Models return instanceProperties.Concat(propertyTypes); } + /// + public bool IsSavingCulture(string culture) => _cultureInfos.IsCultureUpdated(_cultureInfos1, culture); + + /// + public bool HasSavedCulture(string culture) => _cultureInfos1.IsCultureUpdated(_cultureInfos2, culture); + #endregion /// diff --git a/src/Umbraco.Core/Models/ContentCultureInfosCollection.cs b/src/Umbraco.Core/Models/ContentCultureInfosCollection.cs index 82b0ba6475..287182d20e 100644 --- a/src/Umbraco.Core/Models/ContentCultureInfosCollection.cs +++ b/src/Umbraco.Core/Models/ContentCultureInfosCollection.cs @@ -31,7 +31,7 @@ namespace Umbraco.Core.Models foreach (var item in items) Add(new ContentCultureInfos(item)); } - + /// /// Adds or updates a instance. /// @@ -53,7 +53,7 @@ namespace Umbraco.Core.Models Name = name, Date = date }); - } + } } /// diff --git a/src/Umbraco.Core/Models/ContentCultureInfosCollectionExtensions.cs b/src/Umbraco.Core/Models/ContentCultureInfosCollectionExtensions.cs new file mode 100644 index 0000000000..98a0b48d07 --- /dev/null +++ b/src/Umbraco.Core/Models/ContentCultureInfosCollectionExtensions.cs @@ -0,0 +1,12 @@ +namespace Umbraco.Core.Models +{ + public static class ContentCultureInfosCollectionExtensions + { + public static bool IsCultureUpdated(this ContentCultureInfosCollection to, ContentCultureInfosCollection from, string culture) + => to != null && to.ContainsKey(culture) && + (from == null || !from.ContainsKey(culture) || from[culture].Date != to[culture].Date); + + public static bool IsCultureRemoved(this ContentCultureInfosCollection to, ContentCultureInfosCollection from, string culture) + => (to == null || !to.ContainsKey(culture)) && from != null && from.ContainsKey(culture); + } +} \ No newline at end of file diff --git a/src/Umbraco.Core/Models/IContent.cs b/src/Umbraco.Core/Models/IContent.cs index 4363324c8d..d0f47bc2f2 100644 --- a/src/Umbraco.Core/Models/IContent.cs +++ b/src/Umbraco.Core/Models/IContent.cs @@ -177,5 +177,29 @@ namespace Umbraco.Core.Models /// Unpublishing must be finalized via the content service SavePublishing method. /// void UnpublishCulture(string culture = "*"); + + /// + /// Determines whether a culture is being published, during a Publishing event. + /// + /// Outside of a Publishing event handler, the returned value is unspecified. + bool IsPublishingCulture(string culture); + + /// + /// Determines whether a culture is being unpublished, during a Publishing event. + /// + /// Outside of a Publishing event handler, the returned value is unspecified. + bool IsUnpublishingCulture(string culture); + + /// + /// Determines whether a culture has been published, during a Published event. + /// + /// Outside of a Published event handler, the returned value is unspecified. + bool HasPublishedCulture(string culture); + + /// + /// Determines whether a culture has been unpublished, during a Published event. + /// + /// Outside of a Published event handler, the returned value is unspecified. + bool HasUnpublishedCulture(string culture); } } diff --git a/src/Umbraco.Core/Models/IContentBase.cs b/src/Umbraco.Core/Models/IContentBase.cs index 40a1c57097..d2a928f978 100644 --- a/src/Umbraco.Core/Models/IContentBase.cs +++ b/src/Umbraco.Core/Models/IContentBase.cs @@ -143,5 +143,22 @@ namespace Umbraco.Core.Models /// If the content type is variant, then culture can be either '*' or an actual culture, but neither 'null' nor /// 'empty'. If the content type is invariant, then culture can be either '*' or null or empty. Property[] ValidateProperties(string culture = "*"); + + /// + /// Determines whether a culture is being saved, during a Saving event. + /// + /// Outside of a Saving event handler, the returned value is unspecified. + bool IsSavingCulture(string culture); + + /// + /// Determines whether a culture has been saved, during a Saved event. + /// + /// Outside of a Saved event handler, the returned value is unspecified. + bool HasSavedCulture(string culture); + + /// + /// Updates a culture date, if the culture exists. + /// + void TouchCulture(string culture); } } diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs index 6af7031883..70f3bd8071 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs @@ -1188,8 +1188,11 @@ namespace Umbraco.Core.Persistence.Repositories.Implement foreach (var v in contentVariation) content.SetCultureInfo(v.Culture, v.Name, v.Date); if (content.PublishedVersionId > 0 && contentVariations.TryGetValue(content.PublishedVersionId, out contentVariation)) + { foreach (var v in contentVariation) content.SetPublishInfo(v.Culture, v.Name, v.Date); + content.AknPublishInfo(); + } if (documentVariations.TryGetValue(content.Id, out var documentVariation)) foreach (var v in documentVariation.Where(x => x.Edited)) content.SetCultureEdited(v.Culture); diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index 266f34cc37..804fe35dd1 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -4,7 +4,6 @@ using System.Globalization; using System.Linq; using Umbraco.Core.Events; using Umbraco.Core.Exceptions; -using Umbraco.Core.IO; using Umbraco.Core.Logging; using Umbraco.Core.Models; using Umbraco.Core.Models.Membership; @@ -27,18 +26,16 @@ namespace Umbraco.Core.Services.Implement private readonly IContentTypeRepository _contentTypeRepository; private readonly IDocumentBlueprintRepository _documentBlueprintRepository; private readonly ILanguageRepository _languageRepository; - private readonly IMediaFileSystem _mediaFileSystem; private IQuery _queryNotTrashed; #region Constructors public ContentService(IScopeProvider provider, ILogger logger, - IEventMessagesFactory eventMessagesFactory, IMediaFileSystem mediaFileSystem, + IEventMessagesFactory eventMessagesFactory, IDocumentRepository documentRepository, IEntityRepository entityRepository, IAuditRepository auditRepository, IContentTypeRepository contentTypeRepository, IDocumentBlueprintRepository documentBlueprintRepository, ILanguageRepository languageRepository) : base(provider, logger, eventMessagesFactory) { - _mediaFileSystem = mediaFileSystem; _documentRepository = documentRepository; _entityRepository = entityRepository; _auditRepository = auditRepository; diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 4e6e832294..e4fbfb3892 100755 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -388,6 +388,7 @@ + diff --git a/src/Umbraco.Tests/Services/AmbiguousEventTests.cs b/src/Umbraco.Tests/Services/AmbiguousEventTests.cs new file mode 100644 index 0000000000..e137da1188 --- /dev/null +++ b/src/Umbraco.Tests/Services/AmbiguousEventTests.cs @@ -0,0 +1,78 @@ +using System; +using System.Reflection; +using System.Text; +using NUnit.Framework; +using Umbraco.Core.Events; +using Umbraco.Core.Services.Implement; + +namespace Umbraco.Tests.Services +{ + [TestFixture] + public class AmbiguousEventTests + { + [Explicit] + [TestCase(typeof(ContentService))] + [TestCase(typeof(MediaService))] + public void ListAmbiguousEvents(Type serviceType) + { + var typedEventHandler = typeof(TypedEventHandler<,>); + + // get all events + var events = serviceType.GetEvents(BindingFlags.Static | BindingFlags.Public); + + string TypeName(Type type) + { + if (!type.IsGenericType) + return type.Name; + var sb = new StringBuilder(); + TypeNameSb(type, sb); + return sb.ToString(); + } + + void TypeNameSb(Type type, StringBuilder sb) + { + var name = type.Name; + var pos = name.IndexOf('`'); + name = pos > 0 ? name.Substring(0, pos) : name; + sb.Append(name); + if (!type.IsGenericType) + return; + sb.Append("<"); + var first = true; + foreach (var arg in type.GetGenericArguments()) + { + if (first) first = false; + else sb.Append(", "); + TypeNameSb(arg, sb); + } + sb.Append(">"); + } + + foreach (var e in events) + { + // only continue if this is a TypedEventHandler + if (!e.EventHandlerType.IsGenericType) continue; + var typeDef = e.EventHandlerType.GetGenericTypeDefinition(); + if (typedEventHandler != typeDef) continue; + + // get the event args type + var eventArgsType = e.EventHandlerType.GenericTypeArguments[1]; + + // try to find the event back, based upon sender type + args type + // exclude -ing (eg Saving) events, we don't deal with them in EventDefinitionBase (they always trigger) + var found = EventNameExtractor.FindEvents(serviceType, eventArgsType, EventNameExtractor.MatchIngNames); + + if (found.Length == 1) continue; + + if (found.Length == 0) + { + Console.WriteLine($"{typeof(ContentService).Name} {e.Name} {TypeName(eventArgsType)} NotFound"); + continue; + } + + Console.WriteLine($"{typeof(ContentService).Name} {e.Name} {TypeName(eventArgsType)} Ambiguous"); + Console.WriteLine("\t" + string.Join(", ", found)); + } + } + } +} \ No newline at end of file diff --git a/src/Umbraco.Tests/Services/ContentServiceEventTests.cs b/src/Umbraco.Tests/Services/ContentServiceEventTests.cs new file mode 100644 index 0000000000..3576425b9c --- /dev/null +++ b/src/Umbraco.Tests/Services/ContentServiceEventTests.cs @@ -0,0 +1,224 @@ +using System.Linq; +using NUnit.Framework; +using Umbraco.Core.Events; +using Umbraco.Core.Models; +using Umbraco.Core.Persistence.Repositories.Implement; +using Umbraco.Core.Services; +using Umbraco.Core.Services.Implement; +using Umbraco.Tests.TestHelpers.Entities; +using Umbraco.Tests.Testing; + +namespace Umbraco.Tests.Services +{ + [TestFixture] + [UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest, + PublishedRepositoryEvents = true, + WithApplication = true, + Logger = UmbracoTestOptions.Logger.Console)] + public class ContentServiceEventTests : TestWithSomeContentBase + { + public override void SetUp() + { + base.SetUp(); + ContentRepositoryBase.ThrowOnWarning = true; + } + + public override void TearDown() + { + ContentRepositoryBase.ThrowOnWarning = false; + base.TearDown(); + } + + [Test] + public void SavingTest() + { + var languageService = ServiceContext.LocalizationService; + + languageService.Save(new Language("fr-FR")); + + var contentTypeService = ServiceContext.ContentTypeService; + + var contentType = MockedContentTypes.CreateTextPageContentType(); + ServiceContext.FileService.SaveTemplate(contentType.DefaultTemplate); + contentType.Variations = ContentVariation.Culture; + foreach (var propertyType in contentType.PropertyTypes) + propertyType.Variations = ContentVariation.Culture; + contentTypeService.Save(contentType); + + var contentService = ServiceContext.ContentService; + + var document = new Content("content", -1, contentType); + document.SetCultureName("hello", "en-US"); + document.SetCultureName("bonjour", "fr-FR"); + contentService.Save(document); + + // properties: title, bodyText, keywords, description + document.SetValue("title", "title-en", "en-US"); + + // touch the culture - required for IsSaving/HasSaved to work + document.TouchCulture("fr-FR"); + + void OnSaving(IContentService sender, SaveEventArgs e) + { + var saved = e.SavedEntities.First(); + + Assert.AreSame(document, saved); + + Assert.IsTrue(saved.IsSavingCulture("fr-FR")); + Assert.IsFalse(saved.IsSavingCulture("en-UK")); + } + + void OnSaved(IContentService sender, SaveEventArgs e) + { + var saved = e.SavedEntities.First(); + + Assert.AreSame(document, saved); + + Assert.IsTrue(saved.HasSavedCulture("fr-FR")); + Assert.IsFalse(saved.HasSavedCulture("en-UK")); + } + + ContentService.Saving += OnSaving; + ContentService.Saved += OnSaved; + contentService.Save(document); + ContentService.Saving -= OnSaving; + ContentService.Saved -= OnSaved; + } + + [Test] + public void PublishingTest() + { + var languageService = ServiceContext.LocalizationService; + + languageService.Save(new Language("fr-FR")); + + var contentTypeService = ServiceContext.ContentTypeService; + + var contentType = MockedContentTypes.CreateTextPageContentType(); + ServiceContext.FileService.SaveTemplate(contentType.DefaultTemplate); + contentType.Variations = ContentVariation.Culture; + foreach (var propertyType in contentType.PropertyTypes) + propertyType.Variations = ContentVariation.Culture; + contentTypeService.Save(contentType); + + var contentService = ServiceContext.ContentService; + + var document = new Content("content", -1, contentType); + document.SetCultureName("hello", "en-US"); + document.SetCultureName("bonjour", "fr-FR"); + contentService.Save(document); + + // ensure it works and does not throw + Assert.IsFalse(document.WasCulturePublished("fr-FR")); + Assert.IsFalse(document.WasCulturePublished("en-US")); + Assert.IsFalse(document.IsCulturePublished("fr-FR")); + Assert.IsFalse(document.IsCulturePublished("en-US")); + + void OnPublishing(IContentService sender, PublishEventArgs e) + { + var publishing = e.PublishedEntities.First(); + + Assert.AreSame(document, publishing); + + Assert.IsFalse(publishing.IsPublishingCulture("en-US")); + Assert.IsTrue(publishing.IsPublishingCulture("fr-FR")); + } + + void OnPublished(IContentService sender, PublishEventArgs e) + { + var published = e.PublishedEntities.First(); + + Assert.AreSame(document, published); + + Assert.IsFalse(published.HasPublishedCulture("en-US")); + Assert.IsTrue(published.HasPublishedCulture("fr-FR")); + } + + ContentService.Publishing += OnPublishing; + ContentService.Published += OnPublished; + contentService.SaveAndPublish(document, "fr-FR"); + ContentService.Publishing -= OnPublishing; + ContentService.Published -= OnPublished; + + document = (Content) contentService.GetById(document.Id); + + // ensure it works and does not throw + Assert.IsTrue(document.WasCulturePublished("fr-FR")); + Assert.IsFalse(document.WasCulturePublished("en-US")); + Assert.IsTrue(document.IsCulturePublished("fr-FR")); + Assert.IsFalse(document.IsCulturePublished("en-US")); + } + + [Test] + public void UnpublishingTest() + { + var languageService = ServiceContext.LocalizationService; + + languageService.Save(new Language("fr-FR")); + + var contentTypeService = ServiceContext.ContentTypeService; + + var contentType = MockedContentTypes.CreateTextPageContentType(); + ServiceContext.FileService.SaveTemplate(contentType.DefaultTemplate); + contentType.Variations = ContentVariation.Culture; + foreach (var propertyType in contentType.PropertyTypes) + propertyType.Variations = ContentVariation.Culture; + contentTypeService.Save(contentType); + + var contentService = ServiceContext.ContentService; + + var document = new Content("content", -1, contentType); + document.SetCultureName("hello", "en-US"); + document.SetCultureName("bonjour", "fr-FR"); + contentService.SaveAndPublish(document); + + // ensure it works and does not throw + Assert.IsTrue(document.WasCulturePublished("fr-FR")); + Assert.IsTrue(document.WasCulturePublished("en-US")); + Assert.IsTrue(document.IsCulturePublished("fr-FR")); + Assert.IsTrue(document.IsCulturePublished("en-US")); + + void OnPublishing(IContentService sender, PublishEventArgs e) + { + var publishing = e.PublishedEntities.First(); + + Assert.AreSame(document, publishing); + + Assert.IsFalse(publishing.IsPublishingCulture("en-US")); + Assert.IsFalse(publishing.IsPublishingCulture("fr-FR")); + + Assert.IsFalse(publishing.IsUnpublishingCulture("en-US")); + Assert.IsTrue(publishing.IsUnpublishingCulture("fr-FR")); + } + + void OnPublished(IContentService sender, PublishEventArgs e) + { + var published = e.PublishedEntities.First(); + + Assert.AreSame(document, published); + + Assert.IsFalse(published.HasPublishedCulture("en-US")); + Assert.IsFalse(published.HasPublishedCulture("fr-FR")); + + Assert.IsFalse(published.HasUnpublishedCulture("en-US")); + Assert.IsTrue(published.HasUnpublishedCulture("fr-FR")); + } + + document.UnpublishCulture("fr-FR"); + + ContentService.Publishing += OnPublishing; + ContentService.Published += OnPublished; + contentService.SavePublishing(document); + ContentService.Publishing -= OnPublishing; + ContentService.Published -= OnPublished; + + document = (Content) contentService.GetById(document.Id); + + // ensure it works and does not throw + Assert.IsFalse(document.WasCulturePublished("fr-FR")); + Assert.IsTrue(document.WasCulturePublished("en-US")); + Assert.IsFalse(document.IsCulturePublished("fr-FR")); + Assert.IsTrue(document.IsCulturePublished("en-US")); + } + } +} diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index 37d34557bb..e339d8d1b6 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -6,7 +6,6 @@ using System.Threading; using Moq; using NUnit.Framework; using Umbraco.Core; -using Umbraco.Core.Configuration.UmbracoSettings; using Umbraco.Core.Models; using Umbraco.Core.Models.Membership; using Umbraco.Core.Services; @@ -14,14 +13,11 @@ using Umbraco.Tests.TestHelpers.Entities; using Umbraco.Core.Events; using Umbraco.Core.Persistence.Dtos; using Umbraco.Core.Persistence.Repositories.Implement; -using Umbraco.Core.PropertyEditors; using Umbraco.Core.Scoping; using Umbraco.Core.Services.Implement; using Umbraco.Tests.Testing; -using System.Reflection; using Umbraco.Core.Persistence.DatabaseModelDefinitions; using Umbraco.Core.Cache; -using Umbraco.Core.Composing; namespace Umbraco.Tests.Services { @@ -60,32 +56,6 @@ namespace Umbraco.Tests.Services Composition.RegisterUnique(factory => Mock.Of()); } - /// - /// Used to list out all ambiguous events that will require dispatching with a name - /// - [Test, Explicit] - public void List_Ambiguous_Events() - { - var events = ServiceContext.ContentService.GetType().GetEvents(BindingFlags.Static | BindingFlags.Public); - var typedEventHandler = typeof(TypedEventHandler<,>); - foreach(var e in events) - { - //only continue if this is a TypedEventHandler - if (!e.EventHandlerType.IsGenericType) continue; - var typeDef = e.EventHandlerType.GetGenericTypeDefinition(); - if (typedEventHandler != typeDef) continue; - - //get the event arg type - var eventArgType = e.EventHandlerType.GenericTypeArguments[1]; - - var found = EventNameExtractor.FindEvent(typeof(ContentService), eventArgType, EventNameExtractor.MatchIngNames); - if (!found.Success && found.Result.Error == EventNameExtractorError.Ambiguous) - { - Console.WriteLine($"Ambiguous event, source: {typeof(ContentService)}, args: {eventArgType}"); - } - } - } - [Test] public void Create_Blueprint() { @@ -818,7 +788,7 @@ namespace Umbraco.Tests.Services content.PublishCulture(langGB.IsoCode); content.PublishCulture(langFr.IsoCode); Assert.IsTrue(ServiceContext.ContentService.SavePublishing(content).Success); - + //re-get content = ServiceContext.ContentService.GetById(content.Id); Assert.AreEqual(PublishedState.Published, content.PublishedState); diff --git a/src/Umbraco.Tests/TestHelpers/TestObjects.cs b/src/Umbraco.Tests/TestHelpers/TestObjects.cs index 7a9702031b..46af5d9e71 100644 --- a/src/Umbraco.Tests/TestHelpers/TestObjects.cs +++ b/src/Umbraco.Tests/TestHelpers/TestObjects.cs @@ -153,7 +153,7 @@ namespace Umbraco.Tests.TestHelpers var localizationService = GetLazyService(factory, c => new LocalizationService(scopeProvider, logger, eventMessagesFactory, GetRepo(c), GetRepo(c), GetRepo(c))); var userService = GetLazyService(factory, c => new UserService(scopeProvider, logger, eventMessagesFactory, runtimeState, GetRepo(c), GetRepo(c),globalSettings)); var dataTypeService = GetLazyService(factory, c => new DataTypeService(scopeProvider, logger, eventMessagesFactory, GetRepo(c), GetRepo(c), GetRepo(c), GetRepo(c), GetRepo(c))); - var contentService = GetLazyService(factory, c => new ContentService(scopeProvider, logger, eventMessagesFactory, mediaFileSystem, GetRepo(c), GetRepo(c), GetRepo(c), GetRepo(c), GetRepo(c), GetRepo(c))); + var contentService = GetLazyService(factory, c => new ContentService(scopeProvider, logger, eventMessagesFactory, GetRepo(c), GetRepo(c), GetRepo(c), GetRepo(c), GetRepo(c), GetRepo(c))); var notificationService = GetLazyService(factory, c => new NotificationService(scopeProvider, userService.Value, contentService.Value, localizationService.Value, logger, GetRepo(c), globalSettings, umbracoSettings.Content)); var serverRegistrationService = GetLazyService(factory, c => new ServerRegistrationService(scopeProvider, logger, eventMessagesFactory, GetRepo(c))); var memberGroupService = GetLazyService(factory, c => new MemberGroupService(scopeProvider, logger, eventMessagesFactory, GetRepo(c))); @@ -176,7 +176,7 @@ namespace Umbraco.Tests.TestHelpers var propertyEditorCollection = new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty())); var compiledPackageXmlParser = new CompiledPackageXmlParser(new ConflictingPackageData(macroService.Value, fileService.Value)); return new PackagingService( - auditService.Value, + auditService.Value, new PackagesRepository(contentService.Value, contentTypeService.Value, dataTypeService.Value, fileService.Value, macroService.Value, localizationService.Value, new EntityXmlSerializer(contentService.Value, mediaService.Value, dataTypeService.Value, userService.Value, localizationService.Value, contentTypeService.Value, urlSegmentProviders), logger, "createdPackages.config"), new PackagesRepository(contentService.Value, contentTypeService.Value, dataTypeService.Value, fileService.Value, macroService.Value, localizationService.Value, @@ -188,7 +188,7 @@ namespace Umbraco.Tests.TestHelpers new DirectoryInfo(IOHelper.GetRootDirectorySafe()))); }); var relationService = GetLazyService(factory, c => new RelationService(scopeProvider, logger, eventMessagesFactory, entityService.Value, GetRepo(c), GetRepo(c))); - var tagService = GetLazyService(factory, c => new TagService(scopeProvider, logger, eventMessagesFactory, GetRepo(c))); + var tagService = GetLazyService(factory, c => new TagService(scopeProvider, logger, eventMessagesFactory, GetRepo(c))); var redirectUrlService = GetLazyService(factory, c => new RedirectUrlService(scopeProvider, logger, eventMessagesFactory, GetRepo(c))); var consentService = GetLazyService(factory, c => new ConsentService(scopeProvider, logger, eventMessagesFactory, GetRepo(c))); diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 986dad12fd..1703743c6f 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -141,6 +141,8 @@ + + From eeaf8d594ddc569faa5c73df4c4db700102e0943 Mon Sep 17 00:00:00 2001 From: Kenn Jacobsen Date: Thu, 31 Jan 2019 19:43:57 +0100 Subject: [PATCH 02/26] Make NC resilient towards culture variance in elements --- .../PropertyEditors/NestedContentPropertyEditor.cs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/Umbraco.Web/PropertyEditors/NestedContentPropertyEditor.cs b/src/Umbraco.Web/PropertyEditors/NestedContentPropertyEditor.cs index 7ff6439e08..6dee2f78b5 100644 --- a/src/Umbraco.Web/PropertyEditors/NestedContentPropertyEditor.cs +++ b/src/Umbraco.Web/PropertyEditors/NestedContentPropertyEditor.cs @@ -172,18 +172,16 @@ namespace Umbraco.Web.PropertyEditors try { // create a temp property with the value + // - force it to be culture invariant as NC can't handle culture variant element properties + propType.Variations = ContentVariation.Nothing; var tempProp = new Property(propType); - // if the property varies by culture, make sure we save using the current culture - var propCulture = propType.VariesByCulture() || propType.VariesByCultureAndSegment() - ? culture - : null; - tempProp.SetValue(propValues[propAlias] == null ? null : propValues[propAlias].ToString(), propCulture); + tempProp.SetValue(propValues[propAlias] == null ? null : propValues[propAlias].ToString()); // convert that temp property, and store the converted value var propEditor = _propertyEditors[propType.PropertyEditorAlias]; var tempConfig = dataTypeService.GetDataType(propType.DataTypeId).Configuration; var valEditor = propEditor.GetValueEditor(tempConfig); - var convValue = valEditor.ToEditor(tempProp, dataTypeService, propCulture); + var convValue = valEditor.ToEditor(tempProp, dataTypeService); propValues[propAlias] = convValue == null ? null : JToken.FromObject(convValue); } catch (InvalidOperationException) From 90a282c083e8a465db65476e926086c6159c6e62 Mon Sep 17 00:00:00 2001 From: Kenn Jacobsen Date: Fri, 1 Feb 2019 20:09:54 +0100 Subject: [PATCH 03/26] Fix the icon color picker --- .../directives/components/umbcolorswatches.directive.js | 4 ++-- .../infiniteeditors/iconpicker/iconpicker.controller.js | 3 ++- .../views/common/infiniteeditors/iconpicker/iconpicker.html | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/umbcolorswatches.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/umbcolorswatches.directive.js index aac11bfd22..9401cacab1 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/umbcolorswatches.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/umbcolorswatches.directive.js @@ -36,7 +36,7 @@ Use this directive to generate color swatches to pick from. scope.setColor = function (color, $index, $event) { scope.selectedColor = color; if (scope.onSelect) { - scope.onSelect(color.color, $index, $event); + scope.onSelect({color: color, $index: $index, $event: $event}); $event.stopPropagation(); } }; @@ -55,7 +55,7 @@ Use this directive to generate color swatches to pick from. colors: '=?', size: '@', selectedColor: '=', - onSelect: '=', + onSelect: '&', useLabel: '=', useColorClass: '=?' }, diff --git a/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/iconpicker/iconpicker.controller.js b/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/iconpicker/iconpicker.controller.js index 16d6cf23fc..9c7e6de83b 100644 --- a/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/iconpicker/iconpicker.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/iconpicker/iconpicker.controller.js @@ -71,7 +71,8 @@ function IconPickerController($scope, iconHelper, localizationService) { } function selectColor(color, $index, $event) { - $scope.model.color = color; + $scope.model.color = color.value; + vm.color = color.value; } function close() { diff --git a/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/iconpicker/iconpicker.html b/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/iconpicker/iconpicker.html index 55c4317279..b29d9af2fa 100644 --- a/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/iconpicker/iconpicker.html +++ b/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/iconpicker/iconpicker.html @@ -37,7 +37,7 @@ colors="vm.colors" selected-color="vm.color" size="s" - on-select="vm.selectColor"> + on-select="vm.selectColor(color)"> From 6874a5486b328ceb8d747f4fa10cdd33c849a455 Mon Sep 17 00:00:00 2001 From: Kenn Jacobsen Date: Fri, 1 Feb 2019 20:28:27 +0100 Subject: [PATCH 04/26] Fix selected color --- .../infiniteeditors/iconpicker/iconpicker.controller.js | 8 ++++++-- .../common/infiniteeditors/iconpicker/iconpicker.html | 4 ++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/iconpicker/iconpicker.controller.js b/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/iconpicker/iconpicker.controller.js index 9c7e6de83b..471d23ae84 100644 --- a/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/iconpicker/iconpicker.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/iconpicker/iconpicker.controller.js @@ -49,7 +49,7 @@ function IconPickerController($scope, iconHelper, localizationService) { }); // set a default color if nothing is passed in - vm.color = $scope.model.color ? $scope.model.color : vm.colors[0].value; + vm.color = $scope.model.color ? findColor($scope.model.color) : vm.colors[0]; // if an icon is passed in - preselect it vm.icon = $scope.model.icon ? $scope.model.icon : undefined; @@ -70,9 +70,13 @@ function IconPickerController($scope, iconHelper, localizationService) { submit(); } + function findColor(value) { + return _.findWhere(vm.colors, {value: value}); + } + function selectColor(color, $index, $event) { $scope.model.color = color.value; - vm.color = color.value; + vm.color = color; } function close() { diff --git a/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/iconpicker/iconpicker.html b/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/iconpicker/iconpicker.html index b29d9af2fa..3caa6ae03d 100644 --- a/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/iconpicker/iconpicker.html +++ b/src/Umbraco.Web.UI.Client/src/views/common/infiniteeditors/iconpicker/iconpicker.html @@ -44,9 +44,9 @@
-
    +
    • - +
      • From b51d3036e3d76dd7d0d892d73fe7ebc4daa67660 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 4 Feb 2019 16:55:35 +1100 Subject: [PATCH 05/26] Adds overload of SaveAndPublish to accept multiple cultures, renames SavePublishing to CommitDocumentChanges, removes IContent.ChangeContentType --- src/Umbraco.Core/ContentExtensions.cs | 16 +--- src/Umbraco.Core/Models/Content.cs | 4 +- src/Umbraco.Core/Models/ContentBase.cs | 2 +- src/Umbraco.Core/Models/IContent.cs | 19 +---- src/Umbraco.Core/Services/IContentService.cs | 42 +++++++--- .../Services/Implement/ContentService.cs | 46 ++++++++--- src/Umbraco.Tests/Models/ContentTests.cs | 23 +++--- .../ContentServicePublishBranchTests.cs | 8 +- .../Services/ContentServiceTests.cs | 78 +++++++------------ src/Umbraco.Web/Editors/ContentController.cs | 17 ++-- 10 files changed, 122 insertions(+), 133 deletions(-) diff --git a/src/Umbraco.Core/ContentExtensions.cs b/src/Umbraco.Core/ContentExtensions.cs index 5db080f3f3..9fcc12bc10 100644 --- a/src/Umbraco.Core/ContentExtensions.cs +++ b/src/Umbraco.Core/ContentExtensions.cs @@ -67,21 +67,7 @@ namespace Umbraco.Core content.WasCulturePublished(x)) // but was published before .ToList(); } - - /// - /// Returns true if this entity was just published as part of a recent save operation (i.e. it wasn't previously published) - /// - /// - /// - /// - /// This is helpful for determining if the published event will execute during the saved event for a content item. - /// - internal static bool JustPublished(this IContent entity) - { - var dirty = (IRememberBeingDirty)entity; - return dirty.WasPropertyDirty("Published") && entity.Published; - } - + #endregion /// diff --git a/src/Umbraco.Core/Models/Content.cs b/src/Umbraco.Core/Models/Content.cs index 16b28e088a..93f2b08482 100644 --- a/src/Umbraco.Core/Models/Content.cs +++ b/src/Umbraco.Core/Models/Content.cs @@ -420,7 +420,7 @@ namespace Umbraco.Core.Models /// /// New ContentType for this content /// Leaves PropertyTypes intact after change - public void ChangeContentType(IContentType contentType) + internal void ChangeContentType(IContentType contentType) { ContentTypeId = contentType.Id; ContentType = new SimpleContentType(contentType); @@ -437,7 +437,7 @@ namespace Umbraco.Core.Models ///
/// New ContentType for this content /// Boolean indicating whether to clear PropertyTypes upon change - public void ChangeContentType(IContentType contentType, bool clearProperties) + internal void ChangeContentType(IContentType contentType, bool clearProperties) { if(clearProperties) { diff --git a/src/Umbraco.Core/Models/ContentBase.cs b/src/Umbraco.Core/Models/ContentBase.cs index ca1152a9a4..89749da7e2 100644 --- a/src/Umbraco.Core/Models/ContentBase.cs +++ b/src/Umbraco.Core/Models/ContentBase.cs @@ -376,7 +376,7 @@ namespace Umbraco.Core.Models #region Validation /// - public virtual Property[] ValidateProperties(string culture = "*") + public Property[] ValidateProperties(string culture = "*") { var alsoInvariant = culture != null && culture != "*"; diff --git a/src/Umbraco.Core/Models/IContent.cs b/src/Umbraco.Core/Models/IContent.cs index ee38f613a3..1d85a735b5 100644 --- a/src/Umbraco.Core/Models/IContent.cs +++ b/src/Umbraco.Core/Models/IContent.cs @@ -79,7 +79,7 @@ namespace Umbraco.Core.Models /// and the content published name for this culture is non-null. It becomes non-published /// whenever values for this culture are unpublished. /// A culture becomes published as soon as PublishCulture has been invoked, - /// even though the document might now have been saved yet (and can have no identity). + /// even though the document might not have been saved yet (and can have no identity). /// Does not support the '*' wildcard (returns false). /// bool IsCulturePublished(string culture); @@ -137,23 +137,6 @@ namespace Umbraco.Core.Models /// IEnumerable EditedCultures { get; } - // TODO: these two should move to some kind of service - - /// - /// Changes the for the current content object - /// - /// New ContentType for this content - /// Leaves PropertyTypes intact after change - void ChangeContentType(IContentType contentType); - - /// - /// Changes the for the current content object and removes PropertyTypes, - /// which are not part of the new ContentType. - /// - /// New ContentType for this content - /// Boolean indicating whether to clear PropertyTypes upon change - void ChangeContentType(IContentType contentType, bool clearProperties); - /// /// Creates a deep clone of the current entity with its identity/alias and it's property identities reset /// diff --git a/src/Umbraco.Core/Services/IContentService.cs b/src/Umbraco.Core/Services/IContentService.cs index 7fb7450b46..ad97ba2ddc 100644 --- a/src/Umbraco.Core/Services/IContentService.cs +++ b/src/Umbraco.Core/Services/IContentService.cs @@ -336,29 +336,47 @@ namespace Umbraco.Core.Services /// /// /// By default, publishes all variations of the document, but it is possible to specify a culture to be published. - /// When a culture is being published, it includes all varying values along with all invariant values. For - /// anything more complicated, see . + /// When a culture is being published, it includes all varying values along with all invariant values. /// The document is *always* saved, even when publishing fails. /// If the content type is variant, then culture can be either '*' or an actual culture, but neither 'null' nor /// 'empty'. If the content type is invariant, then culture can be either '*' or null or empty. /// + /// + /// + /// + /// + /// PublishResult SaveAndPublish(IContent content, string culture = "*", int userId = 0, bool raiseEvents = true); /// - /// Saves and publishes a publishing document. + /// Saves and publishes a document. /// /// - /// A publishing document is a document with values that are being published, i.e. - /// that have been published or cleared via and - /// . - /// When one needs to publish or unpublish a single culture, or all cultures, using - /// and is the way to go. But if one needs to, say, publish two cultures and unpublish a third - /// one, in one go, then one needs to invoke and - /// on the content itself - this prepares the content, but does not commit anything - and then, invoke - /// to actually commit the changes to the database. + /// By default, publishes all variations of the document, but it is possible to specify a culture to be published. + /// When a culture is being published, it includes all varying values along with all invariant values. /// The document is *always* saved, even when publishing fails. /// - PublishResult SavePublishing(IContent content, int userId = 0, bool raiseEvents = true); + /// + /// The cultures to publish. + /// + /// + /// + PublishResult SaveAndPublish(IContent content, string[] cultures, int userId = 0, bool raiseEvents = true); + + /// + /// Expert: Saves a document and publishes/unpublishes any pending publishing changes made to the document. + /// + /// + /// Pending publishing/unpublishing changes on a document are made with calls to and + /// . + /// When publishing or unpublishing a single culture, or all cultures, use + /// and . But if the flexibility to both publish and unpublish in a single operation is required + /// then this method needs to be used in combination with and + /// on the content itself - this prepares the content, but does not commit anything - and then, invoke + /// to actually commit the changes to the database. + /// The document is *always* saved, even when publishing fails. + /// + PublishResult CommitDocumentChanges(IContent content, int userId = 0, bool raiseEvents = true); /// /// Saves and publishes a document branch. diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index c5745090f9..404adeea72 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -857,7 +857,7 @@ namespace Umbraco.Core.Services.Implement var publishedState = content.PublishedState; if (publishedState != PublishedState.Published && publishedState != PublishedState.Unpublished) - throw new InvalidOperationException($"Cannot save-and-publish (un)publishing content, use the dedicated {nameof(SavePublishing)} method."); + throw new InvalidOperationException($"Cannot save-and-publish (un)publishing content, use the dedicated {nameof(CommitDocumentChanges)} method."); // cannot accept invariant (null or empty) culture for variant content type // cannot accept a specific culture for invariant content type (but '*' is ok) @@ -891,7 +891,31 @@ namespace Umbraco.Core.Services.Implement // finally, "save publishing" // what happens next depends on whether the content can be published or not - return SavePublishing(content, userId, raiseEvents); + return CommitDocumentChanges(content, userId, raiseEvents); + } + + /// + public PublishResult SaveAndPublish(IContent content, string[] cultures, int userId = 0, bool raiseEvents = true) + { + if (content == null) throw new ArgumentNullException(nameof(content)); + if (cultures == null) throw new ArgumentNullException(nameof(cultures)); + + var evtMsgs = EventMessagesFactory.Get(); + + var varies = content.ContentType.VariesByCulture(); + + if (cultures.Length == 0) + { + //no cultures specified and doesn't vary, so publish it, else nothing to publish + return !varies + ? SaveAndPublish(content, userId: userId, raiseEvents: raiseEvents) + : new PublishResult(PublishResultType.FailedPublishNothingToPublish, evtMsgs, content); + } + + if (cultures.Select(content.PublishCulture).Any(isValid => !isValid)) + return new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, content); //fixme: no way to know which one failed? + + return CommitDocumentChanges(content, userId, raiseEvents); } /// @@ -903,7 +927,7 @@ namespace Umbraco.Core.Services.Implement var publishedState = content.PublishedState; if (publishedState != PublishedState.Published && publishedState != PublishedState.Unpublished) - throw new InvalidOperationException($"Cannot save-and-publish (un)publishing content, use the dedicated {nameof(SavePublishing)} method."); + throw new InvalidOperationException($"Cannot save-and-publish (un)publishing content, use the dedicated {nameof(CommitDocumentChanges)} method."); // cannot accept invariant (null or empty) culture for variant content type // cannot accept a specific culture for invariant content type (but '*' is ok) @@ -938,22 +962,22 @@ namespace Umbraco.Core.Services.Implement } // finally, "save publishing" - return SavePublishing(content, userId); + return CommitDocumentChanges(content, userId); } /// - public PublishResult SavePublishing(IContent content, int userId = 0, bool raiseEvents = true) + public PublishResult CommitDocumentChanges(IContent content, int userId = 0, bool raiseEvents = true) { using (var scope = ScopeProvider.CreateScope()) { scope.WriteLock(Constants.Locks.ContentTree); - var result = SavePublishingInternal(scope, content, userId, raiseEvents); + var result = CommitDocumentChangesInternal(scope, content, userId, raiseEvents); scope.Complete(); return result; } } - private PublishResult SavePublishingInternal(IScope scope, IContent content, int userId = 0, bool raiseEvents = true, bool branchOne = false, bool branchRoot = false) + private PublishResult CommitDocumentChangesInternal(IScope scope, IContent content, int userId = 0, bool raiseEvents = true, bool branchOne = false, bool branchRoot = false) { var evtMsgs = EventMessagesFactory.Get(); PublishResult publishResult = null; @@ -961,7 +985,7 @@ namespace Umbraco.Core.Services.Implement // nothing set = republish it all if (content.PublishedState != PublishedState.Publishing && content.PublishedState != PublishedState.Unpublishing) - ((Content)content).PublishedState = PublishedState.Publishing; + ((Content)content).PublishedState = PublishedState.Publishing; //TODO: fix this https://github.com/umbraco/Umbraco-CMS/issues/4234 // state here is either Publishing or Unpublishing // (even though, Publishing to unpublish a culture may end up unpublishing everything) @@ -1217,7 +1241,7 @@ namespace Umbraco.Core.Services.Implement else if (!publishing) result = new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, d); else - result = SavePublishing(d, d.WriterId); + result = CommitDocumentChanges(d, d.WriterId); if (result.Success == false) Logger.Error(null, "Failed to publish document id={DocumentId}, reason={Reason}.", d.Id, result.Result); @@ -1261,7 +1285,7 @@ namespace Umbraco.Core.Services.Implement if (pendingCultures.Count > 0) { - result = SavePublishing(d, d.WriterId); + result = CommitDocumentChanges(d, d.WriterId); if (result.Success == false) Logger.Error(null, "Failed to publish document id={DocumentId}, reason={Reason}.", d.Id, result.Result); yield return result; @@ -1495,7 +1519,7 @@ namespace Umbraco.Core.Services.Implement if (!publishCultures(document, culturesToPublish)) return new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, document); - var result = SavePublishingInternal(scope, document, userId, branchOne: true, branchRoot: isRoot); + var result = CommitDocumentChangesInternal(scope, document, userId, branchOne: true, branchRoot: isRoot); if (result.Success) publishedDocuments.Add(document); return result; diff --git a/src/Umbraco.Tests/Models/ContentTests.cs b/src/Umbraco.Tests/Models/ContentTests.cs index 862925db2c..805c957a2e 100644 --- a/src/Umbraco.Tests/Models/ContentTests.cs +++ b/src/Umbraco.Tests/Models/ContentTests.cs @@ -704,20 +704,23 @@ namespace Umbraco.Tests.Models } [Test] + [Ignore("Need to reimplement this logic for v8")] public void Can_Change_ContentType_On_Content_And_Clear_Old_PropertyTypes() { - // Arrange - var contentType = MockedContentTypes.CreateTextPageContentType(); - var simpleContentType = MockedContentTypes.CreateSimpleContentType(); - var content = MockedContent.CreateTextpageContent(contentType, "Textpage", -1); + throw new NotImplementedException(); - // Act - content.ChangeContentType(simpleContentType, true); + //// Arrange + //var contentType = MockedContentTypes.CreateTextPageContentType(); + //var simpleContentType = MockedContentTypes.CreateSimpleContentType(); + //var content = MockedContent.CreateTextpageContent(contentType, "Textpage", -1); - // Assert - Assert.That(content.Properties.Contains("author"), Is.True); - Assert.That(content.Properties.Contains("keywords"), Is.False); - Assert.That(content.Properties.Contains("description"), Is.False); + //// Act + //content.ChangeContentType(simpleContentType, true); + + //// Assert + //Assert.That(content.Properties.Contains("author"), Is.True); + //Assert.That(content.Properties.Contains("keywords"), Is.False); + //Assert.That(content.Properties.Contains("description"), Is.False); } [Test] diff --git a/src/Umbraco.Tests/Services/ContentServicePublishBranchTests.cs b/src/Umbraco.Tests/Services/ContentServicePublishBranchTests.cs index 4afd5e33eb..d2343d3dea 100644 --- a/src/Umbraco.Tests/Services/ContentServicePublishBranchTests.cs +++ b/src/Umbraco.Tests/Services/ContentServicePublishBranchTests.cs @@ -264,9 +264,7 @@ namespace Umbraco.Tests.Services vRoot.SetValue("vp", "changed.es", "es"); ServiceContext.ContentService.Save(vRoot); // now root has drafts in all cultures - iv1.PublishCulture("de"); - iv1.PublishCulture("ru"); - ServiceContext.ContentService.SavePublishing(iv1); // now iv1 de and ru are published + ServiceContext.ContentService.SaveAndPublish(iv1, new []{"de", "ru"}); // now iv1 de and ru are published iv1.SetValue("ip", "changed"); iv1.SetValue("vp", "changed.de", "de"); @@ -345,10 +343,8 @@ namespace Umbraco.Tests.Services iv11.SetValue("vp", "iv11.es", "es"); ServiceContext.ContentService.Save(iv11); - iv11.PublishCulture("de"); iv11.SetCultureName("iv11.ru", "ru"); - iv11.PublishCulture("ru"); - ServiceContext.ContentService.SavePublishing(iv11); + ServiceContext.ContentService.SaveAndPublish(iv11, new []{"de", "ru"}); Assert.AreEqual("iv11.de", iv11.GetValue("vp", "de", published: true)); Assert.AreEqual("iv11.ru", iv11.GetValue("vp", "ru", published: true)); diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index 017f1f50ec..3e21768f97 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -767,7 +767,7 @@ namespace Umbraco.Tests.Services Assert.IsTrue(content.IsCulturePublished(langUk.IsoCode)); Assert.IsFalse(content.WasCulturePublished(langUk.IsoCode)); //not persisted yet, will be false - var published = ServiceContext.ContentService.SavePublishing(content); + var published = ServiceContext.ContentService.SaveAndPublish(content, new[]{ langFr.IsoCode , langUk.IsoCode }); //re-get content = ServiceContext.ContentService.GetById(content.Id); Assert.IsTrue(published.Success); @@ -819,9 +819,8 @@ namespace Umbraco.Tests.Services IContent content = new Content("content", -1, contentType); content.SetCultureName("content-en", langGB.IsoCode); content.SetCultureName("content-fr", langFr.IsoCode); - content.PublishCulture(langGB.IsoCode); - content.PublishCulture(langFr.IsoCode); - Assert.IsTrue(ServiceContext.ContentService.SavePublishing(content).Success); + + Assert.IsTrue(ServiceContext.ContentService.SaveAndPublish(content, new []{ langGB.IsoCode , langFr.IsoCode }).Success); //re-get content = ServiceContext.ContentService.GetById(content.Id); @@ -861,8 +860,7 @@ namespace Umbraco.Tests.Services IContent content = new Content("content", -1, contentType); content.SetCultureName("content-fr", langFr.IsoCode); - content.PublishCulture(langFr.IsoCode); - var published = ServiceContext.ContentService.SavePublishing(content); + var published = ServiceContext.ContentService.SaveAndPublish(content, langFr.IsoCode); //audit log will only show that french was published var lastLog = ServiceContext.AuditService.GetLogs(content.Id).Last(); Assert.AreEqual($"Published languages: French (France)", lastLog.Comment); @@ -870,8 +868,7 @@ namespace Umbraco.Tests.Services //re-get content = ServiceContext.ContentService.GetById(content.Id); content.SetCultureName("content-en", langGB.IsoCode); - content.PublishCulture(langGB.IsoCode); - published = ServiceContext.ContentService.SavePublishing(content); + published = ServiceContext.ContentService.SaveAndPublish(content, langGB.IsoCode); //audit log will only show that english was published lastLog = ServiceContext.AuditService.GetLogs(content.Id).Last(); Assert.AreEqual($"Published languages: English (United Kingdom)", lastLog.Comment); @@ -895,15 +892,12 @@ namespace Umbraco.Tests.Services IContent content = new Content("content", -1, contentType); content.SetCultureName("content-fr", langFr.IsoCode); content.SetCultureName("content-gb", langGB.IsoCode); - content.PublishCulture(langGB.IsoCode); - content.PublishCulture(langFr.IsoCode); - var published = ServiceContext.ContentService.SavePublishing(content); + var published = ServiceContext.ContentService.SaveAndPublish(content, new[] {langGB.IsoCode, langFr.IsoCode}); Assert.IsTrue(published.Success); //re-get content = ServiceContext.ContentService.GetById(content.Id); - content.UnpublishCulture(langFr.IsoCode); //unpublish non-mandatory lang - var unpublished = ServiceContext.ContentService.SavePublishing(content); + var unpublished = ServiceContext.ContentService.Unpublish(content, langFr.IsoCode); //audit log will only show that french was unpublished var lastLog = ServiceContext.AuditService.GetLogs(content.Id).Last(); Assert.AreEqual($"Unpublished languages: French (France)", lastLog.Comment); @@ -911,8 +905,7 @@ namespace Umbraco.Tests.Services //re-get content = ServiceContext.ContentService.GetById(content.Id); content.SetCultureName("content-en", langGB.IsoCode); - content.UnpublishCulture(langGB.IsoCode); //unpublish mandatory lang - unpublished = ServiceContext.ContentService.SavePublishing(content); + unpublished = ServiceContext.ContentService.Unpublish(content, langGB.IsoCode); //audit log will only show that english was published var logs = ServiceContext.AuditService.GetLogs(content.Id).ToList(); Assert.AreEqual($"Unpublished languages: English (United Kingdom)", logs[logs.Count - 2].Comment); @@ -927,8 +920,7 @@ namespace Umbraco.Tests.Services var content = contentService.GetById(NodeDto.NodeIdSeed + 2); // Act - content.PublishCulture(); - var published = contentService.SavePublishing(content, Constants.Security.SuperUserId); + var published = contentService.SaveAndPublish(content, userId: Constants.Security.SuperUserId); // Assert Assert.That(published.Success, Is.True); @@ -982,8 +974,7 @@ namespace Umbraco.Tests.Services Assert.AreEqual("Home", content.Name); content.Name = "foo"; - content.PublishCulture(); - var published = contentService.SavePublishing(content, Constants.Security.SuperUserId); + var published = contentService.SaveAndPublish(content, userId: Constants.Security.SuperUserId); Assert.That(published.Success, Is.True); Assert.That(content.Published, Is.True); @@ -1030,10 +1021,7 @@ namespace Umbraco.Tests.Services Assert.IsTrue(parentPublished.Success); Assert.IsTrue(parent.Published); - var contentCanPublishValues = content.PublishCulture(); - // content cannot publish values because they are invalid - Assert.IsFalse(contentCanPublishValues); Assert.IsNotEmpty(content.ValidateProperties()); // and therefore cannot be published, @@ -1061,7 +1049,7 @@ namespace Umbraco.Tests.Services content.SetCultureName("name-da", langDa.IsoCode); content.PublishCulture(langFr.IsoCode); - var result = ServiceContext.ContentService.SavePublishing(content); + var result = ServiceContext.ContentService.CommitDocumentChanges(content); Assert.IsTrue(result.Success); content = ServiceContext.ContentService.GetById(content.Id); Assert.IsTrue(content.IsCulturePublished(langFr.IsoCode)); @@ -1070,7 +1058,7 @@ namespace Umbraco.Tests.Services content.UnpublishCulture(langFr.IsoCode); content.PublishCulture(langDa.IsoCode); - result = ServiceContext.ContentService.SavePublishing(content); + result = ServiceContext.ContentService.CommitDocumentChanges(content); Assert.IsTrue(result.Success); Assert.AreEqual(PublishResultType.SuccessMixedCulture, result.Result); @@ -1149,12 +1137,10 @@ namespace Umbraco.Tests.Services contentService.Save(content); var parent = contentService.GetById(NodeDto.NodeIdSeed + 2); - parent.PublishCulture(); - var parentPublished = contentService.SavePublishing(parent, Constants.Security.SuperUserId);//Publish root Home node to enable publishing of 'NodeDto.NodeIdSeed + 3' + var parentPublished = contentService.SaveAndPublish(parent, userId: Constants.Security.SuperUserId);//Publish root Home node to enable publishing of 'NodeDto.NodeIdSeed + 3' // Act - content.PublishCulture(); - var published = contentService.SavePublishing(content, Constants.Security.SuperUserId); + var published = contentService.SaveAndPublish(content, userId: Constants.Security.SuperUserId); // Assert Assert.That(parentPublished.Success, Is.True); @@ -1192,12 +1178,10 @@ namespace Umbraco.Tests.Services contentService.Save(content, Constants.Security.SuperUserId); var parent = contentService.GetById(NodeDto.NodeIdSeed + 2); - parent.PublishCulture(); - var parentPublished = contentService.SavePublishing(parent, Constants.Security.SuperUserId);//Publish root Home node to enable publishing of 'NodeDto.NodeIdSeed + 3' + var parentPublished = contentService.SaveAndPublish(parent, userId: Constants.Security.SuperUserId);//Publish root Home node to enable publishing of 'NodeDto.NodeIdSeed + 3' // Act - content.PublishCulture(); - var published = contentService.SavePublishing(content, Constants.Security.SuperUserId); + var published = contentService.SaveAndPublish(content, userId: Constants.Security.SuperUserId); // Assert Assert.That(parentPublished.Success, Is.True); @@ -1249,8 +1233,7 @@ namespace Umbraco.Tests.Services var content = contentService.GetById(NodeDto.NodeIdSeed + 5); // Act - content.PublishCulture(); - var published = contentService.SavePublishing(content, Constants.Security.SuperUserId); + var published = contentService.SaveAndPublish(content, userId: Constants.Security.SuperUserId); // Assert Assert.That(published.Success, Is.False); @@ -1267,8 +1250,7 @@ namespace Umbraco.Tests.Services content.SetValue("author", "Barack Obama"); // Act - content.PublishCulture(); - var published = contentService.SavePublishing(content, Constants.Security.SuperUserId); + var published = contentService.SaveAndPublish(content, userId: Constants.Security.SuperUserId); // Assert Assert.That(content.HasIdentity, Is.True); @@ -1292,15 +1274,13 @@ namespace Umbraco.Tests.Services content.SetValue("author", "Barack Obama"); // Act - content.PublishCulture(); - var published = contentService.SavePublishing(content, Constants.Security.SuperUserId); + var published = contentService.SaveAndPublish(content, userId: Constants.Security.SuperUserId); var childContent = contentService.Create("Child", content.Id, "umbTextpage", Constants.Security.SuperUserId); // Reset all identity properties childContent.Id = 0; childContent.Path = null; ((Content)childContent).ResetIdentity(); - childContent.PublishCulture(); - var childPublished = contentService.SavePublishing(childContent, Constants.Security.SuperUserId); + var childPublished = contentService.SaveAndPublish(childContent, userId: Constants.Security.SuperUserId); // Assert Assert.That(content.HasIdentity, Is.True); @@ -1639,14 +1619,12 @@ namespace Umbraco.Tests.Services content1.PropertyValues(obj); content1.ResetDirtyProperties(false); ServiceContext.ContentService.Save(content1, Constants.Security.SuperUserId); - content1.PublishCulture(); - Assert.IsTrue(ServiceContext.ContentService.SavePublishing(content1, 0).Success); + Assert.IsTrue(ServiceContext.ContentService.SaveAndPublish(content1, userId: 0).Success); var content2 = MockedContent.CreateBasicContent(contentType); content2.PropertyValues(obj); content2.ResetDirtyProperties(false); ServiceContext.ContentService.Save(content2, Constants.Security.SuperUserId); - content2.PublishCulture(); - Assert.IsTrue(ServiceContext.ContentService.SavePublishing(content2, 0).Success); + Assert.IsTrue(ServiceContext.ContentService.SaveAndPublish(content2, userId: 0).Success); var editorGroup = ServiceContext.UserService.GetUserGroupByAlias("editor"); editorGroup.StartContentId = content1.Id; @@ -2721,9 +2699,7 @@ namespace Umbraco.Tests.Services // act - content.PublishCulture(langFr.IsoCode); - content.PublishCulture(langUk.IsoCode); - contentService.SavePublishing(content); + contentService.SaveAndPublish(content, new[]{ langFr.IsoCode, langUk.IsoCode }); // both FR and UK have been published, // and content has been published, @@ -2827,8 +2803,7 @@ namespace Umbraco.Tests.Services // act // cannot just 'save' since we are changing what's published! - content.UnpublishCulture(langFr.IsoCode); - contentService.SavePublishing(content); + contentService.Unpublish(content, langFr.IsoCode); // content has been published, // the french culture is gone @@ -2919,7 +2894,7 @@ namespace Umbraco.Tests.Services // that HAS to be SavePublishing, because SaveAndPublish would just republish everything! - contentService.SavePublishing(content); + contentService.CommitDocumentChanges(content); // content has been re-published, // everything is back to what it was before being unpublished @@ -2959,8 +2934,7 @@ namespace Umbraco.Tests.Services // act - content.PublishCulture(langUk.IsoCode); - contentService.SavePublishing(content); + contentService.SaveAndPublish(content, langUk.IsoCode); content2 = contentService.GetById(content.Id); diff --git a/src/Umbraco.Web/Editors/ContentController.cs b/src/Umbraco.Web/Editors/ContentController.cs index ee056e38f0..5cbd83d236 100644 --- a/src/Umbraco.Web/Editors/ContentController.cs +++ b/src/Umbraco.Web/Editors/ContentController.cs @@ -1148,7 +1148,6 @@ namespace Umbraco.Web.Editors /// Performs the publishing operation for a content item /// /// - /// /// /// /// if the content is variant this will return an array of cultures that will be published (passed validation rules) @@ -1197,10 +1196,12 @@ namespace Umbraco.Web.Editors if (canPublish) { + var culturesToPublish = cultureVariants.Where(x => x.Publish).Select(x => x.Culture).ToArray(); + //proceed to publish if all validation still succeeds - var publishStatus = Services.ContentService.SavePublishing(contentItem.PersistedContent, Security.CurrentUser.Id); + var publishStatus = Services.ContentService.SaveAndPublish(contentItem.PersistedContent, culturesToPublish, Security.CurrentUser.Id); wasCancelled = publishStatus.Result == PublishResultType.FailedPublishCancelledByEvent; - successfulCultures = contentItem.Variants.Where(x => x.Publish).Select(x => x.Culture).ToArray(); + successfulCultures = culturesToPublish; return publishStatus; } else @@ -1219,6 +1220,10 @@ namespace Umbraco.Web.Editors /// /// /// + /// + /// + /// + /// /// private bool ValidatePublishingMandatoryLanguages( ContentItemSave contentItem, @@ -1240,7 +1245,7 @@ namespace Umbraco.Web.Editors var isPublished = contentItem.PersistedContent.Published && contentItem.PersistedContent.IsCulturePublished(culture); result.Add((mandatoryVariant, isPublished)); - var isPublishing = isPublished ? true : publishingCheck(mandatoryVariant); + var isPublishing = isPublished || publishingCheck(mandatoryVariant); if (isPublished || isPublishing) continue; @@ -1254,7 +1259,7 @@ namespace Umbraco.Web.Editors } /// - /// This will call PublishCulture on the content item for each culture that needs to be published including the invariant culture + /// Call PublishCulture on the content item for each culture to get a validation result for each culture /// /// /// @@ -1311,7 +1316,7 @@ namespace Umbraco.Web.Editors return HandleContentNotFound(id, false); } - var publishResult = Services.ContentService.SavePublishing(foundContent, Security.GetUserId().ResultOr(0)); + var publishResult = Services.ContentService.SaveAndPublish(foundContent, userId: Security.GetUserId().ResultOr(0)); if (publishResult.Success == false) { var notificationModel = new SimpleNotificationModel(); From 09255019b22d4dc8d3c0ea3d29793db18b001b34 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 5 Feb 2019 01:22:52 +1100 Subject: [PATCH 06/26] removes CommitDocumentChanges from the public interface --- src/Umbraco.Core/Services/IContentService.cs | 15 --------------- .../Services/Implement/ContentService.cs | 17 +++++++++++++++-- .../Services/ContentServiceTests.cs | 12 ++++++++---- 3 files changed, 23 insertions(+), 21 deletions(-) diff --git a/src/Umbraco.Core/Services/IContentService.cs b/src/Umbraco.Core/Services/IContentService.cs index ad97ba2ddc..600921ee78 100644 --- a/src/Umbraco.Core/Services/IContentService.cs +++ b/src/Umbraco.Core/Services/IContentService.cs @@ -363,21 +363,6 @@ namespace Umbraco.Core.Services /// PublishResult SaveAndPublish(IContent content, string[] cultures, int userId = 0, bool raiseEvents = true); - /// - /// Expert: Saves a document and publishes/unpublishes any pending publishing changes made to the document. - /// - /// - /// Pending publishing/unpublishing changes on a document are made with calls to and - /// . - /// When publishing or unpublishing a single culture, or all cultures, use - /// and . But if the flexibility to both publish and unpublish in a single operation is required - /// then this method needs to be used in combination with and - /// on the content itself - this prepares the content, but does not commit anything - and then, invoke - /// to actually commit the changes to the database. - /// The document is *always* saved, even when publishing fails. - /// - PublishResult CommitDocumentChanges(IContent content, int userId = 0, bool raiseEvents = true); - /// /// Saves and publishes a document branch. /// diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index 404adeea72..adc6c919f1 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -965,8 +965,21 @@ namespace Umbraco.Core.Services.Implement return CommitDocumentChanges(content, userId); } - /// - public PublishResult CommitDocumentChanges(IContent content, int userId = 0, bool raiseEvents = true) + /// + /// Saves a document and publishes/unpublishes any pending publishing changes made to the document. + /// + /// + /// This is the underlying logic for both publishing and unpublishing any document + /// Pending publishing/unpublishing changes on a document are made with calls to and + /// . + /// When publishing or unpublishing a single culture, or all cultures, use + /// and . But if the flexibility to both publish and unpublish in a single operation is required + /// then this method needs to be used in combination with and + /// on the content itself - this prepares the content, but does not commit anything - and then, invoke + /// to actually commit the changes to the database. + /// The document is *always* saved, even when publishing fails. + /// + internal PublishResult CommitDocumentChanges(IContent content, int userId = 0, bool raiseEvents = true) { using (var scope = ScopeProvider.CreateScope()) { diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index 3e21768f97..7dd6c7ba03 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -1035,6 +1035,8 @@ namespace Umbraco.Tests.Services [Test] public void Can_Publish_And_Unpublish_Cultures_In_Single_Operation() { + //TODO: This is using an internal API - we aren't exposing this publicly (at least for now) but we'll keep the test around + var langFr = new Language("fr"); var langDa = new Language("da"); ServiceContext.LocalizationService.Save(langFr); @@ -1049,7 +1051,7 @@ namespace Umbraco.Tests.Services content.SetCultureName("name-da", langDa.IsoCode); content.PublishCulture(langFr.IsoCode); - var result = ServiceContext.ContentService.CommitDocumentChanges(content); + var result = ((ContentService)ServiceContext.ContentService).CommitDocumentChanges(content); Assert.IsTrue(result.Success); content = ServiceContext.ContentService.GetById(content.Id); Assert.IsTrue(content.IsCulturePublished(langFr.IsoCode)); @@ -1058,7 +1060,7 @@ namespace Umbraco.Tests.Services content.UnpublishCulture(langFr.IsoCode); content.PublishCulture(langDa.IsoCode); - result = ServiceContext.ContentService.CommitDocumentChanges(content); + result = ((ContentService)ServiceContext.ContentService).CommitDocumentChanges(content); Assert.IsTrue(result.Success); Assert.AreEqual(PublishResultType.SuccessMixedCulture, result.Result); @@ -2893,8 +2895,10 @@ namespace Umbraco.Tests.Services // act // that HAS to be SavePublishing, because SaveAndPublish would just republish everything! - - contentService.CommitDocumentChanges(content); + //TODO: This is using an internal API - the test can't pass without this but we want to keep the test here + // will need stephane to have a look at this test at some stage since there is a lot of logic here that we + // want to keep on testing but don't need the public API to do these more complicated things. + ((ContentService)contentService).CommitDocumentChanges(content); // content has been re-published, // everything is back to what it was before being unpublished From a5947356e756bab88beccd2f13fb164146ad5578 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 5 Feb 2019 01:58:18 +1100 Subject: [PATCH 07/26] Removes WasCulturePublished from IContent --- src/Umbraco.Core/ContentExtensions.cs | 10 ++++++++-- src/Umbraco.Core/Models/Content.cs | 17 ++--------------- src/Umbraco.Core/Models/IContent.cs | 8 -------- .../Repositories/IDocumentRepository.cs | 2 ++ .../Implement/DocumentRepository.cs | 3 ++- .../Services/Implement/ContentService.cs | 15 ++++++++++++--- .../Services/ContentServiceTests.cs | 17 +++++------------ 7 files changed, 31 insertions(+), 41 deletions(-) diff --git a/src/Umbraco.Core/ContentExtensions.cs b/src/Umbraco.Core/ContentExtensions.cs index 9fcc12bc10..44e9907210 100644 --- a/src/Umbraco.Core/ContentExtensions.cs +++ b/src/Umbraco.Core/ContentExtensions.cs @@ -56,15 +56,21 @@ namespace Umbraco.Core /// Gets the cultures that have been flagged for unpublishing. /// /// Gets cultures for which content.UnpublishCulture() has been invoked. - internal static IReadOnlyList GetCulturesUnpublishing(this IContent content) + internal static IReadOnlyList GetCulturesUnpublishing(this IContent content, IContent persisted) { + //TODO: The reason we need a ref to the original persisted IContent is to check if it is published + // however, for performance reasons we could pass in a ContentCultureInfosCollection which could be + // resolved from the database much more quickly than resolving an entire IContent object. + // That said, the GetById on the IContentService will return from cache so might not be something to worry about. + + if (!content.Published || !content.ContentType.VariesByCulture() || !content.IsPropertyDirty("PublishCultureInfos")) return Array.Empty(); var culturesChanging = content.CultureInfos.Where(x => x.Value.IsDirty()).Select(x => x.Key); return culturesChanging .Where(x => !content.IsCulturePublished(x) && // is not published anymore - content.WasCulturePublished(x)) // but was published before + persisted != null && persisted.IsCulturePublished(x)) // but was published before .ToList(); } diff --git a/src/Umbraco.Core/Models/Content.cs b/src/Umbraco.Core/Models/Content.cs index 84ccc42a77..63ab65b8fe 100644 --- a/src/Umbraco.Core/Models/Content.cs +++ b/src/Umbraco.Core/Models/Content.cs @@ -19,7 +19,6 @@ namespace Umbraco.Core.Models private bool _published; private PublishedState _publishedState; private ContentCultureInfosCollection _publishInfos; - private ContentCultureInfosCollection _publishInfosOrig; private HashSet _editedCultures; /// @@ -201,11 +200,6 @@ namespace Umbraco.Core.Models // a non-available culture could not become published anyways => _publishInfos != null && _publishInfos.ContainsKey(culture); - /// - public bool WasCulturePublished(string culture) - // just check _publishInfosOrig - a copy of _publishInfos - // a non-available culture could not become published anyways - => _publishInfosOrig != null && _publishInfosOrig.ContainsKey(culture); // adjust dates to sync between version, cultures etc // used by the repo when persisting @@ -216,9 +210,8 @@ namespace Umbraco.Core.Models if (_publishInfos == null || !_publishInfos.TryGetValue(culture, out var publishInfos)) continue; - if (_publishInfosOrig != null && _publishInfosOrig.TryGetValue(culture, out var publishInfosOrig) - && publishInfosOrig.Date == publishInfos.Date) - continue; + //fixme: Removing the logic here for the old WasCulturePublished and the _publishInfosOrig has broken + // the test Can_Rollback_Version_On_Multilingual, but we need to understand what it's doing since I don't _publishInfos.AddOrUpdate(culture, publishInfos.Name, date); @@ -449,12 +442,6 @@ namespace Umbraco.Core.Models // take care of the published state _publishedState = _published ? PublishedState.Published : PublishedState.Unpublished; - // Make a copy of the _publishInfos, this is purely so that we can detect - // if this entity's previous culture publish state (regardless of the rememberDirty flag) - _publishInfosOrig = _publishInfos == null - ? null - : new ContentCultureInfosCollection(_publishInfos); - if (_publishInfos == null) return; foreach (var infos in _publishInfos) diff --git a/src/Umbraco.Core/Models/IContent.cs b/src/Umbraco.Core/Models/IContent.cs index 1d85a735b5..f468df59ab 100644 --- a/src/Umbraco.Core/Models/IContent.cs +++ b/src/Umbraco.Core/Models/IContent.cs @@ -84,14 +84,6 @@ namespace Umbraco.Core.Models /// bool IsCulturePublished(string culture); - /// - /// Gets a value indicating whether a culture was published. - /// - /// - /// Mirrors whenever the content item is saved. - /// - bool WasCulturePublished(string culture); - /// /// Gets the date a culture was published. /// diff --git a/src/Umbraco.Core/Persistence/Repositories/IDocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/IDocumentRepository.cs index fc5382499f..d49c92f1bf 100644 --- a/src/Umbraco.Core/Persistence/Repositories/IDocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/IDocumentRepository.cs @@ -7,6 +7,8 @@ namespace Umbraco.Core.Persistence.Repositories { public interface IDocumentRepository : IContentRepository, IReadRepository { + + /// /// Clears the publishing schedule for all entries having an a date before (lower than, or equal to) a specified date. /// diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs index 6af7031883..9b4175b63f 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs @@ -1127,7 +1127,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement // TODO: shall we get published properties or not? //var publishedVersionId = dto.Published ? dto.PublishedVersionDto.Id : 0; - var publishedVersionId = dto.PublishedVersionDto != null ? dto.PublishedVersionDto.Id : 0; + var publishedVersionId = dto.PublishedVersionDto?.Id ?? 0; var temp = new TempContent(dto.NodeId, versionId, publishedVersionId, contentType); var ltemp = new List> { temp }; @@ -1424,6 +1424,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement } } + // ReSharper disable once ClassNeverInstantiated.Local private class CultureNodeName { public int Id { get; set; } diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index adc6c919f1..9c2721cbb4 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -921,6 +921,8 @@ namespace Umbraco.Core.Services.Implement /// public PublishResult Unpublish(IContent content, string culture = "*", int userId = 0) { + if (content == null) throw new ArgumentNullException(nameof(content)); + var evtMsgs = EventMessagesFactory.Get(); culture = culture.NullOrWhiteSpaceAsNull(); @@ -949,12 +951,16 @@ namespace Umbraco.Core.Services.Implement // all cultures = unpublish whole if (culture == "*" || (!content.ContentType.VariesByCulture() && culture == null)) { + //TODO: Stop casting https://github.com/umbraco/Umbraco-CMS/issues/4234 ((Content)content).PublishedState = PublishedState.Unpublishing; } else { - // if the culture we want to unpublish was already unpublished, nothing to do - if (!content.WasCulturePublished(culture)) + // If the culture we want to unpublish was already unpublished, nothing to do. + // To check for that we need to lookup the persisted content item + var persisted = content.HasIdentity ? GetById(content.Id) : null; + + if (persisted != null && !persisted.IsCulturePublished(culture)) return new PublishResult(PublishResultType.SuccessUnpublishAlready, evtMsgs, content); // unpublish the culture @@ -1025,7 +1031,10 @@ namespace Umbraco.Core.Services.Implement if (publishing) { - culturesUnpublishing = content.GetCulturesUnpublishing(); + //to continue, we need to have a reference to the original IContent item that is currently persisted + var persisted = content.HasIdentity ? GetById(content.Id) : null; + + culturesUnpublishing = content.GetCulturesUnpublishing(persisted); culturesPublishing = variesByCulture ? content.PublishCultureInfos.Where(x => x.Value.IsDirty()).Select(x => x.Key).ToList() : null; diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index 7dd6c7ba03..5bd37c0385 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -763,36 +763,29 @@ namespace Umbraco.Tests.Services content.PublishCulture(langFr.IsoCode); content.PublishCulture(langUk.IsoCode); Assert.IsTrue(content.IsCulturePublished(langFr.IsoCode)); - Assert.IsFalse(content.WasCulturePublished(langFr.IsoCode)); //not persisted yet, will be false Assert.IsTrue(content.IsCulturePublished(langUk.IsoCode)); - Assert.IsFalse(content.WasCulturePublished(langUk.IsoCode)); //not persisted yet, will be false var published = ServiceContext.ContentService.SaveAndPublish(content, new[]{ langFr.IsoCode , langUk.IsoCode }); + Assert.IsTrue(content.IsCulturePublished(langFr.IsoCode)); + Assert.IsTrue(content.IsCulturePublished(langUk.IsoCode)); + //re-get content = ServiceContext.ContentService.GetById(content.Id); Assert.IsTrue(published.Success); Assert.IsTrue(content.IsCulturePublished(langFr.IsoCode)); - Assert.IsTrue(content.WasCulturePublished(langFr.IsoCode)); Assert.IsTrue(content.IsCulturePublished(langUk.IsoCode)); - Assert.IsTrue(content.WasCulturePublished(langUk.IsoCode)); var unpublished = ServiceContext.ContentService.Unpublish(content, langFr.IsoCode); Assert.IsTrue(unpublished.Success); Assert.AreEqual(PublishResultType.SuccessUnpublishCulture, unpublished.Result); - Assert.IsFalse(content.IsCulturePublished(langFr.IsoCode)); - //this is slightly confusing but this will be false because this method is used for checking the state of the current model, - //but the state on the model has changed with the above Unpublish call - Assert.IsFalse(content.WasCulturePublished(langFr.IsoCode)); + Assert.IsTrue(content.IsCulturePublished(langUk.IsoCode)); //re-get content = ServiceContext.ContentService.GetById(content.Id); Assert.IsFalse(content.IsCulturePublished(langFr.IsoCode)); - //this is slightly confusing but this will be false because this method is used for checking the state of a current model, - //but we've re-fetched from the database - Assert.IsFalse(content.WasCulturePublished(langFr.IsoCode)); Assert.IsTrue(content.IsCulturePublished(langUk.IsoCode)); - Assert.IsTrue(content.WasCulturePublished(langUk.IsoCode)); + } From 9f612465cf954eb349acdad06519b1c203dab800 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 5 Feb 2019 14:13:03 +1100 Subject: [PATCH 08/26] Cleans up IContent/IContentBase, no more internal methods or setters, moves manipulation used by the DocumentRepository to ext methods --- src/Umbraco.Core/ContentExtensions.cs | 2 +- src/Umbraco.Core/Models/Content.cs | 207 ++++-------------- src/Umbraco.Core/Models/ContentBase.cs | 75 +++---- .../Models/ContentRepositoryExtensions.cs | 159 ++++++++++++++ src/Umbraco.Core/Models/IContent.cs | 41 +--- src/Umbraco.Core/Models/IContentBase.cs | 6 +- .../Implement/DocumentRepository.cs | 62 +++--- .../Services/Implement/ContentService.cs | 10 +- .../Services/Implement/NotificationService.cs | 8 +- src/Umbraco.Core/Umbraco.Core.csproj | 1 + src/Umbraco.Tests/Models/ContentTests.cs | 12 +- .../PublishedContentHashtableConverter.cs | 4 +- .../NuCache/PublishedSnapshotService.cs | 7 +- 13 files changed, 302 insertions(+), 292 deletions(-) create mode 100644 src/Umbraco.Core/Models/ContentRepositoryExtensions.cs diff --git a/src/Umbraco.Core/ContentExtensions.cs b/src/Umbraco.Core/ContentExtensions.cs index 44e9907210..b2bf29cc5f 100644 --- a/src/Umbraco.Core/ContentExtensions.cs +++ b/src/Umbraco.Core/ContentExtensions.cs @@ -67,7 +67,7 @@ namespace Umbraco.Core if (!content.Published || !content.ContentType.VariesByCulture() || !content.IsPropertyDirty("PublishCultureInfos")) return Array.Empty(); - var culturesChanging = content.CultureInfos.Where(x => x.Value.IsDirty()).Select(x => x.Key); + var culturesChanging = content.CultureInfos.Values.Where(x => x.IsDirty()).Select(x => x.Culture); return culturesChanging .Where(x => !content.IsCulturePublished(x) && // is not published anymore persisted != null && persisted.IsCulturePublished(x)) // but was published before diff --git a/src/Umbraco.Core/Models/Content.cs b/src/Umbraco.Core/Models/Content.cs index 63ab65b8fe..09b0b41839 100644 --- a/src/Umbraco.Core/Models/Content.cs +++ b/src/Umbraco.Core/Models/Content.cs @@ -128,15 +128,16 @@ namespace Umbraco.Core.Models /// /// Gets or sets a value indicating whether this content item is published or not. /// + /// + /// the setter is should only be invoked from + /// - the ContentFactory when creating a content entity from a dto + /// - the ContentRepository when updating a content entity + /// [DataMember] public bool Published { get => _published; - - // the setter is internal and should only be invoked from - // - the ContentFactory when creating a content entity from a dto - // - the ContentRepository when updating a content entity - internal set + set { SetPropertyValueAndDetectChanges(value, ref _published, nameof(Published)); _publishedState = _published ? PublishedState.Published : PublishedState.Unpublished; @@ -162,7 +163,7 @@ namespace Umbraco.Core.Models } [IgnoreDataMember] - public bool Edited { get; internal set; } + public bool Edited { get; set; } /// /// Gets the ContentType used by this content object @@ -172,23 +173,27 @@ namespace Umbraco.Core.Models /// [IgnoreDataMember] - public DateTime? PublishDate { get; internal set; } // set by persistence + public DateTime? PublishDate { get; set; } // set by persistence /// [IgnoreDataMember] - public int? PublisherId { get; internal set; } // set by persistence + public int? PublisherId { get; set; } // set by persistence /// [IgnoreDataMember] - public int? PublishTemplateId { get; internal set; } // set by persistence + public int? PublishTemplateId { get; set; } // set by persistence /// [IgnoreDataMember] - public string PublishName { get; internal set; } // set by persistence + public string PublishName { get; set; } // set by persistence /// [IgnoreDataMember] - public IEnumerable EditedCultures => CultureInfos.Keys.Where(IsCultureEdited); + public IEnumerable EditedCultures + { + get => CultureInfos.Keys.Where(IsCultureEdited); + set => _editedCultures = value == null ? null : new HashSet(value, StringComparer.OrdinalIgnoreCase); + } /// [IgnoreDataMember] @@ -199,27 +204,7 @@ namespace Umbraco.Core.Models // just check _publishInfos // a non-available culture could not become published anyways => _publishInfos != null && _publishInfos.ContainsKey(culture); - - - // adjust dates to sync between version, cultures etc - // used by the repo when persisting - internal void AdjustDates(DateTime date) - { - foreach (var culture in PublishedCultures.ToList()) - { - if (_publishInfos == null || !_publishInfos.TryGetValue(culture, out var publishInfos)) - continue; - - //fixme: Removing the logic here for the old WasCulturePublished and the _publishInfosOrig has broken - // the test Can_Rollback_Version_On_Multilingual, but we need to understand what it's doing since I don't - - _publishInfos.AddOrUpdate(culture, publishInfos.Name, date); - - if (CultureInfos.TryGetValue(culture, out var infos)) - SetCultureInfo(culture, infos.Name, date); - } - } - + /// public bool IsCultureEdited(string culture) => IsCultureAvailable(culture) && // is available, and @@ -228,7 +213,23 @@ namespace Umbraco.Core.Models /// [IgnoreDataMember] - public IReadOnlyDictionary PublishCultureInfos => _publishInfos ?? NoInfos; + public ContentCultureInfosCollection PublishCultureInfos + { + get + { + if (_publishInfos != null) return _publishInfos; + _publishInfos = new ContentCultureInfosCollection(); + _publishInfos.CollectionChanged += PublishNamesCollectionChanged; + return _publishInfos; + } + set + { + if (_publishInfos != null) _publishInfos.CollectionChanged -= PublishNamesCollectionChanged; + _publishInfos = value; + if (_publishInfos != null) + _publishInfos.CollectionChanged += PublishNamesCollectionChanged; + } + } /// public string GetPublishName(string culture) @@ -247,67 +248,7 @@ namespace Umbraco.Core.Models if (_publishInfos == null) return null; return _publishInfos.TryGetValue(culture, out var infos) ? infos.Date : (DateTime?) null; } - - // internal for repository - internal void SetPublishInfo(string culture, string name, DateTime date) - { - if (string.IsNullOrWhiteSpace(name)) - throw new ArgumentNullOrEmptyException(nameof(name)); - - if (culture.IsNullOrWhiteSpace()) - throw new ArgumentNullOrEmptyException(nameof(culture)); - - if (_publishInfos == null) - { - _publishInfos = new ContentCultureInfosCollection(); - _publishInfos.CollectionChanged += PublishNamesCollectionChanged; - } - - _publishInfos.AddOrUpdate(culture, name, date); - } - - private void ClearPublishInfos() - { - _publishInfos = null; - } - - private void ClearPublishInfo(string culture) - { - if (culture.IsNullOrWhiteSpace()) - throw new ArgumentNullOrEmptyException(nameof(culture)); - - if (_publishInfos == null) return; - _publishInfos.Remove(culture); - if (_publishInfos.Count == 0) _publishInfos = null; - - // set the culture to be dirty - it's been modified - TouchCultureInfo(culture); - } - - // sets a publish edited - internal void SetCultureEdited(string culture) - { - if (culture.IsNullOrWhiteSpace()) - throw new ArgumentNullOrEmptyException(nameof(culture)); - if (_editedCultures == null) - _editedCultures = new HashSet(StringComparer.OrdinalIgnoreCase); - _editedCultures.Add(culture.ToLowerInvariant()); - } - - // sets all publish edited - internal void SetCultureEdited(IEnumerable cultures) - { - if (cultures == null) - { - _editedCultures = null; - } - else - { - var editedCultures = new HashSet(cultures.Where(x => !x.IsNullOrWhiteSpace()), StringComparer.OrdinalIgnoreCase); - _editedCultures = editedCultures.Count > 0 ? editedCultures : null; - } - } - + /// /// Handles culture infos collection changes. /// @@ -317,84 +258,10 @@ namespace Umbraco.Core.Models } [IgnoreDataMember] - public int PublishedVersionId { get; internal set; } + public int PublishedVersionId { get; set; } [DataMember] - public bool Blueprint { get; internal set; } - - /// - public bool PublishCulture(string culture = "*") - { - culture = culture.NullOrWhiteSpaceAsNull(); - - // the variation should be supported by the content type properties - // if the content type is invariant, only '*' and 'null' is ok - // if the content type varies, everything is ok because some properties may be invariant - if (!ContentType.SupportsPropertyVariation(culture, "*", true)) - throw new NotSupportedException($"Culture \"{culture}\" is not supported by content type \"{ContentType.Alias}\" with variation \"{ContentType.Variations}\"."); - - // the values we want to publish should be valid - if (ValidateProperties(culture).Any()) - return false; - - var alsoInvariant = false; - if (culture == "*") // all cultures - { - foreach (var c in AvailableCultures) - { - var name = GetCultureName(c); - if (string.IsNullOrWhiteSpace(name)) - return false; - SetPublishInfo(c, name, DateTime.Now); - } - } - else if (culture == null) // invariant culture - { - if (string.IsNullOrWhiteSpace(Name)) - return false; - // PublishName set by repository - nothing to do here - } - else // one single culture - { - var name = GetCultureName(culture); - if (string.IsNullOrWhiteSpace(name)) - return false; - SetPublishInfo(culture, name, DateTime.Now); - alsoInvariant = true; // we also want to publish invariant values - } - - // property.PublishValues only publishes what is valid, variation-wise - foreach (var property in Properties) - { - property.PublishValues(culture); - if (alsoInvariant) - property.PublishValues(null); - } - - _publishedState = PublishedState.Publishing; - return true; - } - - /// - public void UnpublishCulture(string culture = "*") - { - culture = culture.NullOrWhiteSpaceAsNull(); - - // the variation should be supported by the content type properties - if (!ContentType.SupportsPropertyVariation(culture, "*", true)) - throw new NotSupportedException($"Culture \"{culture}\" is not supported by content type \"{ContentType.Alias}\" with variation \"{ContentType.Variations}\"."); - - if (culture == "*") // all cultures - ClearPublishInfos(); - else // one single culture - ClearPublishInfo(culture); - - // property.PublishValues only publishes what is valid, variation-wise - foreach (var property in Properties) - property.UnpublishValues(culture); - - _publishedState = PublishedState.Publishing; - } + public bool Blueprint { get; set; } /// /// Changes the for the current content object diff --git a/src/Umbraco.Core/Models/ContentBase.cs b/src/Umbraco.Core/Models/ContentBase.cs index ad8e179e7a..b9301504c4 100644 --- a/src/Umbraco.Core/Models/ContentBase.cs +++ b/src/Umbraco.Core/Models/ContentBase.cs @@ -17,7 +17,6 @@ namespace Umbraco.Core.Models [DebuggerDisplay("Id: {Id}, Name: {Name}, ContentType: {ContentTypeBase.Alias}")] public abstract class ContentBase : TreeEntityBase, IContentBase { - protected static readonly ContentCultureInfosCollection NoInfos = new ContentCultureInfosCollection(); private int _contentTypeId; protected IContentTypeComposition ContentTypeBase; @@ -69,20 +68,20 @@ namespace Umbraco.Core.Models /// Id of the user who wrote/updated this entity /// [DataMember] - public virtual int WriterId + public int WriterId { get => _writerId; set => SetPropertyValueAndDetectChanges(value, ref _writerId, nameof(WriterId)); } [IgnoreDataMember] - public int VersionId { get; internal set; } + public int VersionId { get; set; } /// /// Integer Id of the default ContentType /// [DataMember] - public virtual int ContentTypeId + public int ContentTypeId { get { @@ -105,11 +104,12 @@ namespace Umbraco.Core.Models /// [DataMember] [DoNotClone] - public virtual PropertyCollection Properties + public PropertyCollection Properties { get => _properties; set { + if (_properties != null) _properties.CollectionChanged -= PropertiesChanged; _properties = value; _properties.CollectionChanged += PropertiesChanged; } @@ -147,7 +147,23 @@ namespace Umbraco.Core.Models /// [DataMember] - public virtual IReadOnlyDictionary CultureInfos => _cultureInfos ?? NoInfos; + public ContentCultureInfosCollection CultureInfos + { + get + { + if (_cultureInfos != null) return _cultureInfos; + _cultureInfos = new ContentCultureInfosCollection(); + _cultureInfos.CollectionChanged += CultureInfosCollectionChanged; + return _cultureInfos; + } + set + { + if (_cultureInfos != null) _cultureInfos.CollectionChanged -= CultureInfosCollectionChanged; + _cultureInfos = value; + if (_cultureInfos != null) + _cultureInfos.CollectionChanged += CultureInfosCollectionChanged; + } + } /// public string GetCultureName(string culture) @@ -182,7 +198,7 @@ namespace Umbraco.Core.Models } else // set { - SetCultureInfo(culture, name, DateTime.Now); + this.SetCultureInfo(culture, name, DateTime.Now); } } else // set on invariant content type @@ -194,13 +210,13 @@ namespace Umbraco.Core.Models } } - protected void ClearCultureInfos() + private void ClearCultureInfos() { _cultureInfos?.Clear(); _cultureInfos = null; } - protected void ClearCultureInfo(string culture) + private void ClearCultureInfo(string culture) { if (culture.IsNullOrWhiteSpace()) throw new ArgumentNullOrEmptyException(nameof(culture)); @@ -211,30 +227,6 @@ namespace Umbraco.Core.Models _cultureInfos = null; } - protected void TouchCultureInfo(string culture) - { - if (_cultureInfos == null || !_cultureInfos.TryGetValue(culture, out var infos)) return; - _cultureInfos.AddOrUpdate(culture, infos.Name, DateTime.Now); - } - - // internal for repository - internal void SetCultureInfo(string culture, string name, DateTime date) - { - if (name.IsNullOrWhiteSpace()) - throw new ArgumentNullOrEmptyException(nameof(name)); - - if (culture.IsNullOrWhiteSpace()) - throw new ArgumentNullOrEmptyException(nameof(culture)); - - if (_cultureInfos == null) - { - _cultureInfos = new ContentCultureInfosCollection(); - _cultureInfos.CollectionChanged += CultureInfosCollectionChanged; - } - - _cultureInfos.AddOrUpdate(culture, name, date); - } - /// /// Handles culture infos collection changes. /// @@ -248,11 +240,11 @@ namespace Umbraco.Core.Models #region Has, Get, Set, Publish Property Value /// - public virtual bool HasProperty(string propertyTypeAlias) + public bool HasProperty(string propertyTypeAlias) => Properties.Contains(propertyTypeAlias); /// - public virtual object GetValue(string propertyTypeAlias, string culture = null, string segment = null, bool published = false) + public object GetValue(string propertyTypeAlias, string culture = null, string segment = null, bool published = false) { return Properties.TryGetValue(propertyTypeAlias, out var property) ? property.GetValue(culture, segment, published) @@ -260,7 +252,7 @@ namespace Umbraco.Core.Models } /// - public virtual TValue GetValue(string propertyTypeAlias, string culture = null, string segment = null, bool published = false) + public TValue GetValue(string propertyTypeAlias, string culture = null, string segment = null, bool published = false) { if (!Properties.TryGetValue(propertyTypeAlias, out var property)) return default; @@ -270,7 +262,7 @@ namespace Umbraco.Core.Models } /// - public virtual void SetValue(string propertyTypeAlias, object value, string culture = null, string segment = null) + public void SetValue(string propertyTypeAlias, object value, string culture = null, string segment = null) { if (Properties.Contains(propertyTypeAlias)) { @@ -292,7 +284,7 @@ namespace Umbraco.Core.Models #region Copy /// - public virtual void CopyFrom(IContent other, string culture = "*") + public void CopyFrom(IContent other, string culture = "*") { if (other.ContentTypeId != ContentTypeId) throw new InvalidOperationException("Cannot copy values from a different content type."); @@ -353,10 +345,11 @@ namespace Umbraco.Core.Models if (culture == null || culture == "*") Name = other.Name; - foreach (var (otherCulture, otherInfos) in other.CultureInfos) + // ReSharper disable once UseDeconstruction + foreach (var cultureInfo in other.CultureInfos) { - if (culture == "*" || culture == otherCulture) - SetCultureName(otherInfos.Name, otherCulture); + if (culture == "*" || culture == cultureInfo.Culture) + SetCultureName(cultureInfo.Name, cultureInfo.Culture); } } diff --git a/src/Umbraco.Core/Models/ContentRepositoryExtensions.cs b/src/Umbraco.Core/Models/ContentRepositoryExtensions.cs new file mode 100644 index 0000000000..79c3dc3f63 --- /dev/null +++ b/src/Umbraco.Core/Models/ContentRepositoryExtensions.cs @@ -0,0 +1,159 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using Umbraco.Core.Exceptions; + +namespace Umbraco.Core.Models +{ + /// + /// Extension methods used to manipulate content variations by the document repository + /// + internal static class ContentRepositoryExtensions + { + public static void SetPublishInfo(this IContent content, string culture, string name, DateTime date) + { + if (string.IsNullOrWhiteSpace(name)) + throw new ArgumentNullOrEmptyException(nameof(name)); + + if (culture.IsNullOrWhiteSpace()) + throw new ArgumentNullOrEmptyException(nameof(culture)); + + content.PublishCultureInfos.AddOrUpdate(culture, name, date); + } + + // adjust dates to sync between version, cultures etc used by the repo when persisting + public static void AdjustDates(this IContent content, DateTime date) + { + foreach (var culture in content.PublishedCultures.ToList()) + { + if (!content.PublishCultureInfos.TryGetValue(culture, out var publishInfos)) + continue; + + //fixme: Removing the logic here for the old WasCulturePublished and the _publishInfosOrig has broken + // the test Can_Rollback_Version_On_Multilingual, but we need to understand what it's doing since I don't + + content.PublishCultureInfos.AddOrUpdate(culture, publishInfos.Name, date); + + if (content.CultureInfos.TryGetValue(culture, out var infos)) + SetCultureInfo(content, culture, infos.Name, date); + } + } + + // sets the edited cultures on the content + public static void SetCultureEdited(this IContent content, IEnumerable cultures) + { + if (cultures == null) + content.EditedCultures = null; + else + { + var editedCultures = new HashSet(cultures.Where(x => !x.IsNullOrWhiteSpace()), StringComparer.OrdinalIgnoreCase); + content.EditedCultures = editedCultures.Count > 0 ? editedCultures : null; + } + } + + public static void SetCultureInfo(this IContentBase content, string culture, string name, DateTime date) + { + if (name.IsNullOrWhiteSpace()) + throw new ArgumentNullOrEmptyException(nameof(name)); + + if (culture.IsNullOrWhiteSpace()) + throw new ArgumentNullOrEmptyException(nameof(culture)); + + content.CultureInfos.AddOrUpdate(culture, name, date); + } + + public static bool PublishCulture(this IContent content, string culture = "*") + { + culture = culture.NullOrWhiteSpaceAsNull(); + + // the variation should be supported by the content type properties + // if the content type is invariant, only '*' and 'null' is ok + // if the content type varies, everything is ok because some properties may be invariant + if (!content.ContentType.SupportsPropertyVariation(culture, "*", true)) + throw new NotSupportedException($"Culture \"{culture}\" is not supported by content type \"{content.ContentType.Alias}\" with variation \"{content.ContentType.Variations}\"."); + + // the values we want to publish should be valid + if (content.ValidateProperties(culture).Any()) + return false; + + var alsoInvariant = false; + if (culture == "*") // all cultures + { + foreach (var c in content.AvailableCultures) + { + var name = content.GetCultureName(c); + if (string.IsNullOrWhiteSpace(name)) + return false; + content.SetPublishInfo(c, name, DateTime.Now); + } + } + else if (culture == null) // invariant culture + { + if (string.IsNullOrWhiteSpace(content.Name)) + return false; + // PublishName set by repository - nothing to do here + } + else // one single culture + { + var name = content.GetCultureName(culture); + if (string.IsNullOrWhiteSpace(name)) + return false; + content.SetPublishInfo(culture, name, DateTime.Now); + alsoInvariant = true; // we also want to publish invariant values + } + + // property.PublishValues only publishes what is valid, variation-wise + foreach (var property in content.Properties) + { + property.PublishValues(culture); + if (alsoInvariant) + property.PublishValues(null); + } + + content.PublishedState = PublishedState.Publishing; + return true; + } + + public static void UnpublishCulture(this IContent content, string culture = "*") + { + culture = culture.NullOrWhiteSpaceAsNull(); + + // the variation should be supported by the content type properties + if (!content.ContentType.SupportsPropertyVariation(culture, "*", true)) + throw new NotSupportedException($"Culture \"{culture}\" is not supported by content type \"{content.ContentType.Alias}\" with variation \"{content.ContentType.Variations}\"."); + + if (culture == "*") // all cultures + content.ClearPublishInfos(); + else // one single culture + content.ClearPublishInfo(culture); + + // property.PublishValues only publishes what is valid, variation-wise + foreach (var property in content.Properties) + property.UnpublishValues(culture); + + content.PublishedState = PublishedState.Publishing; + } + + public static void ClearPublishInfos(this IContent content) + { + content.PublishCultureInfos = null; + } + + public static void ClearPublishInfo(this IContent content, string culture) + { + if (culture.IsNullOrWhiteSpace()) + throw new ArgumentNullOrEmptyException(nameof(culture)); + + content.PublishCultureInfos.Remove(culture); + + // set the culture to be dirty - it's been modified + content.TouchCultureInfo(culture); + } + + public static void TouchCultureInfo(this IContent content, string culture) + { + if (!content.CultureInfos.TryGetValue(culture, out var infos)) return; + content.CultureInfos.AddOrUpdate(culture, infos.Name, DateTime.Now); + } + } +} diff --git a/src/Umbraco.Core/Models/IContent.cs b/src/Umbraco.Core/Models/IContent.cs index f468df59ab..e953bef1eb 100644 --- a/src/Umbraco.Core/Models/IContent.cs +++ b/src/Umbraco.Core/Models/IContent.cs @@ -25,46 +25,46 @@ namespace Umbraco.Core.Models /// /// Gets a value indicating whether the content is published. /// - bool Published { get; } + bool Published { get; set; } - PublishedState PublishedState { get; } + PublishedState PublishedState { get; set; } /// /// Gets a value indicating whether the content has been edited. /// - bool Edited { get; } + bool Edited { get; set; } /// /// Gets the published version identifier. /// - int PublishedVersionId { get; } + int PublishedVersionId { get; set; } /// /// Gets a value indicating whether the content item is a blueprint. /// - bool Blueprint { get; } + bool Blueprint { get; set; } /// /// Gets the template id used to render the published version of the content. /// /// When editing the content, the template can change, but this will not until the content is published. - int? PublishTemplateId { get; } + int? PublishTemplateId { get; set; } /// /// Gets the name of the published version of the content. /// /// When editing the content, the name can change, but this will not until the content is published. - string PublishName { get; } + string PublishName { get; set; } /// /// Gets the identifier of the user who published the content. /// - int? PublisherId { get; } + int? PublisherId { get; set; } /// /// Gets the date and time the content was published. /// - DateTime? PublishDate { get; } + DateTime? PublishDate { get; set; } /// /// Gets the content type of this content. @@ -117,7 +117,7 @@ namespace Umbraco.Core.Models /// Because a dictionary key cannot be null this cannot get the invariant /// name, which must be get via the property. /// - IReadOnlyDictionary PublishCultureInfos { get; } + ContentCultureInfosCollection PublishCultureInfos { get; set; } /// /// Gets the published cultures. @@ -127,30 +127,13 @@ namespace Umbraco.Core.Models /// /// Gets the edited cultures. /// - IEnumerable EditedCultures { get; } + IEnumerable EditedCultures { get; set; } /// /// Creates a deep clone of the current entity with its identity/alias and it's property identities reset /// /// IContent DeepCloneWithResetIdentities(); - - /// - /// Registers a culture to be published. - /// - /// A value indicating whether the culture can be published. - /// - /// Fails if properties don't pass variant validation rules. - /// Publishing must be finalized via the content service SavePublishing method. - /// - bool PublishCulture(string culture = "*"); - - /// - /// Registers a culture to be unpublished. - /// - /// - /// Unpublishing must be finalized via the content service SavePublishing method. - /// - void UnpublishCulture(string culture = "*"); + } } diff --git a/src/Umbraco.Core/Models/IContentBase.cs b/src/Umbraco.Core/Models/IContentBase.cs index 40a1c57097..b3cc266f0a 100644 --- a/src/Umbraco.Core/Models/IContentBase.cs +++ b/src/Umbraco.Core/Models/IContentBase.cs @@ -26,7 +26,7 @@ namespace Umbraco.Core.Models /// /// Gets the version identifier. /// - int VersionId { get; } + int VersionId { get; set; } /// /// Sets the name of the content item for a specified culture. @@ -57,8 +57,8 @@ namespace Umbraco.Core.Models /// Because a dictionary key cannot be null this cannot contain the invariant /// culture name, which must be get or set via the property. /// - IReadOnlyDictionary CultureInfos { get; } - + ContentCultureInfosCollection CultureInfos { get; set; } + /// /// Gets the available cultures. /// diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs index 9b4175b63f..3cf407d304 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs @@ -380,9 +380,10 @@ namespace Umbraco.Core.Persistence.Repositories.Implement content.AdjustDates(contentVersionDto.VersionDate); // names also impact 'edited' - foreach (var (culture, infos) in content.CultureInfos) - if (infos.Name != content.GetPublishName(culture)) - (editedCultures ?? (editedCultures = new HashSet(StringComparer.OrdinalIgnoreCase))).Add(culture); + // ReSharper disable once UseDeconstruction + foreach (var cultureInfo in content.CultureInfos) + if (cultureInfo.Name != content.GetPublishName(cultureInfo.Culture)) + (editedCultures ?? (editedCultures = new HashSet(StringComparer.OrdinalIgnoreCase))).Add(cultureInfo.Culture); // insert content variations Database.BulkInsertRecords(GetContentVariationDtos(content, publishing)); @@ -541,11 +542,12 @@ namespace Umbraco.Core.Persistence.Repositories.Implement content.AdjustDates(contentVersionDto.VersionDate); // names also impact 'edited' - foreach (var (culture, infos) in content.CultureInfos) - if (infos.Name != content.GetPublishName(culture)) + // ReSharper disable once UseDeconstruction + foreach (var cultureInfo in content.CultureInfos) + if (cultureInfo.Name != content.GetPublishName(cultureInfo.Culture)) { edited = true; - (editedCultures ?? (editedCultures = new HashSet(StringComparer.OrdinalIgnoreCase))).Add(culture); + (editedCultures ?? (editedCultures = new HashSet(StringComparer.OrdinalIgnoreCase))).Add(cultureInfo.Culture); // TODO: change tracking // at the moment, we don't do any dirty tracking on property values, so we don't know whether the @@ -951,6 +953,8 @@ namespace Umbraco.Core.Persistence.Repositories.Implement #endregion + + protected override string ApplySystemOrdering(ref Sql sql, Ordering ordering) { // note: 'updater' is the user who created the latest draft version, @@ -1182,6 +1186,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement return result; } + private void SetVariations(Content content, IDictionary> contentVariations, IDictionary> documentVariations) { if (contentVariations.TryGetValue(content.VersionId, out var contentVariation)) @@ -1191,8 +1196,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement foreach (var v in contentVariation) content.SetPublishInfo(v.Culture, v.Name, v.Date); if (documentVariations.TryGetValue(content.Id, out var documentVariation)) - foreach (var v in documentVariation.Where(x => x.Edited)) - content.SetCultureEdited(v.Culture); + content.SetCultureEdited(documentVariation.Where(x => x.Edited).Select(x => x.Culture)); } private IDictionary> GetContentVariations(List> temps) @@ -1262,14 +1266,15 @@ namespace Umbraco.Core.Persistence.Repositories.Implement private IEnumerable GetContentVariationDtos(IContent content, bool publishing) { // create dtos for the 'current' (non-published) version, all cultures - foreach (var (culture, name) in content.CultureInfos) + // ReSharper disable once UseDeconstruction + foreach (var cultureInfo in content.CultureInfos) yield return new ContentVersionCultureVariationDto { VersionId = content.VersionId, - LanguageId = LanguageRepository.GetIdByIsoCode(culture) ?? throw new InvalidOperationException("Not a valid culture."), - Culture = culture, - Name = name.Name, - UpdateDate = content.GetUpdateDate(culture) ?? DateTime.MinValue // we *know* there is a value + LanguageId = LanguageRepository.GetIdByIsoCode(cultureInfo.Culture) ?? throw new InvalidOperationException("Not a valid culture."), + Culture = cultureInfo.Culture, + Name = cultureInfo.Name, + UpdateDate = content.GetUpdateDate(cultureInfo.Culture) ?? DateTime.MinValue // we *know* there is a value }; // if not publishing, we're just updating the 'current' (non-published) version, @@ -1277,14 +1282,15 @@ namespace Umbraco.Core.Persistence.Repositories.Implement if (!publishing) yield break; // create dtos for the 'published' version, for published cultures (those having a name) - foreach (var (culture, name) in content.PublishCultureInfos) + // ReSharper disable once UseDeconstruction + foreach (var cultureInfo in content.PublishCultureInfos) yield return new ContentVersionCultureVariationDto { VersionId = content.PublishedVersionId, - LanguageId = LanguageRepository.GetIdByIsoCode(culture) ?? throw new InvalidOperationException("Not a valid culture."), - Culture = culture, - Name = name.Name, - UpdateDate = content.GetPublishDate(culture) ?? DateTime.MinValue // we *know* there is a value + LanguageId = LanguageRepository.GetIdByIsoCode(cultureInfo.Culture) ?? throw new InvalidOperationException("Not a valid culture."), + Culture = cultureInfo.Culture, + Name = cultureInfo.Name, + UpdateDate = content.GetPublishDate(cultureInfo.Culture) ?? DateTime.MinValue // we *know* there is a value }; } @@ -1344,7 +1350,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement EnsureVariantNamesAreUnique(content, publishing); } - private void EnsureInvariantNameExists(Content content) + private void EnsureInvariantNameExists(IContent content) { if (content.ContentType.VariesByCulture()) { @@ -1358,7 +1364,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement var defaultCulture = LanguageRepository.GetDefaultIsoCode(); content.Name = defaultCulture != null && content.CultureInfos.TryGetValue(defaultCulture, out var cultureName) ? cultureName.Name - : content.CultureInfos.First().Value.Name; + : content.CultureInfos[0].Name; } else { @@ -1368,7 +1374,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement } } - private void EnsureInvariantNameIsUnique(Content content) + private void EnsureInvariantNameIsUnique(IContent content) { content.Name = EnsureUniqueNodeName(content.ParentId, content.Name, content.Id); } @@ -1405,22 +1411,22 @@ namespace Umbraco.Core.Persistence.Repositories.Implement // of whether the name has changed (ie the culture has been updated) - some saving culture // fr-FR could cause culture en-UK name to change - not sure that is clean - foreach (var (culture, name) in content.CultureInfos) + foreach (var cultureInfo in content.CultureInfos) { - var langId = LanguageRepository.GetIdByIsoCode(culture); + var langId = LanguageRepository.GetIdByIsoCode(cultureInfo.Culture); if (!langId.HasValue) continue; if (!names.TryGetValue(langId.Value, out var cultureNames)) continue; // get a unique name var otherNames = cultureNames.Select(x => new SimilarNodeName { Id = x.Id, Name = x.Name }); - var uniqueName = SimilarNodeName.GetUniqueName(otherNames, 0, name.Name); + var uniqueName = SimilarNodeName.GetUniqueName(otherNames, 0, cultureInfo.Name); - if (uniqueName == content.GetCultureName(culture)) continue; + if (uniqueName == content.GetCultureName(cultureInfo.Culture)) continue; // update the name, and the publish name if published - content.SetCultureName(uniqueName, culture); - if (publishing && content.PublishCultureInfos.ContainsKey(culture)) - content.SetPublishInfo(culture, uniqueName, DateTime.Now); + content.SetCultureName(uniqueName, cultureInfo.Culture); + if (publishing && content.PublishCultureInfos.ContainsKey(cultureInfo.Culture)) + content.SetPublishInfo(cultureInfo.Culture, uniqueName, DateTime.Now); //TODO: This is weird, this call will have already been made in the SetCultureName } } diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index 9c2721cbb4..a6ef1b9f36 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -776,7 +776,7 @@ namespace Umbraco.Core.Services.Implement //track the cultures that have changed var culturesChanging = content.ContentType.VariesByCulture() - ? content.CultureInfos.Where(x => x.Value.IsDirty()).Select(x => x.Key).ToList() + ? content.CultureInfos.Values.Where(x => x.IsDirty()).Select(x => x.Culture).ToList() : null; // TODO: Currently there's no way to change track which variant properties have changed, we only have change // tracking enabled on all values on the Property which doesn't allow us to know which variants have changed. @@ -1017,7 +1017,7 @@ namespace Umbraco.Core.Services.Implement IReadOnlyList culturesPublishing = null; IReadOnlyList culturesUnpublishing = null; IReadOnlyList culturesChanging = variesByCulture - ? content.CultureInfos.Where(x => x.Value.IsDirty()).Select(x => x.Key).ToList() + ? content.CultureInfos.Values.Where(x => x.IsDirty()).Select(x => x.Culture).ToList() : null; var isNew = !content.HasIdentity; @@ -1036,7 +1036,7 @@ namespace Umbraco.Core.Services.Implement culturesUnpublishing = content.GetCulturesUnpublishing(persisted); culturesPublishing = variesByCulture - ? content.PublishCultureInfos.Where(x => x.Value.IsDirty()).Select(x => x.Key).ToList() + ? content.PublishCultureInfos.Values.Where(x => x.IsDirty()).Select(x => x.Culture).ToList() : null; // ensure that the document can be published, and publish handling events, business rules, etc @@ -2043,7 +2043,7 @@ namespace Umbraco.Core.Services.Implement //track the cultures changing for auditing var culturesChanging = content.ContentType.VariesByCulture() - ? string.Join(",", content.CultureInfos.Where(x => x.Value.IsDirty()).Select(x => x.Key)) + ? string.Join(",", content.CultureInfos.Values.Where(x => x.IsDirty()).Select(x => x.Culture)) : null; // TODO: Currently there's no way to change track which variant properties have changed, we only have change @@ -2781,7 +2781,7 @@ namespace Umbraco.Core.Services.Implement content.WriterId = userId; var now = DateTime.Now; - var cultures = blueprint.CultureInfos.Any() ? blueprint.CultureInfos.Select(x=>x.Key) : ArrayOfOneNullString; + var cultures = blueprint.CultureInfos.Count > 0 ? blueprint.CultureInfos.Values.Select(x => x.Culture) : ArrayOfOneNullString; foreach (var culture in cultures) { foreach (var property in blueprint.Properties) diff --git a/src/Umbraco.Core/Services/Implement/NotificationService.cs b/src/Umbraco.Core/Services/Implement/NotificationService.cs index d981809364..2b21945ba8 100644 --- a/src/Umbraco.Core/Services/Implement/NotificationService.cs +++ b/src/Umbraco.Core/Services/Implement/NotificationService.cs @@ -345,8 +345,8 @@ namespace Umbraco.Core.Services.Implement { //Create the HTML based summary (ul of culture names) - var culturesChanged = content.CultureInfos.Where(x => x.Value.WasDirty()) - .Select(x => x.Key) + var culturesChanged = content.CultureInfos.Values.Where(x => x.WasDirty()) + .Select(x => x.Culture) .Select(_localizationService.GetLanguageByIsoCode) .WhereNotNull() .Select(x => x.CultureName); @@ -363,8 +363,8 @@ namespace Umbraco.Core.Services.Implement { //Create the text based summary (csv of culture names) - var culturesChanged = string.Join(", ", content.CultureInfos.Where(x => x.Value.WasDirty()) - .Select(x => x.Key) + var culturesChanged = string.Join(", ", content.CultureInfos.Values.Where(x => x.WasDirty()) + .Select(x => x.Culture) .Select(_localizationService.GetLanguageByIsoCode) .WhereNotNull() .Select(x => x.CultureName)); diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index b327a9281d..13db7e2259 100755 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -391,6 +391,7 @@ + diff --git a/src/Umbraco.Tests/Models/ContentTests.cs b/src/Umbraco.Tests/Models/ContentTests.cs index 2e98b31cca..c5dd8a6aea 100644 --- a/src/Umbraco.Tests/Models/ContentTests.cs +++ b/src/Umbraco.Tests/Models/ContentTests.cs @@ -437,16 +437,16 @@ namespace Umbraco.Tests.Models Assert.IsTrue(content.WasPropertyDirty("CultureInfos")); foreach(var culture in content.CultureInfos) { - Assert.IsTrue(culture.Value.WasDirty()); - Assert.IsTrue(culture.Value.WasPropertyDirty("Name")); - Assert.IsTrue(culture.Value.WasPropertyDirty("Date")); + Assert.IsTrue(culture.WasDirty()); + Assert.IsTrue(culture.WasPropertyDirty("Name")); + Assert.IsTrue(culture.WasPropertyDirty("Date")); } Assert.IsTrue(content.WasPropertyDirty("PublishCultureInfos")); foreach (var culture in content.PublishCultureInfos) { - Assert.IsTrue(culture.Value.WasDirty()); - Assert.IsTrue(culture.Value.WasPropertyDirty("Name")); - Assert.IsTrue(culture.Value.WasPropertyDirty("Date")); + Assert.IsTrue(culture.WasDirty()); + Assert.IsTrue(culture.WasPropertyDirty("Name")); + Assert.IsTrue(culture.WasPropertyDirty("Date")); } } diff --git a/src/Umbraco.Web/Macros/PublishedContentHashtableConverter.cs b/src/Umbraco.Web/Macros/PublishedContentHashtableConverter.cs index f40c8a7d98..9c02fa040d 100644 --- a/src/Umbraco.Web/Macros/PublishedContentHashtableConverter.cs +++ b/src/Umbraco.Web/Macros/PublishedContentHashtableConverter.cs @@ -252,8 +252,8 @@ namespace Umbraco.Web.Macros if (_cultureInfos != null) return _cultureInfos; - return _cultureInfos = _inner.PublishCultureInfos - .ToDictionary(x => x.Key, x => new PublishedCultureInfo(x.Key, x.Value.Name, x.Value.Date)); + return _cultureInfos = _inner.PublishCultureInfos.Values + .ToDictionary(x => x.Culture, x => new PublishedCultureInfo(x.Culture, x.Name, x.Date)); } } diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs index f470e5a65a..c503f382dc 100755 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs @@ -1214,10 +1214,11 @@ namespace Umbraco.Web.PublishedCache.NuCache : document.CultureInfos) : content.CultureInfos; - foreach (var (culture, info) in infos) + // ReSharper disable once UseDeconstruction + foreach (var cultureInfo in infos) { - var cultureIsDraft = !published && content is IContent d && d.IsCultureEdited(culture); - cultureData[culture] = new CultureVariation { Name = info.Name, Date = content.GetUpdateDate(culture) ?? DateTime.MinValue, IsDraft = cultureIsDraft }; + var cultureIsDraft = !published && content is IContent d && d.IsCultureEdited(cultureInfo.Culture); + cultureData[cultureInfo.Culture] = new CultureVariation { Name = cultureInfo.Name, Date = content.GetUpdateDate(cultureInfo.Culture) ?? DateTime.MinValue, IsDraft = cultureIsDraft }; } } From 3dab68b70d92c71ee4703aa3e1d65c3f149fc026 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 5 Feb 2019 14:15:03 +1100 Subject: [PATCH 09/26] adds note --- .../Persistence/Repositories/Implement/DocumentRepository.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs index 3cf407d304..5a3a42af27 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs @@ -120,6 +120,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement break; case QueryType.Single: case QueryType.Many: + //fixme: Apparently this is ambiguous? sql = sql.Select(r => r.Select(documentDto => documentDto.ContentDto, r1 => r1.Select(contentDto => contentDto.NodeDto)) From ff497b413e24bb936bc9b4c1d98d993fa0a764b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20Lyngs=C3=B8?= Date: Tue, 5 Feb 2019 11:54:23 +0100 Subject: [PATCH 10/26] V8: UI, refactoring of infinity editing to use CSS to fix issue regarding stacking of layers. --- .../components/editor/umbeditors.directive.js | 190 ++++++------------ .../less/components/editor/umb-editor.less | 69 +++++-- .../application/umb-navigation.html | 5 +- .../views/components/editor/umb-editors.html | 20 +- 4 files changed, 135 insertions(+), 149 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/editor/umbeditors.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/editor/umbeditors.directive.js index 235918735f..4104a663d3 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/editor/umbeditors.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/editor/umbeditors.directive.js @@ -7,161 +7,85 @@ var evts = []; var allowedNumberOfVisibleEditors = 3; - var editorIndent = 60; - + scope.editors = []; - + function addEditor(editor) { + editor.inFront = true; + editor.moveRight = true; + editor.level = 0; + editor.styleIndex = 0; - if (!editor.style) - editor.style = {}; - - editor.animating = true; - - showOverlayOnPrevEditor(); - - var i = allowedNumberOfVisibleEditors; - var len = scope.editors.length; - while(i= allowedNumberOfVisibleEditors) { - animeConfig.left = i * editorIndent; - } else { - animeConfig.left = (i + 1) * editorIndent; - } - - anime(animeConfig); - - i++; - } - + editor.infinityMode = true; // push the new editor to the dom scope.editors.push(editor); - - - var indentValue = scope.editors.length * editorIndent; - - // don't allow indent larger than what - // fits the max number of visible editors - if(scope.editors.length >= allowedNumberOfVisibleEditors) { - indentValue = allowedNumberOfVisibleEditors * editorIndent; - } - - // indent all large editors - if(editor.size !== "small") { - editor.style.left = indentValue + "px"; - } - - editor.style._tx = 100; - editor.style.transform = "translateX("+editor.style._tx+"%)"; - - // animation config - anime({ - targets: editor.style, - _tx: [100, 0], - easing: 'easeOutExpo', - duration: 480, - update: () => { - editor.style.transform = "translateX("+editor.style._tx+"%)"; - scope.$digest(); - }, - complete: function() { - editor.animating = false; - scope.$digest(); - } - }); - - - } - - function removeEditor(editor) { + $timeout(() => { + editor.moveRight = false; + }) editor.animating = true; + setTimeout(revealEditorContent.bind(this, editor), 400); - editor.style._tx = 0; - editor.style.transform = "translateX("+editor.style._tx+"%)"; + updateEditors(); + + } + + function removeEditor(editor) { - // animation config - anime({ - targets: editor.style, - _tx: [0, 100], - easing: 'easeInExpo', - duration: 360, - update: () => { - editor.style.transform = "translateX("+editor.style._tx+"%)"; - scope.$digest(); - }, - complete: function() { - scope.editors.splice(-1,1); - removeOverlayFromPrevEditor(); - scope.$digest(); - } - }); + editor.moveRight = true; + editor.animating = true; + setTimeout(removeEditorFromDOM.bind(this, editor), 400); - expandEditors(); - + updateEditors(-1); } - - function expandEditors() { + + function revealEditorContent(editor) { - var i = allowedNumberOfVisibleEditors + 1; - var len = scope.editors.length-1; + editor.animating = false; + + scope.$digest(); + + } + + function removeEditorFromDOM(editor) { + + // push the new editor to the dom + var index = scope.editors.indexOf(editor); + if (index !== -1) { + scope.editors.splice(index, 1); + } + + updateEditors(); + + scope.$digest(); + + } + + /** update layer positions. With ability to offset positions, needed for when an item is moving out, then we dont want it to influence positions */ + function updateEditors(offset) { + + offset = offset || 0;// fallback value. + + var len = scope.editors.length; + var calcLen = len + offset; + var ceiling = Math.min(calcLen, allowedNumberOfVisibleEditors); + var origin = Math.max(calcLen-1, 0)-ceiling; + var i = 0; while(i= ceiling; i++; } - - - } - // show backdrop on previous editor - function showOverlayOnPrevEditor() { - var numberOfEditors = scope.editors.length; - if(numberOfEditors > 0) { - scope.editors[numberOfEditors - 1].showOverlay = true; - } } - - function removeOverlayFromPrevEditor() { - var numberOfEditors = scope.editors.length; - if(numberOfEditors > 0) { - scope.editors[numberOfEditors - 1].showOverlay = false; - } - } - + evts.push(eventsService.on("appState.editors.open", function (name, args) { addEditor(args.editor); })); diff --git a/src/Umbraco.Web.UI.Client/src/less/components/editor/umb-editor.less b/src/Umbraco.Web.UI.Client/src/less/components/editor/umb-editor.less index 6dd77c56b1..f1fa8245ea 100644 --- a/src/Umbraco.Web.UI.Client/src/less/components/editor/umb-editor.less +++ b/src/Umbraco.Web.UI.Client/src/less/components/editor/umb-editor.less @@ -4,6 +4,7 @@ right: 0; bottom: 0; left: 0; + overflow: hidden; } .umb-editor { @@ -17,7 +18,49 @@ } .umb-editor--animating { - will-change: transform, width, left; + //will-change: transform, width, left; +} +.umb-editor--infinityMode { + transform: none; + will-change: transform; + transition: transform 400ms ease-in-out; + &.moveRight { + transform: translateX(110%); + } +} + +.umb-editor--outOfRange { + //left:0; + transform: none; + display: none; + will-change: auto; + transition: display 0s 320ms; +} +.umb-editor--level0 { + //left:0; + transform: none; +} +.umb-editor--level1 { + //left:60px; + transform: translateX(60px); +} +.umb-editor--level2 { + //left:120px; + transform: translateX(120px); +} +.umb-editor--level3 { + //left:180px; + transform: translateX(180px); +} + +.umb-editor--n1 { + right:60px; +} +.umb-editor--n2 { + right:120px; +} +.umb-editor--n3 { + right:180px; } // hide all infinite editors by default @@ -28,20 +71,14 @@ .umb-editor--small { width: 500px; + will-change: transform; left: auto; - + .umb-editor-container { max-width: 500px; } } -@keyframes umb-editor__overlay_fade_opacity { - from { - opacity:0; - } - to { - opacity:1; - } -} + .umb-editor__overlay { position: absolute; top: 0; @@ -50,6 +87,14 @@ left: 0; background: rgba(0,0,0,0.4); z-index: @zIndexEditor; - - animation:umb-editor__overlay_fade_opacity 600ms; + visibility: hidden; + opacity: 0; + transition: opacity 320ms 20ms, visibility 0s 400ms; +} + +#contentcolumn > .umb-editor__overlay, +.--notInFront .umb-editor__overlay { + visibility: visible; + opacity: 1; + transition: opacity 320ms 20ms, visibility 0s; } diff --git a/src/Umbraco.Web.UI.Client/src/views/components/application/umb-navigation.html b/src/Umbraco.Web.UI.Client/src/views/components/application/umb-navigation.html index 275c814761..829582329f 100644 --- a/src/Umbraco.Web.UI.Client/src/views/components/application/umb-navigation.html +++ b/src/Umbraco.Web.UI.Client/src/views/components/application/umb-navigation.html @@ -1,7 +1,8 @@
-