diff --git a/src/Umbraco.Core/ContentExtensions.cs b/src/Umbraco.Core/ContentExtensions.cs index b2bf29cc5f..174b4233f4 100644 --- a/src/Umbraco.Core/ContentExtensions.cs +++ b/src/Umbraco.Core/ContentExtensions.cs @@ -52,27 +52,7 @@ namespace Umbraco.Core return ContentStatus.Unpublished; } - /// - /// 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, 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.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 - .ToList(); - } + #endregion diff --git a/src/Umbraco.Core/Events/CancellableEnumerableObjectEventArgs.cs b/src/Umbraco.Core/Events/CancellableEnumerableObjectEventArgs.cs new file mode 100644 index 0000000000..1a651ef348 --- /dev/null +++ b/src/Umbraco.Core/Events/CancellableEnumerableObjectEventArgs.cs @@ -0,0 +1,52 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Security.Permissions; + +namespace Umbraco.Core.Events +{ + [HostProtection(SecurityAction.LinkDemand, SharedState = true)] + public class CancellableEnumerableObjectEventArgs : CancellableObjectEventArgs>, IEquatable> + { + public CancellableEnumerableObjectEventArgs(IEnumerable eventObject, bool canCancel, EventMessages messages, IDictionary additionalData) + : base(eventObject, canCancel, messages, additionalData) + { } + + public CancellableEnumerableObjectEventArgs(IEnumerable eventObject, bool canCancel, EventMessages eventMessages) + : base(eventObject, canCancel, eventMessages) + { } + + public CancellableEnumerableObjectEventArgs(IEnumerable eventObject, EventMessages eventMessages) + : base(eventObject, eventMessages) + { } + + public CancellableEnumerableObjectEventArgs(IEnumerable eventObject, bool canCancel) + : base(eventObject, canCancel) + { } + + public CancellableEnumerableObjectEventArgs(IEnumerable eventObject) + : base(eventObject) + { } + + public bool Equals(CancellableEnumerableObjectEventArgs other) + { + if (other is null) return false; + if (ReferenceEquals(this, other)) return true; + + return EventObject.SequenceEqual(other.EventObject); + } + + public override bool Equals(object obj) + { + if (obj is null) return false; + if (ReferenceEquals(this, obj)) return true; + if (obj.GetType() != this.GetType()) return false; + return Equals((CancellableEnumerableObjectEventArgs)obj); + } + + public override int GetHashCode() + { + return HashCodeHelper.GetHashCode(EventObject); + } + } +} \ No newline at end of file diff --git a/src/Umbraco.Core/Events/CancellableEventArgs.cs b/src/Umbraco.Core/Events/CancellableEventArgs.cs index 0f3091c46a..19f576478f 100644 --- a/src/Umbraco.Core/Events/CancellableEventArgs.cs +++ b/src/Umbraco.Core/Events/CancellableEventArgs.cs @@ -12,7 +12,7 @@ namespace Umbraco.Core.Events public class CancellableEventArgs : EventArgs, IEquatable { private bool _cancel; - private Dictionary _eventState; + private IDictionary _eventState; private static readonly ReadOnlyDictionary EmptyAdditionalData = new ReadOnlyDictionary(new Dictionary()); @@ -89,7 +89,7 @@ namespace Umbraco.Core.Events /// /// Returns the EventMessages object which is used to add messages to the message collection for this event /// - public EventMessages Messages { get; private set; } + public EventMessages Messages { get; } /// /// In some cases raised evens might need to contain additional arbitrary readonly data which can be read by event subscribers @@ -98,7 +98,7 @@ namespace Umbraco.Core.Events /// This allows for a bit of flexibility in our event raising - it's not pretty but we need to maintain backwards compatibility /// so we cannot change the strongly typed nature for some events. /// - public ReadOnlyDictionary AdditionalData { get; private set; } + public ReadOnlyDictionary AdditionalData { get; internal set; } /// /// This can be used by event subscribers to store state in the event args so they easily deal with custom state data between a starting ("ing") @@ -106,7 +106,8 @@ namespace Umbraco.Core.Events /// public IDictionary EventState { - get { return _eventState ?? (_eventState = new Dictionary()); } + get => _eventState ?? (_eventState = new Dictionary()); + internal set => _eventState = value; } public bool Equals(CancellableEventArgs other) diff --git a/src/Umbraco.Core/Events/CancellableObjectEventArgs.cs b/src/Umbraco.Core/Events/CancellableObjectEventArgs.cs index a64a399249..27ffb1b75d 100644 --- a/src/Umbraco.Core/Events/CancellableObjectEventArgs.cs +++ b/src/Umbraco.Core/Events/CancellableObjectEventArgs.cs @@ -1,7 +1,6 @@ using System; using System.Collections; using System.Collections.Generic; -using System.Linq; using System.Security.Permissions; using Umbraco.Core.Models; @@ -128,49 +127,4 @@ namespace Umbraco.Core.Events return !Equals(left, right); } } - - [HostProtection(SecurityAction.LinkDemand, SharedState = true)] - public class CancellableEnumerableObjectEventArgs : CancellableObjectEventArgs>, IEquatable> - { - public CancellableEnumerableObjectEventArgs(IEnumerable eventObject, bool canCancel, EventMessages messages, IDictionary additionalData) - : base(eventObject, canCancel, messages, additionalData) - { } - - public CancellableEnumerableObjectEventArgs(IEnumerable eventObject, bool canCancel, EventMessages eventMessages) - : base(eventObject, canCancel, eventMessages) - { } - - public CancellableEnumerableObjectEventArgs(IEnumerable eventObject, EventMessages eventMessages) - : base(eventObject, eventMessages) - { } - - public CancellableEnumerableObjectEventArgs(IEnumerable eventObject, bool canCancel) - : base(eventObject, canCancel) - { } - - public CancellableEnumerableObjectEventArgs(IEnumerable eventObject) - : base(eventObject) - { } - - public bool Equals(CancellableEnumerableObjectEventArgs other) - { - if (other is null) return false; - if (ReferenceEquals(this, other)) return true; - - return EventObject.SequenceEqual(other.EventObject); - } - - public override bool Equals(object obj) - { - if (obj is null) return false; - if (ReferenceEquals(this, obj)) return true; - if (obj.GetType() != this.GetType()) return false; - return Equals((CancellableEnumerableObjectEventArgs)obj); - } - - public override int GetHashCode() - { - return HashCodeHelper.GetHashCode(EventObject); - } - } } diff --git a/src/Umbraco.Core/Events/ContentPublishedEventArgs.cs b/src/Umbraco.Core/Events/ContentPublishedEventArgs.cs new file mode 100644 index 0000000000..589e447ba7 --- /dev/null +++ b/src/Umbraco.Core/Events/ContentPublishedEventArgs.cs @@ -0,0 +1,29 @@ +using System; +using System.Collections.Generic; +using Umbraco.Core.Models; + +namespace Umbraco.Core.Events +{ + public class ContentPublishedEventArgs : PublishEventArgs + { + public ContentPublishedEventArgs(IEnumerable eventObject, bool canCancel, EventMessages eventMessages) + : base(eventObject, canCancel, eventMessages) + { + } + + /// + /// Determines whether a culture has been published, during a Published event. + /// + public bool HasPublishedCulture(IContent content, string culture) + => content.WasPropertyDirty("_changedCulture_" + culture); + + /// + /// Determines whether a culture has been unpublished, during a Published event. + /// + public bool HasUnpublishedCulture(IContent content, string culture) + => content.WasPropertyDirty("_unpublishedCulture_" + culture); + + + + } +} diff --git a/src/Umbraco.Core/Events/ContentPublishingEventArgs.cs b/src/Umbraco.Core/Events/ContentPublishingEventArgs.cs new file mode 100644 index 0000000000..35cbf63441 --- /dev/null +++ b/src/Umbraco.Core/Events/ContentPublishingEventArgs.cs @@ -0,0 +1,33 @@ +using System; +using System.Linq; +using System.Collections.Generic; +using Umbraco.Core.Models; + +namespace Umbraco.Core.Events +{ + public class ContentPublishingEventArgs : PublishEventArgs + { + /// + /// Creates a new + /// + /// + /// + public ContentPublishingEventArgs(IEnumerable eventObject, EventMessages eventMessages) + : base(eventObject, eventMessages) + { + } + + /// + /// Determines whether a culture is being published, during a Publishing event. + /// + public bool IsPublishingCulture(IContent content, string culture) + => content.PublishCultureInfos.TryGetValue(culture, out var cultureInfo) && cultureInfo.IsDirty(); + + /// + /// Determines whether a culture is being unpublished, during a Publishing event. + /// + public bool IsUnpublishingCulture(IContent content, string culture) + => content.IsPropertyDirty("_unpublishedCulture_" + culture); //bit of a hack since we know that the content implementation tracks changes this way + + } +} diff --git a/src/Umbraco.Core/Events/ContentSavedEventArgs.cs b/src/Umbraco.Core/Events/ContentSavedEventArgs.cs new file mode 100644 index 0000000000..4d4085b064 --- /dev/null +++ b/src/Umbraco.Core/Events/ContentSavedEventArgs.cs @@ -0,0 +1,30 @@ +using System; +using System.Collections.Generic; +using Umbraco.Core.Models; + +namespace Umbraco.Core.Events +{ + public class ContentSavedEventArgs : SaveEventArgs + { + #region Constructors + + /// + /// Creates a new + /// + /// + /// + /// + public ContentSavedEventArgs(IEnumerable eventObject, EventMessages messages, IDictionary additionalData) + : base(eventObject, false, messages, additionalData) + { + } + + #endregion + + /// + /// Determines whether a culture has been saved, during a Saved event. + /// + public bool HasSavedCulture(IContent content, string culture) + => content.WasPropertyDirty("_updatedCulture_" + culture); + } +} diff --git a/src/Umbraco.Core/Events/ContentSavingEventArgs.cs b/src/Umbraco.Core/Events/ContentSavingEventArgs.cs new file mode 100644 index 0000000000..aa62f64349 --- /dev/null +++ b/src/Umbraco.Core/Events/ContentSavingEventArgs.cs @@ -0,0 +1,66 @@ +using System.Linq; +using System.Collections.Generic; +using Umbraco.Core.Models; + +namespace Umbraco.Core.Events +{ + public class ContentSavingEventArgs : SaveEventArgs + { + #region Factory Methods + /// + /// Converts to while preserving all args state + /// + /// + public ContentSavedEventArgs ToContentSavedEventArgs() + { + return new ContentSavedEventArgs(EventObject, Messages, AdditionalData) + { + EventState = EventState + }; + } + + /// + /// Converts to while preserving all args state + /// + /// + public ContentPublishedEventArgs ToContentPublishedEventArgs() + { + return new ContentPublishedEventArgs(EventObject, false, Messages) + { + EventState = EventState, + AdditionalData = AdditionalData + }; + } + + /// + /// Converts to while preserving all args state + /// + /// + public ContentPublishingEventArgs ToContentPublishingEventArgs() + { + return new ContentPublishingEventArgs(EventObject, Messages) + { + EventState = EventState, + AdditionalData = AdditionalData + }; + } + #endregion + + #region Constructors + + public ContentSavingEventArgs(IEnumerable eventObject, EventMessages eventMessages) : base(eventObject, eventMessages) + { + } + + public ContentSavingEventArgs(IContent eventObject, EventMessages eventMessages) : base(eventObject, eventMessages) + { + } + + #endregion + + /// + /// Determines whether a culture is being saved, during a Saving event. + /// + public bool IsSavingCulture(IContent content, string culture) => content.CultureInfos.TryGetValue(culture, out var cultureInfo) && cultureInfo.IsDirty(); + } +} diff --git a/src/Umbraco.Core/Events/PublishEventArgs.cs b/src/Umbraco.Core/Events/PublishEventArgs.cs index 599bae1639..4be655873e 100644 --- a/src/Umbraco.Core/Events/PublishEventArgs.cs +++ b/src/Umbraco.Core/Events/PublishEventArgs.cs @@ -10,12 +10,10 @@ namespace Umbraco.Core.Events /// /// /// - /// /// - public PublishEventArgs(IEnumerable eventObject, bool canCancel, bool isAllPublished, EventMessages eventMessages) + public PublishEventArgs(IEnumerable eventObject, bool canCancel, EventMessages eventMessages) : base(eventObject, canCancel, eventMessages) { - IsAllRepublished = isAllPublished; } /// @@ -43,12 +41,10 @@ namespace Umbraco.Core.Events /// /// /// - /// /// - public PublishEventArgs(TEntity eventObject, bool canCancel, bool isAllPublished, EventMessages eventMessages) + public PublishEventArgs(TEntity eventObject, bool canCancel, EventMessages eventMessages) : base(new List { eventObject }, canCancel, eventMessages) { - IsAllRepublished = isAllPublished; } /// @@ -60,7 +56,6 @@ namespace Umbraco.Core.Events public PublishEventArgs(IEnumerable eventObject, bool canCancel, bool isAllPublished) : base(eventObject, canCancel) { - IsAllRepublished = isAllPublished; } /// @@ -90,24 +85,18 @@ namespace Umbraco.Core.Events public PublishEventArgs(TEntity eventObject, bool canCancel, bool isAllPublished) : base(new List { eventObject }, canCancel) { - IsAllRepublished = isAllPublished; } /// /// Returns all entities that were published during the operation /// - public IEnumerable PublishedEntities - { - get { return EventObject; } - } - - public bool IsAllRepublished { get; private set; } + public IEnumerable PublishedEntities => EventObject; public bool Equals(PublishEventArgs other) { if (ReferenceEquals(null, other)) return false; if (ReferenceEquals(this, other)) return true; - return base.Equals(other) && IsAllRepublished == other.IsAllRepublished; + return base.Equals(other); } public override bool Equals(object obj) @@ -122,7 +111,7 @@ namespace Umbraco.Core.Events { unchecked { - return (base.GetHashCode() * 397) ^ IsAllRepublished.GetHashCode(); + return (base.GetHashCode() * 397); } } diff --git a/src/Umbraco.Core/Events/SaveEventArgs.cs b/src/Umbraco.Core/Events/SaveEventArgs.cs index 8c3fe82e2f..0b061c2227 100644 --- a/src/Umbraco.Core/Events/SaveEventArgs.cs +++ b/src/Umbraco.Core/Events/SaveEventArgs.cs @@ -1,5 +1,4 @@ using System.Collections.Generic; -using System.Linq; namespace Umbraco.Core.Events { @@ -113,9 +112,6 @@ namespace Umbraco.Core.Events /// /// Returns all entities that were saved during the operation /// - public IEnumerable SavedEntities - { - get { return EventObject; } - } + public IEnumerable SavedEntities => EventObject; } } diff --git a/src/Umbraco.Core/Models/Content.cs b/src/Umbraco.Core/Models/Content.cs index 928a109b7a..90c994c866 100644 --- a/src/Umbraco.Core/Models/Content.cs +++ b/src/Umbraco.Core/Models/Content.cs @@ -19,7 +19,14 @@ namespace Umbraco.Core.Models private bool _published; private PublishedState _publishedState; private HashSet _editedCultures; - private ContentCultureInfosCollection _publishInfos, _publishInfos1, _publishInfos2; + private ContentCultureInfosCollection _publishInfos; + + #region Used for change tracking + + private (HashSet addedCultures, HashSet removedCultures, HashSet updatedCultures) _currentPublishCultureChanges; + private (HashSet addedCultures, HashSet removedCultures, HashSet updatedCultures) _previousPublishCultureChanges; + + #endregion /// /// Constructor for creating a Content object @@ -92,7 +99,7 @@ namespace Umbraco.Core.Models } set { - if(_schedule != null) + if (_schedule != null) _schedule.CollectionChanged -= ScheduleCollectionChanged; SetPropertyValueAndDetectChanges(value, ref _schedule, nameof(ContentSchedule)); if (_schedule != null) @@ -204,7 +211,7 @@ namespace Umbraco.Core.Models // just check _publishInfos // a non-available culture could not become published anyways => _publishInfos != null && _publishInfos.ContainsKey(culture); - + /// public bool IsCultureEdited(string culture) => IsCultureAvailable(culture) && // is available, and @@ -246,26 +253,8 @@ namespace Umbraco.Core.Models if (culture.IsNullOrWhiteSpace()) return PublishDate; if (!ContentTypeBase.VariesByCulture()) return null; if (_publishInfos == null) return null; - return _publishInfos.TryGetValue(culture, out var infos) ? infos.Date : (DateTime?) null; + return _publishInfos.TryGetValue(culture, out var infos) ? infos.Date : (DateTime?)null; } - - // 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); /// /// Handles culture infos collection changes. @@ -273,6 +262,40 @@ namespace Umbraco.Core.Models private void PublishNamesCollectionChanged(object sender, NotifyCollectionChangedEventArgs e) { OnPropertyChanged(nameof(PublishCultureInfos)); + + //we don't need to handle other actions, only add/remove, however we could implement Replace and track updated cultures in _updatedCultures too + //which would allows us to continue doing WasCulturePublished, but don't think we need it anymore + switch (e.Action) + { + case NotifyCollectionChangedAction.Add: + { + var cultureInfo = e.NewItems.Cast().First(); + if (_currentPublishCultureChanges.addedCultures == null) _currentPublishCultureChanges.addedCultures = new HashSet(StringComparer.InvariantCultureIgnoreCase); + if (_currentPublishCultureChanges.updatedCultures == null) _currentPublishCultureChanges.updatedCultures = new HashSet(StringComparer.InvariantCultureIgnoreCase); + _currentPublishCultureChanges.addedCultures.Add(cultureInfo.Culture); + _currentPublishCultureChanges.updatedCultures.Add(cultureInfo.Culture); + _currentPublishCultureChanges.removedCultures?.Remove(cultureInfo.Culture); + break; + } + case NotifyCollectionChangedAction.Remove: + { + //remove listening for changes + var cultureInfo = e.OldItems.Cast().First(); + if (_currentPublishCultureChanges.removedCultures == null) _currentPublishCultureChanges.removedCultures = new HashSet(StringComparer.InvariantCultureIgnoreCase); + _currentPublishCultureChanges.removedCultures.Add(cultureInfo.Culture); + _currentPublishCultureChanges.updatedCultures?.Remove(cultureInfo.Culture); + _currentPublishCultureChanges.addedCultures?.Remove(cultureInfo.Culture); + break; + } + case NotifyCollectionChangedAction.Replace: + { + //replace occurs when an Update occurs + var cultureInfo = e.NewItems.Cast().First(); + if (_currentPublishCultureChanges.updatedCultures == null) _currentPublishCultureChanges.updatedCultures = new HashSet(StringComparer.InvariantCultureIgnoreCase); + _currentPublishCultureChanges.updatedCultures.Add(cultureInfo.Culture); + break; + } + } } [IgnoreDataMember] @@ -305,7 +328,7 @@ namespace Umbraco.Core.Models /// Boolean indicating whether to clear PropertyTypes upon change internal void ChangeContentType(IContentType contentType, bool clearProperties) { - if(clearProperties) + if (clearProperties) { ContentTypeId = contentType.Id; ContentType = new SimpleContentType(contentType); @@ -320,21 +343,92 @@ namespace Umbraco.Core.Models ChangeContentType(contentType); } + public override void ResetWereDirtyProperties() + { + base.ResetWereDirtyProperties(); + _previousPublishCultureChanges.updatedCultures = null; + _previousPublishCultureChanges.removedCultures = null; + _previousPublishCultureChanges.addedCultures = null; + } + public override void ResetDirtyProperties(bool rememberDirty) { base.ResetDirtyProperties(rememberDirty); + if (rememberDirty) + { + + _previousPublishCultureChanges.addedCultures = _currentPublishCultureChanges.addedCultures == null ? null : new HashSet(_currentPublishCultureChanges.addedCultures, StringComparer.InvariantCultureIgnoreCase); + _previousPublishCultureChanges.removedCultures = _currentPublishCultureChanges.removedCultures == null ? null : new HashSet(_currentPublishCultureChanges.removedCultures, StringComparer.InvariantCultureIgnoreCase); + _previousPublishCultureChanges.updatedCultures = _currentPublishCultureChanges.updatedCultures == null ? null : new HashSet(_currentPublishCultureChanges.updatedCultures, StringComparer.InvariantCultureIgnoreCase); + } + else + { + _previousPublishCultureChanges.addedCultures = null; + _previousPublishCultureChanges.removedCultures = null; + _previousPublishCultureChanges.updatedCultures = null; + } + _currentPublishCultureChanges.addedCultures?.Clear(); + _currentPublishCultureChanges.removedCultures?.Clear(); + _currentPublishCultureChanges.updatedCultures?.Clear(); + // take care of the published state _publishedState = _published ? PublishedState.Published : PublishedState.Unpublished; - _publishInfos2 = _publishInfos1; - if (_publishInfos == null) return; foreach (var infos in _publishInfos) infos.ResetDirtyProperties(rememberDirty); } + /// + /// Overridden to check special keys. + public override bool IsPropertyDirty(string propertyName) + { + //Special check here since we want to check if the request is for changed cultures + if (propertyName.StartsWith("_publishedCulture_")) + { + var culture = propertyName.TrimStart("_publishedCulture_"); + return _currentPublishCultureChanges.addedCultures?.Contains(culture) ?? false; + } + if (propertyName.StartsWith("_unpublishedCulture_")) + { + var culture = propertyName.TrimStart("_unpublishedCulture_"); + return _currentPublishCultureChanges.removedCultures?.Contains(culture) ?? false; + } + if (propertyName.StartsWith("_changedCulture_")) + { + var culture = propertyName.TrimStart("_changedCulture_"); + return _currentPublishCultureChanges.updatedCultures?.Contains(culture) ?? false; + } + + return base.IsPropertyDirty(propertyName); + } + + /// + /// Overridden to check special keys. + public override bool WasPropertyDirty(string propertyName) + { + //Special check here since we want to check if the request is for changed cultures + if (propertyName.StartsWith("_publishedCulture_")) + { + var culture = propertyName.TrimStart("_publishedCulture_"); + return _previousPublishCultureChanges.addedCultures?.Contains(culture) ?? false; + } + if (propertyName.StartsWith("_unpublishedCulture_")) + { + var culture = propertyName.TrimStart("_unpublishedCulture_"); + return _previousPublishCultureChanges.removedCultures?.Contains(culture) ?? false; + } + if (propertyName.StartsWith("_changedCulture_")) + { + var culture = propertyName.TrimStart("_changedCulture_"); + return _previousPublishCultureChanges.updatedCultures?.Contains(culture) ?? false; + } + + return base.WasPropertyDirty(propertyName); + } + /// /// Creates a deep clone of the current entity with its identity and it's property identities reset /// @@ -365,7 +459,7 @@ namespace Umbraco.Core.Models if (clonedContent._publishInfos != null) { clonedContent._publishInfos.CollectionChanged -= PublishNamesCollectionChanged; //clear this event handler if any - clonedContent._publishInfos = (ContentCultureInfosCollection) _publishInfos.DeepClone(); //manually deep clone + clonedContent._publishInfos = (ContentCultureInfosCollection)_publishInfos.DeepClone(); //manually deep clone clonedContent._publishInfos.CollectionChanged += clonedContent.PublishNamesCollectionChanged; //re-assign correct event handler } diff --git a/src/Umbraco.Core/Models/ContentBase.cs b/src/Umbraco.Core/Models/ContentBase.cs index 620efbb4b8..521e0ebff8 100644 --- a/src/Umbraco.Core/Models/ContentBase.cs +++ b/src/Umbraco.Core/Models/ContentBase.cs @@ -22,7 +22,14 @@ namespace Umbraco.Core.Models protected IContentTypeComposition ContentTypeBase; private int _writerId; private PropertyCollection _properties; - private ContentCultureInfosCollection _cultureInfos, _cultureInfos1, _cultureInfos2; + private ContentCultureInfosCollection _cultureInfos; + + #region Used for change tracking + + private (HashSet addedCultures, HashSet removedCultures, HashSet updatedCultures) _currentCultureChanges; + private (HashSet addedCultures, HashSet removedCultures, HashSet updatedCultures) _previousCultureChanges; + + #endregion /// /// Initializes a new instance of the class. @@ -64,6 +71,8 @@ namespace Umbraco.Core.Models OnPropertyChanged(nameof(Properties)); } + + /// /// Id of the user who wrote/updated this entity /// @@ -180,7 +189,7 @@ namespace Umbraco.Core.Models if (culture.IsNullOrWhiteSpace()) return null; if (!ContentTypeBase.VariesByCulture()) return null; if (_cultureInfos == null) return null; - return _cultureInfos.TryGetValue(culture, out var infos) ? infos.Date : (DateTime?) null; + return _cultureInfos.TryGetValue(culture, out var infos) ? infos.Date : (DateTime?)null; } /// @@ -210,12 +219,6 @@ namespace Umbraco.Core.Models } } - private void ClearCultureInfos() - { - _cultureInfos?.Clear(); - _cultureInfos = null; - } - private void ClearCultureInfo(string culture) { if (culture.IsNullOrWhiteSpace()) @@ -233,6 +236,38 @@ namespace Umbraco.Core.Models private void CultureInfosCollectionChanged(object sender, NotifyCollectionChangedEventArgs e) { OnPropertyChanged(nameof(CultureInfos)); + + switch (e.Action) + { + case NotifyCollectionChangedAction.Add: + { + var cultureInfo = e.NewItems.Cast().First(); + if (_currentCultureChanges.addedCultures == null) _currentCultureChanges.addedCultures = new HashSet(StringComparer.InvariantCultureIgnoreCase); + if (_currentCultureChanges.updatedCultures == null) _currentCultureChanges.updatedCultures = new HashSet(StringComparer.InvariantCultureIgnoreCase); + _currentCultureChanges.addedCultures.Add(cultureInfo.Culture); + _currentCultureChanges.updatedCultures.Add(cultureInfo.Culture); + _currentCultureChanges.removedCultures?.Remove(cultureInfo.Culture); + break; + } + case NotifyCollectionChangedAction.Remove: + { + //remove listening for changes + var cultureInfo = e.OldItems.Cast().First(); + if (_currentCultureChanges.removedCultures == null) _currentCultureChanges.removedCultures = new HashSet(StringComparer.InvariantCultureIgnoreCase); + _currentCultureChanges.removedCultures.Add(cultureInfo.Culture); + _currentCultureChanges.updatedCultures?.Remove(cultureInfo.Culture); + _currentCultureChanges.addedCultures?.Remove(cultureInfo.Culture); + break; + } + case NotifyCollectionChangedAction.Replace: + { + //replace occurs when an Update occurs + var cultureInfo = e.NewItems.Cast().First(); + if (_currentCultureChanges.updatedCultures == null) _currentCultureChanges.updatedCultures = new HashSet(StringComparer.InvariantCultureIgnoreCase); + _currentCultureChanges.updatedCultures.Add(cultureInfo.Culture); + break; + } + } } #endregion @@ -281,124 +316,42 @@ namespace Umbraco.Core.Models #endregion - #region Copy - - /// - public void CopyFrom(IContent other, string culture = "*") - { - if (other.ContentTypeId != ContentTypeId) - throw new InvalidOperationException("Cannot copy values from a different content type."); - - culture = culture?.ToLowerInvariant().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 (!ContentTypeBase.SupportsPropertyVariation(culture, "*", true)) - throw new NotSupportedException($"Culture \"{culture}\" is not supported by content type \"{ContentTypeBase.Alias}\" with variation \"{ContentTypeBase.Variations}\"."); - - // copying from the same Id and VersionPk - var copyingFromSelf = Id == other.Id && VersionId == other.VersionId; - var published = copyingFromSelf; - - // note: use property.SetValue(), don't assign pvalue.EditValue, else change tracking fails - - // clear all existing properties for the specified culture - foreach (var property in Properties) - { - // each property type may or may not support the variation - if (!property.PropertyType.SupportsVariation(culture, "*", wildcards: true)) - continue; - - foreach (var pvalue in property.Values) - if (property.PropertyType.SupportsVariation(pvalue.Culture, pvalue.Segment, wildcards: true) && - (culture == "*" || pvalue.Culture.InvariantEquals(culture))) - { - property.SetValue(null, pvalue.Culture, pvalue.Segment); - } - } - - // copy properties from 'other' - var otherProperties = other.Properties; - foreach (var otherProperty in otherProperties) - { - if (!otherProperty.PropertyType.SupportsVariation(culture, "*", wildcards: true)) - continue; - - var alias = otherProperty.PropertyType.Alias; - foreach (var pvalue in otherProperty.Values) - { - if (otherProperty.PropertyType.SupportsVariation(pvalue.Culture, pvalue.Segment, wildcards: true) && - (culture == "*" || pvalue.Culture.InvariantEquals(culture))) - { - var value = published ? pvalue.PublishedValue : pvalue.EditedValue; - SetValue(alias, value, pvalue.Culture, pvalue.Segment); - } - } - } - - // copy names, too - - if (culture == "*") - ClearCultureInfos(); - - if (culture == null || culture == "*") - Name = other.Name; - - // ReSharper disable once UseDeconstruction - foreach (var cultureInfo in other.CultureInfos) - { - if (culture == "*" || culture == cultureInfo.Culture) - SetCultureName(cultureInfo.Name, cultureInfo.Culture); - } - } - - #endregion - - #region Validation - - /// - public Property[] ValidateProperties(string culture = "*") - { - // select invalid properties - return Properties.Where(x => - { - // if culture is null, we validate invariant properties only - // if culture is '*' we validate both variant and invariant properties, automatically - // if culture is specific eg 'en-US' we both too, but explicitly - - var varies = x.PropertyType.VariesByCulture(); - - if (culture == null) - return !(varies || x.IsValid(null)); // validate invariant property, invariant culture - - if (culture == "*") - return !x.IsValid(culture); // validate property, all cultures - - return varies - ? !x.IsValid(culture) // validate variant property, explicit culture - : !x.IsValid(null); // validate invariant property, explicit culture - }) - .ToArray(); - } - - #endregion - #region Dirty + public override void ResetWereDirtyProperties() + { + base.ResetWereDirtyProperties(); + _previousCultureChanges.addedCultures = null; + _previousCultureChanges.removedCultures = null; + _previousCultureChanges.updatedCultures = null; + } + /// /// Overridden to include user properties. public override void ResetDirtyProperties(bool rememberDirty) { base.ResetDirtyProperties(rememberDirty); + if (rememberDirty) + { + _previousCultureChanges.addedCultures = _currentCultureChanges.addedCultures == null ? null : new HashSet(_currentCultureChanges.addedCultures, StringComparer.InvariantCultureIgnoreCase); + _previousCultureChanges.removedCultures = _currentCultureChanges.removedCultures == null ? null : new HashSet(_currentCultureChanges.removedCultures, StringComparer.InvariantCultureIgnoreCase); + _previousCultureChanges.updatedCultures = _currentCultureChanges.updatedCultures == null ? null : new HashSet(_currentCultureChanges.updatedCultures, StringComparer.InvariantCultureIgnoreCase); + } + else + { + _previousCultureChanges.addedCultures = null; + _previousCultureChanges.removedCultures = null; + _previousCultureChanges.updatedCultures = null; + } + _currentCultureChanges.addedCultures?.Clear(); + _currentCultureChanges.removedCultures?.Clear(); + _currentCultureChanges.updatedCultures?.Clear(); + // also reset dirty changes made to user's properties 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; @@ -443,6 +396,23 @@ namespace Umbraco.Core.Models if (base.IsPropertyDirty(propertyName)) return true; + //Special check here since we want to check if the request is for changed cultures + if (propertyName.StartsWith("_addedCulture_")) + { + var culture = propertyName.TrimStart("_addedCulture_"); + return _currentCultureChanges.addedCultures?.Contains(culture) ?? false; + } + if (propertyName.StartsWith("_removedCulture_")) + { + var culture = propertyName.TrimStart("_removedCulture_"); + return _currentCultureChanges.removedCultures?.Contains(culture) ?? false; + } + if (propertyName.StartsWith("_updatedCulture_")) + { + var culture = propertyName.TrimStart("_updatedCulture_"); + return _currentCultureChanges.updatedCultures?.Contains(culture) ?? false; + } + return Properties.Contains(propertyName) && Properties[propertyName].IsDirty(); } @@ -453,6 +423,23 @@ namespace Umbraco.Core.Models if (base.WasPropertyDirty(propertyName)) return true; + //Special check here since we want to check if the request is for changed cultures + if (propertyName.StartsWith("_addedCulture_")) + { + var culture = propertyName.TrimStart("_addedCulture_"); + return _previousCultureChanges.addedCultures?.Contains(culture) ?? false; + } + if (propertyName.StartsWith("_removedCulture_")) + { + var culture = propertyName.TrimStart("_removedCulture_"); + return _previousCultureChanges.removedCultures?.Contains(culture) ?? false; + } + if (propertyName.StartsWith("_updatedCulture_")) + { + var culture = propertyName.TrimStart("_updatedCulture_"); + return _previousCultureChanges.updatedCultures?.Contains(culture) ?? false; + } + return Properties.Contains(propertyName) && Properties[propertyName].WasDirty(); } @@ -474,12 +461,6 @@ 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 /// @@ -496,7 +477,7 @@ namespace Umbraco.Core.Models if (clonedContent._cultureInfos != null) { clonedContent._cultureInfos.CollectionChanged -= CultureInfosCollectionChanged; //clear this event handler if any - clonedContent._cultureInfos = (ContentCultureInfosCollection) _cultureInfos.DeepClone(); //manually deep clone + clonedContent._cultureInfos = (ContentCultureInfosCollection)_cultureInfos.DeepClone(); //manually deep clone clonedContent._cultureInfos.CollectionChanged += clonedContent.CultureInfosCollectionChanged; //re-assign correct event handler } @@ -504,7 +485,7 @@ namespace Umbraco.Core.Models if (clonedContent._properties != null) { clonedContent._properties.CollectionChanged -= PropertiesChanged; //clear this event handler if any - clonedContent._properties = (PropertyCollection) _properties.DeepClone(); //manually deep clone + clonedContent._properties = (PropertyCollection)_properties.DeepClone(); //manually deep clone clonedContent._properties.CollectionChanged += clonedContent.PropertiesChanged; //re-assign correct event handler } } diff --git a/src/Umbraco.Core/Models/ContentCultureInfosCollectionExtensions.cs b/src/Umbraco.Core/Models/ContentCultureInfosCollectionExtensions.cs deleted file mode 100644 index 98a0b48d07..0000000000 --- a/src/Umbraco.Core/Models/ContentCultureInfosCollectionExtensions.cs +++ /dev/null @@ -1,12 +0,0 @@ -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/ContentRepositoryExtensions.cs b/src/Umbraco.Core/Models/ContentRepositoryExtensions.cs index 79c3dc3f63..31e040f2d4 100644 --- a/src/Umbraco.Core/Models/ContentRepositoryExtensions.cs +++ b/src/Umbraco.Core/Models/ContentRepositoryExtensions.cs @@ -10,6 +10,127 @@ namespace Umbraco.Core.Models /// internal static class ContentRepositoryExtensions { + /// + /// Gets the cultures that have been flagged for unpublishing. + /// + /// Gets cultures for which content.UnpublishCulture() has been invoked. + public static IReadOnlyList GetCulturesUnpublishing(this IContent content) + { + if (!content.Published || !content.ContentType.VariesByCulture() || !content.IsPropertyDirty("PublishCultureInfos")) + return Array.Empty(); + + var culturesUnpublishing = content.CultureInfos.Values + .Where(x => content.IsPropertyDirty("_unpublishedCulture_" + x.Culture)) + .Select(x => x.Culture); + + return culturesUnpublishing.ToList(); + } + + /// + /// Copies values from another document. + /// + public static void CopyFrom(this IContent content, IContent other, string culture = "*") + { + if (other.ContentTypeId != content.ContentTypeId) + throw new InvalidOperationException("Cannot copy values from a different content type."); + + culture = culture?.ToLowerInvariant().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}\"."); + + // copying from the same Id and VersionPk + var copyingFromSelf = content.Id == other.Id && content.VersionId == other.VersionId; + var published = copyingFromSelf; + + // note: use property.SetValue(), don't assign pvalue.EditValue, else change tracking fails + + // clear all existing properties for the specified culture + foreach (var property in content.Properties) + { + // each property type may or may not support the variation + if (!property.PropertyType.SupportsVariation(culture, "*", wildcards: true)) + continue; + + foreach (var pvalue in property.Values) + if (property.PropertyType.SupportsVariation(pvalue.Culture, pvalue.Segment, wildcards: true) && + (culture == "*" || pvalue.Culture.InvariantEquals(culture))) + { + property.SetValue(null, pvalue.Culture, pvalue.Segment); + } + } + + // copy properties from 'other' + var otherProperties = other.Properties; + foreach (var otherProperty in otherProperties) + { + if (!otherProperty.PropertyType.SupportsVariation(culture, "*", wildcards: true)) + continue; + + var alias = otherProperty.PropertyType.Alias; + foreach (var pvalue in otherProperty.Values) + { + if (otherProperty.PropertyType.SupportsVariation(pvalue.Culture, pvalue.Segment, wildcards: true) && + (culture == "*" || pvalue.Culture.InvariantEquals(culture))) + { + var value = published ? pvalue.PublishedValue : pvalue.EditedValue; + content.SetValue(alias, value, pvalue.Culture, pvalue.Segment); + } + } + } + + // copy names, too + + if (culture == "*") + { + content.CultureInfos.Clear(); + content.CultureInfos = null; + } + + + if (culture == null || culture == "*") + content.Name = other.Name; + + // ReSharper disable once UseDeconstruction + foreach (var cultureInfo in other.CultureInfos) + { + if (culture == "*" || culture == cultureInfo.Culture) + content.SetCultureName(cultureInfo.Name, cultureInfo.Culture); + } + } + + /// + /// Validates the content item's properties pass variant rules + /// + /// 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. + public static Property[] ValidateProperties(this IContentBase content, string culture = "*") + { + // select invalid properties + return content.Properties.Where(x => + { + // if culture is null, we validate invariant properties only + // if culture is '*' we validate both variant and invariant properties, automatically + // if culture is specific eg 'en-US' we both too, but explicitly + + var varies = x.PropertyType.VariesByCulture(); + + if (culture == null) + return !(varies || x.IsValid(null)); // validate invariant property, invariant culture + + if (culture == "*") + return !x.IsValid(culture); // validate property, all cultures + + return varies + ? !x.IsValid(culture) // validate variant property, explicit culture + : !x.IsValid(null); // validate invariant property, explicit culture + }) + .ToArray(); + } + public static void SetPublishInfo(this IContent content, string culture, string name, DateTime date) { if (string.IsNullOrWhiteSpace(name)) @@ -32,6 +153,8 @@ namespace Umbraco.Core.Models //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 + //fixme: Because this is being called, we end up updating a culture which triggers the dirty change tracking + // which ends up in error because the culture is not actually being updated which causes the tests to fail content.PublishCultureInfos.AddOrUpdate(culture, publishInfos.Name, date); if (content.CultureInfos.TryGetValue(culture, out var infos)) @@ -147,10 +270,13 @@ namespace Umbraco.Core.Models content.PublishCultureInfos.Remove(culture); // set the culture to be dirty - it's been modified - content.TouchCultureInfo(culture); + content.TouchCulture(culture); } - public static void TouchCultureInfo(this IContent content, string culture) + /// + /// Updates a culture date, if the culture exists. + /// + public static void TouchCulture(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/Entities/BeingDirtyBase.cs b/src/Umbraco.Core/Models/Entities/BeingDirtyBase.cs index a598ba1b3d..a1f4bad9a1 100644 --- a/src/Umbraco.Core/Models/Entities/BeingDirtyBase.cs +++ b/src/Umbraco.Core/Models/Entities/BeingDirtyBase.cs @@ -65,7 +65,7 @@ namespace Umbraco.Core.Models.Entities } /// - public void ResetWereDirtyProperties() + public virtual void ResetWereDirtyProperties() { // note: cannot .Clear() because when memberwise-cloning this will be the SAME // instance as the one on the clone, so we need to create a new instance. diff --git a/src/Umbraco.Core/Models/IContent.cs b/src/Umbraco.Core/Models/IContent.cs index bd72029c3f..e953bef1eb 100644 --- a/src/Umbraco.Core/Models/IContent.cs +++ b/src/Umbraco.Core/Models/IContent.cs @@ -135,29 +135,5 @@ namespace Umbraco.Core.Models /// IContent DeepCloneWithResetIdentities(); - - /// - /// 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 e6cda5de7b..1ebbfa1d49 100644 --- a/src/Umbraco.Core/Models/IContentBase.cs +++ b/src/Umbraco.Core/Models/IContentBase.cs @@ -132,33 +132,5 @@ namespace Umbraco.Core.Models /// Values 'null' and 'empty' are equivalent for culture and segment. void SetValue(string propertyTypeAlias, object value, string culture = null, string segment = null); - /// - /// Copies values from another document. - /// - void CopyFrom(IContent other, string culture = "*"); - - /// - /// Validates the content item's properties pass variant rules - /// - /// 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/Models/Macro.cs b/src/Umbraco.Core/Models/Macro.cs index bc71d3dc2b..4f9e79f482 100644 --- a/src/Umbraco.Core/Models/Macro.cs +++ b/src/Umbraco.Core/Models/Macro.cs @@ -142,13 +142,15 @@ namespace Umbraco.Core.Models } public override void ResetDirtyProperties(bool rememberDirty) - { + { + base.ResetDirtyProperties(rememberDirty); + _addedProperties.Clear(); _removedProperties.Clear(); - base.ResetDirtyProperties(rememberDirty); + foreach (var prop in Properties) { - ((BeingDirtyBase)prop).ResetDirtyProperties(rememberDirty); + prop.ResetDirtyProperties(rememberDirty); } } diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs index c02a26ef7b..a4acb824b7 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs @@ -1197,7 +1197,6 @@ namespace Umbraco.Core.Persistence.Repositories.Implement { foreach (var v in contentVariation) content.SetPublishInfo(v.Culture, v.Name, v.Date); - content.AknPublishInfo(); } if (documentVariations.TryGetValue(content.Id, out var documentVariation)) content.SetCultureEdited(documentVariation.Where(x => x.Edited).Select(x => x.Culture)); diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index cbeada6411..ce7417722d 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -307,15 +307,16 @@ namespace Umbraco.Core.Services.Implement if (withIdentity) { + var evtMsgs = EventMessagesFactory.Get(); + // if saving is cancelled, content remains without an identity - var saveEventArgs = new SaveEventArgs(content); + var saveEventArgs = new ContentSavingEventArgs(content, evtMsgs); if (scope.Events.DispatchCancelable(Saving, this, saveEventArgs, "Saving")) return; _documentRepository.Save(content); - saveEventArgs.CanCancel = false; - scope.Events.Dispatch(Saved, this, saveEventArgs, "Saved"); + scope.Events.Dispatch(Saved, this, saveEventArgs.ToContentSavedEventArgs(), "Saved"); scope.Events.Dispatch(TreeChanged, this, new TreeChange(content, TreeChangeTypes.RefreshNode).ToEventArgs()); } @@ -758,7 +759,7 @@ namespace Umbraco.Core.Services.Implement using (var scope = ScopeProvider.CreateScope()) { - var saveEventArgs = new SaveEventArgs(content, evtMsgs); + var saveEventArgs = new ContentSavingEventArgs(content, evtMsgs); if (raiseEvents && scope.Events.DispatchCancelable(Saving, this, saveEventArgs, "Saving")) { scope.Complete(); @@ -784,8 +785,7 @@ namespace Umbraco.Core.Services.Implement if (raiseEvents) { - saveEventArgs.CanCancel = false; - scope.Events.Dispatch(Saved, this, saveEventArgs, "Saved"); + scope.Events.Dispatch(Saved, this, saveEventArgs.ToContentSavedEventArgs(), "Saved"); } var changeType = TreeChangeTypes.RefreshNode; scope.Events.Dispatch(TreeChanged, this, new TreeChange(content, changeType).ToEventArgs()); @@ -814,7 +814,7 @@ namespace Umbraco.Core.Services.Implement using (var scope = ScopeProvider.CreateScope()) { - var saveEventArgs = new SaveEventArgs(contentsA, evtMsgs); + var saveEventArgs = new ContentSavingEventArgs(contentsA, evtMsgs); if (raiseEvents && scope.Events.DispatchCancelable(Saving, this, saveEventArgs, "Saving")) { scope.Complete(); @@ -835,8 +835,7 @@ namespace Umbraco.Core.Services.Implement if (raiseEvents) { - saveEventArgs.CanCancel = false; - scope.Events.Dispatch(Saved, this, saveEventArgs, "Saved"); + scope.Events.Dispatch(Saved, this, saveEventArgs.ToContentSavedEventArgs(), "Saved"); } scope.Events.Dispatch(TreeChanged, this, treeChanges.ToEventArgs()); Audit(AuditType.Save, userId == -1 ? 0 : userId, Constants.System.Root, "Saved multiple content"); @@ -948,8 +947,7 @@ 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; + content.PublishedState = PublishedState.Unpublishing; } else { @@ -1001,7 +999,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; //TODO: fix this https://github.com/umbraco/Umbraco-CMS/issues/4234 + content.PublishedState = PublishedState.Publishing; // state here is either Publishing or Unpublishing // (even though, Publishing to unpublish a culture may end up unpublishing everything) @@ -1022,26 +1020,23 @@ namespace Umbraco.Core.Services.Implement var previouslyPublished = content.HasIdentity && content.Published; // always save - var saveEventArgs = new SaveEventArgs(content, evtMsgs); + var saveEventArgs = new ContentSavingEventArgs(content, evtMsgs); if (raiseEvents && scope.Events.DispatchCancelable(Saving, this, saveEventArgs, "Saving")) return new PublishResult(PublishResultType.FailedPublishCancelledByEvent, evtMsgs, content); if (publishing) { - //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); + culturesUnpublishing = content.GetCulturesUnpublishing(); culturesPublishing = variesByCulture ? 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 - publishResult = StrategyCanPublish(scope, content, userId, /*checkPath:*/ (!branchOne || branchRoot), culturesPublishing, culturesUnpublishing, evtMsgs); + publishResult = StrategyCanPublish(scope, content, /*checkPath:*/ (!branchOne || branchRoot), culturesPublishing, culturesUnpublishing, evtMsgs, saveEventArgs); if (publishResult.Success) { // note: StrategyPublish flips the PublishedState to Publishing! - publishResult = StrategyPublish(scope, content, userId, culturesPublishing, culturesUnpublishing, evtMsgs); + publishResult = StrategyPublish(content, culturesPublishing, culturesUnpublishing, evtMsgs); } else { @@ -1061,7 +1056,9 @@ namespace Umbraco.Core.Services.Implement // 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; + //TODO: why? this seems odd, were just setting the exact same value that it already has + // instead do we want to just set the PublishState? + content.Published = content.Published; } } @@ -1077,14 +1074,16 @@ namespace Umbraco.Core.Services.Implement // 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); + unpublishResult = StrategyCanUnpublish(scope, content, evtMsgs); if (unpublishResult.Success) unpublishResult = StrategyUnpublish(scope, content, userId, evtMsgs); else { // 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; + //TODO: why? this seems odd, were just setting the exact same value that it already has + // instead do we want to just set the PublishState? + content.Published = content.Published; } } else @@ -1107,8 +1106,7 @@ namespace Umbraco.Core.Services.Implement // raise the Saved event, always if (raiseEvents) { - saveEventArgs.CanCancel = false; - scope.Events.Dispatch(Saved, this, saveEventArgs, "Saved"); + scope.Events.Dispatch(Saved, this, saveEventArgs.ToContentSavedEventArgs(), "Saved"); } if (unpublishing) // we have tried to unpublish - won't happen in a branch @@ -1151,16 +1149,16 @@ namespace Umbraco.Core.Services.Implement if (!branchOne) // for branches, handled by SaveAndPublishBranch { scope.Events.Dispatch(TreeChanged, this, new TreeChange(content, changeType).ToEventArgs()); - scope.Events.Dispatch(Published, this, new PublishEventArgs(content, false, false), "Published"); + scope.Events.Dispatch(Published, this, saveEventArgs.ToContentPublishedEventArgs(), nameof(Published)); } - // if was not published and now is... descendants that were 'published' (but + // it was not published and now is... descendants that were 'published' (but // had an unpublished ancestor) are 're-published' ie not explicitly published // but back as 'published' nevertheless 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"); + scope.Events.Dispatch(Published, this, new ContentPublishedEventArgs(descendants, false, evtMsgs), "Published"); } switch (publishResult.Result) @@ -1456,7 +1454,7 @@ namespace Umbraco.Core.Services.Implement if (!document.HasIdentity) throw new InvalidOperationException("Cannot not branch-publish a new document."); - var publishedState = ((Content)document).PublishedState; + var publishedState = document.PublishedState; if (publishedState == PublishedState.Publishing) throw new InvalidOperationException("Cannot mix PublishCulture and SaveAndPublishBranch."); @@ -1510,7 +1508,9 @@ namespace Umbraco.Core.Services.Implement // trigger events for the entire branch scope.Events.Dispatch(TreeChanged, this, new TreeChange(document, TreeChangeTypes.RefreshBranch).ToEventArgs()); - scope.Events.Dispatch(Published, this, new PublishEventArgs(publishedDocuments, false, false), "Published"); + + //fixme - in the SaveAndPublishBranchOne -> CommitDocumentChangesInternal publishing/published is going to be raised there, so are we raising it 2x for the same thing? + scope.Events.Dispatch(Published, this, new ContentPublishedEventArgs(publishedDocuments, false, evtMsgs), nameof(Published)); scope.Complete(); } @@ -1772,7 +1772,7 @@ namespace Umbraco.Core.Services.Implement { // however, it had been masked when being trashed, so there's no need for // any special event here - just change its state - ((Content)content).PublishedState = PublishedState.Unpublishing; + content.PublishedState = PublishedState.Unpublishing; } PerformMoveLocked(content, parentId, parent, userId, moves, trashed); @@ -1949,7 +1949,7 @@ namespace Umbraco.Core.Services.Implement // a copy is not published (but not really unpublishing either) // update the create author and last edit author if (copy.Published) - ((Content)copy).Published = false; + copy.Published = false; copy.CreatorId = userId; copy.WriterId = userId; @@ -1993,7 +1993,7 @@ namespace Umbraco.Core.Services.Implement // a copy is not published (but not really unpublishing either) // update the create author and last edit author if (descendantCopy.Published) - ((Content)descendantCopy).Published = false; + descendantCopy.Published = false; descendantCopy.CreatorId = userId; descendantCopy.WriterId = userId; @@ -2130,7 +2130,7 @@ namespace Umbraco.Core.Services.Implement private OperationResult Sort(IScope scope, IContent[] itemsA, int userId, EventMessages evtMsgs, bool raiseEvents) { - var saveEventArgs = new SaveEventArgs(itemsA); + var saveEventArgs = new ContentSavingEventArgs(itemsA, evtMsgs); if (raiseEvents) { //raise cancelable sorting event @@ -2172,15 +2172,16 @@ namespace Umbraco.Core.Services.Implement if (raiseEvents) { + var savedEventsArgs = saveEventArgs.ToContentSavedEventArgs(); //first saved, then sorted - scope.Events.Dispatch(Saved, this, saveEventArgs, nameof(Saved)); - scope.Events.Dispatch(Sorted, this, saveEventArgs, nameof(Sorted)); + scope.Events.Dispatch(Saved, this, savedEventsArgs, nameof(Saved)); + scope.Events.Dispatch(Sorted, this, savedEventsArgs, nameof(Sorted)); } scope.Events.Dispatch(TreeChanged, this, saved.Select(x => new TreeChange(x, TreeChangeTypes.RefreshNode)).ToEventArgs()); if (raiseEvents && published.Any()) - scope.Events.Dispatch(Published, this, new PublishEventArgs(published, false, false), "Published"); + scope.Events.Dispatch(Published, this, new ContentPublishedEventArgs(published, false, evtMsgs), "Published"); Audit(AuditType.Sort, userId, 0, "Sorting content performed by user"); return OperationResult.Succeed(evtMsgs); @@ -2271,12 +2272,12 @@ namespace Umbraco.Core.Services.Implement /// /// Occurs before Save /// - public static event TypedEventHandler> Saving; + public static event TypedEventHandler Saving; /// /// Occurs after Save /// - public static event TypedEventHandler> Saved; + public static event TypedEventHandler Saved; /// /// Occurs after Create @@ -2350,12 +2351,12 @@ namespace Umbraco.Core.Services.Implement /// /// Occurs before publish /// - public static event TypedEventHandler> Publishing; + public static event TypedEventHandler Publishing; /// /// Occurs after publish /// - public static event TypedEventHandler> Published; + public static event TypedEventHandler Published; /// /// Occurs before unpublish @@ -2391,14 +2392,16 @@ namespace Umbraco.Core.Services.Implement /// /// /// - /// /// + /// /// + /// + /// /// - private PublishResult StrategyCanPublish(IScope scope, IContent content, int userId, bool checkPath, IReadOnlyList culturesPublishing, IReadOnlyList culturesUnpublishing, EventMessages evtMsgs) + private PublishResult StrategyCanPublish(IScope scope, IContent content, bool checkPath, IReadOnlyList culturesPublishing, IReadOnlyCollection culturesUnpublishing, EventMessages evtMsgs, ContentSavingEventArgs savingEventArgs) { // raise Publishing event - if (scope.Events.DispatchCancelable(Publishing, this, new PublishEventArgs(content, evtMsgs))) + if (scope.Events.DispatchCancelable(Publishing, this, savingEventArgs.ToContentPublishingEventArgs())) { Logger.Info("Document {ContentName} (id={ContentId}) cannot be published: {Reason}", content.Name, content.Id, "publishing was cancelled"); return new PublishResult(PublishResultType.FailedPublishCancelledByEvent, evtMsgs, content); @@ -2425,7 +2428,7 @@ namespace Umbraco.Core.Services.Implement // ensure that the document has published values // either because it is 'publishing' or because it already has a published version - if (((Content)content).PublishedState != PublishedState.Publishing && content.PublishedVersionId == 0) + if (content.PublishedState != PublishedState.Publishing && content.PublishedVersionId == 0) { Logger.Info("Document {ContentName} (id={ContentId}) cannot be published: {Reason}", content.Name, content.Id, "document does not have published values"); return new PublishResult(PublishResultType.FailedPublishNothingToPublish, evtMsgs, content); @@ -2481,20 +2484,20 @@ namespace Umbraco.Core.Services.Implement /// /// Publishes a document /// - /// /// - /// + /// /// + /// /// /// /// It is assumed that all publishing checks have passed before calling this method like /// - private PublishResult StrategyPublish(IScope scope, IContent content, int userId, - IReadOnlyList culturesPublishing, IReadOnlyList culturesUnpublishing, + private PublishResult StrategyPublish(IContent content, + IReadOnlyCollection culturesPublishing, IReadOnlyCollection culturesUnpublishing, EventMessages evtMsgs) { // change state to publishing - ((Content)content).PublishedState = PublishedState.Publishing; + content.PublishedState = PublishedState.Publishing; //if this is a variant then we need to log which cultures have been published/unpublished and return an appropriate result if (content.ContentType.VariesByCulture()) @@ -2529,10 +2532,9 @@ namespace Umbraco.Core.Services.Implement /// /// /// - /// /// /// - private PublishResult StrategyCanUnpublish(IScope scope, IContent content, int userId, EventMessages evtMsgs) + private PublishResult StrategyCanUnpublish(IScope scope, IContent content, EventMessages evtMsgs) { // raise Unpublishing event if (scope.Events.DispatchCancelable(Unpublishing, this, new PublishEventArgs(content, evtMsgs))) @@ -2573,7 +2575,7 @@ namespace Umbraco.Core.Services.Implement Logger.Info("Document {ContentName} (id={ContentId}) had its release date removed, because it was unpublished.", content.Name, content.Id); // change state to unpublishing - ((Content)content).PublishedState = PublishedState.Unpublishing; + content.PublishedState = PublishedState.Unpublishing; Logger.Info("Document {ContentName} (id={ContentId}) has been unpublished.", content.Name, content.Id); return attempt; @@ -2591,7 +2593,7 @@ namespace Umbraco.Core.Services.Implement /// Deletes content items of the specified type, and only that type. Does *not* handle content types /// inheritance and compositions, which need to be managed outside of this method. /// - /// Id of the + /// Id of the /// Optional Id of the user issuing the delete operation public void DeleteOfTypes(IEnumerable contentTypeIds, int userId = 0) { @@ -2710,7 +2712,7 @@ namespace Umbraco.Core.Services.Implement scope.ReadLock(Constants.Locks.ContentTree); var blueprint = _documentBlueprintRepository.Get(id); if (blueprint != null) - ((Content)blueprint).Blueprint = true; + blueprint.Blueprint = true; return blueprint; } } @@ -2722,7 +2724,7 @@ namespace Umbraco.Core.Services.Implement scope.ReadLock(Constants.Locks.ContentTree); var blueprint = _documentBlueprintRepository.Get(id); if (blueprint != null) - ((Content)blueprint).Blueprint = true; + blueprint.Blueprint = true; return blueprint; } } @@ -2733,7 +2735,7 @@ namespace Umbraco.Core.Services.Implement if (content.ParentId != -1) content.ParentId = -1; - ((Content)content).Blueprint = true; + content.Blueprint = true; using (var scope = ScopeProvider.CreateScope()) { @@ -2809,7 +2811,7 @@ namespace Umbraco.Core.Services.Implement } return _documentBlueprintRepository.Get(query).Select(x => { - ((Content)x).Blueprint = true; + x.Blueprint = true; return x; }); } @@ -2828,7 +2830,7 @@ namespace Umbraco.Core.Services.Implement var blueprints = _documentBlueprintRepository.Get(query).Select(x => { - ((Content)x).Blueprint = true; + x.Blueprint = true; return x; }).ToArray(); diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 6ddfe1afd6..84103067e9 100755 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -299,6 +299,11 @@ + + + + + @@ -389,7 +394,6 @@ - diff --git a/src/Umbraco.Tests/Services/ContentServiceEventTests.cs b/src/Umbraco.Tests/Services/ContentServiceEventTests.cs index 3576425b9c..b4fde295a7 100644 --- a/src/Umbraco.Tests/Services/ContentServiceEventTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceEventTests.cs @@ -47,35 +47,35 @@ namespace Umbraco.Tests.Services var contentService = ServiceContext.ContentService; - var document = new Content("content", -1, contentType); + IContent document = new Content("content", -1, contentType); document.SetCultureName("hello", "en-US"); document.SetCultureName("bonjour", "fr-FR"); contentService.Save(document); + //re-get - dirty properties need resetting + document = contentService.GetById(document.Id); + // 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) + void OnSaving(IContentService sender, ContentSavingEventArgs e) { var saved = e.SavedEntities.First(); Assert.AreSame(document, saved); - Assert.IsTrue(saved.IsSavingCulture("fr-FR")); - Assert.IsFalse(saved.IsSavingCulture("en-UK")); + Assert.IsTrue(e.IsSavingCulture(saved, "fr-FR")); + Assert.IsFalse(e.IsSavingCulture(saved, "en-UK")); } - void OnSaved(IContentService sender, SaveEventArgs e) + void OnSaved(IContentService sender, ContentSavedEventArgs e) { var saved = e.SavedEntities.First(); Assert.AreSame(document, saved); - Assert.IsTrue(saved.HasSavedCulture("fr-FR")); - Assert.IsFalse(saved.HasSavedCulture("en-UK")); + Assert.IsTrue(e.HasSavedCulture(saved, "fr-FR")); + Assert.IsFalse(e.HasSavedCulture(saved, "en-UK")); } ContentService.Saving += OnSaving; @@ -103,35 +103,35 @@ namespace Umbraco.Tests.Services var contentService = ServiceContext.ContentService; - var document = new Content("content", -1, contentType); + IContent 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) + //re-get - dirty properties need resetting + document = contentService.GetById(document.Id); + + void OnPublishing(IContentService sender, ContentPublishingEventArgs e) { var publishing = e.PublishedEntities.First(); Assert.AreSame(document, publishing); - Assert.IsFalse(publishing.IsPublishingCulture("en-US")); - Assert.IsTrue(publishing.IsPublishingCulture("fr-FR")); + Assert.IsFalse(e.IsPublishingCulture(publishing, "en-US")); + Assert.IsTrue(e.IsPublishingCulture(publishing, "fr-FR")); } - void OnPublished(IContentService sender, PublishEventArgs e) + void OnPublished(IContentService sender, ContentPublishedEventArgs e) { var published = e.PublishedEntities.First(); Assert.AreSame(document, published); - Assert.IsFalse(published.HasPublishedCulture("en-US")); - Assert.IsTrue(published.HasPublishedCulture("fr-FR")); + Assert.IsFalse(e.HasPublishedCulture(published, "en-US")); + Assert.IsTrue(e.HasPublishedCulture(published, "fr-FR")); } ContentService.Publishing += OnPublishing; @@ -140,11 +140,9 @@ namespace Umbraco.Tests.Services ContentService.Publishing -= OnPublishing; ContentService.Published -= OnPublished; - document = (Content) contentService.GetById(document.Id); + document = 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")); } @@ -165,58 +163,55 @@ namespace Umbraco.Tests.Services propertyType.Variations = ContentVariation.Culture; contentTypeService.Save(contentType); - var contentService = ServiceContext.ContentService; + var contentService = (ContentService)ServiceContext.ContentService; - var document = new Content("content", -1, contentType); + IContent 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) + //re-get - dirty properties need resetting + document = contentService.GetById(document.Id); + + void OnPublishing(IContentService sender, ContentPublishingEventArgs e) { var publishing = e.PublishedEntities.First(); Assert.AreSame(document, publishing); - Assert.IsFalse(publishing.IsPublishingCulture("en-US")); - Assert.IsFalse(publishing.IsPublishingCulture("fr-FR")); + Assert.IsFalse(e.IsPublishingCulture(publishing, "en-US")); + Assert.IsFalse(e.IsPublishingCulture(publishing, "fr-FR")); - Assert.IsFalse(publishing.IsUnpublishingCulture("en-US")); - Assert.IsTrue(publishing.IsUnpublishingCulture("fr-FR")); + Assert.IsFalse(e.IsUnpublishingCulture(publishing, "en-US")); + Assert.IsTrue(e.IsUnpublishingCulture(publishing, "fr-FR")); } - void OnPublished(IContentService sender, PublishEventArgs e) + void OnPublished(IContentService sender, ContentPublishedEventArgs e) { var published = e.PublishedEntities.First(); Assert.AreSame(document, published); - Assert.IsFalse(published.HasPublishedCulture("en-US")); - Assert.IsFalse(published.HasPublishedCulture("fr-FR")); + Assert.IsFalse(e.HasPublishedCulture(published, "en-US")); + Assert.IsFalse(e.HasPublishedCulture(published, "fr-FR")); - Assert.IsFalse(published.HasUnpublishedCulture("en-US")); - Assert.IsTrue(published.HasUnpublishedCulture("fr-FR")); + Assert.IsFalse(e.HasUnpublishedCulture(published, "en-US")); + Assert.IsTrue(e.HasUnpublishedCulture(published, "fr-FR")); } document.UnpublishCulture("fr-FR"); ContentService.Publishing += OnPublishing; ContentService.Published += OnPublished; - contentService.SavePublishing(document); + contentService.CommitDocumentChanges(document); ContentService.Publishing -= OnPublishing; ContentService.Published -= OnPublished; - document = (Content) contentService.GetById(document.Id); + document = 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")); }