diff --git a/src/Umbraco.Core/Cache/IValueEditorCache.cs b/src/Umbraco.Core/Cache/IValueEditorCache.cs new file mode 100644 index 0000000000..f283d730b5 --- /dev/null +++ b/src/Umbraco.Core/Cache/IValueEditorCache.cs @@ -0,0 +1,12 @@ +using System.Collections.Generic; +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); + public void ClearCache(IEnumerable dataTypeIds); + } +} diff --git a/src/Umbraco.Core/Cache/ValueEditorCache.cs b/src/Umbraco.Core/Cache/ValueEditorCache.cs new file mode 100644 index 0000000000..44aa83d44d --- /dev/null +++ b/src/Umbraco.Core/Cache/ValueEditorCache.cs @@ -0,0 +1,61 @@ +using System.Collections.Generic; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.PropertyEditors; + +namespace Umbraco.Cms.Core.Cache +{ + public class ValueEditorCache : IValueEditorCache + { + private readonly Dictionary> _valueEditorCache; + private readonly object _dictionaryLocker; + + public ValueEditorCache() + { + _valueEditorCache = new Dictionary>(); + _dictionaryLocker = new object(); + } + + public IDataValueEditor GetValueEditor(IDataEditor editor, IDataType dataType) + { + // Lock just in case multiple threads uses the cache at the same time. + lock (_dictionaryLocker) + { + // 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); + _valueEditorCache[editor.Alias] = new Dictionary { [dataType.Id] = valueEditor }; + return valueEditor; + } + } + + public void ClearCache(IEnumerable dataTypeIds) + { + lock (_dictionaryLocker) + { + // 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/Cache/ValueEditorCacheRefresher.cs b/src/Umbraco.Core/Cache/ValueEditorCacheRefresher.cs new file mode 100644 index 0000000000..c815ca7a71 --- /dev/null +++ b/src/Umbraco.Core/Cache/ValueEditorCacheRefresher.cs @@ -0,0 +1,57 @@ +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 sealed class ValueEditorCacheRefresher : PayloadCacheRefresherBase + { + private readonly IValueEditorCache _valueEditorCache; + + public ValueEditorCacheRefresher( + AppCaches appCaches, + IJsonSerializer serializer, + IEventAggregator eventAggregator, + ICacheRefresherNotificationFactory factory, + IValueEditorCache valueEditorCache) : base(appCaches, serializer, eventAggregator, factory) + { + _valueEditorCache = valueEditorCache; + } + + public static readonly Guid UniqueId = Guid.Parse("D28A1DBB-2308-4918-9A92-2F8689B6CBFE"); + public override Guid RefresherUniqueId => UniqueId; + public override string Name => "ValueEditorCacheRefresher"; + + 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 648af8f082..6f6a53df66 100644 --- a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs +++ b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs @@ -253,6 +253,9 @@ namespace Umbraco.Cms.Core.DependencyInjection // register a basic/noop published snapshot service to be replaced Services.AddSingleton(); + + // Register ValueEditorCache used for validation + Services.AddSingleton(); } } } 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) 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.Integration/Umbraco.Infrastructure/Services/ContentServiceTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServiceTests.cs index 60441a013d..d10e269fad 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,6 +73,8 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services private IJsonSerializer Serializer => GetRequiredService(); + private IValueEditorCache ValueEditorCache => GetRequiredService(); + [SetUp] public void Setup() => ContentRepositoryBase.ThrowOnWarning = true; @@ -1162,7 +1165,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.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; + } + } +} 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()); } } } 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]