From 113b6598edacce78f828c0bff9a8dc514caadf49 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 12 Jul 2021 09:46:56 -0600 Subject: [PATCH 01/13] removes raiseEvent params --- .../Events/IScopedNotificationPublisher.cs | 7 ++ .../Events/ScopedNotificationPublisher.cs | 59 +++++++++++- src/Umbraco.Core/Services/IContentService.cs | 16 ++-- .../Services/IContentServiceBase.cs | 4 +- src/Umbraco.Core/Services/IDataTypeService.cs | 12 +-- src/Umbraco.Core/Services/IMediaService.cs | 10 +-- .../Services/IMemberGroupService.cs | 2 +- .../Services/IMembershipMemberService.cs | 10 +-- src/Umbraco.Core/Services/IUserService.cs | 6 +- .../Services/Implement/ContentService.cs | 89 +++++++++---------- .../Services/Implement/DataTypeService.cs | 21 +---- .../Services/Implement/MediaService.cs | 30 +++---- .../Services/Implement/MemberGroupService.cs | 9 +- .../Services/Implement/MemberService.cs | 19 ++-- .../Services/Implement/UserService.cs | 37 +++----- .../Umbraco.Infrastructure.csproj | 4 - 16 files changed, 167 insertions(+), 168 deletions(-) diff --git a/src/Umbraco.Core/Events/IScopedNotificationPublisher.cs b/src/Umbraco.Core/Events/IScopedNotificationPublisher.cs index 7df9167ce6..75fbf83860 100644 --- a/src/Umbraco.Core/Events/IScopedNotificationPublisher.cs +++ b/src/Umbraco.Core/Events/IScopedNotificationPublisher.cs @@ -1,6 +1,7 @@ // Copyright (c) Umbraco. // See LICENSE for more details. +using System; using System.Threading.Tasks; using Umbraco.Cms.Core.Notifications; @@ -8,6 +9,12 @@ namespace Umbraco.Cms.Core.Events { public interface IScopedNotificationPublisher { + /// + /// Supresses all notifications from being added/created until the result object is disposed. + /// + /// + IDisposable Supress(); + /// /// Publishes a cancelable notification to the notification subscribers /// diff --git a/src/Umbraco.Core/Events/ScopedNotificationPublisher.cs b/src/Umbraco.Core/Events/ScopedNotificationPublisher.cs index c9b0080218..6ea7ee5b6a 100644 --- a/src/Umbraco.Core/Events/ScopedNotificationPublisher.cs +++ b/src/Umbraco.Core/Events/ScopedNotificationPublisher.cs @@ -12,6 +12,8 @@ namespace Umbraco.Cms.Core.Events { private readonly IEventAggregator _eventAggregator; private readonly List _notificationOnScopeCompleted; + private readonly object _locker = new object(); + private bool _isSuppressed = false; public ScopedNotificationPublisher(IEventAggregator eventAggregator) { @@ -26,6 +28,11 @@ namespace Umbraco.Cms.Core.Events throw new ArgumentNullException(nameof(notification)); } + if (_isSuppressed) + { + return false; + } + _eventAggregator.Publish(notification); return notification.Cancel; } @@ -37,6 +44,11 @@ namespace Umbraco.Cms.Core.Events throw new ArgumentNullException(nameof(notification)); } + if (_isSuppressed) + { + return false; + } + await _eventAggregator.PublishAsync(notification); return notification.Cancel; } @@ -48,6 +60,11 @@ namespace Umbraco.Cms.Core.Events throw new ArgumentNullException(nameof(notification)); } + if (_isSuppressed) + { + return; + } + _notificationOnScopeCompleted.Add(notification); } @@ -57,7 +74,7 @@ namespace Umbraco.Cms.Core.Events { if (completed) { - foreach (var notification in _notificationOnScopeCompleted) + foreach (INotification notification in _notificationOnScopeCompleted) { _eventAggregator.Publish(notification); } @@ -68,5 +85,45 @@ namespace Umbraco.Cms.Core.Events _notificationOnScopeCompleted.Clear(); } } + + public IDisposable Supress() + { + lock(_locker) + { + if (_isSuppressed) + { + throw new InvalidOperationException("Notifications are already suppressed"); + } + return new Suppressor(this); + } + } + + private class Suppressor : IDisposable + { + private bool _disposedValue; + private readonly ScopedNotificationPublisher _scopedNotificationPublisher; + + public Suppressor(ScopedNotificationPublisher scopedNotificationPublisher) + { + _scopedNotificationPublisher = scopedNotificationPublisher; + _scopedNotificationPublisher._isSuppressed = true; + } + + protected virtual void Dispose(bool disposing) + { + if (!_disposedValue) + { + if (disposing) + { + lock (_scopedNotificationPublisher._locker) + { + _scopedNotificationPublisher._isSuppressed = false; + } + } + _disposedValue = true; + } + } + public void Dispose() => Dispose(disposing: true); + } } } diff --git a/src/Umbraco.Core/Services/IContentService.cs b/src/Umbraco.Core/Services/IContentService.cs index cef8a8c6d6..e58664fef8 100644 --- a/src/Umbraco.Core/Services/IContentService.cs +++ b/src/Umbraco.Core/Services/IContentService.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.Membership; @@ -236,13 +236,13 @@ namespace Umbraco.Cms.Core.Services /// /// Saves a document. /// - OperationResult Save(IContent content, int userId = Constants.Security.SuperUserId, bool raiseEvents = true); + OperationResult Save(IContent content, int userId = Constants.Security.SuperUserId); /// /// Saves documents. /// // TODO: why only 1 result not 1 per content?! - OperationResult Save(IEnumerable contents, int userId = Constants.Security.SuperUserId, bool raiseEvents = true); + OperationResult Save(IEnumerable contents, int userId = Constants.Security.SuperUserId); /// /// Deletes a document. @@ -325,12 +325,12 @@ namespace Umbraco.Cms.Core.Services /// /// Sorts documents. /// - OperationResult Sort(IEnumerable items, int userId = Constants.Security.SuperUserId, bool raiseEvents = true); + OperationResult Sort(IEnumerable items, int userId = Constants.Security.SuperUserId); /// /// Sorts documents. /// - OperationResult Sort(IEnumerable ids, int userId = Constants.Security.SuperUserId, bool raiseEvents = true); + OperationResult Sort(IEnumerable ids, int userId = Constants.Security.SuperUserId); #endregion @@ -349,8 +349,7 @@ namespace Umbraco.Cms.Core.Services /// The document to publish. /// The culture to publish. /// The identifier of the user performing the action. - /// A value indicating whether to raise events. - PublishResult SaveAndPublish(IContent content, string culture = "*", int userId = Constants.Security.SuperUserId, bool raiseEvents = true); + PublishResult SaveAndPublish(IContent content, string culture = "*", int userId = Constants.Security.SuperUserId); /// /// Saves and publishes a document. @@ -363,8 +362,7 @@ namespace Umbraco.Cms.Core.Services /// The document to publish. /// The cultures to publish. /// The identifier of the user performing the action. - /// A value indicating whether to raise events. - PublishResult SaveAndPublish(IContent content, string[] cultures, int userId = Constants.Security.SuperUserId, bool raiseEvents = true); + PublishResult SaveAndPublish(IContent content, string[] cultures, int userId = Constants.Security.SuperUserId); /// /// Saves and publishes a document branch. diff --git a/src/Umbraco.Core/Services/IContentServiceBase.cs b/src/Umbraco.Core/Services/IContentServiceBase.cs index 9ab7fc7acc..a62d039abb 100644 --- a/src/Umbraco.Core/Services/IContentServiceBase.cs +++ b/src/Umbraco.Core/Services/IContentServiceBase.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using Umbraco.Cms.Core.Models; @@ -8,7 +8,7 @@ namespace Umbraco.Cms.Core.Services where TItem: class, IContentBase { TItem GetById(Guid key); - Attempt Save(IEnumerable contents, int userId = Constants.Security.SuperUserId, bool raiseEvents = true); + Attempt Save(IEnumerable contents, int userId = Constants.Security.SuperUserId); } /// diff --git a/src/Umbraco.Core/Services/IDataTypeService.cs b/src/Umbraco.Core/Services/IDataTypeService.cs index f8e91d07d8..c7b13c04e1 100644 --- a/src/Umbraco.Core/Services/IDataTypeService.cs +++ b/src/Umbraco.Core/Services/IDataTypeService.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using Umbraco.Cms.Core.Models; @@ -68,15 +68,7 @@ namespace Umbraco.Cms.Core.Services /// to save /// Id of the user issuing the save void Save(IEnumerable dataTypeDefinitions, int userId = Constants.Security.SuperUserId); - - /// - /// Saves a collection of - /// - /// to save - /// Id of the user issuing the save - /// Boolean indicating whether or not to raise events - void Save(IEnumerable dataTypeDefinitions, int userId, bool raiseEvents); - + /// /// Deletes an /// diff --git a/src/Umbraco.Core/Services/IMediaService.cs b/src/Umbraco.Core/Services/IMediaService.cs index fc4d4fd612..6c9bf21ecc 100644 --- a/src/Umbraco.Core/Services/IMediaService.cs +++ b/src/Umbraco.Core/Services/IMediaService.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.IO; using Umbraco.Cms.Core.Models; @@ -198,16 +198,14 @@ namespace Umbraco.Cms.Core.Services /// /// The to save /// Id of the User saving the Media - /// Optional boolean indicating whether or not to raise events. - Attempt Save(IMedia media, int userId = Constants.Security.SuperUserId, bool raiseEvents = true); + Attempt Save(IMedia media, int userId = Constants.Security.SuperUserId); /// /// Saves a collection of objects /// /// Collection of to save /// Id of the User saving the Media - /// Optional boolean indicating whether or not to raise events. - Attempt Save(IEnumerable medias, int userId = Constants.Security.SuperUserId, bool raiseEvents = true); + Attempt Save(IEnumerable medias, int userId = Constants.Security.SuperUserId); /// /// Gets an object by its 'UniqueId' @@ -304,7 +302,7 @@ namespace Umbraco.Cms.Core.Services /// /// /// True if sorting succeeded, otherwise False - bool Sort(IEnumerable items, int userId = Constants.Security.SuperUserId, bool raiseEvents = true); + bool Sort(IEnumerable items, int userId = Constants.Security.SuperUserId); /// /// Creates an object using the alias of the diff --git a/src/Umbraco.Core/Services/IMemberGroupService.cs b/src/Umbraco.Core/Services/IMemberGroupService.cs index 16028ded3f..0b72906c2f 100644 --- a/src/Umbraco.Core/Services/IMemberGroupService.cs +++ b/src/Umbraco.Core/Services/IMemberGroupService.cs @@ -11,7 +11,7 @@ namespace Umbraco.Cms.Core.Services IMemberGroup GetById(Guid id); IEnumerable GetByIds(IEnumerable ids); IMemberGroup GetByName(string name); - void Save(IMemberGroup memberGroup, bool raiseEvents = true); + void Save(IMemberGroup memberGroup); void Delete(IMemberGroup memberGroup); } } diff --git a/src/Umbraco.Core/Services/IMembershipMemberService.cs b/src/Umbraco.Core/Services/IMembershipMemberService.cs index c91eba5250..6093c0a4fe 100644 --- a/src/Umbraco.Core/Services/IMembershipMemberService.cs +++ b/src/Umbraco.Core/Services/IMembershipMemberService.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.Membership; @@ -125,18 +125,14 @@ namespace Umbraco.Cms.Core.Services /// /// An can be of type or /// or to Save - /// Optional parameter to raise events. - /// Default is True otherwise set to False to not raise events - void Save(T entity, bool raiseEvents = true); + void Save(T entity); /// /// Saves a list of objects /// /// An can be of type or /// to save - /// Optional parameter to raise events. - /// Default is True otherwise set to False to not raise events - void Save(IEnumerable entities, bool raiseEvents = true); + void Save(IEnumerable entities); /// /// Finds a list of objects by a partial email string diff --git a/src/Umbraco.Core/Services/IUserService.cs b/src/Umbraco.Core/Services/IUserService.cs index a4af73e084..25ce49b65d 100644 --- a/src/Umbraco.Core/Services/IUserService.cs +++ b/src/Umbraco.Core/Services/IUserService.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using Umbraco.Cms.Core.Models.Membership; using Umbraco.Cms.Core.Persistence.Querying; @@ -243,9 +243,7 @@ namespace Umbraco.Cms.Core.Services /// If null than no changes are made to the users who are assigned to this group, however if a value is passed in /// than all users will be removed from this group and only these users will be added /// - /// Optional parameter to raise events. - /// Default is True otherwise set to False to not raise events - void Save(IUserGroup userGroup, int[] userIds = null, bool raiseEvents = true); + void Save(IUserGroup userGroup, int[] userIds = null); /// /// Deletes a UserGroup diff --git a/src/Umbraco.Infrastructure/Services/Implement/ContentService.cs b/src/Umbraco.Infrastructure/Services/Implement/ContentService.cs index 3d4c6dfbbe..b6fa009d2e 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/ContentService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/ContentService.cs @@ -381,8 +381,7 @@ namespace Umbraco.Cms.Core.Services.Implement /// /// /// - Attempt IContentServiceBase.Save(IEnumerable contents, int userId, - bool raiseEvents) => Attempt.Succeed(Save(contents, userId, raiseEvents)); + Attempt IContentServiceBase.Save(IEnumerable contents, int userId) => Attempt.Succeed(Save(contents, userId)); /// /// Gets objects by Ids @@ -756,7 +755,7 @@ namespace Umbraco.Cms.Core.Services.Implement #region Save, Publish, Unpublish /// - public OperationResult Save(IContent content, int userId = Cms.Core.Constants.Security.SuperUserId, bool raiseEvents = true) + public OperationResult Save(IContent content, int userId = Cms.Core.Constants.Security.SuperUserId) { PublishedState publishedState = content.PublishedState; if (publishedState != PublishedState.Published && publishedState != PublishedState.Unpublished) @@ -774,7 +773,7 @@ namespace Umbraco.Cms.Core.Services.Implement using (IScope scope = ScopeProvider.CreateScope()) { var savingNotification = new ContentSavingNotification(content, eventMessages); - if (raiseEvents && scope.Notifications.PublishCancelable(savingNotification)) + if (scope.Notifications.PublishCancelable(savingNotification)) { scope.Complete(); return OperationResult.Cancel(eventMessages); @@ -800,10 +799,12 @@ namespace Umbraco.Cms.Core.Services.Implement _documentRepository.Save(content); - if (raiseEvents) - { - scope.Notifications.Publish(new ContentSavedNotification(content, eventMessages).WithStateFrom(savingNotification)); - } + scope.Notifications.Publish(new ContentSavedNotification(content, eventMessages).WithStateFrom(savingNotification)); + + // TODO: we had code here to FORCE that this event can never be suppressed. But that just doesn't make a ton of sense?! + // I understand that if its suppressed that the caches aren't updated, but that would be expected. If someone + // is supressing events then I think it's expected that nothing will happen. They are probably doing it for perf + // reasons like bulk import and in those cases we don't want this occuring. scope.Notifications.Publish(new ContentTreeChangeNotification(content, TreeChangeTypes.RefreshNode, eventMessages)); if (culturesChanging != null) @@ -823,7 +824,7 @@ namespace Umbraco.Cms.Core.Services.Implement } /// - public OperationResult Save(IEnumerable contents, int userId = Cms.Core.Constants.Security.SuperUserId, bool raiseEvents = true) + public OperationResult Save(IEnumerable contents, int userId = Cms.Core.Constants.Security.SuperUserId) { EventMessages eventMessages = EventMessagesFactory.Get(); IContent[] contentsA = contents.ToArray(); @@ -831,7 +832,7 @@ namespace Umbraco.Cms.Core.Services.Implement using (IScope scope = ScopeProvider.CreateScope()) { var savingNotification = new ContentSavingNotification(contentsA, eventMessages); - if (raiseEvents && scope.Notifications.PublishCancelable(savingNotification)) + if (scope.Notifications.PublishCancelable(savingNotification)) { scope.Complete(); return OperationResult.Cancel(eventMessages); @@ -850,11 +851,10 @@ namespace Umbraco.Cms.Core.Services.Implement _documentRepository.Save(content); } - if (raiseEvents) - { - scope.Notifications.Publish(new ContentSavedNotification(contentsA, eventMessages).WithStateFrom(savingNotification)); - } + scope.Notifications.Publish(new ContentSavedNotification(contentsA, eventMessages).WithStateFrom(savingNotification)); + // TODO: See note above about supressing events scope.Notifications.Publish(new ContentTreeChangeNotification(contentsA, TreeChangeTypes.RefreshNode, eventMessages)); + Audit(AuditType.Save, userId == -1 ? 0 : userId, Cms.Core.Constants.System.Root, "Saved multiple content"); scope.Complete(); @@ -864,7 +864,7 @@ namespace Umbraco.Cms.Core.Services.Implement } /// - public PublishResult SaveAndPublish(IContent content, string culture = "*", int userId = Cms.Core.Constants.Security.SuperUserId, bool raiseEvents = true) + public PublishResult SaveAndPublish(IContent content, string culture = "*", int userId = Cms.Core.Constants.Security.SuperUserId) { var evtMsgs = EventMessagesFactory.Get(); @@ -912,14 +912,14 @@ namespace Umbraco.Cms.Core.Services.Implement // we don't care about the response here, this response will be rechecked below but we need to set the culture info values now. content.PublishCulture(impact); - var result = CommitDocumentChangesInternal(scope, content, evtMsgs, allLangs, savingNotification.State, userId, raiseEvents); + var result = CommitDocumentChangesInternal(scope, content, evtMsgs, allLangs, savingNotification.State, userId); scope.Complete(); return result; } } /// - public PublishResult SaveAndPublish(IContent content, string[] cultures, int userId = 0, bool raiseEvents = true) + public PublishResult SaveAndPublish(IContent content, string[] cultures, int userId = 0) { if (content == null) throw new ArgumentNullException(nameof(content)); if (cultures == null) throw new ArgumentNullException(nameof(cultures)); @@ -938,7 +938,7 @@ namespace Umbraco.Cms.Core.Services.Implement var evtMsgs = EventMessagesFactory.Get(); var savingNotification = new ContentSavingNotification(content, evtMsgs); - if (raiseEvents && scope.Notifications.PublishCancelable(savingNotification)) + if (scope.Notifications.PublishCancelable(savingNotification)) { return new PublishResult(PublishResultType.FailedPublishCancelledByEvent, evtMsgs, content); } @@ -948,7 +948,7 @@ namespace Umbraco.Cms.Core.Services.Implement if (cultures.Length == 0 && !varies) { //no cultures specified and doesn't vary, so publish it, else nothing to publish - return SaveAndPublish(content, userId: userId, raiseEvents: raiseEvents); + return SaveAndPublish(content, userId: userId); } if (cultures.Any(x => x == null || x == "*")) @@ -959,9 +959,11 @@ namespace Umbraco.Cms.Core.Services.Implement // publish the culture(s) // we don't care about the response here, this response will be rechecked below but we need to set the culture info values now. foreach (var impact in impacts) + { content.PublishCulture(impact); + } - var result = CommitDocumentChangesInternal(scope, content, evtMsgs, allLangs, savingNotification.State, userId, raiseEvents); + var result = CommitDocumentChangesInternal(scope, content, evtMsgs, allLangs, savingNotification.State, userId); scope.Complete(); return result; } @@ -1067,7 +1069,7 @@ namespace Umbraco.Cms.Core.Services.Implement /// The document is *always* saved, even when publishing fails. /// internal PublishResult CommitDocumentChanges(IContent content, - int userId = Cms.Core.Constants.Security.SuperUserId, bool raiseEvents = true) + int userId = Cms.Core.Constants.Security.SuperUserId) { using (var scope = ScopeProvider.CreateScope()) { @@ -1083,7 +1085,7 @@ namespace Umbraco.Cms.Core.Services.Implement var allLangs = _languageRepository.GetMany().ToList(); - var result = CommitDocumentChangesInternal(scope, content, evtMsgs, allLangs, savingNotification.State, userId, raiseEvents); + var result = CommitDocumentChangesInternal(scope, content, evtMsgs, allLangs, savingNotification.State, userId); scope.Complete(); return result; } @@ -2383,7 +2385,7 @@ namespace Umbraco.Cms.Core.Services.Implement /// /// /// Result indicating what action was taken when handling the command. - public OperationResult Sort(IEnumerable items, int userId = Cms.Core.Constants.Security.SuperUserId, bool raiseEvents = true) + public OperationResult Sort(IEnumerable items, int userId = Cms.Core.Constants.Security.SuperUserId) { var evtMsgs = EventMessagesFactory.Get(); @@ -2394,7 +2396,7 @@ namespace Umbraco.Cms.Core.Services.Implement { scope.WriteLock(Cms.Core.Constants.Locks.ContentTree); - var ret = Sort(scope, itemsA, userId, evtMsgs, raiseEvents); + var ret = Sort(scope, itemsA, userId, evtMsgs); scope.Complete(); return ret; } @@ -2412,7 +2414,7 @@ namespace Umbraco.Cms.Core.Services.Implement /// /// /// Result indicating what action was taken when handling the command. - public OperationResult Sort(IEnumerable ids, int userId = Cms.Core.Constants.Security.SuperUserId, bool raiseEvents = true) + public OperationResult Sort(IEnumerable ids, int userId = Cms.Core.Constants.Security.SuperUserId) { var evtMsgs = EventMessagesFactory.Get(); @@ -2424,29 +2426,27 @@ namespace Umbraco.Cms.Core.Services.Implement scope.WriteLock(Cms.Core.Constants.Locks.ContentTree); var itemsA = GetByIds(idsA).ToArray(); - var ret = Sort(scope, itemsA, userId, evtMsgs, raiseEvents); + var ret = Sort(scope, itemsA, userId, evtMsgs); scope.Complete(); return ret; } } - private OperationResult Sort(IScope scope, IContent[] itemsA, int userId, EventMessages eventMessages, bool raiseEvents) + private OperationResult Sort(IScope scope, IContent[] itemsA, int userId, EventMessages eventMessages) { var sortingNotification = new ContentSortingNotification(itemsA, eventMessages); var savingNotification = new ContentSavingNotification(itemsA, eventMessages); - if (raiseEvents) - { - // raise cancelable sorting event - if (scope.Notifications.PublishCancelable(sortingNotification)) - { - return OperationResult.Cancel(eventMessages); - } - // raise cancelable saving event - if (scope.Notifications.PublishCancelable(savingNotification)) - { - return OperationResult.Cancel(eventMessages); - } + // raise cancelable sorting event + if (scope.Notifications.PublishCancelable(sortingNotification)) + { + return OperationResult.Cancel(eventMessages); + } + + // raise cancelable saving event + if (scope.Notifications.PublishCancelable(savingNotification)) + { + return OperationResult.Cancel(eventMessages); } var published = new List(); @@ -2479,16 +2479,13 @@ namespace Umbraco.Cms.Core.Services.Implement _documentRepository.Save(content); } - if (raiseEvents) - { - //first saved, then sorted - scope.Notifications.Publish(new ContentSavedNotification(itemsA, eventMessages).WithStateFrom(savingNotification)); - scope.Notifications.Publish(new ContentSortedNotification(itemsA, eventMessages).WithStateFrom(sortingNotification)); - } + //first saved, then sorted + scope.Notifications.Publish(new ContentSavedNotification(itemsA, eventMessages).WithStateFrom(savingNotification)); + scope.Notifications.Publish(new ContentSortedNotification(itemsA, eventMessages).WithStateFrom(sortingNotification)); scope.Notifications.Publish(new ContentTreeChangeNotification(saved, TreeChangeTypes.RefreshNode, eventMessages)); - if (raiseEvents && published.Any()) + if (published.Any()) { scope.Notifications.Publish(new ContentPublishedNotification(published, eventMessages)); } diff --git a/src/Umbraco.Infrastructure/Services/Implement/DataTypeService.cs b/src/Umbraco.Infrastructure/Services/Implement/DataTypeService.cs index b20be692df..50caca0ec8 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/DataTypeService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/DataTypeService.cs @@ -434,18 +434,7 @@ namespace Umbraco.Cms.Core.Services.Implement /// /// to save /// Id of the user issuing the save - public void Save(IEnumerable dataTypeDefinitions, int userId = Cms.Core.Constants.Security.SuperUserId) - { - Save(dataTypeDefinitions, userId, true); - } - - /// - /// Saves a collection of - /// - /// to save - /// Id of the user issuing the save - /// Boolean indicating whether or not to raise events - public void Save(IEnumerable dataTypeDefinitions, int userId, bool raiseEvents) + public void Save(IEnumerable dataTypeDefinitions, int userId) { var evtMsgs = EventMessagesFactory.Get(); var dataTypeDefinitionsA = dataTypeDefinitions.ToArray(); @@ -453,7 +442,7 @@ namespace Umbraco.Cms.Core.Services.Implement using (var scope = ScopeProvider.CreateScope()) { var savingDataTypeNotification = new DataTypeSavingNotification(dataTypeDefinitions, evtMsgs); - if (raiseEvents && scope.Notifications.PublishCancelable(savingDataTypeNotification)) + if (scope.Notifications.PublishCancelable(savingDataTypeNotification)) { scope.Complete(); return; @@ -465,10 +454,8 @@ namespace Umbraco.Cms.Core.Services.Implement _dataTypeRepository.Save(dataTypeDefinition); } - if (raiseEvents) - { - scope.Notifications.Publish(new DataTypeSavedNotification(dataTypeDefinitions, evtMsgs).WithStateFrom(savingDataTypeNotification)); - } + scope.Notifications.Publish(new DataTypeSavedNotification(dataTypeDefinitions, evtMsgs).WithStateFrom(savingDataTypeNotification)); + Audit(AuditType.Save, userId, -1); scope.Complete(); diff --git a/src/Umbraco.Infrastructure/Services/Implement/MediaService.cs b/src/Umbraco.Infrastructure/Services/Implement/MediaService.cs index 34d1c2a5ce..bdf672ce7a 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/MediaService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/MediaService.cs @@ -654,14 +654,14 @@ namespace Umbraco.Cms.Core.Services.Implement /// The to save /// Id of the User saving the Media /// Optional boolean indicating whether or not to raise events. - public Attempt Save(IMedia media, int userId = Cms.Core.Constants.Security.SuperUserId, bool raiseEvents = true) + public Attempt Save(IMedia media, int userId = Cms.Core.Constants.Security.SuperUserId) { EventMessages eventMessages = EventMessagesFactory.Get(); using (IScope scope = ScopeProvider.CreateScope()) { var savingNotification = new MediaSavingNotification(media, eventMessages); - if (raiseEvents && scope.Notifications.PublishCancelable(savingNotification)) + if (scope.Notifications.PublishCancelable(savingNotification)) { scope.Complete(); return OperationResult.Attempt.Cancel(eventMessages); @@ -685,10 +685,8 @@ namespace Umbraco.Cms.Core.Services.Implement } _mediaRepository.Save(media); - if (raiseEvents) - { - scope.Notifications.Publish(new MediaSavedNotification(media, eventMessages).WithStateFrom(savingNotification)); - } + scope.Notifications.Publish(new MediaSavedNotification(media, eventMessages).WithStateFrom(savingNotification)); + // TODO: See note about supressing events in content service scope.Notifications.Publish(new MediaTreeChangeNotification(media, TreeChangeTypes.RefreshNode, eventMessages)); Audit(AuditType.Save, userId, media.Id); @@ -704,7 +702,7 @@ namespace Umbraco.Cms.Core.Services.Implement /// Collection of to save /// Id of the User saving the Media /// Optional boolean indicating whether or not to raise events. - public Attempt Save(IEnumerable medias, int userId = Cms.Core.Constants.Security.SuperUserId, bool raiseEvents = true) + public Attempt Save(IEnumerable medias, int userId = Cms.Core.Constants.Security.SuperUserId) { EventMessages messages = EventMessagesFactory.Get(); IMedia[] mediasA = medias.ToArray(); @@ -712,7 +710,7 @@ namespace Umbraco.Cms.Core.Services.Implement using (IScope scope = ScopeProvider.CreateScope()) { var savingNotification = new MediaSavingNotification(mediasA, messages); - if (raiseEvents && scope.Notifications.PublishCancelable(savingNotification)) + if (scope.Notifications.PublishCancelable(savingNotification)) { scope.Complete(); return OperationResult.Attempt.Cancel(messages); @@ -731,10 +729,8 @@ namespace Umbraco.Cms.Core.Services.Implement _mediaRepository.Save(media); } - if (raiseEvents) - { - scope.Notifications.Publish(new MediaSavedNotification(mediasA, messages).WithStateFrom(savingNotification)); - } + scope.Notifications.Publish(new MediaSavedNotification(mediasA, messages).WithStateFrom(savingNotification)); + // TODO: See note about supressing events in content service scope.Notifications.Publish(new MediaTreeChangeNotification(treeChanges, messages)); Audit(AuditType.Save, userId == -1 ? 0 : userId, Cms.Core.Constants.System.Root, "Bulk save media"); @@ -1095,7 +1091,7 @@ namespace Umbraco.Cms.Core.Services.Implement /// /// /// True if sorting succeeded, otherwise False - public bool Sort(IEnumerable items, int userId = Cms.Core.Constants.Security.SuperUserId, bool raiseEvents = true) + public bool Sort(IEnumerable items, int userId = Cms.Core.Constants.Security.SuperUserId) { IMedia[] itemsA = items.ToArray(); if (itemsA.Length == 0) @@ -1108,7 +1104,7 @@ namespace Umbraco.Cms.Core.Services.Implement using (IScope scope = ScopeProvider.CreateScope()) { var savingNotification = new MediaSavingNotification(itemsA, messages); - if (raiseEvents && scope.Notifications.PublishCancelable(savingNotification)) + if (scope.Notifications.PublishCancelable(savingNotification)) { scope.Complete(); return false; @@ -1135,10 +1131,8 @@ namespace Umbraco.Cms.Core.Services.Implement _mediaRepository.Save(media); } - if (raiseEvents) - { - scope.Notifications.Publish(new MediaSavedNotification(itemsA, messages).WithStateFrom(savingNotification)); - } + scope.Notifications.Publish(new MediaSavedNotification(itemsA, messages).WithStateFrom(savingNotification)); + // TODO: See note about supressing events in content service scope.Notifications.Publish(new MediaTreeChangeNotification(saved, TreeChangeTypes.RefreshNode, messages)); Audit(AuditType.Sort, userId, 0); diff --git a/src/Umbraco.Infrastructure/Services/Implement/MemberGroupService.cs b/src/Umbraco.Infrastructure/Services/Implement/MemberGroupService.cs index 096ff164a0..9d68415ad5 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/MemberGroupService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/MemberGroupService.cs @@ -64,7 +64,7 @@ namespace Umbraco.Cms.Core.Services.Implement } } - public void Save(IMemberGroup memberGroup, bool raiseEvents = true) + public void Save(IMemberGroup memberGroup) { if (string.IsNullOrWhiteSpace(memberGroup.Name)) { @@ -76,7 +76,7 @@ namespace Umbraco.Cms.Core.Services.Implement using (var scope = ScopeProvider.CreateScope()) { var savingNotification = new MemberGroupSavingNotification(memberGroup, evtMsgs); - if (raiseEvents && scope.Notifications.PublishCancelable(savingNotification)) + if (scope.Notifications.PublishCancelable(savingNotification)) { scope.Complete(); return; @@ -85,10 +85,7 @@ namespace Umbraco.Cms.Core.Services.Implement _memberGroupRepository.Save(memberGroup); scope.Complete(); - if (raiseEvents) - { - scope.Notifications.Publish(new MemberGroupSavedNotification(memberGroup, evtMsgs).WithStateFrom(savingNotification)); - } + scope.Notifications.Publish(new MemberGroupSavedNotification(memberGroup, evtMsgs).WithStateFrom(savingNotification)); } } diff --git a/src/Umbraco.Infrastructure/Services/Implement/MemberService.cs b/src/Umbraco.Infrastructure/Services/Implement/MemberService.cs index 94476ff1e1..f6aac98682 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/MemberService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/MemberService.cs @@ -767,7 +767,7 @@ namespace Umbraco.Cms.Core.Services.Implement } /// - public void Save(IMember member, bool raiseEvents = true) + public void Save(IMember member) { // trimming username and email to make sure we have no trailing space member.Username = member.Username.Trim(); @@ -778,7 +778,7 @@ namespace Umbraco.Cms.Core.Services.Implement using (IScope scope = ScopeProvider.CreateScope()) { var savingNotification = new MemberSavingNotification(member, evtMsgs); - if (raiseEvents && scope.Notifications.PublishCancelable(savingNotification)) + if (scope.Notifications.PublishCancelable(savingNotification)) { scope.Complete(); return; @@ -793,10 +793,7 @@ namespace Umbraco.Cms.Core.Services.Implement _memberRepository.Save(member); - if (raiseEvents) - { - scope.Notifications.Publish(new MemberSavedNotification(member, evtMsgs).WithStateFrom(savingNotification)); - } + scope.Notifications.Publish(new MemberSavedNotification(member, evtMsgs).WithStateFrom(savingNotification)); Audit(AuditType.Save, 0, member.Id); @@ -805,7 +802,7 @@ namespace Umbraco.Cms.Core.Services.Implement } /// - public void Save(IEnumerable members, bool raiseEvents = true) + public void Save(IEnumerable members) { var membersA = members.ToArray(); @@ -814,7 +811,7 @@ namespace Umbraco.Cms.Core.Services.Implement using (var scope = ScopeProvider.CreateScope()) { var savingNotification = new MemberSavingNotification(membersA, evtMsgs); - if (raiseEvents && scope.Notifications.PublishCancelable(savingNotification)) + if (scope.Notifications.PublishCancelable(savingNotification)) { scope.Complete(); return; @@ -831,10 +828,8 @@ namespace Umbraco.Cms.Core.Services.Implement _memberRepository.Save(member); } - if (raiseEvents) - { - scope.Notifications.Publish(new MemberSavedNotification(membersA, evtMsgs).WithStateFrom(savingNotification)); - } + scope.Notifications.Publish(new MemberSavedNotification(membersA, evtMsgs).WithStateFrom(savingNotification)); + Audit(AuditType.Save, 0, -1, "Save multiple Members"); scope.Complete(); diff --git a/src/Umbraco.Infrastructure/Services/Implement/UserService.cs b/src/Umbraco.Infrastructure/Services/Implement/UserService.cs index 743b4816da..978dbb5bda 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/UserService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/UserService.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Data.Common; using System.Globalization; @@ -273,16 +273,14 @@ namespace Umbraco.Cms.Core.Services.Implement /// Saves an /// /// to Save - /// Optional parameter to raise events. - /// Default is True otherwise set to False to not raise events - public void Save(IUser entity, bool raiseEvents = true) + public void Save(IUser entity) { var evtMsgs = EventMessagesFactory.Get(); using (var scope = ScopeProvider.CreateScope()) { var savingNotification = new UserSavingNotification(entity, evtMsgs); - if (raiseEvents && scope.Notifications.PublishCancelable(savingNotification)) + if (scope.Notifications.PublishCancelable(savingNotification)) { scope.Complete(); return; @@ -297,10 +295,7 @@ namespace Umbraco.Cms.Core.Services.Implement try { _userRepository.Save(entity); - if (raiseEvents) - { - scope.Notifications.Publish(new UserSavedNotification(entity, evtMsgs).WithStateFrom(savingNotification)); - } + scope.Notifications.Publish(new UserSavedNotification(entity, evtMsgs).WithStateFrom(savingNotification)); scope.Complete(); } @@ -321,9 +316,7 @@ namespace Umbraco.Cms.Core.Services.Implement /// Saves a list of objects /// /// to save - /// Optional parameter to raise events. - /// Default is True otherwise set to False to not raise events - public void Save(IEnumerable entities, bool raiseEvents = true) + public void Save(IEnumerable entities) { var evtMsgs = EventMessagesFactory.Get(); @@ -332,7 +325,7 @@ namespace Umbraco.Cms.Core.Services.Implement using (var scope = ScopeProvider.CreateScope()) { var savingNotification = new UserSavingNotification(entitiesA, evtMsgs); - if (raiseEvents && scope.Notifications.PublishCancelable(savingNotification)) + if (scope.Notifications.PublishCancelable(savingNotification)) { scope.Complete(); return; @@ -350,10 +343,7 @@ namespace Umbraco.Cms.Core.Services.Implement } - if (raiseEvents) - { - scope.Notifications.Publish(new UserSavedNotification(entitiesA, evtMsgs).WithStateFrom(savingNotification)); - } + scope.Notifications.Publish(new UserSavedNotification(entitiesA, evtMsgs).WithStateFrom(savingNotification)); //commit the whole lot in one go scope.Complete(); @@ -818,7 +808,7 @@ namespace Umbraco.Cms.Core.Services.Implement /// /// Optional parameter to raise events. /// Default is True otherwise set to False to not raise events - public void Save(IUserGroup userGroup, int[] userIds = null, bool raiseEvents = true) + public void Save(IUserGroup userGroup, int[] userIds = null) { var evtMsgs = EventMessagesFactory.Get(); @@ -843,7 +833,7 @@ namespace Umbraco.Cms.Core.Services.Implement // this is the default/expected notification for the IUserGroup entity being saved var savingNotification = new UserGroupSavingNotification(userGroup, evtMsgs); - if (raiseEvents && scope.Notifications.PublishCancelable(savingNotification)) + if (scope.Notifications.PublishCancelable(savingNotification)) { scope.Complete(); return; @@ -851,7 +841,7 @@ namespace Umbraco.Cms.Core.Services.Implement // this is an additional notification for special auditing var savingUserGroupWithUsersNotification = new UserGroupWithUsersSavingNotification(userGroupWithUsers, evtMsgs); - if (raiseEvents && scope.Notifications.PublishCancelable(savingUserGroupWithUsersNotification)) + if (scope.Notifications.PublishCancelable(savingUserGroupWithUsersNotification)) { scope.Complete(); return; @@ -859,11 +849,8 @@ namespace Umbraco.Cms.Core.Services.Implement _userGroupRepository.AddOrUpdateGroupWithUsers(userGroup, userIds); - if (raiseEvents) - { - scope.Notifications.Publish(new UserGroupSavedNotification(userGroup, evtMsgs).WithStateFrom(savingNotification)); - scope.Notifications.Publish(new UserGroupWithUsersSavedNotification(userGroupWithUsers, evtMsgs).WithStateFrom(savingUserGroupWithUsersNotification)); - } + scope.Notifications.Publish(new UserGroupSavedNotification(userGroup, evtMsgs).WithStateFrom(savingNotification)); + scope.Notifications.Publish(new UserGroupWithUsersSavedNotification(userGroupWithUsers, evtMsgs).WithStateFrom(savingUserGroupWithUsersNotification)); scope.Complete(); } diff --git a/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj b/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj index d759c8da9b..712323656d 100644 --- a/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj +++ b/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj @@ -111,10 +111,6 @@ - - - - From ff9645c289fc48b0bf6c07f6e899e9b91fc4c724 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 12 Jul 2021 10:20:59 -0600 Subject: [PATCH 02/13] Updates usages. --- .../Packaging/PackageDataInstallation.cs | 2 +- .../Umbraco.Core/Services/SectionServiceTests.cs | 11 ++++++++--- .../Security/MemberManagerTests.cs | 2 +- .../Security/MemberRoleStoreTests.cs | 16 +++++----------- .../Security/MemberUserStoreTests.cs | 14 +++++--------- .../Controllers/MemberControllerUnitTests.cs | 2 +- 6 files changed, 21 insertions(+), 26 deletions(-) diff --git a/src/Umbraco.Infrastructure/Packaging/PackageDataInstallation.cs b/src/Umbraco.Infrastructure/Packaging/PackageDataInstallation.cs index e680471486..2fc562b00d 100644 --- a/src/Umbraco.Infrastructure/Packaging/PackageDataInstallation.cs +++ b/src/Umbraco.Infrastructure/Packaging/PackageDataInstallation.cs @@ -1030,7 +1030,7 @@ namespace Umbraco.Cms.Infrastructure.Packaging if (dataTypes.Count > 0) { - _dataTypeService.Save(dataTypes, userId, true); + _dataTypeService.Save(dataTypes, userId); } return dataTypes; diff --git a/src/Umbraco.Tests.Integration/Umbraco.Core/Services/SectionServiceTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Core/Services/SectionServiceTests.cs index b9b3d0569e..1e9a80faf6 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Core/Services/SectionServiceTests.cs +++ b/src/Umbraco.Tests.Integration/Umbraco.Core/Services/SectionServiceTests.cs @@ -1,11 +1,13 @@ // Copyright (c) Umbraco. // See LICENSE for more details. +using System; using System.Linq; using System.Threading; using NUnit.Framework; using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.Models.Membership; +using Umbraco.Cms.Core.Scoping; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Tests.Common.Testing; using Umbraco.Cms.Tests.Integration.Testing; @@ -39,6 +41,9 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Core.Services private IUser CreateTestUser() { + using IScope scope = ScopeProvider.CreateScope(autoComplete: true); + using IDisposable _ = scope.Notifications.Supress(); + var globalSettings = new GlobalSettings(); var user = new User(globalSettings) { @@ -46,7 +51,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Core.Services Username = "testUser", Email = "testuser@test.com", }; - UserService.Save(user, false); + UserService.Save(user); var userGroupA = new UserGroup(ShortStringHelper) { @@ -57,7 +62,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Core.Services userGroupA.AddAllowedSection("settings"); // TODO: This is failing the test - UserService.Save(userGroupA, new[] { user.Id }, false); + UserService.Save(userGroupA, new[] { user.Id }); var userGroupB = new UserGroup(ShortStringHelper) { @@ -66,7 +71,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Core.Services }; userGroupB.AddAllowedSection("settings"); userGroupB.AddAllowedSection("member"); - UserService.Save(userGroupB, new[] { user.Id }, false); + UserService.Save(userGroupB, new[] { user.Id }); return UserService.GetUserById(user.Id); } diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberManagerTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberManagerTests.cs index c8f90050e2..86e6462472 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberManagerTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberManagerTests.cs @@ -246,7 +246,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security private void MockMemberServiceForCreateMember(IMember fakeMember) { _mockMemberService.Setup(x => x.CreateMember(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())).Returns(fakeMember); - _mockMemberService.Setup(x => x.Save(fakeMember, false)); + _mockMemberService.Setup(x => x.Save(fakeMember)); } } } diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberRoleStoreTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberRoleStoreTests.cs index 412de11a9e..a82534c940 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberRoleStoreTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberRoleStoreTests.cs @@ -62,9 +62,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security IMemberGroup mockMemberGroup = Mock.Of(m => m.Name == "fakeGroupName" && m.CreatorId == 77); - bool raiseEvents = false; - - _mockMemberGroupService.Setup(x => x.Save(mockMemberGroup, raiseEvents)); + _mockMemberGroupService.Setup(x => x.Save(mockMemberGroup)); // act IdentityResult identityResult = await sut.CreateAsync(fakeRole, fakeCancellationToken); @@ -72,7 +70,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security // assert Assert.IsTrue(identityResult.Succeeded); Assert.IsTrue(!identityResult.Errors.Any()); - _mockMemberGroupService.Verify(x => x.Save(It.IsAny(), It.IsAny())); + _mockMemberGroupService.Verify(x => x.Save(It.IsAny())); } [Test] @@ -86,10 +84,8 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security IMemberGroup mockMemberGroup = Mock.Of(m => m.Name == "fakeGroupName" && m.CreatorId == 777); - bool raiseEvents = false; - _mockMemberGroupService.Setup(x => x.GetById(777)).Returns(mockMemberGroup); - _mockMemberGroupService.Setup(x => x.Save(mockMemberGroup, raiseEvents)); + _mockMemberGroupService.Setup(x => x.Save(mockMemberGroup)); // act IdentityResult identityResult = await sut.UpdateAsync(fakeRole, fakeCancellationToken); @@ -111,10 +107,8 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security IMemberGroup mockMemberGroup = Mock.Of(m => m.Name == "fakeGroupName" && m.CreatorId == 777); - bool raiseEvents = false; - _mockMemberGroupService.Setup(x => x.GetById(777)).Returns(mockMemberGroup); - _mockMemberGroupService.Setup(x => x.Save(mockMemberGroup, raiseEvents)); + _mockMemberGroupService.Setup(x => x.Save(mockMemberGroup)); // act IdentityResult identityResult = await sut.UpdateAsync(fakeRole, fakeCancellationToken); @@ -122,7 +116,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security // assert Assert.IsTrue(identityResult.Succeeded); Assert.IsTrue(!identityResult.Errors.Any()); - _mockMemberGroupService.Verify(x => x.Save(It.IsAny(), It.IsAny())); + _mockMemberGroupService.Verify(x => x.Save(It.IsAny())); _mockMemberGroupService.Verify(x => x.GetById(777)); } diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberUserStoreTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberUserStoreTests.cs index 3fdb3c27f9..7a63ff4bc4 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberUserStoreTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/MemberUserStoreTests.cs @@ -90,7 +90,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security m.HasIdentity == false); _mockMemberService.Setup(x => x.CreateMember(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())).Returns(mockMember); - _mockMemberService.Setup(x => x.Save(mockMember, It.IsAny())); + _mockMemberService.Setup(x => x.Save(mockMember)); // act IdentityResult actual = await sut.CreateAsync(null); @@ -118,10 +118,8 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security m.ContentTypeAlias == fakeMemberType.Alias && m.HasIdentity == true); - bool raiseEvents = false; - _mockMemberService.Setup(x => x.CreateMember(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())).Returns(mockMember); - _mockMemberService.Setup(x => x.Save(mockMember, raiseEvents)); + _mockMemberService.Setup(x => x.Save(mockMember)); // act IdentityResult identityResult = await sut.CreateAsync(fakeUser, CancellationToken.None); @@ -130,7 +128,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security Assert.IsTrue(identityResult.Succeeded); Assert.IsTrue(!identityResult.Errors.Any()); _mockMemberService.Verify(x => x.CreateMember(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())); - _mockMemberService.Verify(x => x.Save(mockMember, It.IsAny())); + _mockMemberService.Verify(x => x.Save(mockMember)); } [Test] @@ -175,9 +173,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security m.RawPasswordValue == "xyz" && m.SecurityStamp == "xyz"); - bool raiseEvents = false; - - _mockMemberService.Setup(x => x.Save(mockMember, raiseEvents)); + _mockMemberService.Setup(x => x.Save(mockMember)); _mockMemberService.Setup(x => x.GetById(123)).Returns(mockMember); // act @@ -200,7 +196,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Security Assert.AreEqual(fakeUser.SecurityStamp, mockMember.SecurityStamp); Assert.AreNotEqual(DateTime.MinValue, mockMember.EmailConfirmedDate.Value); - _mockMemberService.Verify(x => x.Save(mockMember, It.IsAny())); + _mockMemberService.Verify(x => x.Save(mockMember)); _mockMemberService.Verify(x => x.GetById(123)); _mockMemberService.Verify(x => x.ReplaceRoles(new[] { 123 }, new[] { "role1", "role2" })); } diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs index 8006bda3a4..e017844b2b 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs @@ -417,7 +417,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers Mock.Get(umbracoMembersUserManager) .Verify(u => u.AddToRolesAsync(membersIdentityUser, new[] { roleName })); Mock.Get(memberService) - .Verify(m => m.Save(It.IsAny(), true)); + .Verify(m => m.Save(It.IsAny())); AssertMemberDisplayPropertiesAreEqual(memberDisplay, result.Value); } From a15eab1e6b789c0831981a2ac5968ac0da5a7305 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 12 Jul 2021 10:43:50 -0600 Subject: [PATCH 03/13] cleanup --- src/Umbraco.Core/Services/IMediaService.cs | 1 - .../Services/Implement/ContentService.cs | 13 +++---------- .../Services/Implement/MediaService.cs | 3 --- .../Services/Implement/UserService.cs | 1 - 4 files changed, 3 insertions(+), 15 deletions(-) diff --git a/src/Umbraco.Core/Services/IMediaService.cs b/src/Umbraco.Core/Services/IMediaService.cs index 6c9bf21ecc..19b509134e 100644 --- a/src/Umbraco.Core/Services/IMediaService.cs +++ b/src/Umbraco.Core/Services/IMediaService.cs @@ -300,7 +300,6 @@ namespace Umbraco.Cms.Core.Services /// /// /// - /// /// True if sorting succeeded, otherwise False bool Sort(IEnumerable items, int userId = Constants.Security.SuperUserId); diff --git a/src/Umbraco.Infrastructure/Services/Implement/ContentService.cs b/src/Umbraco.Infrastructure/Services/Implement/ContentService.cs index b6fa009d2e..609a6bd426 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/ContentService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/ContentService.cs @@ -379,7 +379,6 @@ namespace Umbraco.Cms.Core.Services.Implement /// /// /// - /// /// Attempt IContentServiceBase.Save(IEnumerable contents, int userId) => Attempt.Succeed(Save(contents, userId)); @@ -1099,7 +1098,6 @@ namespace Umbraco.Cms.Core.Services.Implement /// /// /// - /// /// /// /// @@ -1113,8 +1111,8 @@ namespace Umbraco.Cms.Core.Services.Implement private PublishResult CommitDocumentChangesInternal(IScope scope, IContent content, EventMessages eventMessages, IReadOnlyCollection allLangs, IDictionary notificationState, - int userId = Cms.Core.Constants.Security.SuperUserId, - bool raiseEvents = true, bool branchOne = false, bool branchRoot = false) + int userId = Constants.Security.SuperUserId, + bool branchOne = false, bool branchRoot = false) { if (scope == null) { @@ -1271,10 +1269,7 @@ namespace Umbraco.Cms.Core.Services.Implement SaveDocument(content); // raise the Saved event, always - if (raiseEvents) - { - scope.Notifications.Publish(new ContentSavedNotification(content, eventMessages).WithState(notificationState)); - } + scope.Notifications.Publish(new ContentSavedNotification(content, eventMessages).WithState(notificationState)); if (unpublishing) // we have tried to unpublish - won't happen in a branch { @@ -2383,7 +2378,6 @@ namespace Umbraco.Cms.Core.Services.Implement /// /// /// - /// /// Result indicating what action was taken when handling the command. public OperationResult Sort(IEnumerable items, int userId = Cms.Core.Constants.Security.SuperUserId) { @@ -2412,7 +2406,6 @@ namespace Umbraco.Cms.Core.Services.Implement /// /// /// - /// /// Result indicating what action was taken when handling the command. public OperationResult Sort(IEnumerable ids, int userId = Cms.Core.Constants.Security.SuperUserId) { diff --git a/src/Umbraco.Infrastructure/Services/Implement/MediaService.cs b/src/Umbraco.Infrastructure/Services/Implement/MediaService.cs index bdf672ce7a..501f675fdc 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/MediaService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/MediaService.cs @@ -653,7 +653,6 @@ namespace Umbraco.Cms.Core.Services.Implement /// /// The to save /// Id of the User saving the Media - /// Optional boolean indicating whether or not to raise events. public Attempt Save(IMedia media, int userId = Cms.Core.Constants.Security.SuperUserId) { EventMessages eventMessages = EventMessagesFactory.Get(); @@ -701,7 +700,6 @@ namespace Umbraco.Cms.Core.Services.Implement /// /// Collection of to save /// Id of the User saving the Media - /// Optional boolean indicating whether or not to raise events. public Attempt Save(IEnumerable medias, int userId = Cms.Core.Constants.Security.SuperUserId) { EventMessages messages = EventMessagesFactory.Get(); @@ -1089,7 +1087,6 @@ namespace Umbraco.Cms.Core.Services.Implement /// /// /// - /// /// True if sorting succeeded, otherwise False public bool Sort(IEnumerable items, int userId = Cms.Core.Constants.Security.SuperUserId) { diff --git a/src/Umbraco.Infrastructure/Services/Implement/UserService.cs b/src/Umbraco.Infrastructure/Services/Implement/UserService.cs index 978dbb5bda..0500c18bdd 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/UserService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/UserService.cs @@ -806,7 +806,6 @@ namespace Umbraco.Cms.Core.Services.Implement /// If null than no changes are made to the users who are assigned to this group, however if a value is passed in /// than all users will be removed from this group and only these users will be added /// - /// Optional parameter to raise events. /// Default is True otherwise set to False to not raise events public void Save(IUserGroup userGroup, int[] userIds = null) { From e8bc2f6dac663b156d064e66c5e14ea012757a96 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 12 Jul 2021 10:58:50 -0600 Subject: [PATCH 04/13] adds tests --- .../Scoping/SupressNotificationsTests.cs | 79 +++++++++++++++++++ .../Services/UserServiceTests.cs | 1 + 2 files changed, 80 insertions(+) create mode 100644 src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/SupressNotificationsTests.cs diff --git a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/SupressNotificationsTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/SupressNotificationsTests.cs new file mode 100644 index 0000000000..1481f303a7 --- /dev/null +++ b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/SupressNotificationsTests.cs @@ -0,0 +1,79 @@ +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using System; +using NUnit.Framework; +using Umbraco.Cms.Core.DependencyInjection; +using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Notifications; +using Umbraco.Cms.Core.Scoping; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Tests.Common.Builders; +using Umbraco.Cms.Tests.Common.Testing; +using Umbraco.Cms.Tests.Integration.Testing; + +namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping +{ + [TestFixture] + [UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)] + public class SupressNotificationsTests : UmbracoIntegrationTest + { + private IContentService ContentService => GetRequiredService(); + + private IContentTypeService ContentTypeService => GetRequiredService(); + + protected override void CustomTestSetup(IUmbracoBuilder builder) + { + base.CustomTestSetup(builder); + + builder.AddNotificationHandler(); + builder.AddNotificationHandler(); + } + + [Test] + public void GivenScope_WhenNotificationsSuppressed_ThenNotificationsDoNotExecute() + { + using IScope scope = ScopeProvider.CreateScope(autoComplete: true); + using IDisposable _ = scope.Notifications.Supress(); + + ContentType contentType = ContentTypeBuilder.CreateBasicContentType(); + ContentTypeService.Save(contentType); + Content content = ContentBuilder.CreateBasicContent(contentType); + ContentService.Save(content); + + Assert.Pass(); + } + + [Test] + public void GivenNestedScope_WhenOuterNotificationsSuppressed_ThenNotificationsDoNotExecute() + { + using (IScope parentScope = ScopeProvider.CreateScope(autoComplete: true)) + { + using IDisposable _ = parentScope.Notifications.Supress(); + + using (IScope childScope = ScopeProvider.CreateScope(autoComplete: true)) + { + ContentType contentType = ContentTypeBuilder.CreateBasicContentType(); + ContentTypeService.Save(contentType); + Content content = ContentBuilder.CreateBasicContent(contentType); + ContentService.Save(content); + + Assert.Pass(); + } + } + } + + private class TestContentNotificationHandler : INotificationHandler + { + public void Handle(ContentSavingNotification notification) + => Assert.Fail("Notification was sent"); + } + + private class TestContentTypeNotificationHandler : INotificationHandler + { + public void Handle(ContentTypeSavingNotification notification) + => Assert.Fail("Notification was sent"); + } + } +} diff --git a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/UserServiceTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/UserServiceTests.cs index f1d1af20c0..62b8456fc7 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/UserServiceTests.cs +++ b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/UserServiceTests.cs @@ -23,6 +23,7 @@ using Umbraco.Extensions; namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services { + /// /// Tests covering the UserService /// From cd20701929de05ba9a4de920c1a186ed8a091369 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 12 Jul 2021 11:13:01 -0600 Subject: [PATCH 05/13] another test --- .../Scoping/SupressNotificationsTests.cs | 35 ++++++++++++++++--- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/SupressNotificationsTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/SupressNotificationsTests.cs index 1481f303a7..4c3fa23504 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/SupressNotificationsTests.cs +++ b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/SupressNotificationsTests.cs @@ -20,8 +20,9 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping public class SupressNotificationsTests : UmbracoIntegrationTest { private IContentService ContentService => GetRequiredService(); - private IContentTypeService ContentTypeService => GetRequiredService(); + private IMediaTypeService MediaTypeService => GetRequiredService(); + private IMediaService MediaService => GetRequiredService(); protected override void CustomTestSetup(IUmbracoBuilder builder) { @@ -29,6 +30,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping builder.AddNotificationHandler(); builder.AddNotificationHandler(); + builder.AddNotificationHandler(); } [Test] @@ -41,8 +43,6 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping ContentTypeService.Save(contentType); Content content = ContentBuilder.CreateBasicContent(contentType); ContentService.Save(content); - - Assert.Pass(); } [Test] @@ -58,18 +58,43 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping ContentTypeService.Save(contentType); Content content = ContentBuilder.CreateBasicContent(contentType); ContentService.Save(content); - - Assert.Pass(); } } } + [Test] + public void GivenSuppressedNotifications_WhenDisposed_ThenNotificationsExecute() + { + int asserted = 0; + using (IScope scope = ScopeProvider.CreateScope(autoComplete: true)) + { + using IDisposable suppressed = scope.Notifications.Supress(); + + MediaType mediaType = MediaTypeBuilder.CreateImageMediaType("test"); + MediaTypeService.Save(mediaType); + + suppressed.Dispose(); + + asserted = TestContext.CurrentContext.AssertCount; + Media media = MediaBuilder.CreateMediaImage(mediaType, -1); + MediaService.Save(media); + } + + Assert.AreEqual(asserted + 1, TestContext.CurrentContext.AssertCount); + } + private class TestContentNotificationHandler : INotificationHandler { public void Handle(ContentSavingNotification notification) => Assert.Fail("Notification was sent"); } + private class TestMediaNotificationHandler : INotificationHandler + { + public void Handle(MediaSavedNotification notification) + => Assert.IsNotNull(notification); + } + private class TestContentTypeNotificationHandler : INotificationHandler { public void Handle(ContentTypeSavingNotification notification) From dcae6924075bafe09bbe85ac6a662836a8e11653 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 12 Jul 2021 11:54:38 -0600 Subject: [PATCH 06/13] Fix typo, adds [CannotSuppressNotification] --- .../Events/IScopedNotificationPublisher.cs | 2 +- .../Events/ScopedNotificationPublisher.cs | 12 ++++++++---- .../CannotSuppressNotificationAttribute.cs | 9 +++++++++ .../ContentRefreshNotification.cs | 2 ++ .../Notifications/MediaRefreshNotification.cs | 1 + .../MemberRefreshNotification.cs | 1 + .../ScopedEntityRemoveNotification.cs | 1 + .../Services/SectionServiceTests.cs | 2 +- .../Scoping/SupressNotificationsTests.cs | 19 ++++++++++++++++--- 9 files changed, 40 insertions(+), 9 deletions(-) create mode 100644 src/Umbraco.Core/Notifications/CannotSuppressNotificationAttribute.cs diff --git a/src/Umbraco.Core/Events/IScopedNotificationPublisher.cs b/src/Umbraco.Core/Events/IScopedNotificationPublisher.cs index 75fbf83860..8ac4f4312d 100644 --- a/src/Umbraco.Core/Events/IScopedNotificationPublisher.cs +++ b/src/Umbraco.Core/Events/IScopedNotificationPublisher.cs @@ -13,7 +13,7 @@ namespace Umbraco.Cms.Core.Events /// Supresses all notifications from being added/created until the result object is disposed. /// /// - IDisposable Supress(); + IDisposable Suppress(); /// /// Publishes a cancelable notification to the notification subscribers diff --git a/src/Umbraco.Core/Events/ScopedNotificationPublisher.cs b/src/Umbraco.Core/Events/ScopedNotificationPublisher.cs index 6ea7ee5b6a..cf6260291b 100644 --- a/src/Umbraco.Core/Events/ScopedNotificationPublisher.cs +++ b/src/Umbraco.Core/Events/ScopedNotificationPublisher.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Reflection; using System.Threading.Tasks; using Umbraco.Cms.Core.Notifications; @@ -28,7 +29,7 @@ namespace Umbraco.Cms.Core.Events throw new ArgumentNullException(nameof(notification)); } - if (_isSuppressed) + if (CanSuppress(notification) && _isSuppressed) { return false; } @@ -44,7 +45,7 @@ namespace Umbraco.Cms.Core.Events throw new ArgumentNullException(nameof(notification)); } - if (_isSuppressed) + if (CanSuppress(notification) && _isSuppressed) { return false; } @@ -60,7 +61,7 @@ namespace Umbraco.Cms.Core.Events throw new ArgumentNullException(nameof(notification)); } - if (_isSuppressed) + if (CanSuppress(notification) && _isSuppressed) { return; } @@ -86,7 +87,7 @@ namespace Umbraco.Cms.Core.Events } } - public IDisposable Supress() + public IDisposable Suppress() { lock(_locker) { @@ -98,6 +99,9 @@ namespace Umbraco.Cms.Core.Events } } + private bool CanSuppress(INotification notification) + => notification.GetType().GetCustomAttribute() == null; + private class Suppressor : IDisposable { private bool _disposedValue; diff --git a/src/Umbraco.Core/Notifications/CannotSuppressNotificationAttribute.cs b/src/Umbraco.Core/Notifications/CannotSuppressNotificationAttribute.cs new file mode 100644 index 0000000000..8279ae4caf --- /dev/null +++ b/src/Umbraco.Core/Notifications/CannotSuppressNotificationAttribute.cs @@ -0,0 +1,9 @@ +using System; + +namespace Umbraco.Cms.Core.Notifications +{ + [AttributeUsage(AttributeTargets.Class)] + internal sealed class CannotSuppressNotificationAttribute : Attribute + { + } +} diff --git a/src/Umbraco.Core/Notifications/ContentRefreshNotification.cs b/src/Umbraco.Core/Notifications/ContentRefreshNotification.cs index 6957da7f70..f2c81d3fac 100644 --- a/src/Umbraco.Core/Notifications/ContentRefreshNotification.cs +++ b/src/Umbraco.Core/Notifications/ContentRefreshNotification.cs @@ -5,7 +5,9 @@ using Umbraco.Cms.Core.Models; namespace Umbraco.Cms.Core.Notifications { + [Obsolete("This is only used for the internal cache and will change, use saved notifications instead")] + [CannotSuppressNotification] [EditorBrowsable(EditorBrowsableState.Never)] public class ContentRefreshNotification : EntityRefreshNotification { diff --git a/src/Umbraco.Core/Notifications/MediaRefreshNotification.cs b/src/Umbraco.Core/Notifications/MediaRefreshNotification.cs index 1c8b8b9bea..4fe0f82d33 100644 --- a/src/Umbraco.Core/Notifications/MediaRefreshNotification.cs +++ b/src/Umbraco.Core/Notifications/MediaRefreshNotification.cs @@ -5,6 +5,7 @@ using Umbraco.Cms.Core.Models; namespace Umbraco.Cms.Core.Notifications { + [CannotSuppressNotification] [Obsolete("This is only used for the internal cache and will change, use tree change notifications instead")] [EditorBrowsable(EditorBrowsableState.Never)] public class MediaRefreshNotification : EntityRefreshNotification diff --git a/src/Umbraco.Core/Notifications/MemberRefreshNotification.cs b/src/Umbraco.Core/Notifications/MemberRefreshNotification.cs index a22c48348f..d4dfeef68f 100644 --- a/src/Umbraco.Core/Notifications/MemberRefreshNotification.cs +++ b/src/Umbraco.Core/Notifications/MemberRefreshNotification.cs @@ -5,6 +5,7 @@ using Umbraco.Cms.Core.Models; namespace Umbraco.Cms.Core.Notifications { + [CannotSuppressNotification] [Obsolete("This is only used for the internal cache and will change, use tree change notifications instead")] [EditorBrowsable(EditorBrowsableState.Never)] public class MemberRefreshNotification : EntityRefreshNotification diff --git a/src/Umbraco.Core/Notifications/ScopedEntityRemoveNotification.cs b/src/Umbraco.Core/Notifications/ScopedEntityRemoveNotification.cs index 307ae2103c..a86ea659bb 100644 --- a/src/Umbraco.Core/Notifications/ScopedEntityRemoveNotification.cs +++ b/src/Umbraco.Core/Notifications/ScopedEntityRemoveNotification.cs @@ -5,6 +5,7 @@ using Umbraco.Cms.Core.Models; namespace Umbraco.Cms.Core.Notifications { + [CannotSuppressNotification] [Obsolete("This is only used for the internal cache and will change, use tree change notifications instead")] [EditorBrowsable(EditorBrowsableState.Never)] public class ScopedEntityRemoveNotification : ObjectNotification diff --git a/src/Umbraco.Tests.Integration/Umbraco.Core/Services/SectionServiceTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Core/Services/SectionServiceTests.cs index 1e9a80faf6..d1f25a8f82 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Core/Services/SectionServiceTests.cs +++ b/src/Umbraco.Tests.Integration/Umbraco.Core/Services/SectionServiceTests.cs @@ -42,7 +42,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Core.Services private IUser CreateTestUser() { using IScope scope = ScopeProvider.CreateScope(autoComplete: true); - using IDisposable _ = scope.Notifications.Supress(); + using IDisposable _ = scope.Notifications.Suppress(); var globalSettings = new GlobalSettings(); var user = new User(globalSettings) diff --git a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/SupressNotificationsTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/SupressNotificationsTests.cs index 4c3fa23504..c40249b886 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/SupressNotificationsTests.cs +++ b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/SupressNotificationsTests.cs @@ -37,7 +37,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping public void GivenScope_WhenNotificationsSuppressed_ThenNotificationsDoNotExecute() { using IScope scope = ScopeProvider.CreateScope(autoComplete: true); - using IDisposable _ = scope.Notifications.Supress(); + using IDisposable _ = scope.Notifications.Suppress(); ContentType contentType = ContentTypeBuilder.CreateBasicContentType(); ContentTypeService.Save(contentType); @@ -50,7 +50,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping { using (IScope parentScope = ScopeProvider.CreateScope(autoComplete: true)) { - using IDisposable _ = parentScope.Notifications.Supress(); + using IDisposable _ = parentScope.Notifications.Suppress(); using (IScope childScope = ScopeProvider.CreateScope(autoComplete: true)) { @@ -68,7 +68,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping int asserted = 0; using (IScope scope = ScopeProvider.CreateScope(autoComplete: true)) { - using IDisposable suppressed = scope.Notifications.Supress(); + using IDisposable suppressed = scope.Notifications.Suppress(); MediaType mediaType = MediaTypeBuilder.CreateImageMediaType("test"); MediaTypeService.Save(mediaType); @@ -83,6 +83,19 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping Assert.AreEqual(asserted + 1, TestContext.CurrentContext.AssertCount); } + [Test] + public void GivenSuppressedNotificationsOnParent_WhenChildSupresses_ThenExceptionIsThrown() + { + using (IScope parentScope = ScopeProvider.CreateScope(autoComplete: true)) + using (IDisposable parentSuppressed = parentScope.Notifications.Suppress()) + { + using (IScope childScope = ScopeProvider.CreateScope(autoComplete: true)) + { + Assert.Throws(() => childScope.Notifications.Suppress()); + } + } + } + private class TestContentNotificationHandler : INotificationHandler { public void Handle(ContentSavingNotification notification) From 1fc4bf6dcde744d1b4ec67e1c590ef08118ac170 Mon Sep 17 00:00:00 2001 From: Shannon Deminick Date: Wed, 14 Jul 2021 01:19:06 +1000 Subject: [PATCH 07/13] Update src/Umbraco.Core/Events/IScopedNotificationPublisher.cs Co-authored-by: Andy Butland --- src/Umbraco.Core/Events/IScopedNotificationPublisher.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Core/Events/IScopedNotificationPublisher.cs b/src/Umbraco.Core/Events/IScopedNotificationPublisher.cs index 8ac4f4312d..58fdafc341 100644 --- a/src/Umbraco.Core/Events/IScopedNotificationPublisher.cs +++ b/src/Umbraco.Core/Events/IScopedNotificationPublisher.cs @@ -10,7 +10,7 @@ namespace Umbraco.Cms.Core.Events public interface IScopedNotificationPublisher { /// - /// Supresses all notifications from being added/created until the result object is disposed. + /// Suppresses all notifications from being added/created until the result object is disposed. /// /// IDisposable Suppress(); From 47a8a3f5ffa71f76c92f09357e8ec1ad2246acaf Mon Sep 17 00:00:00 2001 From: Shannon Deminick Date: Wed, 14 Jul 2021 01:19:14 +1000 Subject: [PATCH 08/13] Update src/Umbraco.Infrastructure/Services/Implement/MediaService.cs Co-authored-by: Andy Butland --- src/Umbraco.Infrastructure/Services/Implement/MediaService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Infrastructure/Services/Implement/MediaService.cs b/src/Umbraco.Infrastructure/Services/Implement/MediaService.cs index 501f675fdc..f26bb68342 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/MediaService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/MediaService.cs @@ -685,7 +685,7 @@ namespace Umbraco.Cms.Core.Services.Implement _mediaRepository.Save(media); scope.Notifications.Publish(new MediaSavedNotification(media, eventMessages).WithStateFrom(savingNotification)); - // TODO: See note about supressing events in content service + // TODO: See note about suppressing events in content service scope.Notifications.Publish(new MediaTreeChangeNotification(media, TreeChangeTypes.RefreshNode, eventMessages)); Audit(AuditType.Save, userId, media.Id); From 127c0e1be0cea803da641262aefd5bcf76faa000 Mon Sep 17 00:00:00 2001 From: Shannon Deminick Date: Wed, 14 Jul 2021 01:19:19 +1000 Subject: [PATCH 09/13] Update src/Umbraco.Infrastructure/Services/Implement/MediaService.cs Co-authored-by: Andy Butland --- src/Umbraco.Infrastructure/Services/Implement/MediaService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Infrastructure/Services/Implement/MediaService.cs b/src/Umbraco.Infrastructure/Services/Implement/MediaService.cs index f26bb68342..9dbf33a4d1 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/MediaService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/MediaService.cs @@ -728,7 +728,7 @@ namespace Umbraco.Cms.Core.Services.Implement } scope.Notifications.Publish(new MediaSavedNotification(mediasA, messages).WithStateFrom(savingNotification)); - // TODO: See note about supressing events in content service + // TODO: See note about suppressing events in content service scope.Notifications.Publish(new MediaTreeChangeNotification(treeChanges, messages)); Audit(AuditType.Save, userId == -1 ? 0 : userId, Cms.Core.Constants.System.Root, "Bulk save media"); From a39f5da65b0e2fc74b7636a8fbbfa1d5b7d8d701 Mon Sep 17 00:00:00 2001 From: Shannon Deminick Date: Wed, 14 Jul 2021 01:19:26 +1000 Subject: [PATCH 10/13] Update src/Umbraco.Infrastructure/Services/Implement/MediaService.cs Co-authored-by: Andy Butland --- src/Umbraco.Infrastructure/Services/Implement/MediaService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Infrastructure/Services/Implement/MediaService.cs b/src/Umbraco.Infrastructure/Services/Implement/MediaService.cs index 9dbf33a4d1..0239ad8e60 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/MediaService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/MediaService.cs @@ -1129,7 +1129,7 @@ namespace Umbraco.Cms.Core.Services.Implement } scope.Notifications.Publish(new MediaSavedNotification(itemsA, messages).WithStateFrom(savingNotification)); - // TODO: See note about supressing events in content service + // TODO: See note about suppressing events in content service scope.Notifications.Publish(new MediaTreeChangeNotification(saved, TreeChangeTypes.RefreshNode, messages)); Audit(AuditType.Sort, userId, 0); From e30bc55ffede0e7d80bb8df12f83718a56c77c6d Mon Sep 17 00:00:00 2001 From: Shannon Deminick Date: Wed, 14 Jul 2021 01:19:35 +1000 Subject: [PATCH 11/13] Update src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/SupressNotificationsTests.cs Co-authored-by: Andy Butland --- .../Umbraco.Infrastructure/Scoping/SupressNotificationsTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/SupressNotificationsTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/SupressNotificationsTests.cs index c40249b886..4d71959417 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/SupressNotificationsTests.cs +++ b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/SupressNotificationsTests.cs @@ -17,7 +17,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping { [TestFixture] [UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)] - public class SupressNotificationsTests : UmbracoIntegrationTest + public class SuppressNotificationsTests : UmbracoIntegrationTest { private IContentService ContentService => GetRequiredService(); private IContentTypeService ContentTypeService => GetRequiredService(); From 3e586991df01b0030ba58b8aecbf2940faa37e88 Mon Sep 17 00:00:00 2001 From: Shannon Deminick Date: Wed, 14 Jul 2021 01:19:40 +1000 Subject: [PATCH 12/13] Update src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/SupressNotificationsTests.cs Co-authored-by: Andy Butland --- .../Umbraco.Infrastructure/Scoping/SupressNotificationsTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/SupressNotificationsTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/SupressNotificationsTests.cs index 4d71959417..c5cff66d0a 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/SupressNotificationsTests.cs +++ b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/SupressNotificationsTests.cs @@ -84,7 +84,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping } [Test] - public void GivenSuppressedNotificationsOnParent_WhenChildSupresses_ThenExceptionIsThrown() + public void GivenSuppressedNotificationsOnParent_WhenChildSuppresses_ThenExceptionIsThrown() { using (IScope parentScope = ScopeProvider.CreateScope(autoComplete: true)) using (IDisposable parentSuppressed = parentScope.Notifications.Suppress()) From 2fc0fc685020a91b2cf710b55a810c782ace2c68 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 14 Jul 2021 11:56:30 -0600 Subject: [PATCH 13/13] Removes CannotSuppressNotificationAttribute --- src/Umbraco.Core/Events/ScopedNotificationPublisher.cs | 9 +++------ .../Notifications/CannotSuppressNotificationAttribute.cs | 9 --------- .../Notifications/ContentRefreshNotification.cs | 1 - .../Notifications/MediaRefreshNotification.cs | 1 - .../Notifications/MemberRefreshNotification.cs | 1 - .../Notifications/ScopedEntityRemoveNotification.cs | 1 - 6 files changed, 3 insertions(+), 19 deletions(-) delete mode 100644 src/Umbraco.Core/Notifications/CannotSuppressNotificationAttribute.cs diff --git a/src/Umbraco.Core/Events/ScopedNotificationPublisher.cs b/src/Umbraco.Core/Events/ScopedNotificationPublisher.cs index cf6260291b..7261b514bf 100644 --- a/src/Umbraco.Core/Events/ScopedNotificationPublisher.cs +++ b/src/Umbraco.Core/Events/ScopedNotificationPublisher.cs @@ -29,7 +29,7 @@ namespace Umbraco.Cms.Core.Events throw new ArgumentNullException(nameof(notification)); } - if (CanSuppress(notification) && _isSuppressed) + if (_isSuppressed) { return false; } @@ -45,7 +45,7 @@ namespace Umbraco.Cms.Core.Events throw new ArgumentNullException(nameof(notification)); } - if (CanSuppress(notification) && _isSuppressed) + if (_isSuppressed) { return false; } @@ -61,7 +61,7 @@ namespace Umbraco.Cms.Core.Events throw new ArgumentNullException(nameof(notification)); } - if (CanSuppress(notification) && _isSuppressed) + if (_isSuppressed) { return; } @@ -99,9 +99,6 @@ namespace Umbraco.Cms.Core.Events } } - private bool CanSuppress(INotification notification) - => notification.GetType().GetCustomAttribute() == null; - private class Suppressor : IDisposable { private bool _disposedValue; diff --git a/src/Umbraco.Core/Notifications/CannotSuppressNotificationAttribute.cs b/src/Umbraco.Core/Notifications/CannotSuppressNotificationAttribute.cs deleted file mode 100644 index 8279ae4caf..0000000000 --- a/src/Umbraco.Core/Notifications/CannotSuppressNotificationAttribute.cs +++ /dev/null @@ -1,9 +0,0 @@ -using System; - -namespace Umbraco.Cms.Core.Notifications -{ - [AttributeUsage(AttributeTargets.Class)] - internal sealed class CannotSuppressNotificationAttribute : Attribute - { - } -} diff --git a/src/Umbraco.Core/Notifications/ContentRefreshNotification.cs b/src/Umbraco.Core/Notifications/ContentRefreshNotification.cs index f2c81d3fac..b9cda7722c 100644 --- a/src/Umbraco.Core/Notifications/ContentRefreshNotification.cs +++ b/src/Umbraco.Core/Notifications/ContentRefreshNotification.cs @@ -7,7 +7,6 @@ namespace Umbraco.Cms.Core.Notifications { [Obsolete("This is only used for the internal cache and will change, use saved notifications instead")] - [CannotSuppressNotification] [EditorBrowsable(EditorBrowsableState.Never)] public class ContentRefreshNotification : EntityRefreshNotification { diff --git a/src/Umbraco.Core/Notifications/MediaRefreshNotification.cs b/src/Umbraco.Core/Notifications/MediaRefreshNotification.cs index 4fe0f82d33..1c8b8b9bea 100644 --- a/src/Umbraco.Core/Notifications/MediaRefreshNotification.cs +++ b/src/Umbraco.Core/Notifications/MediaRefreshNotification.cs @@ -5,7 +5,6 @@ using Umbraco.Cms.Core.Models; namespace Umbraco.Cms.Core.Notifications { - [CannotSuppressNotification] [Obsolete("This is only used for the internal cache and will change, use tree change notifications instead")] [EditorBrowsable(EditorBrowsableState.Never)] public class MediaRefreshNotification : EntityRefreshNotification diff --git a/src/Umbraco.Core/Notifications/MemberRefreshNotification.cs b/src/Umbraco.Core/Notifications/MemberRefreshNotification.cs index d4dfeef68f..a22c48348f 100644 --- a/src/Umbraco.Core/Notifications/MemberRefreshNotification.cs +++ b/src/Umbraco.Core/Notifications/MemberRefreshNotification.cs @@ -5,7 +5,6 @@ using Umbraco.Cms.Core.Models; namespace Umbraco.Cms.Core.Notifications { - [CannotSuppressNotification] [Obsolete("This is only used for the internal cache and will change, use tree change notifications instead")] [EditorBrowsable(EditorBrowsableState.Never)] public class MemberRefreshNotification : EntityRefreshNotification diff --git a/src/Umbraco.Core/Notifications/ScopedEntityRemoveNotification.cs b/src/Umbraco.Core/Notifications/ScopedEntityRemoveNotification.cs index a86ea659bb..307ae2103c 100644 --- a/src/Umbraco.Core/Notifications/ScopedEntityRemoveNotification.cs +++ b/src/Umbraco.Core/Notifications/ScopedEntityRemoveNotification.cs @@ -5,7 +5,6 @@ using Umbraco.Cms.Core.Models; namespace Umbraco.Cms.Core.Notifications { - [CannotSuppressNotification] [Obsolete("This is only used for the internal cache and will change, use tree change notifications instead")] [EditorBrowsable(EditorBrowsableState.Never)] public class ScopedEntityRemoveNotification : ObjectNotification