From 306ed56027210a767d7acc74e991b5885f0135ba Mon Sep 17 00:00:00 2001 From: Nikolaj Date: Mon, 23 Aug 2021 14:28:44 +0200 Subject: [PATCH 1/8] Create a cache for value editors --- src/Umbraco.Core/Cache/IValueEditorCache.cs | 10 ++++ src/Umbraco.Core/Cache/ValueEditorCache.cs | 57 +++++++++++++++++++ .../DependencyInjection/UmbracoBuilder.cs | 6 ++ .../Implement/PropertyValidationService.cs | 11 +++- .../Umbraco.Core/Models/VariationTests.cs | 4 +- 5 files changed, 85 insertions(+), 3 deletions(-) create mode 100644 src/Umbraco.Core/Cache/IValueEditorCache.cs create mode 100644 src/Umbraco.Core/Cache/ValueEditorCache.cs diff --git a/src/Umbraco.Core/Cache/IValueEditorCache.cs b/src/Umbraco.Core/Cache/IValueEditorCache.cs new file mode 100644 index 0000000000..72b85746c4 --- /dev/null +++ b/src/Umbraco.Core/Cache/IValueEditorCache.cs @@ -0,0 +1,10 @@ +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.PropertyEditors; + +namespace Umbraco.Cms.Core.Cache +{ + public interface IValueEditorCache + { + public IDataValueEditor GetValueEditor(IDataEditor dataEditor, IDataType dataType); + } +} diff --git a/src/Umbraco.Core/Cache/ValueEditorCache.cs b/src/Umbraco.Core/Cache/ValueEditorCache.cs new file mode 100644 index 0000000000..81f9348d60 --- /dev/null +++ b/src/Umbraco.Core/Cache/ValueEditorCache.cs @@ -0,0 +1,57 @@ +using System.Collections.Generic; +using System.Linq; +using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Notifications; +using Umbraco.Cms.Core.PropertyEditors; + +namespace Umbraco.Cms.Core.Cache +{ + public class ValueEditorCache : IValueEditorCache, + INotificationHandler, + INotificationHandler + { + private readonly Dictionary> _valueEditorCache = new(); + + public IDataValueEditor GetValueEditor(IDataEditor editor, IDataType dataType) + { + // Instead of creating a value editor immediately check if we've already created one and use that. + IDataValueEditor valueEditor; + if (_valueEditorCache.TryGetValue(editor.Alias, out Dictionary dataEditorCache)) + { + if (dataEditorCache.TryGetValue(dataType.Id, out valueEditor)) + { + return valueEditor; + } + + valueEditor = editor.GetValueEditor(dataType.Configuration); + dataEditorCache[dataType.Id] = valueEditor; + return valueEditor; + + } + + valueEditor = editor.GetValueEditor(dataType.Configuration); + _valueEditorCache[editor.Alias] = new Dictionary { [dataType.Id] = valueEditor }; + return valueEditor; + } + + public void Handle(DataTypeSavedNotification notification) => + ClearCache(notification.SavedEntities.Select(x => x.Id)); + + public void Handle(DataTypeDeletedNotification notification) => + ClearCache(notification.DeletedEntities.Select(x => x.Id)); + + private void ClearCache(IEnumerable dataTypeIds) + { + // If a datatype is saved or deleted we have to clear any value editors based on their ID from the cache, + // since it could mean that their configuration has changed. + foreach (var id in dataTypeIds) + { + foreach (Dictionary editors in _valueEditorCache.Values) + { + editors.Remove(id); + } + } + } + } +} diff --git a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs index 648af8f082..35239121c0 100644 --- a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs +++ b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs @@ -253,6 +253,12 @@ namespace Umbraco.Cms.Core.DependencyInjection // register a basic/noop published snapshot service to be replaced Services.AddSingleton(); + + // Register ValueEditorCache used for validation + Services.AddSingleton(); + Services + .AddNotificationHandler() + .AddNotificationHandler(); } } } diff --git a/src/Umbraco.Infrastructure/Services/Implement/PropertyValidationService.cs b/src/Umbraco.Infrastructure/Services/Implement/PropertyValidationService.cs index 5fd7971976..70580153de 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/PropertyValidationService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/PropertyValidationService.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.ComponentModel.DataAnnotations; using System.Linq; +using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.PropertyEditors; using Umbraco.Extensions; @@ -13,12 +14,18 @@ namespace Umbraco.Cms.Core.Services.Implement private readonly PropertyEditorCollection _propertyEditors; private readonly IDataTypeService _dataTypeService; private readonly ILocalizedTextService _textService; + private readonly IValueEditorCache _valueEditorCache; - public PropertyValidationService(PropertyEditorCollection propertyEditors, IDataTypeService dataTypeService, ILocalizedTextService textService) + public PropertyValidationService( + PropertyEditorCollection propertyEditors, + IDataTypeService dataTypeService, + ILocalizedTextService textService, + IValueEditorCache valueEditorCache) { _propertyEditors = propertyEditors; _dataTypeService = dataTypeService; _textService = textService; + _valueEditorCache = valueEditorCache; } /// @@ -58,7 +65,7 @@ namespace Umbraco.Cms.Core.Services.Implement _textService.Localize("validation", "invalidPattern"), }; - var valueEditor = editor.GetValueEditor(dataType.Configuration); + IDataValueEditor valueEditor = _valueEditorCache.GetValueEditor(editor, dataType); foreach (var validationResult in valueEditor.Validate(postedValue, isRequired, validationRegExp)) { // If we've got custom error messages, we'll replace the default ones that will have been applied in the call to Validate(). diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/Models/VariationTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Models/VariationTests.cs index 892ef696c3..c2d5b785c9 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Core/Models/VariationTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Models/VariationTests.cs @@ -6,6 +6,7 @@ using Microsoft.Extensions.Logging.Abstractions; using Moq; using NUnit.Framework; using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Hosting; using Umbraco.Cms.Core.IO; using Umbraco.Cms.Core.Models; @@ -603,7 +604,8 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Models return new PropertyValidationService( propertyEditorCollection, dataTypeService, - localizedTextService); + localizedTextService, + new ValueEditorCache()); } } } From c690d5f3bab8c67f47365f2b83f222633505a6e6 Mon Sep 17 00:00:00 2001 From: Nikolaj Date: Mon, 23 Aug 2021 15:06:47 +0200 Subject: [PATCH 2/8] Add clarifying comment in ValueEditorCache --- src/Umbraco.Core/Cache/ValueEditorCache.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Core/Cache/ValueEditorCache.cs b/src/Umbraco.Core/Cache/ValueEditorCache.cs index 81f9348d60..4ee41f9e55 100644 --- a/src/Umbraco.Core/Cache/ValueEditorCache.cs +++ b/src/Umbraco.Core/Cache/ValueEditorCache.cs @@ -15,7 +15,9 @@ namespace Umbraco.Cms.Core.Cache public IDataValueEditor GetValueEditor(IDataEditor editor, IDataType dataType) { - // Instead of creating a value editor immediately check if we've already created one and use that. + // We try and get the dictionary based on the IDataEditor alias, + // this is here just in case a data type can have more than one value data editor. + // If this is not the case this could be simplified quite a bit, by just using the inner dictionary only. IDataValueEditor valueEditor; if (_valueEditorCache.TryGetValue(editor.Alias, out Dictionary dataEditorCache)) { From 6c21cb30d3bf8f1563dc5a382242017b19b13de6 Mon Sep 17 00:00:00 2001 From: Nikolaj Date: Tue, 24 Aug 2021 10:38:38 +0200 Subject: [PATCH 3/8] Add locks for the dictionary. --- src/Umbraco.Core/Cache/ValueEditorCache.cs | 49 ++++++++++++++-------- 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/src/Umbraco.Core/Cache/ValueEditorCache.cs b/src/Umbraco.Core/Cache/ValueEditorCache.cs index 4ee41f9e55..44a813f129 100644 --- a/src/Umbraco.Core/Cache/ValueEditorCache.cs +++ b/src/Umbraco.Core/Cache/ValueEditorCache.cs @@ -11,30 +11,40 @@ namespace Umbraco.Cms.Core.Cache INotificationHandler, INotificationHandler { - private readonly Dictionary> _valueEditorCache = new(); + private readonly Dictionary> _valueEditorCache; + private readonly object _dictionaryLocker; + + public ValueEditorCache() + { + _valueEditorCache = new Dictionary>(); + _dictionaryLocker = new object(); + } public IDataValueEditor GetValueEditor(IDataEditor editor, IDataType dataType) { - // We try and get the dictionary based on the IDataEditor alias, - // this is here just in case a data type can have more than one value data editor. - // If this is not the case this could be simplified quite a bit, by just using the inner dictionary only. - IDataValueEditor valueEditor; - if (_valueEditorCache.TryGetValue(editor.Alias, out Dictionary dataEditorCache)) + // Lock just in case multiple threads uses the cache at the same time. + lock (_dictionaryLocker) { - if (dataEditorCache.TryGetValue(dataType.Id, out valueEditor)) + // We try and get the dictionary based on the IDataEditor alias, + // this is here just in case a data type can have more than one value data editor. + // If this is not the case this could be simplified quite a bit, by just using the inner dictionary only. + IDataValueEditor valueEditor; + if (_valueEditorCache.TryGetValue(editor.Alias, out Dictionary dataEditorCache)) { + if (dataEditorCache.TryGetValue(dataType.Id, out valueEditor)) + { + return valueEditor; + } + + valueEditor = editor.GetValueEditor(dataType.Configuration); + dataEditorCache[dataType.Id] = valueEditor; return valueEditor; } valueEditor = editor.GetValueEditor(dataType.Configuration); - dataEditorCache[dataType.Id] = valueEditor; + _valueEditorCache[editor.Alias] = new Dictionary { [dataType.Id] = valueEditor }; return valueEditor; - } - - valueEditor = editor.GetValueEditor(dataType.Configuration); - _valueEditorCache[editor.Alias] = new Dictionary { [dataType.Id] = valueEditor }; - return valueEditor; } public void Handle(DataTypeSavedNotification notification) => @@ -45,13 +55,16 @@ namespace Umbraco.Cms.Core.Cache private void ClearCache(IEnumerable dataTypeIds) { - // If a datatype is saved or deleted we have to clear any value editors based on their ID from the cache, - // since it could mean that their configuration has changed. - foreach (var id in dataTypeIds) + lock (_dictionaryLocker) { - foreach (Dictionary editors in _valueEditorCache.Values) + // If a datatype is saved or deleted we have to clear any value editors based on their ID from the cache, + // since it could mean that their configuration has changed. + foreach (var id in dataTypeIds) { - editors.Remove(id); + foreach (Dictionary editors in _valueEditorCache.Values) + { + editors.Remove(id); + } } } } From 65b69dfa4af1417510e3f34eb84a28a5cf888ce2 Mon Sep 17 00:00:00 2001 From: Nikolaj Date: Tue, 24 Aug 2021 11:02:37 +0200 Subject: [PATCH 4/8] Create separate refresher for ValueEditorCache Notification handlers are scoped. --- src/Umbraco.Core/Cache/IValueEditorCache.cs | 4 +++- src/Umbraco.Core/Cache/ValueEditorCache.cs | 15 ++----------- .../Cache/ValueEditorCacheRefresher.cs | 21 +++++++++++++++++++ .../DependencyInjection/UmbracoBuilder.cs | 4 ++-- 4 files changed, 28 insertions(+), 16 deletions(-) create mode 100644 src/Umbraco.Core/Cache/ValueEditorCacheRefresher.cs diff --git a/src/Umbraco.Core/Cache/IValueEditorCache.cs b/src/Umbraco.Core/Cache/IValueEditorCache.cs index 72b85746c4..f283d730b5 100644 --- a/src/Umbraco.Core/Cache/IValueEditorCache.cs +++ b/src/Umbraco.Core/Cache/IValueEditorCache.cs @@ -1,4 +1,5 @@ -using Umbraco.Cms.Core.Models; +using System.Collections.Generic; +using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.PropertyEditors; namespace Umbraco.Cms.Core.Cache @@ -6,5 +7,6 @@ namespace Umbraco.Cms.Core.Cache public interface IValueEditorCache { public IDataValueEditor GetValueEditor(IDataEditor dataEditor, IDataType dataType); + public void ClearCache(IEnumerable dataTypeIds); } } diff --git a/src/Umbraco.Core/Cache/ValueEditorCache.cs b/src/Umbraco.Core/Cache/ValueEditorCache.cs index 44a813f129..44aa83d44d 100644 --- a/src/Umbraco.Core/Cache/ValueEditorCache.cs +++ b/src/Umbraco.Core/Cache/ValueEditorCache.cs @@ -1,15 +1,10 @@ using System.Collections.Generic; -using System.Linq; -using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Models; -using Umbraco.Cms.Core.Notifications; using Umbraco.Cms.Core.PropertyEditors; namespace Umbraco.Cms.Core.Cache { - public class ValueEditorCache : IValueEditorCache, - INotificationHandler, - INotificationHandler + public class ValueEditorCache : IValueEditorCache { private readonly Dictionary> _valueEditorCache; private readonly object _dictionaryLocker; @@ -47,13 +42,7 @@ namespace Umbraco.Cms.Core.Cache } } - public void Handle(DataTypeSavedNotification notification) => - ClearCache(notification.SavedEntities.Select(x => x.Id)); - - public void Handle(DataTypeDeletedNotification notification) => - ClearCache(notification.DeletedEntities.Select(x => x.Id)); - - private void ClearCache(IEnumerable dataTypeIds) + public void ClearCache(IEnumerable dataTypeIds) { lock (_dictionaryLocker) { diff --git a/src/Umbraco.Core/Cache/ValueEditorCacheRefresher.cs b/src/Umbraco.Core/Cache/ValueEditorCacheRefresher.cs new file mode 100644 index 0000000000..04330a1973 --- /dev/null +++ b/src/Umbraco.Core/Cache/ValueEditorCacheRefresher.cs @@ -0,0 +1,21 @@ +using System.Linq; +using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.Notifications; + +namespace Umbraco.Cms.Core.Cache +{ + public class ValueEditorCacheRefresher : + INotificationHandler, + INotificationHandler + { + private readonly IValueEditorCache _valueEditorCache; + + public ValueEditorCacheRefresher(IValueEditorCache valueEditorCache) => _valueEditorCache = valueEditorCache; + + public void Handle(DataTypeSavedNotification notification) => + _valueEditorCache.ClearCache(notification.SavedEntities.Select(x => x.Id)); + + public void Handle(DataTypeDeletedNotification notification) => + _valueEditorCache.ClearCache(notification.DeletedEntities.Select(x => x.Id)); + } +} diff --git a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs index 35239121c0..3212787fdb 100644 --- a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs +++ b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs @@ -257,8 +257,8 @@ namespace Umbraco.Cms.Core.DependencyInjection // Register ValueEditorCache used for validation Services.AddSingleton(); Services - .AddNotificationHandler() - .AddNotificationHandler(); + .AddNotificationHandler() + .AddNotificationHandler(); } } } From 1c4893d3c8d0504f8a9dd43cd7970bc0effffe27 Mon Sep 17 00:00:00 2001 From: Nikolaj Date: Tue, 24 Aug 2021 11:02:51 +0200 Subject: [PATCH 5/8] Fix Tests --- .../Services/ContentServiceTests.cs | 9 +++++++-- .../Services/PropertyValidationServiceTests.cs | 3 ++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServiceTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServiceTests.cs index 60441a013d..75b2e62bf9 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServiceTests.cs @@ -9,6 +9,7 @@ using System.Threading; using Moq; using NUnit.Framework; using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Models; @@ -72,13 +73,17 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services private IJsonSerializer Serializer => GetRequiredService(); + private IValueEditorCache ValueEditorCache => GetRequiredService(); + [SetUp] public void Setup() => ContentRepositoryBase.ThrowOnWarning = true; protected override void CustomTestSetup(IUmbracoBuilder builder) => builder .AddNotificationHandler() .AddNotificationHandler() - .AddNotificationHandler(); + .AddNotificationHandler() + .AddNotificationHandler() + .AddNotificationHandler(); [TearDown] public void Teardown() => ContentRepositoryBase.ThrowOnWarning = false; @@ -1162,7 +1167,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services Assert.IsFalse(content.HasIdentity); // content cannot publish values because they are invalid - var propertyValidationService = new PropertyValidationService(PropertyEditorCollection, DataTypeService, TextService); + var propertyValidationService = new PropertyValidationService(PropertyEditorCollection, DataTypeService, TextService, ValueEditorCache); bool isValid = propertyValidationService.IsPropertyDataValid(content, out IProperty[] invalidProperties, CultureImpact.Invariant); Assert.IsFalse(isValid); Assert.IsNotEmpty(invalidProperties); diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Services/PropertyValidationServiceTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Services/PropertyValidationServiceTests.cs index bfbe6258cc..942ba8cf4a 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Services/PropertyValidationServiceTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Services/PropertyValidationServiceTests.cs @@ -5,6 +5,7 @@ using System.Threading; using Moq; using NUnit.Framework; using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Hosting; using Umbraco.Cms.Core.IO; using Umbraco.Cms.Core.Models; @@ -45,7 +46,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Services var propEditors = new PropertyEditorCollection(new DataEditorCollection(() => new[] { dataEditor })); - validationService = new PropertyValidationService(propEditors, dataTypeService.Object, Mock.Of()); + validationService = new PropertyValidationService(propEditors, dataTypeService.Object, Mock.Of(), new ValueEditorCache()); } [Test] From 90bc927d669860fad24f2c0e53b5c3c99468e1b4 Mon Sep 17 00:00:00 2001 From: Nikolaj Date: Tue, 24 Aug 2021 14:15:18 +0200 Subject: [PATCH 6/8] Add unit tests --- .../Cache/ValueEditorCacheTests.cs | 133 ++++++++++++++++++ 1 file changed, 133 insertions(+) create mode 100644 src/Umbraco.Tests.UnitTests/Umbraco.Core/Cache/ValueEditorCacheTests.cs diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/Cache/ValueEditorCacheTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Cache/ValueEditorCacheTests.cs new file mode 100644 index 0000000000..94cd9924f9 --- /dev/null +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Cache/ValueEditorCacheTests.cs @@ -0,0 +1,133 @@ +using System.Collections.Generic; +using Moq; +using NUnit.Framework; +using Umbraco.Cms.Core.Cache; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.PropertyEditors; + +namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Cache +{ + [TestFixture] + public class ValueEditorCacheTests + { + + [Test] + public void Caches_ValueEditor() + { + var sut = new ValueEditorCache(); + + var dataEditor = new FakeDataEditor("TestEditor"); + IDataType dataType = CreateDataTypeMock(1).Object; + + // Request the same value editor twice + IDataValueEditor firstEditor = sut.GetValueEditor(dataEditor, dataType); + IDataValueEditor secondEditor = sut.GetValueEditor(dataEditor, dataType); + + Assert.Multiple(() => + { + Assert.AreSame(firstEditor, secondEditor); + Assert.AreEqual(1, dataEditor.ValueEditorCount, "GetValueEditor invoked more than once."); + }); + } + + [Test] + public void Different_Data_Editors_Returns_Different_Value_Editors() + { + var sut = new ValueEditorCache(); + + var dataEditor1 = new FakeDataEditor("Editor1"); + var dataEditor2 = new FakeDataEditor("Editor2"); + IDataType dataType = CreateDataTypeMock(1).Object; + + IDataValueEditor firstEditor = sut.GetValueEditor(dataEditor1, dataType); + IDataValueEditor secondEditor = sut.GetValueEditor(dataEditor2, dataType); + + Assert.AreNotSame(firstEditor, secondEditor); + } + + [Test] + public void Different_Data_Types_Returns_Different_Value_Editors() + { + var sut = new ValueEditorCache(); + var dataEditor = new FakeDataEditor("Editor"); + IDataType dataType1 = CreateDataTypeMock(1).Object; + IDataType dataType2 = CreateDataTypeMock(2).Object; + + IDataValueEditor firstEditor = sut.GetValueEditor(dataEditor, dataType1); + IDataValueEditor secondEditor = sut.GetValueEditor(dataEditor, dataType2); + + Assert.AreNotSame(firstEditor, secondEditor); + } + + [Test] + public void Clear_Cache_Removes_Specific_Editors() + { + var sut = new ValueEditorCache(); + + var dataEditor1 = new FakeDataEditor("Editor 1"); + var dataEditor2 = new FakeDataEditor("Editor 2"); + + IDataType dataType1 = CreateDataTypeMock(1).Object; + IDataType dataType2 = CreateDataTypeMock(2).Object; + + // Load the editors into cache + IDataValueEditor editor1DataType1 = sut.GetValueEditor(dataEditor1, dataType1); + IDataValueEditor editor1Datatype2 = sut.GetValueEditor(dataEditor1, dataType2); + IDataValueEditor editor2DataType1 = sut.GetValueEditor(dataEditor2, dataType1); + IDataValueEditor editor2Datatype2 = sut.GetValueEditor(dataEditor2, dataType2); + + sut.ClearCache(new []{dataType1.Id}); + + // New value editor objects should be created after it's cleared + Assert.AreNotSame(editor1DataType1, sut.GetValueEditor(dataEditor1, dataType1), "Value editor was not cleared from cache"); + Assert.AreNotSame(editor2DataType1, sut.GetValueEditor(dataEditor2, dataType1), "Value editor was not cleared from cache"); + + // But the value editors for data type 2 should be the same + Assert.AreSame(editor1Datatype2, sut.GetValueEditor(dataEditor1, dataType2), "Too many editors was cleared from cache"); + Assert.AreSame(editor2Datatype2, sut.GetValueEditor(dataEditor2, dataType2), "Too many editors was cleared from cache"); + } + + + private Mock CreateDataTypeMock(int id) + { + var mock = new Mock(); + mock.Setup(x => x.Id).Returns(id); + return mock; + } + + /// + /// A fake IDataEditor + /// + /// + /// This is necessary to ensure that different objects are returned from GetValueEditor + /// + private class FakeDataEditor : IDataEditor + { + public FakeDataEditor(string alias) + { + Alias = alias; + } + + public string Alias { get; } + public EditorType Type { get; } + public string Name { get; } + public string Icon { get; } + public string Group { get; } + public bool IsDeprecated { get; } + + public IDataValueEditor GetValueEditor() + { + ValueEditorCount++; + return Mock.Of(); + } + public IDataValueEditor GetValueEditor(object configuration) => GetValueEditor(); + + public IDictionary DefaultConfiguration { get; } + public IConfigurationEditor GetConfigurationEditor() => throw new System.NotImplementedException(); + + public IPropertyIndexValueFactory PropertyIndexValueFactory { get; } + + public int ValueEditorCount; + } + } +} From d448ddd7dfb1b1ab2ae1829a32a0d0fdc03ee8d9 Mon Sep 17 00:00:00 2001 From: Nikolaj Date: Mon, 30 Aug 2021 10:45:47 +0200 Subject: [PATCH 7/8] Make ValueEditorCacheRefresher a distributed cache refresher --- .../Cache/ValueEditorCacheRefresher.cs | 54 +++++++++++++++---- .../DependencyInjection/UmbracoBuilder.cs | 3 -- .../Cache/DistributedCacheBinder_Handlers.cs | 2 + .../Cache/DistributedCacheExtensions.cs | 16 ++++++ 4 files changed, 63 insertions(+), 12 deletions(-) diff --git a/src/Umbraco.Core/Cache/ValueEditorCacheRefresher.cs b/src/Umbraco.Core/Cache/ValueEditorCacheRefresher.cs index 04330a1973..c815ca7a71 100644 --- a/src/Umbraco.Core/Cache/ValueEditorCacheRefresher.cs +++ b/src/Umbraco.Core/Cache/ValueEditorCacheRefresher.cs @@ -1,21 +1,57 @@ -using System.Linq; +using System; +using System.Collections.Generic; +using System.Linq; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Notifications; +using Umbraco.Cms.Core.Serialization; namespace Umbraco.Cms.Core.Cache { - public class ValueEditorCacheRefresher : - INotificationHandler, - INotificationHandler + public sealed class ValueEditorCacheRefresher : PayloadCacheRefresherBase { private readonly IValueEditorCache _valueEditorCache; - public ValueEditorCacheRefresher(IValueEditorCache valueEditorCache) => _valueEditorCache = valueEditorCache; + public ValueEditorCacheRefresher( + AppCaches appCaches, + IJsonSerializer serializer, + IEventAggregator eventAggregator, + ICacheRefresherNotificationFactory factory, + IValueEditorCache valueEditorCache) : base(appCaches, serializer, eventAggregator, factory) + { + _valueEditorCache = valueEditorCache; + } - public void Handle(DataTypeSavedNotification notification) => - _valueEditorCache.ClearCache(notification.SavedEntities.Select(x => x.Id)); + public static readonly Guid UniqueId = Guid.Parse("D28A1DBB-2308-4918-9A92-2F8689B6CBFE"); + public override Guid RefresherUniqueId => UniqueId; + public override string Name => "ValueEditorCacheRefresher"; - public void Handle(DataTypeDeletedNotification notification) => - _valueEditorCache.ClearCache(notification.DeletedEntities.Select(x => x.Id)); + public override void Refresh(DataTypeCacheRefresher.JsonPayload[] payloads) + { + IEnumerable ids = payloads.Select(x => x.Id); + _valueEditorCache.ClearCache(ids); + } + + // these events should never trigger + // everything should be PAYLOAD/JSON + + public override void RefreshAll() + { + throw new NotSupportedException(); + } + + public override void Refresh(int id) + { + throw new NotSupportedException(); + } + + public override void Refresh(Guid id) + { + throw new NotSupportedException(); + } + + public override void Remove(int id) + { + throw new NotSupportedException(); + } } } diff --git a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs index 3212787fdb..6f6a53df66 100644 --- a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs +++ b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs @@ -256,9 +256,6 @@ namespace Umbraco.Cms.Core.DependencyInjection // Register ValueEditorCache used for validation Services.AddSingleton(); - Services - .AddNotificationHandler() - .AddNotificationHandler(); } } } diff --git a/src/Umbraco.Infrastructure/Cache/DistributedCacheBinder_Handlers.cs b/src/Umbraco.Infrastructure/Cache/DistributedCacheBinder_Handlers.cs index 35845f3cd0..6e6f549b03 100644 --- a/src/Umbraco.Infrastructure/Cache/DistributedCacheBinder_Handlers.cs +++ b/src/Umbraco.Infrastructure/Cache/DistributedCacheBinder_Handlers.cs @@ -130,6 +130,7 @@ namespace Umbraco.Cms.Core.Cache { _distributedCache.RefreshDataTypeCache(entity); } + _distributedCache.RefreshValueEditorCache(notification.SavedEntities); } public void Handle(DataTypeDeletedNotification notification) @@ -138,6 +139,7 @@ namespace Umbraco.Cms.Core.Cache { _distributedCache.RemoveDataTypeCache(entity); } + _distributedCache.RefreshValueEditorCache(notification.DeletedEntities); } #endregion diff --git a/src/Umbraco.Infrastructure/Cache/DistributedCacheExtensions.cs b/src/Umbraco.Infrastructure/Cache/DistributedCacheExtensions.cs index a0ba0ff128..ceac767a8c 100644 --- a/src/Umbraco.Infrastructure/Cache/DistributedCacheExtensions.cs +++ b/src/Umbraco.Infrastructure/Cache/DistributedCacheExtensions.cs @@ -1,6 +1,7 @@ // Copyright (c) Umbraco. // See LICENSE for more details. +using System.Collections.Generic; using System.Linq; using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Models; @@ -106,6 +107,21 @@ namespace Umbraco.Extensions #endregion + #region ValueEditorCache + + public static void RefreshValueEditorCache(this DistributedCache dc, IEnumerable dataTypes) + { + if (dataTypes is null) + { + return; + } + + var payloads = dataTypes.Select(x => new DataTypeCacheRefresher.JsonPayload(x.Id, x.Key, false)); + dc.RefreshByPayload(ValueEditorCacheRefresher.UniqueId, payloads); + } + + #endregion + #region ContentCache public static void RefreshAllContentCache(this DistributedCache dc) From 3d2dae075d2bd61ee73064caac6fb097f6f422de Mon Sep 17 00:00:00 2001 From: Nikolaj Date: Mon, 30 Aug 2021 10:57:32 +0200 Subject: [PATCH 8/8] Remove notification registration for ValueEditorCacheRefresher --- .../Umbraco.Infrastructure/Services/ContentServiceTests.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServiceTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServiceTests.cs index 75b2e62bf9..d10e269fad 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServiceTests.cs @@ -81,9 +81,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services protected override void CustomTestSetup(IUmbracoBuilder builder) => builder .AddNotificationHandler() .AddNotificationHandler() - .AddNotificationHandler() - .AddNotificationHandler() - .AddNotificationHandler(); + .AddNotificationHandler(); [TearDown] public void Teardown() => ContentRepositoryBase.ThrowOnWarning = false;