From e91d485d1c6ba559b6912d963d6c6e8df6b991cb Mon Sep 17 00:00:00 2001 From: Stephan Date: Tue, 6 Nov 2018 15:24:55 +0100 Subject: [PATCH] 3336 - bulk branch publish with variants --- src/Umbraco.Core/Services/IContentService.cs | 47 +- .../Services/Implement/ContentService.cs | 431 ++++++++++-------- .../ContentServicePublishBranchTests.cs | 368 +++++++++++++++ .../Services/ContentServiceTests.cs | 4 - src/Umbraco.Tests/Umbraco.Tests.csproj | 1 + 5 files changed, 653 insertions(+), 198 deletions(-) create mode 100644 src/Umbraco.Tests/Services/ContentServicePublishBranchTests.cs diff --git a/src/Umbraco.Core/Services/IContentService.cs b/src/Umbraco.Core/Services/IContentService.cs index 275957e453..7220afe388 100644 --- a/src/Umbraco.Core/Services/IContentService.cs +++ b/src/Umbraco.Core/Services/IContentService.cs @@ -352,12 +352,34 @@ namespace Umbraco.Core.Services /// PublishResult SavePublishing(IContent content, int userId = 0, bool raiseEvents = true); + /* + fixme - document this better + test + If the item being published is Invariant and it has Variant descendants and + we are NOT forcing publishing of anything not published - the result will be that the Variant cultures that are + already published (but may contain a draft) are published. Any cultures that don't have a published version are not published + fixme: now, if publishing '*' then all cultures + If the item being published is Invariant and it has Variant descendants and + we ARE forcing publishing of anything not published - the result will be that all Variant cultures are + published regardless of whether they don't have any current published versions + + If the item being published is Variant and it has Invariant descendants and + we are NOT forcing publishing of anything not published - the result will be that all Invariant documents are + published that already have a published versions, regardless of what cultures are selected to be published + If the item being published is Variant and it has Invariant descendants and + we ARE forcing publishing of anything not published - the result will be that all Invariant documents are + published regardless of whether they have a published version or not and regardless of what cultures are selected to be published + */ + /// /// Saves and publishes a document branch. /// + /// The root document. + /// A value indicating whether to force-publish documents that are not already published. + /// A culture, or "*" for all cultures. + /// The identifier of the user performing the operation. /// /// Unless specified, all cultures are re-published. Otherwise, one culture can be specified. To act on more - /// that one culture, see the other overload of this method. + /// than one culture, see the other overloads of this method. /// The parameter determines which documents are published. When false, /// only those documents that are already published, are republished. When true, all documents are /// published. @@ -367,19 +389,38 @@ namespace Umbraco.Core.Services /// /// Saves and publishes a document branch. /// + /// The root document. + /// A value indicating whether to force-publish documents that are not already published. + /// The cultures to publish. + /// The identifier of the user performing the operation. + /// + /// The parameter determines which documents are published. When false, + /// only those documents that are already published, are republished. When true, all documents are + /// published. + /// + IEnumerable SaveAndPublishBranch(IContent content, bool force, string[] cultures, int userId = 0); + + /// + /// Saves and publishes a document branch. + /// + /// The root document. + /// A value indicating whether to force-publish documents that are not already published. + /// A function determining whether a document has changes to publish. + /// A function publishing cultures. + /// The identifier of the user performing the operation. /// /// The parameter determines which documents are published. When false, /// only those documents that are already published, are republished. When true, all documents are /// published. /// The parameter is a function which determines whether a document has - /// values to publish (else there is no need to publish it). If one wants to publish only a selection of + /// changes to publish (else there is no need to publish it). If one wants to publish only a selection of /// cultures, one may want to check that only properties for these cultures have changed. Otherwise, other /// cultures may trigger an unwanted republish. /// The parameter is a function to execute to publish cultures, on /// each document. It can publish all, one, or a selection of cultures. It returns a boolean indicating /// whether the cultures could be published. /// - IEnumerable SaveAndPublishBranch(IContent content, bool force, Func editing, Func publishCultures, int userId = 0); + IEnumerable SaveAndPublishBranch(IContent content, bool force, Func> shouldPublish, Func, bool> publishCultures, int userId = 0); /// /// Unpublishes a document. diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index 2b585cebad..b4647114c0 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -987,6 +987,17 @@ namespace Umbraco.Core.Services.Implement /// public PublishResult SavePublishing(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); + scope.Complete(); + return result; + } + } + + private PublishResult SavePublishingInternal(IScope scope, IContent content, int userId = 0, bool raiseEvents = true) { var evtMsgs = EventMessagesFactory.Get(); PublishResult publishResult = null; @@ -994,7 +1005,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; // state here is either Publishing or Unpublishing var publishing = content.PublishedState == PublishedState.Publishing; @@ -1002,169 +1013,156 @@ namespace Umbraco.Core.Services.Implement IEnumerable culturesChanging = null; - using (var scope = ScopeProvider.CreateScope()) + // is the content going to end up published, or unpublished? + if (publishing && content.ContentType.VariesByCulture()) { - // is the content going to end up published, or unpublished? - if (publishing && content.ContentType.VariesByCulture()) + var publishedCultures = content.PublishedCultures.ToList(); + var cannotBePublished = publishedCultures.Count == 0; // no published cultures = cannot be published + if (!cannotBePublished) { - var publishedCultures = content.PublishedCultures.ToList(); - var cannotBePublished = publishedCultures.Count == 0; // no published cultures = cannot be published - if (!cannotBePublished) - { - var mandatoryCultures = _languageRepository.GetMany().Where(x => x.IsMandatory).Select(x => x.IsoCode); - cannotBePublished = mandatoryCultures.Any(x => !publishedCultures.Contains(x, StringComparer.OrdinalIgnoreCase)); // missing mandatory culture = cannot be published - } - - if (cannotBePublished) - { - publishing = false; - unpublishing = content.Published; // if not published yet, nothing to do - - // we may end up in a state where we won't publish nor unpublish - // keep going, though, as we want to save anways - } - else - { - culturesChanging = content.PublishCultureInfos.Where(x => x.Value.IsDirty()).Select(x => x.Key).ToList(); - } + var mandatoryCultures = _languageRepository.GetMany().Where(x => x.IsMandatory).Select(x => x.IsoCode); + cannotBePublished = mandatoryCultures.Any(x => !publishedCultures.Contains(x, StringComparer.OrdinalIgnoreCase)); // missing mandatory culture = cannot be published } - var isNew = !content.HasIdentity; - var changeType = isNew ? TreeChangeTypes.RefreshNode : TreeChangeTypes.RefreshBranch; - var previouslyPublished = content.HasIdentity && content.Published; - - scope.WriteLock(Constants.Locks.ContentTree); - - // always save - var saveEventArgs = new SaveEventArgs(content, evtMsgs); - if (raiseEvents && scope.Events.DispatchCancelable(Saving, this, saveEventArgs, "Saving")) + if (cannotBePublished) { - scope.Complete(); - return new PublishResult(PublishResultType.FailedCancelledByEvent, evtMsgs, content); + publishing = false; + unpublishing = content.Published; // if not published yet, nothing to do + + // we may end up in a state where we won't publish nor unpublish + // keep going, though, as we want to save anyways } - - if (publishing) + else { - // ensure that the document can be published, and publish + culturesChanging = content.PublishCultureInfos.Where(x => x.Value.IsDirty()).Select(x => x.Key).ToList(); + } + } + + var isNew = !content.HasIdentity; + var changeType = isNew ? TreeChangeTypes.RefreshNode : TreeChangeTypes.RefreshBranch; + var previouslyPublished = content.HasIdentity && content.Published; + + // always save + var saveEventArgs = new SaveEventArgs(content, evtMsgs); + if (raiseEvents && scope.Events.DispatchCancelable(Saving, this, saveEventArgs, "Saving")) + return new PublishResult(PublishResultType.FailedCancelledByEvent, evtMsgs, content); + + if (publishing) + { + // ensure that the document can be published, and publish + // handling events, business rules, etc + // note: StrategyPublish flips the PublishedState to Publishing! + publishResult = StrategyCanPublish(scope, content, userId, /*checkPath:*/ true, evtMsgs); + if (publishResult.Success) + publishResult = StrategyPublish(scope, content, /*canPublish:*/ true, userId, evtMsgs); + if (!publishResult.Success) + ((Content)content).Published = content.Published; // reset published state = save unchanged + } + + if (unpublishing) + { + var newest = GetById(content.Id); // ensure we have the newest version - in scope + if (content.VersionId != newest.VersionId) // but use the original object if it's already the newest version + content = newest; + + if (content.Published) + { + // ensure that the document can be unpublished, and unpublish // handling events, business rules, etc - // note: StrategyPublish flips the PublishedState to Publishing! - publishResult = StrategyCanPublish(scope, content, userId, /*checkPath:*/ true, evtMsgs); - if (publishResult.Success) - publishResult = StrategyPublish(scope, content, /*canPublish:*/ true, userId, evtMsgs); - if (!publishResult.Success) - ((Content) content).Published = content.Published; // reset published state = save unchanged + // note: StrategyUnpublish flips the PublishedState to Unpublishing! + // note: This unpublishes the entire document (not different variants) + unpublishResult = StrategyCanUnpublish(scope, content, userId, evtMsgs); + if (unpublishResult.Success) + unpublishResult = StrategyUnpublish(scope, content, true, userId, evtMsgs); + if (!unpublishResult.Success) + ((Content)content).Published = content.Published; // reset published state = save unchanged + } + else + { + // already unpublished - optimistic concurrency collision, really, + // and I am not sure at all what we should do, better die fast, else + // we may end up corrupting the db + throw new InvalidOperationException("Concurrency collision."); + } + } + + // save, always + if (content.HasIdentity == false) + content.CreatorId = userId; + content.WriterId = userId; + + // saving does NOT change the published version, unless PublishedState is Publishing or Unpublishing + _documentRepository.Save(content); + + // raise the Saved event, always + if (raiseEvents) + { + saveEventArgs.CanCancel = false; + scope.Events.Dispatch(Saved, this, saveEventArgs, "Saved"); + } + + if (unpublishing) // we have tried to unpublish + { + if (unpublishResult.Success) // and succeeded, trigger events + { + // events and audit + scope.Events.Dispatch(Unpublished, this, new PublishEventArgs(content, false, false), "Unpublished"); + scope.Events.Dispatch(TreeChanged, this, new TreeChange(content, TreeChangeTypes.RefreshBranch).ToEventArgs()); + Audit(AuditType.Unpublish, userId, content.Id); + return new PublishResult(PublishResultType.Success, evtMsgs, content); } - if (unpublishing) - { - var newest = GetById(content.Id); // ensure we have the newest version - in scope - if (content.VersionId != newest.VersionId) // but use the original object if it's already the newest version - content = newest; + // or, failed + scope.Events.Dispatch(TreeChanged, this, new TreeChange(content, changeType).ToEventArgs()); + return new PublishResult(PublishResultType.FailedToUnpublish, evtMsgs, content); // bah + } - if (content.Published) + if (publishing) // we have tried to publish + { + if (publishResult.Success) // and succeeded, trigger events + { + if (isNew == false && previouslyPublished == false) + changeType = TreeChangeTypes.RefreshBranch; // whole branch + + // invalidate the node/branch + scope.Events.Dispatch(TreeChanged, this, new TreeChange(content, changeType).ToEventArgs()); + scope.Events.Dispatch(Published, this, new PublishEventArgs(content, false, false), "Published"); + + // if was not published and now is... descendants that were 'published' (but + // had an unpublished ancestor) are 're-published' ie not explicitely published + // but back as 'published' nevertheless + if (isNew == false && previouslyPublished == false && HasChildren(content.Id)) { - // ensure that the document can be unpublished, and unpublish - // handling events, business rules, etc - // note: StrategyUnpublish flips the PublishedState to Unpublishing! - // note: This unpublishes the entire document (not different variants) - unpublishResult = StrategyCanUnpublish(scope, content, userId, evtMsgs); - if (unpublishResult.Success) - unpublishResult = StrategyUnpublish(scope, content, true, userId, evtMsgs); - if (!unpublishResult.Success) - ((Content) content).Published = content.Published; // reset published state = save unchanged + var descendants = GetPublishedDescendantsLocked(content).ToArray(); + scope.Events.Dispatch(Published, this, new PublishEventArgs(descendants, false, false), "Published"); + } + + if (culturesChanging != null) + { + var langs = string.Join(", ", _languageRepository.GetMany() + .Where(x => culturesChanging.InvariantContains(x.IsoCode)) + .Select(x => x.CultureName)); + Audit(AuditType.PublishVariant, userId, content.Id, $"Published languages: {langs}", langs); } else - { - // already unpublished - optimistic concurrency collision, really, - // and I am not sure at all what we should do, better die fast, else - // we may end up corrupting the db - throw new InvalidOperationException("Concurrency collision."); - } - } + Audit(AuditType.Publish, userId, content.Id); - // save, always - if (content.HasIdentity == false) - content.CreatorId = userId; - content.WriterId = userId; - - // saving does NOT change the published version, unless PublishedState is Publishing or Unpublishing - _documentRepository.Save(content); - - // raise the Saved event, always - if (raiseEvents) - { - saveEventArgs.CanCancel = false; - scope.Events.Dispatch(Saved, this, saveEventArgs, "Saved"); - } - - if (unpublishing) // we have tried to unpublish - { - if (unpublishResult.Success) // and succeeded, trigger events - { - // events and audit - scope.Events.Dispatch(Unpublished, this, new PublishEventArgs(content, false, false), "Unpublished"); - scope.Events.Dispatch(TreeChanged, this, new TreeChange(content, TreeChangeTypes.RefreshBranch).ToEventArgs()); - Audit(AuditType.Unpublish, userId, content.Id); - scope.Complete(); - return new PublishResult(PublishResultType.Success, evtMsgs, content); - } - - // or, failed - scope.Events.Dispatch(TreeChanged, this, new TreeChange(content, changeType).ToEventArgs()); - scope.Complete(); // compete the save - return new PublishResult(PublishResultType.FailedToUnpublish, evtMsgs, content); // bah - } - - if (publishing) // we have tried to publish - { - if (publishResult.Success) // and succeeded, trigger events - { - if (isNew == false && previouslyPublished == false) - changeType = TreeChangeTypes.RefreshBranch; // whole branch - - // invalidate the node/branch - scope.Events.Dispatch(TreeChanged, this, new TreeChange(content, changeType).ToEventArgs()); - scope.Events.Dispatch(Published, this, new PublishEventArgs(content, false, false), "Published"); - - // if was not published and now is... descendants that were 'published' (but - // had an unpublished ancestor) are 're-published' ie not explicitely published - // but back as 'published' nevertheless - if (isNew == false && previouslyPublished == false && HasChildren(content.Id)) - { - var descendants = GetPublishedDescendantsLocked(content).ToArray(); - scope.Events.Dispatch(Published, this, new PublishEventArgs(descendants, false, false), "Published"); - } - - if (culturesChanging != null) - { - var langs = string.Join(", ", _languageRepository.GetMany() - .Where(x => culturesChanging.InvariantContains(x.IsoCode)) - .Select(x => x.CultureName)); - Audit(AuditType.PublishVariant, userId, content.Id, $"Published languages: {langs}", langs); - } - else - Audit(AuditType.Publish, userId, content.Id); - - scope.Complete(); - return publishResult; - } - - // or, failed - scope.Events.Dispatch(TreeChanged, this, new TreeChange(content, changeType).ToEventArgs()); - scope.Complete(); // compete the save return publishResult; } - // both publishing and unpublishing are false - // this means that we wanted to publish, in a variant scenario, a document that - // was not published yet, and we could not, due to cultures issues - // - // raise event (we saved), report - + // or, failed scope.Events.Dispatch(TreeChanged, this, new TreeChange(content, changeType).ToEventArgs()); - scope.Complete(); // compete the save - return new PublishResult(PublishResultType.FailedByCulture, evtMsgs, content); + return publishResult; } + + // both publishing and unpublishing are false + // this means that we wanted to publish, in a variant scenario, a document that + // was not published yet, and we could not, due to cultures issues + // + // raise event (we saved), report + + scope.Events.Dispatch(TreeChanged, this, new TreeChange(content, changeType).ToEventArgs()); + return new PublishResult(PublishResultType.FailedByCulture, evtMsgs, content); } /// @@ -1218,16 +1216,44 @@ namespace Umbraco.Core.Services.Implement // note: EditedValue and PublishedValue are objects here, so it is important to .Equals() // and not to == them, else we would be comparing references, and that is a bad thing - bool IsEditing(IContent c, string l) - => c.PublishName != c.Name || - c.PublishedCultures.Where(x => x.InvariantEquals(l)).Any(x => c.GetCultureName(x) != c.GetPublishName(x)) || - c.Properties.Any(x => x.Values.Where(y => culture == "*" || y.Culture.InvariantEquals(l)).Any(y => !y.EditedValue.Equals(y.PublishedValue))); + // determines whether the document is edited, and thus needs to be published, + // for the specified culture (it may be edited for other cultures and that + // should not trigger a publish). + HashSet ShouldPublish(IContent c) + { + if (c.ContentType.VariesByCulture()) + { + // variant content type + // add culture if edited, and already published or forced + if (c.IsCultureEdited(culture) && (c.IsCulturePublished(culture) || force)) + return new HashSet { culture.ToLowerInvariant() }; + } + else + { + // invariant content type + // add "*" if edited, and already published or forced + if (c.Edited && (c.Published || force)) + return new HashSet { "*" }; + } - return SaveAndPublishBranch(content, force, document => IsEditing(document, culture), document => document.PublishCulture(culture), userId); + return new HashSet(); + } + + // publish the specified cultures + bool PublishCultures(IContent c, HashSet culturesToPublish) + { + // variant content type - publish specified cultures + // invariant content type - publish only the invariant culture + return c.ContentType.VariesByCulture() + ? culturesToPublish.All(c.PublishCulture) + : c.PublishCulture(); + } + + return SaveAndPublishBranch(content, force, ShouldPublish, PublishCultures, userId); } - // fixme - make this public once we know it works + document - private IEnumerable SaveAndPublishBranch(IContent content, bool force, string[] cultures, int userId = 0) + /// + public IEnumerable SaveAndPublishBranch(IContent content, bool force, string[] cultures, int userId = 0) { // note: EditedValue and PublishedValue are objects here, so it is important to .Equals() // and not to == them, else we would be comparing references, and that is a bad thing @@ -1237,35 +1263,47 @@ namespace Umbraco.Core.Services.Implement // determines whether the document is edited, and thus needs to be published, // for the specified cultures (it may be edited for other cultures and that // should not trigger a publish). - bool IsEdited(IContent c) + HashSet ShouldPublish(IContent c) { - if (cultures.Length == 0) + var culturesToPublish = new HashSet(); + + if (c.ContentType.VariesByCulture()) { - // nothing = everything - return c.PublishName != c.Name || - c.PublishedCultures.Any(x => c.GetCultureName(x) != c.GetPublishName(x)) || - c.Properties.Any(x => x.Values.Any(y => !y.EditedValue.Equals(y.PublishedValue))); + // variant content type + // add cultures which are edited, and already published or forced + foreach (var culture in cultures) + { + if (c.IsCultureEdited(culture) && (c.IsCulturePublished(culture) || force)) + culturesToPublish.Add(culture.ToLowerInvariant()); + } + } + else + { + // invariant content type + // add "*" if edited, and already published or forced + if (c.Edited && (c.Published || force)) + culturesToPublish.Add("*"); } - return c.PublishName != c.Name || - c.PublishedCultures.Where(x => cultures.Contains(x, StringComparer.InvariantCultureIgnoreCase)).Any(x => c.GetCultureName(x) != c.GetPublishName(x)) || - c.Properties.Any(x => x.Values.Where(y => cultures.Contains(y.Culture, StringComparer.InvariantCultureIgnoreCase)).Any(y => !y.EditedValue.Equals(y.PublishedValue))); + return culturesToPublish; } // publish the specified cultures - bool PublishCultures(IContent c) + bool PublishCultures(IContent c, HashSet culturesToPublish) { - return cultures.Length == 0 - ? c.PublishCulture() // nothing = everything - : cultures.All(c.PublishCulture); + // variant content type - publish specified cultures + // invariant content type - publish only the invariant culture + return c.ContentType.VariesByCulture() + ? culturesToPublish.All(c.PublishCulture) + : c.PublishCulture(); } - return SaveAndPublishBranch(content, force, IsEdited, PublishCultures, userId); + return SaveAndPublishBranch(content, force, ShouldPublish, PublishCultures, userId); } /// public IEnumerable SaveAndPublishBranch(IContent document, bool force, - Func editing, Func publishCultures, int userId = 0) + Func> shouldPublish, Func, bool> publishCultures, int userId = 0) { var evtMsgs = EventMessagesFactory.Get(); var results = new List(); @@ -1278,14 +1316,14 @@ namespace Umbraco.Core.Services.Implement // fixme events?! if (!document.HasIdentity) - throw new InvalidOperationException("Do not branch-publish a new document."); + throw new InvalidOperationException("Cannot not branch-publish a new document."); var publishedState = ((Content) document).PublishedState; if (publishedState == PublishedState.Publishing) - throw new InvalidOperationException("Do not publish values when publishing branches."); + throw new InvalidOperationException("Cannot mix PublishCulture and SaveAndPublishBranch."); // deal with the branch root - if it fails, abort - var result = SaveAndPublishBranchOne(scope, document, editing, publishCultures, true, publishedDocuments, evtMsgs, userId); + var result = SaveAndPublishBranchOne(scope, document, shouldPublish, publishCultures, true, publishedDocuments, evtMsgs, userId); results.Add(result); if (!result.Success) return results; @@ -1293,35 +1331,36 @@ namespace Umbraco.Core.Services.Implement // if one fails, abort its branch var exclude = new HashSet(); - const int pageSize = 500; + int count; var page = 0; - var total = long.MaxValue; - while (page * pageSize < total) + const int pageSize = 100; + do { - var descendants = GetPagedDescendants(document.Id, page++, pageSize, out total); - - foreach (var d in descendants) + count = 0; + // important to order by Path ASC so make it explicit in case defaults change + // ReSharper disable once RedundantArgumentDefaultValue + foreach (var d in GetPagedDescendants(document.Id, page, pageSize, out _, ordering: Ordering.By("Path", Direction.Ascending))) { - // if parent is excluded, exclude document and ignore - // if not forcing, and not publishing, exclude document and ignore - if (exclude.Contains(d.ParentId) || !force && !d.Published) + count++; + + // if parent is excluded, exclude child too + if (exclude.Contains(d.ParentId)) { exclude.Add(d.Id); continue; } - // no need to check path here, - // 1. because we know the parent is path-published (we just published it) - // 2. because it would not work as nothing's been written out to the db until the uow completes - result = SaveAndPublishBranchOne(scope, d, editing, publishCultures, false, publishedDocuments, evtMsgs, userId); + // no need to check path here, parent has to be published here + result = SaveAndPublishBranchOne(scope, d, shouldPublish, publishCultures, false, publishedDocuments, evtMsgs, userId); results.Add(result); if (result.Success) continue; - // abort branch + // if we could not publish the document, cut its branch exclude.Add(d.Id); } - } - + + page++; + } while (count > 0); scope.Events.Dispatch(TreeChanged, this, new TreeChange(document, TreeChangeTypes.RefreshBranch).ToEventArgs()); scope.Events.Dispatch(Published, this, new PublishEventArgs(publishedDocuments, false, false), "Published"); @@ -1333,21 +1372,31 @@ namespace Umbraco.Core.Services.Implement return results; } + // shouldPublish: a function determining whether the document has changes that need to be published + // note - 'force' is handled by 'editing' + // publishValues: a function publishing values (using the appropriate PublishCulture calls) private PublishResult SaveAndPublishBranchOne(IScope scope, IContent document, - Func editing, Func publishValues, + Func> shouldPublish, Func, bool> publishCultures, bool checkPath, - List publishedDocuments, + ICollection publishedDocuments, EventMessages evtMsgs, int userId) { - // if already published, and values haven't changed - i.e. not changing anything - // nothing to do - fixme - unless we *want* to bump dates? - if (document.Published && (editing == null || !editing(document))) + // use 'shouldPublish' to determine whether there are changes to be published + // if the document has no changes to be published - nothing to + // for an invariant content, shouldPublish may contain "*" to indicate changes + var culturesToPublish = shouldPublish?.Invoke(document); + if (culturesToPublish == null || culturesToPublish.Count == 0) return new PublishResult(PublishResultType.SuccessAlready, evtMsgs, document); + // there are changes to be published - publish them with 'publishCultures' + // publish & check if values are valid - if (publishValues != null && !publishValues(document)) + if (publishCultures != null && !publishCultures(document, culturesToPublish)) return new PublishResult(PublishResultType.FailedContentInvalid, evtMsgs, document); + return SavePublishingInternal(scope, document, userId); + + // fixme deal with the rest of this code! // check if we can publish var result = StrategyCanPublish(scope, document, userId, checkPath, evtMsgs); if (!result.Success) diff --git a/src/Umbraco.Tests/Services/ContentServicePublishBranchTests.cs b/src/Umbraco.Tests/Services/ContentServicePublishBranchTests.cs new file mode 100644 index 0000000000..699bf1e5a6 --- /dev/null +++ b/src/Umbraco.Tests/Services/ContentServicePublishBranchTests.cs @@ -0,0 +1,368 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using NUnit.Framework; +using Umbraco.Core; +using Umbraco.Core.Models; +using Umbraco.Core.Services; +using Umbraco.Tests.TestHelpers; +using Umbraco.Tests.Testing; + +// ReSharper disable CommentTypo +// ReSharper disable StringLiteralTypo +namespace Umbraco.Tests.Services +{ + [TestFixture] + [UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest, PublishedRepositoryEvents = true, WithApplication = true)] + public class ContentServicePublishBranchTests : TestWithDatabaseBase + { + [TestCase(1)] // use overload w/ culture: "*" + [TestCase(2)] // use overload w/ cultures: new [] { "*" } + public void Can_Publish_InvariantBranch(int method) + { + CreateTypes(out var iContentType, out _); + + IContent iRoot = new Content("iroot", -1, iContentType); + iRoot.SetValue("ip", "iroot"); + ServiceContext.ContentService.Save(iRoot); + IContent ii1 = new Content("ii1", iRoot, iContentType); + ii1.SetValue("ip", "vii1"); + ServiceContext.ContentService.Save(ii1); + IContent ii2 = new Content("ii2", iRoot, iContentType); + ii2.SetValue("ip", "vii2"); + ServiceContext.ContentService.Save(ii2); + + // iroot !published !edited + // ii1 !published !edited + // ii2 !published !edited + + // !force = publishes those that are actually published, and have changes + // here: nothing + + var r = SaveAndPublishInvariantBranch(iRoot, false, method).ToArray(); + AssertPublishResults(r, x => x.Content.Name, + "iroot", "ii1", "ii2"); + AssertPublishResults(r, x => x.Result, + PublishResultType.SuccessAlready, + PublishResultType.SuccessAlready, + PublishResultType.SuccessAlready); + + // prepare + + ServiceContext.ContentService.SaveAndPublish(iRoot); + ServiceContext.ContentService.SaveAndPublish(ii1); + + IContent ii11 = new Content("ii11", ii1, iContentType); + ii11.SetValue("ip", "vii11"); + ServiceContext.ContentService.SaveAndPublish(ii11); + IContent ii12 = new Content("ii12", ii1, iContentType); + ii11.SetValue("ip", "vii12"); + ServiceContext.ContentService.Save(ii12); + + ServiceContext.ContentService.SaveAndPublish(ii2); + IContent ii21 = new Content("ii21", ii2, iContentType); + ii21.SetValue("ip", "vii21"); + ServiceContext.ContentService.SaveAndPublish(ii21); + IContent ii22 = new Content("ii22", ii2, iContentType); + ii22.SetValue("ip", "vii22"); + ServiceContext.ContentService.Save(ii22); + ServiceContext.ContentService.Unpublish(ii2); + + // iroot published !edited + // ii1 published !edited + // ii11 published !edited + // ii12 !published !edited + // ii2 !published !edited + // ii21 (published) !edited + // ii22 !published !edited + + // !force = publishes those that are actually published, and have changes + // here: nothing + + r = SaveAndPublishInvariantBranch(iRoot, false, method).ToArray(); + AssertPublishResults(r, x => x.Content.Name, + "iroot", "ii1", "ii11", "ii12", "ii2", "ii21", "ii22"); + AssertPublishResults(r, x => x.Result, + PublishResultType.SuccessAlready, + PublishResultType.SuccessAlready, + PublishResultType.SuccessAlready, + PublishResultType.SuccessAlready, + PublishResultType.SuccessAlready, + PublishResultType.SuccessAlready, + PublishResultType.SuccessAlready); + + // prepare + + iRoot.SetValue("ip", "changed"); + ServiceContext.ContentService.Save(iRoot); + ii11.SetValue("ip", "changed"); + ServiceContext.ContentService.Save(ii11); + + // iroot published edited *** + // ii1 published !edited + // ii11 published edited *** + // ii12 !published !edited + // ii2 !published !edited + // ii21 (published) !edited + // ii22 !published !edited + + // !force = publishes those that are actually published, and have changes + // here: iroot and ii11 + + r = SaveAndPublishInvariantBranch(iRoot, false, method).ToArray(); + AssertPublishResults(r, x => x.Content.Name, + "iroot", "ii1", "ii11", "ii12", "ii2", "ii21", "ii22"); + AssertPublishResults(r, x => x.Result, + PublishResultType.Success, + PublishResultType.SuccessAlready, + PublishResultType.Success, + PublishResultType.SuccessAlready, + PublishResultType.SuccessAlready, + PublishResultType.SuccessAlready, + PublishResultType.SuccessAlready); + + // force = publishes everything that has changes + // here: ii12, ii2, ii22 - ii21 was published already but masked + + r = SaveAndPublishInvariantBranch(iRoot, true, method).ToArray(); + AssertPublishResults(r, x => x.Content.Name, + "iroot", "ii1", "ii11", "ii12", "ii2", "ii21", "ii22"); + AssertPublishResults(r, x => x.Result, + PublishResultType.SuccessAlready, + PublishResultType.SuccessAlready, + PublishResultType.SuccessAlready, + PublishResultType.Success, + PublishResultType.Success, + PublishResultType.SuccessAlready, // was masked + PublishResultType.Success); + + ii21 = ServiceContext.ContentService.GetById(ii21.Id); + Assert.IsTrue(ii21.Published); + } + + [Test] + public void Can_Publish_VariantBranch() + { + CreateTypes(out _, out var vContentType); + + IContent vRoot = new Content("vroot", -1, vContentType, "de"); + vRoot.SetCultureName("vroot.de", "de"); + vRoot.SetCultureName("vroot.ru", "ru"); + vRoot.SetCultureName("vroot.es", "es"); + vRoot.SetValue("ip", "vroot"); + vRoot.SetValue("vp", "vroot.de", "de"); + vRoot.SetValue("vp", "vroot.ru", "ru"); + vRoot.SetValue("vp", "vroot.es", "es"); + ServiceContext.ContentService.Save(vRoot); + + IContent iv1 = new Content("iv1", vRoot, vContentType, "de"); + iv1.SetCultureName("iv1.de", "de"); + iv1.SetCultureName("iv1.ru", "ru"); + iv1.SetCultureName("iv1.es", "es"); + iv1.SetValue("ip", "iv1"); + iv1.SetValue("vp", "iv1.de", "de"); + iv1.SetValue("vp", "iv1.ru", "ru"); + iv1.SetValue("vp", "iv1.es", "es"); + ServiceContext.ContentService.Save(iv1); + + IContent iv2 = new Content("iv2", vRoot, vContentType, "de"); + iv2.SetCultureName("iv2.de", "de"); + iv2.SetCultureName("iv2.ru", "ru"); + iv2.SetCultureName("iv2.es", "es"); + iv2.SetValue("ip", "iv2"); + iv2.SetValue("vp", "iv2.de", "de"); + iv2.SetValue("vp", "iv2.ru", "ru"); + iv2.SetValue("vp", "iv2.es", "es"); + ServiceContext.ContentService.Save(iv2); + + // vroot !published !edited + // iv1 !published !edited + // iv2 !published !edited + + // !force = publishes those that are actually published, and have changes + // here: nothing + + var r = ServiceContext.ContentService.SaveAndPublishBranch(vRoot, false).ToArray(); + AssertPublishResults(r, x => x.Content.Name, + "vroot.de", "iv1.de", "iv2.de"); + AssertPublishResults(r, x => x.Result, + PublishResultType.SuccessAlready, + PublishResultType.SuccessAlready, + PublishResultType.SuccessAlready); + + // prepare + ServiceContext.ContentService.SaveAndPublish(vRoot, "de"); + vRoot.SetValue("ip", "changed"); + vRoot.SetValue("vp", "changed.de", "de"); + vRoot.SetValue("vp", "changed.ru", "ru"); + vRoot.SetValue("vp", "changed.es", "es"); + ServiceContext.ContentService.Save(vRoot); + iv1.PublishCulture("de"); + iv1.PublishCulture("ru"); + ServiceContext.ContentService.SavePublishing(iv1); + iv1.SetValue("ip", "changed"); + iv1.SetValue("vp", "changed.de", "de"); + iv1.SetValue("vp", "changed.ru", "ru"); + iv1.SetValue("vp", "changed.es", "es"); + ServiceContext.ContentService.Save(iv1); + + // validate + Assert.IsTrue(vRoot.Published); + Assert.IsTrue(vRoot.IsCulturePublished("de")); + Assert.IsFalse(vRoot.IsCulturePublished("ru")); + Assert.IsFalse(vRoot.IsCulturePublished("es")); + Assert.IsTrue(iv1.Published); + Assert.IsTrue(iv1.IsCulturePublished("de")); + Assert.IsTrue(iv1.IsCulturePublished("ru")); + Assert.IsFalse(vRoot.IsCulturePublished("es")); + + r = ServiceContext.ContentService.SaveAndPublishBranch(vRoot, false, "de").ToArray(); + AssertPublishResults(r, x => x.Content.Name, + "vroot.de", "iv1.de", "iv2.de"); + AssertPublishResults(r, x => x.Result, + PublishResultType.Success, + PublishResultType.Success, + PublishResultType.SuccessAlready); + + // reload - SaveAndPublishBranch has modified other instances + Reload(ref iv1); + Reload(ref iv2); + + // de is published, ru and es have not been published + Assert.IsTrue(vRoot.Published); + Assert.IsTrue(vRoot.IsCulturePublished("de")); + Assert.IsFalse(vRoot.IsCulturePublished("ru")); + Assert.IsFalse(vRoot.IsCulturePublished("es")); + Assert.AreEqual("changed", vRoot.GetValue("ip", published: true)); // publishing de implies publishing invariants + Assert.AreEqual("changed.de", vRoot.GetValue("vp", "de", published: true)); + + // de and ru are published, es has not been published + Assert.IsTrue(iv1.Published); + Assert.IsTrue(iv1.IsCulturePublished("de")); + Assert.IsTrue(iv1.IsCulturePublished("ru")); + Assert.IsFalse(vRoot.IsCulturePublished("es")); + Assert.AreEqual("changed", iv1.GetValue("ip", published: true)); + Assert.AreEqual("changed.de", iv1.GetValue("vp", "de", published: true)); + Assert.AreEqual("iv1.ru", iv1.GetValue("vp", "ru", published: true)); + + } + + [Test] + public void Can_Publish_MixedBranch() + { + // invariant root -> variant -> invariant + // variant root -> variant -> invariant + // variant root -> invariant -> variant + + CreateTypes(out var iContentType, out var vContentType); + + // invariant root -> invariant -> variant + IContent iRoot = new Content("iroot", -1, iContentType); + iRoot.SetValue("ip", "iroot"); + ServiceContext.ContentService.SaveAndPublish(iRoot); + IContent ii1 = new Content("ii1", iRoot, iContentType); + ii1.SetValue("ip", "vii1"); + ServiceContext.ContentService.SaveAndPublish(ii1); + ii1.SetValue("ip", "changed"); + ServiceContext.ContentService.Save(ii1); + IContent iv11 = new Content("iv11.de", ii1, vContentType, "de"); + iv11.SetValue("ip", "iv11"); + iv11.SetValue("vp", "iv11.de", "de"); + iv11.SetValue("vp", "iv11.ru", "ru"); + 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); + + Assert.AreEqual("iv11.de", iv11.GetValue("vp", "de", published: true)); + Assert.AreEqual("iv11.ru", iv11.GetValue("vp", "ru", published: true)); + + iv11.SetValue("ip", "changed"); + iv11.SetValue("vp", "changed.de", "de"); + iv11.SetValue("vp", "changed.ru", "ru"); + ServiceContext.ContentService.Save(iv11); + + var r = ServiceContext.ContentService.SaveAndPublishBranch(iRoot, false, "de").ToArray(); + AssertPublishResults(r, x => x.Content.Name, + "iroot", "ii1", "iv11.de"); + AssertPublishResults(r, x => x.Result, + PublishResultType.SuccessAlready, + PublishResultType.Success, + PublishResultType.Success); + + // reload - SaveAndPublishBranch has modified other instances + Reload(ref ii1); + Reload(ref iv11); + + // the invariant child has been published + // the variant child has been published for 'de' only + + Assert.AreEqual("changed", ii1.GetValue("ip", published: true)); + Assert.AreEqual("changed", iv11.GetValue("ip", published: true)); + Assert.AreEqual("changed.de", iv11.GetValue("vp", "de", published: true)); + Assert.AreEqual("iv11.ru", iv11.GetValue("vp", "ru", published: true)); + } + + private void AssertPublishResults(PublishResult[] values, Func getter, params T[] expected) + { + Assert.AreEqual(expected.Length, values.Length); + for (var i = 0; i < values.Length; i++) + { + var value = getter(values[i]); + Assert.AreEqual(expected[i], value, $"Expected {expected[i]} at {i} but got {value}."); + } + } + + private void Reload(ref IContent document) + => document = ServiceContext.ContentService.GetById(document.Id); + + private void CreateTypes(out IContentType iContentType, out IContentType vContentType) + { + var langDe = new Language("de") { IsDefault = true }; + ServiceContext.LocalizationService.Save(langDe); + var langRu = new Language("ru"); + ServiceContext.LocalizationService.Save(langRu); + var langEs = new Language("es"); + ServiceContext.LocalizationService.Save(langEs); + + iContentType = new ContentType(-1) + { + Alias = "ict", + Name = "Invariant Content Type", + Variations = ContentVariation.Nothing + }; + iContentType.AddPropertyType(new PropertyType(Constants.PropertyEditors.Aliases.TextBox, ValueStorageType.Nvarchar, "ip") { Variations = ContentVariation.Nothing }); + ServiceContext.ContentTypeService.Save(iContentType); + + vContentType = new ContentType(-1) + { + Alias = "vct", + Name = "Variant Content Type", + Variations = ContentVariation.Culture + }; + vContentType.AddPropertyType(new PropertyType(Constants.PropertyEditors.Aliases.TextBox, ValueStorageType.Nvarchar, "ip") { Variations = ContentVariation.Nothing }); + vContentType.AddPropertyType(new PropertyType(Constants.PropertyEditors.Aliases.TextBox, ValueStorageType.Nvarchar, "vp") { Variations = ContentVariation.Culture }); + ServiceContext.ContentTypeService.Save(vContentType); + } + + private IEnumerable SaveAndPublishInvariantBranch(IContent content, bool force, int method) + { + // ReSharper disable RedundantArgumentDefaultValue + // ReSharper disable ArgumentsStyleOther + switch (method) + { + case 1: + return ServiceContext.ContentService.SaveAndPublishBranch(content, force, culture: "*"); + case 2: + return ServiceContext.ContentService.SaveAndPublishBranch(content, force, cultures: new [] { "*" }); + default: + throw new ArgumentOutOfRangeException(nameof(method)); + } + // ReSharper restore RedundantArgumentDefaultValue + // ReSharper restore ArgumentsStyleOther + } + } +} diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index d5d74ee754..5f0a8db0b5 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -1,5 +1,4 @@ using System; -using System.Collections; using System.Collections.Generic; using System.Diagnostics; using System.Linq; @@ -12,18 +11,15 @@ using Umbraco.Core.IO; using Umbraco.Core.Models; using Umbraco.Core.Models.Membership; using LightInject; -using Umbraco.Core.Persistence.Repositories; using Umbraco.Core.Services; using Umbraco.Tests.TestHelpers.Entities; using Umbraco.Core.Events; -using Umbraco.Core.Logging; 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 Umbraco.Web.PropertyEditors; using System.Reflection; using Umbraco.Core.Persistence.DatabaseModelDefinitions; diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 156bc06a14..34ce14039e 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -129,6 +129,7 @@ +