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