From 34949f98e9f105cf07352d602a0ac5e155252d22 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Thu, 21 Oct 2021 16:09:34 +0200 Subject: [PATCH 1/3] Instead of only using first event, we combine events of same type into a single event with multiple arguments --- .../Cache/DistributedCacheBinderTests.cs | 72 ++++++++++++-- .../Cache/DistributedCacheBinder.cs | 94 +++++++++++++++++++ 2 files changed, 159 insertions(+), 7 deletions(-) diff --git a/src/Umbraco.Tests/Cache/DistributedCacheBinderTests.cs b/src/Umbraco.Tests/Cache/DistributedCacheBinderTests.cs index 00a33c0b6c52..45bb0ffb4072 100644 --- a/src/Umbraco.Tests/Cache/DistributedCacheBinderTests.cs +++ b/src/Umbraco.Tests/Cache/DistributedCacheBinderTests.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Linq; using System.Threading; using Moq; @@ -10,6 +11,7 @@ using Umbraco.Core.Models.Membership; using Umbraco.Core.Services; using Umbraco.Core.Services.Changes; +using Umbraco.Tests.TestHelpers.Entities; using Umbraco.Tests.Testing; using Umbraco.Tests.Testing.Objects.Accessors; using Umbraco.Web; @@ -170,15 +172,71 @@ public void CanHandleEvent() [Test] public void OnlyHandlesOnContentTypeEvent() { - var definitions = new IEventDefinition[] + var num = 30; + var contentTypes = Enumerable.Repeat(MockedContentTypes.CreateBasicContentType(), num); + var mediaTypes = Enumerable.Repeat(MockedContentTypes.CreateImageMediaType(), num); + var memberTypes = Enumerable.Repeat(MockedContentTypes.CreateSimpleMemberType(), num); + var definitionsContent = contentTypes.SelectMany(x=> new IEventDefinition[] { - new EventDefinition.EventArgs>(null, Current.Services.ContentTypeService, new ContentTypeChange.EventArgs(Enumerable.Empty>()), "Changed"), - new EventDefinition>(null, Current.Services.ContentTypeService, new SaveEventArgs(Enumerable.Empty()), "Saved"), - new EventDefinition.EventArgs>(null, Current.Services.ContentTypeService, new ContentTypeChange.EventArgs(Enumerable.Empty>()), "Changed"), - new EventDefinition>(null, Current.Services.ContentTypeService, new SaveEventArgs(Enumerable.Empty()), "Saved"), - }; + new EventDefinition.EventArgs>(null, Current.Services.ContentTypeService, new ContentTypeChange.EventArgs(new ContentTypeChange(x, ContentTypeChangeTypes.Create)), "Changed"), + new EventDefinition>(null, Current.Services.ContentTypeService, new SaveEventArgs(x), "Saved"), + }); + + var definitionsMedia =mediaTypes.SelectMany(x=> new IEventDefinition[] + { + new EventDefinition.EventArgs>(null, Current.Services.MediaTypeService, new ContentTypeChange.EventArgs(new ContentTypeChange(x, ContentTypeChangeTypes.Create)), "Changed"), + new EventDefinition>(null, Current.Services.MediaTypeService, new SaveEventArgs(x), "Saved"), + }); + var definitionsMember =memberTypes.SelectMany(x=> new IEventDefinition[] + { + new EventDefinition.EventArgs>(null, Current.Services.MemberTypeService, new ContentTypeChange.EventArgs(new ContentTypeChange(x, ContentTypeChangeTypes.Create)), "Changed"), + new EventDefinition>(null, Current.Services.MemberTypeService, new SaveEventArgs(x), "Saved"), + }); + + var definitions = new List(); + definitions.AddRange(definitionsContent); + definitions.AddRange(definitionsMedia); + definitions.AddRange(definitionsMember); + var result = DistributedCacheBinder.GetReducedEventList(definitions); - Assert.AreEqual(1, result.Count()); + + Assert.Multiple(() => + { + Assert.AreEqual(num*6, definitions.Count(), "Precondition is we have many definitions"); + Assert.AreEqual(6, result.Count(), "Unexpect number of reduced definitions"); + foreach (var eventDefinition in result) + { + if (eventDefinition.Args is SaveEventArgs saveContentEventArgs) + { + Assert.AreEqual(num, saveContentEventArgs.SavedEntities.Count()); + } + + if (eventDefinition.Args is ContentTypeChange.EventArgs changeContentEventArgs) + { + Assert.AreEqual(num, changeContentEventArgs.Changes.Count()); + } + + if (eventDefinition.Args is SaveEventArgs saveMediaEventArgs) + { + Assert.AreEqual(num, saveMediaEventArgs.SavedEntities.Count()); + } + + if (eventDefinition.Args is ContentTypeChange.EventArgs changeMediaEventArgs) + { + Assert.AreEqual(num, changeMediaEventArgs.Changes.Count()); + } + + if (eventDefinition.Args is SaveEventArgs saveMemberEventArgs) + { + Assert.AreEqual(num, saveMemberEventArgs.SavedEntities.Count()); + } + + if (eventDefinition.Args is ContentTypeChange.EventArgs changeMemberEventArgs) + { + Assert.AreEqual(num, changeMemberEventArgs.Changes.Count()); + } + } + }); } } } diff --git a/src/Umbraco.Web/Cache/DistributedCacheBinder.cs b/src/Umbraco.Web/Cache/DistributedCacheBinder.cs index e3a5a01d54d8..5412fe37a57f 100644 --- a/src/Umbraco.Web/Cache/DistributedCacheBinder.cs +++ b/src/Umbraco.Web/Cache/DistributedCacheBinder.cs @@ -6,6 +6,9 @@ using Umbraco.Core; using Umbraco.Core.Events; using Umbraco.Core.Logging; +using Umbraco.Core.Models; +using Umbraco.Core.Services; +using Umbraco.Core.Services.Changes; namespace Umbraco.Web.Cache { @@ -90,6 +93,97 @@ internal static IEnumerable GetReducedEventList(IEnumerable(); + var grouped = events.GroupBy(x => x.GetType()); + + foreach (var group in grouped) + { + if (group.Key == typeof(EventDefinition>)) + { + var groupedGroups = group.GroupBy(x => (x.EventName, x.Sender)); + + foreach (var groupedGroup in groupedGroups) + { + reducedEvents.Add(new EventDefinition>( + null, + (IContentTypeService)groupedGroup.Key.Sender, + new SaveEventArgs(groupedGroup.SelectMany(x => ((SaveEventArgs)x.Args).SavedEntities)), + groupedGroup.Key.EventName)); + } + } + else if (group.Key == typeof(EventDefinition.EventArgs>)) + { + var groupedGroups = group.GroupBy(x => (x.EventName, x.Sender)); + + foreach (var groupedGroup in groupedGroups) + { + reducedEvents.Add(new EventDefinition.EventArgs>( + null, + (IContentTypeService)groupedGroup.Key.Sender, + new ContentTypeChange.EventArgs(groupedGroup.SelectMany(x => ((ContentTypeChange.EventArgs)x.Args).Changes)), + groupedGroup.Key.EventName)); + } + + } + else if (group.Key == typeof(EventDefinition>)) + { + var groupedGroups = group.GroupBy(x => (x.EventName, x.Sender)); + + foreach (var groupedGroup in groupedGroups) + { + reducedEvents.Add(new EventDefinition>( + null, + (IMediaTypeService)groupedGroup.Key.Sender, + new SaveEventArgs(groupedGroup.SelectMany(x => ((SaveEventArgs)x.Args).SavedEntities)), + groupedGroup.Key.EventName)); + } + } + else if (group.Key == typeof(EventDefinition.EventArgs>)) + { + var groupedGroups = group.GroupBy(x => (x.EventName, x.Sender)); + + foreach (var groupedGroup in groupedGroups) + { + reducedEvents.Add(new EventDefinition.EventArgs>( + null, + (IMediaTypeService)groupedGroup.Key.Sender, + new ContentTypeChange.EventArgs(groupedGroup.SelectMany(x => ((ContentTypeChange.EventArgs)x.Args).Changes)), + groupedGroup.Key.EventName)); + } + } + else if (group.Key == typeof(EventDefinition>)) + { + var groupedGroups = group.GroupBy(x => (x.EventName, x.Sender)); + + foreach (var groupedGroup in groupedGroups) + { + reducedEvents.Add(new EventDefinition>( + null, + (IMemberTypeService)groupedGroup.Key.Sender, + new SaveEventArgs(groupedGroup.SelectMany(x => ((SaveEventArgs)x.Args).SavedEntities)), + groupedGroup.Key.EventName)); + } + } + else if (group.Key == typeof(EventDefinition.EventArgs>)) + { + var groupedGroups = group.GroupBy(x => (x.EventName, x.Sender)); + + foreach (var groupedGroup in groupedGroups) + { + reducedEvents.Add(new EventDefinition.EventArgs>( + null, + (IMemberTypeService)groupedGroup.Key.Sender, + new ContentTypeChange.EventArgs(groupedGroup.SelectMany(x => ((ContentTypeChange.EventArgs)x.Args).Changes)), + groupedGroup.Key.EventName)); + } + } + else + { + reducedEvents.AddRange(group); + } + } + + return reducedEvents; + var gotDoumentType = false; var gotMediaType = false; var gotMemberType = false; From 0e4c20a39f2b0cddcce1d6d1b72ecc5576ace63f Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Thu, 21 Oct 2021 16:50:58 +0200 Subject: [PATCH 2/3] Added generic method to DRY up grouping logic. --- .../Cache/DistributedCacheBinderTests.cs | 12 +- .../Cache/DistributedCacheBinder.cs | 137 +++++------------- 2 files changed, 43 insertions(+), 106 deletions(-) diff --git a/src/Umbraco.Tests/Cache/DistributedCacheBinderTests.cs b/src/Umbraco.Tests/Cache/DistributedCacheBinderTests.cs index 45bb0ffb4072..0cbeed835675 100644 --- a/src/Umbraco.Tests/Cache/DistributedCacheBinderTests.cs +++ b/src/Umbraco.Tests/Cache/DistributedCacheBinderTests.cs @@ -170,24 +170,24 @@ public void CanHandleEvent() } [Test] - public void OnlyHandlesOnContentTypeEvent() + public void GroupsContentTypeEvents() { var num = 30; var contentTypes = Enumerable.Repeat(MockedContentTypes.CreateBasicContentType(), num); var mediaTypes = Enumerable.Repeat(MockedContentTypes.CreateImageMediaType(), num); var memberTypes = Enumerable.Repeat(MockedContentTypes.CreateSimpleMemberType(), num); - var definitionsContent = contentTypes.SelectMany(x=> new IEventDefinition[] + var definitionsContent = contentTypes.SelectMany(x => new IEventDefinition[] { new EventDefinition.EventArgs>(null, Current.Services.ContentTypeService, new ContentTypeChange.EventArgs(new ContentTypeChange(x, ContentTypeChangeTypes.Create)), "Changed"), new EventDefinition>(null, Current.Services.ContentTypeService, new SaveEventArgs(x), "Saved"), }); - var definitionsMedia =mediaTypes.SelectMany(x=> new IEventDefinition[] + var definitionsMedia = mediaTypes.SelectMany(x => new IEventDefinition[] { new EventDefinition.EventArgs>(null, Current.Services.MediaTypeService, new ContentTypeChange.EventArgs(new ContentTypeChange(x, ContentTypeChangeTypes.Create)), "Changed"), new EventDefinition>(null, Current.Services.MediaTypeService, new SaveEventArgs(x), "Saved"), }); - var definitionsMember =memberTypes.SelectMany(x=> new IEventDefinition[] + var definitionsMember = memberTypes.SelectMany(x => new IEventDefinition[] { new EventDefinition.EventArgs>(null, Current.Services.MemberTypeService, new ContentTypeChange.EventArgs(new ContentTypeChange(x, ContentTypeChangeTypes.Create)), "Changed"), new EventDefinition>(null, Current.Services.MemberTypeService, new SaveEventArgs(x), "Saved"), @@ -202,8 +202,8 @@ public void OnlyHandlesOnContentTypeEvent() Assert.Multiple(() => { - Assert.AreEqual(num*6, definitions.Count(), "Precondition is we have many definitions"); - Assert.AreEqual(6, result.Count(), "Unexpect number of reduced definitions"); + Assert.AreEqual(num * 6, definitions.Count(), "Precondition is we have many definitions"); + Assert.AreEqual(6, result.Count(), "Unexpected number of reduced definitions"); foreach (var eventDefinition in result) { if (eventDefinition.Args is SaveEventArgs saveContentEventArgs) diff --git a/src/Umbraco.Web/Cache/DistributedCacheBinder.cs b/src/Umbraco.Web/Cache/DistributedCacheBinder.cs index 5412fe37a57f..074867888661 100644 --- a/src/Umbraco.Web/Cache/DistributedCacheBinder.cs +++ b/src/Umbraco.Web/Cache/DistributedCacheBinder.cs @@ -69,9 +69,7 @@ public void HandleEvents(IEnumerable events) using (_umbracoContextFactory.EnsureUmbracoContext()) { // When it comes to content types types, a change to any single one will trigger a reload of the content and media caches. - // As far as I (AB) can tell, there's no type specific logic here, they all clear caches for all content types, and trigger a reload of all content and media. - // We also have events registered for Changed and Saved, which do the same thing, so really only need one of these. - // Hence if we have more than one document or media types, we can and should only handle one of the events for one, to avoid repeated cache reloads. + // We can reduce the impact of that by grouping the events to invoke just one per type, providing a collection of the individual arguments. foreach (var e in GetReducedEventList(events)) { var handler = FindHandler(e); @@ -91,7 +89,7 @@ public void HandleEvents(IEnumerable events) // Internal for tests internal static IEnumerable GetReducedEventList(IEnumerable events) { - var reducedEvents = new List(); + var groupedEvents = new List(); var grouped = events.GroupBy(x => x.GetType()); @@ -99,128 +97,67 @@ internal static IEnumerable GetReducedEventList(IEnumerable>)) { - var groupedGroups = group.GroupBy(x => (x.EventName, x.Sender)); - - foreach (var groupedGroup in groupedGroups) - { - reducedEvents.Add(new EventDefinition>( - null, - (IContentTypeService)groupedGroup.Key.Sender, - new SaveEventArgs(groupedGroup.SelectMany(x => ((SaveEventArgs)x.Args).SavedEntities)), - groupedGroup.Key.EventName)); - } + GroupSaveEvents(groupedEvents, group); } else if (group.Key == typeof(EventDefinition.EventArgs>)) { - var groupedGroups = group.GroupBy(x => (x.EventName, x.Sender)); - - foreach (var groupedGroup in groupedGroups) - { - reducedEvents.Add(new EventDefinition.EventArgs>( - null, - (IContentTypeService)groupedGroup.Key.Sender, - new ContentTypeChange.EventArgs(groupedGroup.SelectMany(x => ((ContentTypeChange.EventArgs)x.Args).Changes)), - groupedGroup.Key.EventName)); - } - + GroupChangeEvents(groupedEvents, group); } else if (group.Key == typeof(EventDefinition>)) { - var groupedGroups = group.GroupBy(x => (x.EventName, x.Sender)); - - foreach (var groupedGroup in groupedGroups) - { - reducedEvents.Add(new EventDefinition>( - null, - (IMediaTypeService)groupedGroup.Key.Sender, - new SaveEventArgs(groupedGroup.SelectMany(x => ((SaveEventArgs)x.Args).SavedEntities)), - groupedGroup.Key.EventName)); - } + GroupSaveEvents(groupedEvents, group); } else if (group.Key == typeof(EventDefinition.EventArgs>)) { - var groupedGroups = group.GroupBy(x => (x.EventName, x.Sender)); - - foreach (var groupedGroup in groupedGroups) - { - reducedEvents.Add(new EventDefinition.EventArgs>( - null, - (IMediaTypeService)groupedGroup.Key.Sender, - new ContentTypeChange.EventArgs(groupedGroup.SelectMany(x => ((ContentTypeChange.EventArgs)x.Args).Changes)), - groupedGroup.Key.EventName)); - } + GroupChangeEvents(groupedEvents, group); } else if (group.Key == typeof(EventDefinition>)) { - var groupedGroups = group.GroupBy(x => (x.EventName, x.Sender)); - - foreach (var groupedGroup in groupedGroups) - { - reducedEvents.Add(new EventDefinition>( - null, - (IMemberTypeService)groupedGroup.Key.Sender, - new SaveEventArgs(groupedGroup.SelectMany(x => ((SaveEventArgs)x.Args).SavedEntities)), - groupedGroup.Key.EventName)); - } + GroupSaveEvents(groupedEvents, group); } else if (group.Key == typeof(EventDefinition.EventArgs>)) { - var groupedGroups = group.GroupBy(x => (x.EventName, x.Sender)); - - foreach (var groupedGroup in groupedGroups) - { - reducedEvents.Add(new EventDefinition.EventArgs>( - null, - (IMemberTypeService)groupedGroup.Key.Sender, - new ContentTypeChange.EventArgs(groupedGroup.SelectMany(x => ((ContentTypeChange.EventArgs)x.Args).Changes)), - groupedGroup.Key.EventName)); - } + GroupChangeEvents(groupedEvents, group); } else { - reducedEvents.AddRange(group); + groupedEvents.AddRange(group); } } - return reducedEvents; + return groupedEvents; + } - var gotDoumentType = false; - var gotMediaType = false; - var gotMemberType = false; + private static void GroupSaveEvents(List groupedEvents, IGrouping group) + where TService : IContentTypeBaseService + where TType : IContentTypeBase + { + var groupedGroups = group.GroupBy(x => (x.EventName, x.Sender)); - foreach (var evt in events) + foreach (var groupedGroup in groupedGroups) { - if (evt.Sender.ToString().Contains(nameof(Core.Services.Implement.ContentTypeService))) - { - if (gotDoumentType == false) - { - reducedEvents.Add(evt); - gotDoumentType = true; - } - } - else if (evt.Sender.ToString().Contains(nameof(Core.Services.Implement.MediaTypeService))) - { - if (gotMediaType == false) - { - reducedEvents.Add(evt); - gotMediaType = true; - } - } - else if (evt.Sender.ToString().Contains(nameof(Core.Services.Implement.MemberTypeService))) - { - if (gotMemberType == false) - { - reducedEvents.Add(evt); - gotMemberType = true; - } - } - else - { - reducedEvents.Add(evt); - } + groupedEvents.Add(new EventDefinition>( + null, + (TService)groupedGroup.Key.Sender, + new SaveEventArgs(groupedGroup.SelectMany(x => ((SaveEventArgs)x.Args).SavedEntities)), + groupedGroup.Key.EventName)); } + } + + private static void GroupChangeEvents(List groupedEvents, IGrouping group) + where TService : IContentTypeBaseService + where TType : class, IContentTypeComposition + { + var groupedGroups = group.GroupBy(x => (x.EventName, x.Sender)); - return reducedEvents; + foreach (var groupedGroup in groupedGroups) + { + groupedEvents.Add(new EventDefinition.EventArgs>( + null, + (TService)groupedGroup.Key.Sender, + new ContentTypeChange.EventArgs(groupedGroup.SelectMany(x => ((ContentTypeChange.EventArgs)x.Args).Changes)), + groupedGroup.Key.EventName)); + } } } } From c9dd7cfeca0c5148f23d0981c8eccefb20dac613 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Thu, 21 Oct 2021 17:07:20 +0200 Subject: [PATCH 3/3] Renamed method to better reflect new functionality. --- src/Umbraco.Tests/Cache/DistributedCacheBinderTests.cs | 2 +- src/Umbraco.Web/Cache/DistributedCacheBinder.cs | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Tests/Cache/DistributedCacheBinderTests.cs b/src/Umbraco.Tests/Cache/DistributedCacheBinderTests.cs index 0cbeed835675..7037e1bea897 100644 --- a/src/Umbraco.Tests/Cache/DistributedCacheBinderTests.cs +++ b/src/Umbraco.Tests/Cache/DistributedCacheBinderTests.cs @@ -198,7 +198,7 @@ public void GroupsContentTypeEvents() definitions.AddRange(definitionsMedia); definitions.AddRange(definitionsMember); - var result = DistributedCacheBinder.GetReducedEventList(definitions); + var result = DistributedCacheBinder.GetGroupedEventList(definitions); Assert.Multiple(() => { diff --git a/src/Umbraco.Web/Cache/DistributedCacheBinder.cs b/src/Umbraco.Web/Cache/DistributedCacheBinder.cs index 074867888661..bfb1a01a6919 100644 --- a/src/Umbraco.Web/Cache/DistributedCacheBinder.cs +++ b/src/Umbraco.Web/Cache/DistributedCacheBinder.cs @@ -70,7 +70,8 @@ public void HandleEvents(IEnumerable events) { // When it comes to content types types, a change to any single one will trigger a reload of the content and media caches. // We can reduce the impact of that by grouping the events to invoke just one per type, providing a collection of the individual arguments. - foreach (var e in GetReducedEventList(events)) + var groupedEvents = GetGroupedEventList(events); + foreach (var e in groupedEvents) { var handler = FindHandler(e); if (handler == null) @@ -87,7 +88,7 @@ public void HandleEvents(IEnumerable events) } // Internal for tests - internal static IEnumerable GetReducedEventList(IEnumerable events) + internal static IEnumerable GetGroupedEventList(IEnumerable events) { var groupedEvents = new List();