From 76b1cd6573a2b2f0f441bb7dfc65db9770247202 Mon Sep 17 00:00:00 2001 From: Stephan Date: Tue, 13 Nov 2018 11:24:30 +0100 Subject: [PATCH 1/4] Cleanup --- .../Repositories/IDocumentRepository.cs | 9 +-- .../Services/Implement/ContentService.cs | 65 +++++++++++-------- 2 files changed, 40 insertions(+), 34 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/IDocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/IDocumentRepository.cs index 685f8643cb..fc5382499f 100644 --- a/src/Umbraco.Core/Persistence/Repositories/IDocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/IDocumentRepository.cs @@ -8,15 +8,13 @@ namespace Umbraco.Core.Persistence.Repositories public interface IDocumentRepository : IContentRepository, IReadRepository { /// - /// Clears the publishing schedule for all entries before this date + /// Clears the publishing schedule for all entries having an a date before (lower than, or equal to) a specified date. /// - /// void ClearSchedule(DateTime date); /// - /// Gets a collection of objects, which has an expiration date less than or equal to today. + /// Gets objects having an expiration date before (lower than, or equal to) a specified date. /// - /// /// /// The content returned from this method may be culture variant, in which case the resulting should be queried /// for which culture(s) have been scheduled. @@ -24,9 +22,8 @@ namespace Umbraco.Core.Persistence.Repositories IEnumerable GetContentForExpiration(DateTime date); /// - /// Gets a collection of objects, which has a release date less than or equal to today. + /// Gets objects having a release date before (lower than, or equal to) a specified date. /// - /// An Enumerable list of objects /// /// The content returned from this method may be culture variant, in which case the resulting should be queried /// for which culture(s) have been scheduled. diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index 9e5d2a22a8..e74d69d141 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -1164,6 +1164,12 @@ namespace Umbraco.Core.Services.Implement /// public IEnumerable PerformScheduledPublish(DateTime date) + => PerformScheduledPublishInternal(date).ToList(); + + // beware! this method yields results, so the returned IEnumerable *must* be + // enumerated for anything to happen - dangerous, so private + exposed via + // the public method above, which forces ToList(). + private IEnumerable PerformScheduledPublishInternal(DateTime date) { var evtMsgs = EventMessagesFactory.Get(); @@ -1171,61 +1177,64 @@ namespace Umbraco.Core.Services.Implement { scope.WriteLock(Constants.Locks.ContentTree); - var now = date; - - foreach (var d in _documentRepository.GetContentForRelease(now)) + foreach (var d in _documentRepository.GetContentForRelease(date)) { PublishResult result; if (d.ContentType.VariesByCulture()) { //find which cultures have pending schedules - var pendingCultures = d.ContentSchedule.GetPending(ContentScheduleChange.Start, now) + var pendingCultures = d.ContentSchedule.GetPending(ContentScheduleChange.Start, date) .Select(x => x.Culture) .Distinct() .ToList(); - foreach (var c in pendingCultures) + var publishing = true; + foreach (var culture in pendingCultures) { //Clear this schedule for this culture - d.ContentSchedule.Clear(c, ContentScheduleChange.Start, now); - if (!d.Trashed) - d.PublishCulture(c); //set the culture to be published + d.ContentSchedule.Clear(culture, ContentScheduleChange.Start, date); + + if (d.Trashed) continue; // won't publish + + publishing &= d.PublishCulture(culture); //set the culture to be published + if (!publishing) break; // no point continuing } - if (pendingCultures.Count > 0) - { - if (!d.Trashed) - result = SavePublishing(d, d.WriterId); - else - result = new PublishResult(PublishResultType.FailedPublishPathNotPublished, evtMsgs, d); + if (d.Trashed) + result = new PublishResult(PublishResultType.FailedPublishIsTrashed, evtMsgs, d); + else if (!publishing) + result = new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, d); + else + result = SavePublishing(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; - } + if (result.Success == false) + Logger.Error(null, "Failed to publish document id={DocumentId}, reason={Reason}.", d.Id, result.Result); + + yield return result; } else { //Clear this schedule - d.ContentSchedule.Clear(ContentScheduleChange.Start, now); - if (!d.Trashed) - result = SaveAndPublish(d, userId: d.WriterId); - else - result = new PublishResult(PublishResultType.FailedPublishPathNotPublished, evtMsgs, d); + d.ContentSchedule.Clear(ContentScheduleChange.Start, date); + + result = d.Trashed + ? new PublishResult(PublishResultType.FailedPublishIsTrashed, evtMsgs, d) + : SaveAndPublish(d, userId: d.WriterId); if (result.Success == false) Logger.Error(null, "Failed to publish document id={DocumentId}, reason={Reason}.", d.Id, result.Result); + yield return result; } } - foreach (var d in _documentRepository.GetContentForExpiration(now)) + foreach (var d in _documentRepository.GetContentForExpiration(date)) { PublishResult result; if (d.ContentType.VariesByCulture()) { //find which cultures have pending schedules - var pendingCultures = d.ContentSchedule.GetPending(ContentScheduleChange.End, now) + var pendingCultures = d.ContentSchedule.GetPending(ContentScheduleChange.End, date) .Select(x => x.Culture) .Distinct() .ToList(); @@ -1233,7 +1242,7 @@ namespace Umbraco.Core.Services.Implement foreach (var c in pendingCultures) { //Clear this schedule for this culture - d.ContentSchedule.Clear(c, ContentScheduleChange.End, now); + d.ContentSchedule.Clear(c, ContentScheduleChange.End, date); //set the culture to be published d.UnpublishCulture(c); } @@ -1249,7 +1258,7 @@ namespace Umbraco.Core.Services.Implement else { //Clear this schedule - d.ContentSchedule.Clear(ContentScheduleChange.End, now); + d.ContentSchedule.Clear(ContentScheduleChange.End, date); result = Unpublish(d, userId: d.WriterId); if (result.Success == false) Logger.Error(null, "Failed to unpublish document id={DocumentId}, reason={Reason}.", d.Id, result.Result); @@ -1259,7 +1268,7 @@ namespace Umbraco.Core.Services.Implement } - _documentRepository.ClearSchedule(now); + _documentRepository.ClearSchedule(date); scope.Complete(); } From 6cb20c1d791658585980bacac46834ada836ca74 Mon Sep 17 00:00:00 2001 From: Stephan Date: Tue, 13 Nov 2018 11:49:46 +0100 Subject: [PATCH 2/4] Deal with events and contexts in scheduled publishing --- .../Cache/CacheRefresherComponent.cs | 27 ++------------ .../Scheduling/ScheduledPublishing.cs | 35 ++++++++++++++----- src/Umbraco.Web/UmbracoContext.cs | 35 +++++++++++++++++++ 3 files changed, 65 insertions(+), 32 deletions(-) diff --git a/src/Umbraco.Web/Cache/CacheRefresherComponent.cs b/src/Umbraco.Web/Cache/CacheRefresherComponent.cs index 2a427a8cc8..a020b0d76f 100644 --- a/src/Umbraco.Web/Cache/CacheRefresherComponent.cs +++ b/src/Umbraco.Web/Cache/CacheRefresherComponent.cs @@ -222,26 +222,9 @@ namespace Umbraco.Web.Cache internal static void HandleEvents(IEnumerable events) { - // fixme remove this in v8, this is a backwards compat hack and is needed because when we are using Deploy, the events will be raised on a background - //thread which means that cache refreshers will also execute on a background thread and in many cases developers may be using UmbracoContext.Current in their - //cache refresher handlers, so before we execute all of the events, we'll ensure a context - UmbracoContext tempContext = null; - if (UmbracoContext.Current == null) - { - var httpContext = new HttpContextWrapper(HttpContext.Current ?? new HttpContext(new SimpleWorkerRequest("temp.aspx", "", new StringWriter()))); - tempContext = UmbracoContext.EnsureContext( - Current.UmbracoContextAccessor, - httpContext, - null, - new WebSecurity(httpContext, Current.Services.UserService, UmbracoConfig.For.GlobalSettings()), - UmbracoConfig.For.UmbracoSettings(), - Current.UrlProviders, - UmbracoConfig.For.GlobalSettings(), - Current.Container.GetInstance(), - true); - } - - try + // ensure we run with an UmbracoContext, because this may run in a background task, + // yet developers may be using the 'current' UmbracoContext in the event handlers + using (UmbracoContext.EnsureContext()) { foreach (var e in events) { @@ -251,10 +234,6 @@ namespace Umbraco.Web.Cache handler.Invoke(null, new[] { e.Sender, e.Args }); } } - finally - { - tempContext?.Dispose(); - } } #endregion diff --git a/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs b/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs index 0dc882c9ca..4d5d5a467a 100644 --- a/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs +++ b/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs @@ -53,17 +53,36 @@ namespace Umbraco.Web.Scheduling try { - // run - // fixme context & events during scheduled publishing? - // in v7 we create an UmbracoContext and an HttpContext, and cache instructions - // are batched, and we have to explicitly flush them, how is it going to work here? - var result = _contentService.PerformScheduledPublish(DateTime.Now).ToList(); - if (result.Count > 0) - foreach(var grouped in result.GroupBy(x => x.Result)) - _logger.Info("Scheduled publishing result: '{StatusCount}' items with status {Status}", grouped.Count(), grouped.Key); + // ensure we run with an UmbracoContext, because this may run in a background task, + // yet developers may be using the 'current' UmbracoContext in the event handlers + // + // fixme + // - or maybe not, CacheRefresherComponent already ensures a context when handling events + // - UmbracoContext 'current' needs to be refactored and cleaned up + // - batched messenger should not depend on a current HttpContext + // but then what should be its "scope"? could we attach it to scopes? + // - and we should definitively *not* have to flush it here (should be auto) + // + using (var tempContext = UmbracoContext.EnsureContext()) + { + try + { + // run + var result = _contentService.PerformScheduledPublish(DateTime.Now); + foreach (var grouped in result.GroupBy(x => x.Result)) + _logger.Info("Scheduled publishing result: '{StatusCount}' items with status {Status}", grouped.Count(), grouped.Key); + } + finally + { + // if running on a temp context, we have to flush the messenger + if (tempContext != null && Composing.Current.ServerMessenger is BatchedDatabaseServerMessenger m) + m.FlushBatch(); + } + } } catch (Exception ex) { + // important to catch *everything* to ensure the task repeats _logger.Error(ex, "Failed."); } diff --git a/src/Umbraco.Web/UmbracoContext.cs b/src/Umbraco.Web/UmbracoContext.cs index 831e3c5cdc..1961a98ce6 100644 --- a/src/Umbraco.Web/UmbracoContext.cs +++ b/src/Umbraco.Web/UmbracoContext.cs @@ -1,6 +1,8 @@ using System; using System.Collections.Generic; +using System.IO; using System.Web; +using System.Web.Hosting; using Umbraco.Core; using Umbraco.Core.Configuration; using Umbraco.Core.Configuration.UmbracoSettings; @@ -10,6 +12,7 @@ using Umbraco.Web.PublishedCache; using Umbraco.Web.Routing; using Umbraco.Web.Runtime; using Umbraco.Web.Security; +using LightInject; namespace Umbraco.Web { @@ -91,6 +94,35 @@ namespace Umbraco.Web return umbracoContextAccessor.UmbracoContext = new UmbracoContext(httpContext, publishedSnapshotService, webSecurity, umbracoSettings, urlProviders, globalSettings, variationContextAccessor); } + /// + /// Gets a disposable object representing the presence of a current UmbracoContext. + /// + /// + /// The disposable object should be used in a using block: using (UmbracoContext.EnsureContext()) { ... }. + /// If an actual current UmbracoContext is already present, the disposable object is null and this method does nothing. + /// Otherwise, a temporary, dummy UmbracoContext is created and registered in the accessor. And disposed and removed from the accessor. + /// + internal static IDisposable EnsureContext() // keep this internal for now! + { + if (Composing.Current.UmbracoContext != null) return null; + + var httpContext = new HttpContextWrapper(System.Web.HttpContext.Current ?? new HttpContext(new SimpleWorkerRequest("temp.aspx", "", new StringWriter()))); + + return EnsureContext( + Composing.Current.UmbracoContextAccessor, + httpContext, + null, + new WebSecurity(httpContext, Composing.Current.Services.UserService, UmbracoConfig.For.GlobalSettings()), + UmbracoConfig.For.UmbracoSettings(), + Composing.Current.UrlProviders, + UmbracoConfig.For.GlobalSettings(), + Composing.Current.Container.GetInstance(), + true); + + // when the context will be disposed, it will be removed from the accessor + // (see DisposeResources) + } + // initializes a new instance of the UmbracoContext class // internal for unit tests // otherwise it's used by EnsureContext above @@ -215,6 +247,9 @@ namespace Umbraco.Web /// public HttpContextBase HttpContext { get; } + /// + /// Gets the variation context accessor. + /// public IVariationContextAccessor VariationContextAccessor { get; } /// From 0376e48f51abafd5fd0a0e499b5dd54b95236b41 Mon Sep 17 00:00:00 2001 From: Stephan Date: Tue, 13 Nov 2018 11:59:27 +0100 Subject: [PATCH 3/4] Clear fixme --- .../Services/Implement/ContentService.cs | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index e74d69d141..e17b7d973d 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -1988,29 +1988,29 @@ namespace Umbraco.Core.Services.Implement var culturesChanging = content.ContentType.VariesByCulture() ? string.Join(",", content.CultureInfos.Where(x => x.Value.IsDirty()).Select(x => x.Key)) : 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. // in this particular case, determining which cultures have changed works with the above with names since it will // have always changed if it's been saved in the back office but that's not really fail safe. //Save before raising event - // fixme - nesting uow? var saveResult = Save(content, userId); - if (saveResult.Success) - { - sendToPublishEventArgs.CanCancel = false; - scope.Events.Dispatch(SentToPublish, this, sendToPublishEventArgs); - - if (culturesChanging != null) - Audit(AuditType.SendToPublishVariant, userId, content.Id, $"Send To Publish for cultures: {culturesChanging}", culturesChanging); - else - Audit(AuditType.SendToPublish, content.WriterId, content.Id); - } - - // fixme here, on only on success? + // always complete (but maybe return a failed status) scope.Complete(); + if (!saveResult.Success) + return saveResult.Success; + + sendToPublishEventArgs.CanCancel = false; + scope.Events.Dispatch(SentToPublish, this, sendToPublishEventArgs); + + if (culturesChanging != null) + Audit(AuditType.SendToPublishVariant, userId, content.Id, $"Send To Publish for cultures: {culturesChanging}", culturesChanging); + else + Audit(AuditType.SendToPublish, content.WriterId, content.Id); + return saveResult.Success; } } From 0cd8f3787cb37010be1076afb96d252d80126a0e Mon Sep 17 00:00:00 2001 From: Stephan Date: Tue, 13 Nov 2018 12:05:02 +0100 Subject: [PATCH 4/4] Clear fixmes --- src/Umbraco.Core/Models/Content.cs | 12 ++-- src/Umbraco.Core/Models/ContentBase.cs | 13 ++-- .../Services/Implement/ContentService.cs | 65 ++++++++----------- .../Services/PublishResultType.cs | 7 +- 4 files changed, 47 insertions(+), 50 deletions(-) diff --git a/src/Umbraco.Core/Models/Content.cs b/src/Umbraco.Core/Models/Content.cs index 11ef3cf37f..3f6e387dec 100644 --- a/src/Umbraco.Core/Models/Content.cs +++ b/src/Umbraco.Core/Models/Content.cs @@ -217,7 +217,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); + => _publishInfosOrig != null && _publishInfosOrig.ContainsKey(culture); // adjust dates to sync between version, cultures etc // used by the repo when persisting @@ -297,13 +297,10 @@ namespace Umbraco.Core.Models if (_publishInfos == null) return; _publishInfos.Remove(culture); - - //we need to set the culture name to be dirty so we know it's being modified - //fixme is there a better way to do this, not as far as i know. - // fixme why do we need this? - SetCultureName(GetCultureName(culture), culture); - if (_publishInfos.Count == 0) _publishInfos = null; + + // set the culture to be dirty - it's been modified + TouchCultureInfo(culture); } // sets a publish edited @@ -522,7 +519,6 @@ namespace Umbraco.Core.Models clonedContent._schedule = (ContentScheduleCollection)_schedule.DeepClone(); //manually deep clone clonedContent._schedule.CollectionChanged += clonedContent.ScheduleCollectionChanged; //re-assign correct event handler } - } } } diff --git a/src/Umbraco.Core/Models/ContentBase.cs b/src/Umbraco.Core/Models/ContentBase.cs index 7e70238d2f..b0c786d4b0 100644 --- a/src/Umbraco.Core/Models/ContentBase.cs +++ b/src/Umbraco.Core/Models/ContentBase.cs @@ -159,7 +159,7 @@ namespace Umbraco.Core.Models /// [DataMember] public virtual IReadOnlyDictionary CultureInfos => _cultureInfos ?? NoInfos; - + /// public string GetCultureName(string culture) { @@ -222,6 +222,12 @@ 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) { @@ -235,7 +241,7 @@ namespace Umbraco.Core.Models { _cultureInfos = new ContentCultureInfosCollection(); _cultureInfos.CollectionChanged += CultureInfosCollectionChanged; - } + } _cultureInfos.AddOrUpdate(culture, name, date); } @@ -368,7 +374,7 @@ namespace Umbraco.Core.Models #endregion #region Validation - + /// public virtual Property[] ValidateProperties(string culture = "*") { @@ -496,7 +502,6 @@ namespace Umbraco.Core.Models clonedContent._properties = (PropertyCollection) _properties.DeepClone(); //manually deep clone clonedContent._properties.CollectionChanged += clonedContent.PropertiesChanged; //re-assign correct event handler } - } } } diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index e17b7d973d..69ada21b35 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -955,7 +955,7 @@ namespace Umbraco.Core.Services.Implement } } - private PublishResult SavePublishingInternal(IScope scope, IContent content, int userId = 0, bool raiseEvents = true) + private PublishResult SavePublishingInternal(IScope scope, IContent content, int userId = 0, bool raiseEvents = true, bool branchOne = false, bool branchRoot = false) { var evtMsgs = EventMessagesFactory.Get(); PublishResult publishResult = null; @@ -996,7 +996,7 @@ namespace Umbraco.Core.Services.Implement : null; // ensure that the document can be published, and publish handling events, business rules, etc - publishResult = StrategyCanPublish(scope, content, userId, /*checkPath:*/ true, culturesPublishing, culturesUnpublishing, evtMsgs); + publishResult = StrategyCanPublish(scope, content, userId, /*checkPath:*/ (!branchOne || branchRoot), culturesPublishing, culturesUnpublishing, evtMsgs); if (publishResult.Success) { // note: StrategyPublish flips the PublishedState to Publishing! @@ -1004,7 +1004,11 @@ namespace Umbraco.Core.Services.Implement } else { - //check for mandatory culture missing, if this is the case we'll switch the unpublishing flag + // in a branch, just give up + if (branchOne && !branchRoot) + return publishResult; + + //check for mandatory culture missing, and then unpublish document as a whole if (publishResult.Result == PublishResultType.FailedPublishMandatoryCultureMissing) { publishing = false; @@ -1015,15 +1019,17 @@ namespace Umbraco.Core.Services.Implement } //fixme - casting - ((Content)content).Published = content.Published; // reset published state = save unchanged - fixme doh? + // reset published state from temp values (publishing, unpublishing) to original value + // (published, unpublished) in order to save the document, unchanged + ((Content)content).Published = content.Published; } } - if (unpublishing) + if (unpublishing) // won't happen in a branch { 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; // fixme confusing should just die here - else we'll miss some changes + if (content.VersionId != newest.VersionId) + return new PublishResult(PublishResultType.FailedPublishConcurrencyViolation, evtMsgs, content); if (content.Published) { @@ -1037,7 +1043,9 @@ namespace Umbraco.Core.Services.Implement else { //fixme - casting - ((Content)content).Published = content.Published; // reset published state = save unchanged + // reset published state from temp values (publishing, unpublishing) to original value + // (published, unpublished) in order to save the document, unchanged + ((Content)content).Published = content.Published; } } else @@ -1064,7 +1072,7 @@ namespace Umbraco.Core.Services.Implement scope.Events.Dispatch(Saved, this, saveEventArgs, "Saved"); } - if (unpublishing) // we have tried to unpublish + if (unpublishing) // we have tried to unpublish - won't happen in a branch { if (unpublishResult.Success) // and succeeded, trigger events { @@ -1101,13 +1109,14 @@ namespace Umbraco.Core.Services.Implement changeType = TreeChangeTypes.RefreshBranch; // whole branch // invalidate the node/branch - scope.Events.Dispatch(TreeChanged, this, new TreeChange(content, changeType).ToEventArgs()); + if (!branchOne || branchRoot) + 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)) + if (!branchOne && isNew == false && previouslyPublished == false && HasChildren(content.Id)) { var descendants = GetPublishedDescendantsLocked(content).ToArray(); scope.Events.Dispatch(Published, this, new PublishEventArgs(descendants, false, false), "Published"); @@ -1140,11 +1149,14 @@ namespace Umbraco.Core.Services.Implement return publishResult; } - } + // should not happen + if (branchOne && !branchRoot) + throw new Exception("panic"); + //if publishing didn't happen or if it has failed, we still need to log which cultures were saved - if (publishResult == null || !publishResult.Success) + if (!branchOne && (publishResult == null || !publishResult.Success)) { if (culturesChanging != null) { @@ -1154,7 +1166,9 @@ namespace Umbraco.Core.Services.Implement Audit(AuditType.SaveVariant, userId, content.Id, $"Saved languages: {langs}", langs); } else + { Audit(AuditType.Save, userId, content.Id); + } } // or, failed @@ -1429,8 +1443,6 @@ namespace Umbraco.Core.Services.Implement 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"); Audit(AuditType.Publish, userId, document.Id, "Branch published"); scope.Complete(); @@ -1461,28 +1473,7 @@ namespace Umbraco.Core.Services.Implement if (publishCultures != null && !publishCultures(document, culturesToPublish)) return new PublishResult(PublishResultType.FailedPublishContentInvalid, evtMsgs, document); - // fixme - this is totally kinda wrong - - var culturesPublishing = document.ContentType.VariesByCulture() - ? document.PublishCultureInfos.Where(x => x.Value.IsDirty()).Select(x => x.Key).ToList() - : null; - var result = StrategyCanPublish(scope, document, userId, /*checkPath:*/ isRoot, culturesPublishing, Array.Empty(), evtMsgs); - if (!result.Success) - return result; - result = StrategyPublish(scope, document, userId, culturesPublishing, Array.Empty(), evtMsgs); - if (!result.Success) - throw new Exception("panic"); - if (document.HasIdentity == false) - document.CreatorId = userId; - document.WriterId = userId; - _documentRepository.Save(document); - publishedDocuments.Add(document); - // fixme - but then, we have all the audit thing to run - // so... it would be better to re-run the internal thing? - return result; - - // we want _some part_ of it but not all of it - return SavePublishingInternal(scope, document, userId); + return SavePublishingInternal(scope, document, userId, branchOne: true, branchRoot: isRoot); } #endregion diff --git a/src/Umbraco.Core/Services/PublishResultType.cs b/src/Umbraco.Core/Services/PublishResultType.cs index 7462855754..f79dab91d3 100644 --- a/src/Umbraco.Core/Services/PublishResultType.cs +++ b/src/Umbraco.Core/Services/PublishResultType.cs @@ -113,7 +113,7 @@ FailedPublishContentInvalid = FailedPublish | 8, /// - /// The document cannot be published because it has no publishing flags or values. + /// The document could not be published because it has no publishing flags or values. /// FailedPublishNothingToPublish = FailedPublish | 9, // in ContentService.StrategyCanPublish - fixme weird @@ -122,6 +122,11 @@ /// FailedPublishMandatoryCultureMissing = FailedPublish | 10, // in ContentService.SavePublishing + /// + /// The document could not be published because it has been modified by another user. + /// + FailedPublishConcurrencyViolation = FailedPublish | 11, + #endregion #region Failed - Unpublish