Performance improvement: Reusable data editors (#12921)

* Introduce opt-in option for reusable data editors

* Verified RTE as reusable

* Make attribute property naming more explicit + update comments

* Test file upload and image cropper

* Add unit tests

(cherry picked from commit 44122c6509)
This commit is contained in:
Kenn Jacobsen
2022-08-31 11:03:34 +02:00
committed by Sebastiaan Janssen
parent 6302630d39
commit 147c60f04b
36 changed files with 213 additions and 43 deletions

View File

@@ -21,7 +21,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
"Content Picker",
"contentpicker",
ValueType = ValueTypes.String,
Group = Constants.PropertyEditors.Groups.Pickers)]
Group = Constants.PropertyEditors.Groups.Pickers,
ValueEditorIsReusable = true)]
public class ContentPickerPropertyEditor : DataEditor
{
private readonly IEditorConfigurationParser _editorConfigurationParser;

View File

@@ -20,6 +20,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
[DataContract]
public class DataEditor : IDataEditor
{
private readonly bool _canReuseValueEditor;
private IDataValueEditor? _reusableValueEditor;
private IDictionary<string, object>? _defaultConfiguration;
/// <summary>
@@ -48,6 +50,8 @@ public class DataEditor : IDataEditor
Icon = Attribute.Icon;
Group = Attribute.Group;
IsDeprecated = Attribute.IsDeprecated;
_canReuseValueEditor = Attribute.ValueEditorIsReusable;
}
/// <summary>
@@ -118,18 +122,14 @@ public class DataEditor : IDataEditor
/// instance is returned. Otherwise, a new instance is created by CreateValueEditor.
/// </para>
/// <para>
/// The instance created by CreateValueEditor is not cached, i.e.
/// a new instance is created each time the property value is retrieved. The
/// property editor is a singleton, and the value editor cannot be a singleton
/// since it depends on the datatype configuration.
/// </para>
/// <para>
/// Technically, it could be cached by datatype but let's keep things
/// simple enough for now.
/// The instance created by CreateValueEditor is cached if allowed by the DataEditor
/// attribute (<see cref="DataEditorAttribute.ValueEditorIsReusable"/> == true).
/// </para>
/// </remarks>
// TODO: point of that one? shouldn't we always configure?
public IDataValueEditor GetValueEditor() => ExplicitValueEditor ?? CreateValueEditor();
public IDataValueEditor GetValueEditor() => ExplicitValueEditor
?? (_canReuseValueEditor
? _reusableValueEditor ??= CreateValueEditor()
: CreateValueEditor());
/// <inheritdoc />
/// <remarks>

View File

@@ -178,4 +178,10 @@ public sealed class DataEditorAttribute : Attribute
/// </summary>
/// <remarks>A deprecated editor is still supported but not proposed in the UI.</remarks>
public bool IsDeprecated { get; set; }
/// <summary>
/// Gets or sets a value indicating whether the value editor can be reused (cached).
/// </summary>
/// <remarks>While most value editors can be reused, complex editors (e.g. block based editors) might not be applicable for reuse.</remarks>
public bool ValueEditorIsReusable { get; set; }
}

View File

@@ -11,7 +11,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
EditorType.PropertyValue | EditorType.MacroParameter,
"Decimal",
"decimal",
ValueType = ValueTypes.Decimal)]
ValueType = ValueTypes.Decimal,
ValueEditorIsReusable = true)]
public class DecimalPropertyEditor : DataEditor
{
/// <summary>

View File

@@ -11,7 +11,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
"Eye Dropper Color Picker",
"eyedropper",
Icon = "icon-colorpicker",
Group = Constants.PropertyEditors.Groups.Pickers)]
Group = Constants.PropertyEditors.Groups.Pickers,
ValueEditorIsReusable = true)]
public class EyeDropperColorPickerPropertyEditor : DataEditor
{
private readonly IEditorConfigurationParser _editorConfigurationParser;

View File

@@ -11,7 +11,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
EditorType.PropertyValue | EditorType.MacroParameter,
"Numeric",
"integer",
ValueType = ValueTypes.Integer)]
ValueType = ValueTypes.Integer,
ValueEditorIsReusable = true)]
public class IntegerPropertyEditor : DataEditor
{
public IntegerPropertyEditor(

View File

@@ -18,7 +18,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
Constants.PropertyEditors.Aliases.Label,
"Label",
"readonlyvalue",
Icon = "icon-readonly")]
Icon = "icon-readonly",
ValueEditorIsReusable = true)]
public class LabelPropertyEditor : DataEditor
{
private readonly IEditorConfigurationParser _editorConfigurationParser;

View File

@@ -17,7 +17,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
"markdowneditor",
ValueType = ValueTypes.Text,
Group = Constants.PropertyEditors.Groups.RichContent,
Icon = "icon-code")]
Icon = "icon-code",
ValueEditorIsReusable = true)]
public class MarkdownPropertyEditor : DataEditor
{
private readonly IEditorConfigurationParser _editorConfigurationParser;

View File

@@ -6,7 +6,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
"membergrouppicker",
ValueType = ValueTypes.Text,
Group = Constants.PropertyEditors.Groups.People,
Icon = Constants.Icons.MemberGroup)]
Icon = Constants.Icons.MemberGroup,
ValueEditorIsReusable = true)]
public class MemberGroupPickerPropertyEditor : DataEditor
{
public MemberGroupPickerPropertyEditor(

View File

@@ -6,7 +6,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
"memberpicker",
ValueType = ValueTypes.String,
Group = Constants.PropertyEditors.Groups.People,
Icon = Constants.Icons.Member)]
Icon = Constants.Icons.Member,
ValueEditorIsReusable = true)]
public class MemberPickerPropertyEditor : DataEditor
{
public MemberPickerPropertyEditor(

View File

@@ -6,7 +6,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
"userpicker",
ValueType = ValueTypes.Integer,
Group = Constants.PropertyEditors.Groups.People,
Icon = Constants.Icons.User)]
Icon = Constants.Icons.User,
ValueEditorIsReusable = true)]
public class UserPickerPropertyEditor : DataEditor
{
public UserPickerPropertyEditor(

View File

@@ -57,7 +57,7 @@ public static partial class UmbracoBuilderExtensions
builder.Services.AddTransient<IExamineIndexCountService, ExamineIndexCountService>();
builder.Services.AddUnique<IUserDataService, SystemInformationTelemetryProvider>();
builder.Services.AddTransient<IUsageInformationService, UsageInformationService>();
builder.Services.AddTransient<IEditorConfigurationParser, EditorConfigurationParser>();
builder.Services.AddSingleton<IEditorConfigurationParser, EditorConfigurationParser>();
return builder;
}

View File

@@ -17,7 +17,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
"blocklist",
ValueType = ValueTypes.Json,
Group = Constants.PropertyEditors.Groups.Lists,
Icon = "icon-thumbnail-list")]
Icon = "icon-thumbnail-list",
ValueEditorIsReusable = false)]
public class BlockListPropertyEditor : BlockEditorPropertyEditor
{
private readonly IEditorConfigurationParser _editorConfigurationParser;

View File

@@ -17,7 +17,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
"Checkbox list",
"checkboxlist",
Icon = "icon-bulleted-list",
Group = Constants.PropertyEditors.Groups.Lists)]
Group = Constants.PropertyEditors.Groups.Lists,
ValueEditorIsReusable = true)]
public class CheckBoxListPropertyEditor : DataEditor
{
private readonly IEditorConfigurationParser _editorConfigurationParser;

View File

@@ -14,7 +14,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
"Color Picker",
"colorpicker",
Icon = "icon-colorpicker",
Group = Constants.PropertyEditors.Groups.Pickers)]
Group = Constants.PropertyEditors.Groups.Pickers,
ValueEditorIsReusable = true)]
public class ColorPickerPropertyEditor : DataEditor
{
private readonly IEditorConfigurationParser _editorConfigurationParser;

View File

@@ -18,7 +18,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
"Date/Time",
"datepicker",
ValueType = ValueTypes.DateTime,
Icon = "icon-time")]
Icon = "icon-time",
ValueEditorIsReusable = true)]
public class DateTimePropertyEditor : DataEditor
{
private readonly IEditorConfigurationParser _editorConfigurationParser;

View File

@@ -14,7 +14,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
"Dropdown",
"dropdownFlexible",
Group = Constants.PropertyEditors.Groups.Lists,
Icon = "icon-indent")]
Icon = "icon-indent",
ValueEditorIsReusable = true)]
public class DropDownFlexiblePropertyEditor : DataEditor
{
private readonly IEditorConfigurationParser _editorConfigurationParser;

View File

@@ -12,7 +12,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
EditorType.PropertyValue | EditorType.MacroParameter,
"Email address",
"email",
Icon = "icon-message")]
Icon = "icon-message",
ValueEditorIsReusable = true)]
public class EmailAddressPropertyEditor : DataEditor
{
private readonly IIOHelper _ioHelper;

View File

@@ -21,7 +21,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
"File upload",
"fileupload",
Group = Constants.PropertyEditors.Groups.Media,
Icon = "icon-download-alt")]
Icon = "icon-download-alt",
ValueEditorIsReusable = true)]
public class FileUploadPropertyEditor : DataEditor, IMediaUrlGenerator,
INotificationHandler<ContentCopiedNotification>, INotificationHandler<ContentDeletedNotification>,
INotificationHandler<MediaDeletedNotification>, INotificationHandler<MediaSavingNotification>,

View File

@@ -28,7 +28,8 @@ namespace Umbraco.Cms.Core.PropertyEditors
HideLabel = true,
ValueType = ValueTypes.Json,
Icon = "icon-layout",
Group = Constants.PropertyEditors.Groups.RichContent)]
Group = Constants.PropertyEditors.Groups.RichContent,
ValueEditorIsReusable = false)]
public class GridPropertyEditor : DataEditor
{
private readonly IBackOfficeSecurityAccessor _backOfficeSecurityAccessor;

View File

@@ -28,7 +28,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
ValueType = ValueTypes.Json,
HideLabel = false,
Group = Constants.PropertyEditors.Groups.Media,
Icon = "icon-crop")]
Icon = "icon-crop",
ValueEditorIsReusable = true)]
public class ImageCropperPropertyEditor : DataEditor, IMediaUrlGenerator,
INotificationHandler<ContentCopiedNotification>, INotificationHandler<ContentDeletedNotification>,
INotificationHandler<MediaDeletedNotification>, INotificationHandler<MediaSavingNotification>,

View File

@@ -17,7 +17,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
"listview",
HideLabel = true,
Group = Constants.PropertyEditors.Groups.Lists,
Icon = Constants.Icons.ListView)]
Icon = Constants.Icons.ListView,
ValueEditorIsReusable = true)]
public class ListViewPropertyEditor : DataEditor
{
private readonly IEditorConfigurationParser _editorConfigurationParser;

View File

@@ -24,7 +24,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
"mediapicker3",
ValueType = ValueTypes.Json,
Group = Constants.PropertyEditors.Groups.Media,
Icon = Constants.Icons.MediaImage)]
Icon = Constants.Icons.MediaImage,
ValueEditorIsReusable = true)]
public class MediaPicker3PropertyEditor : DataEditor
{
private readonly IEditorConfigurationParser _editorConfigurationParser;

View File

@@ -26,7 +26,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
ValueType = ValueTypes.Text,
Group = Constants.PropertyEditors.Groups.Media,
Icon = Constants.Icons.MediaImage,
IsDeprecated = false)]
IsDeprecated = false,
ValueEditorIsReusable = true)]
public class MediaPickerPropertyEditor : DataEditor
{
private readonly IEditorConfigurationParser _editorConfigurationParser;

View File

@@ -18,7 +18,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
"contentpicker",
ValueType = ValueTypes.Text,
Group = Constants.PropertyEditors.Groups.Pickers,
Icon = "icon-page-add")]
Icon = "icon-page-add",
ValueEditorIsReusable = true)]
public class MultiNodeTreePickerPropertyEditor : DataEditor
{
private readonly IEditorConfigurationParser _editorConfigurationParser;

View File

@@ -16,7 +16,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
"multiurlpicker",
ValueType = ValueTypes.Json,
Group = Constants.PropertyEditors.Groups.Pickers,
Icon = "icon-link")]
Icon = "icon-link",
ValueEditorIsReusable = true)]
public class MultiUrlPickerPropertyEditor : DataEditor
{
private readonly IEditorConfigurationParser _editorConfigurationParser;

View File

@@ -25,7 +25,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
"multipletextbox",
ValueType = ValueTypes.Text,
Group = Constants.PropertyEditors.Groups.Lists,
Icon = "icon-ordered-list")]
Icon = "icon-ordered-list",
ValueEditorIsReusable = true)]
public class MultipleTextStringPropertyEditor : DataEditor
{
private readonly IEditorConfigurationParser _editorConfigurationParser;

View File

@@ -25,7 +25,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
"nestedcontent",
ValueType = ValueTypes.Json,
Group = Constants.PropertyEditors.Groups.Lists,
Icon = "icon-thumbnail-list")]
Icon = "icon-thumbnail-list",
ValueEditorIsReusable = false)]
public class NestedContentPropertyEditor : DataEditor
{
public const string ContentTypeAliasPropertyKey = "ncContentTypeAlias";

View File

@@ -17,7 +17,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
"radiobuttons",
ValueType = ValueTypes.String,
Group = Constants.PropertyEditors.Groups.Lists,
Icon = "icon-target")]
Icon = "icon-target",
ValueEditorIsReusable = true)]
public class RadioButtonsPropertyEditor : DataEditor
{
private readonly IEditorConfigurationParser _editorConfigurationParser;

View File

@@ -29,7 +29,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
ValueType = ValueTypes.Text,
HideLabel = false,
Group = Constants.PropertyEditors.Groups.RichContent,
Icon = "icon-browser-window")]
Icon = "icon-browser-window",
ValueEditorIsReusable = true)]
public class RichTextPropertyEditor : DataEditor
{
private readonly IBackOfficeSecurityAccessor _backOfficeSecurityAccessor;

View File

@@ -15,7 +15,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
Constants.PropertyEditors.Aliases.Slider,
"Slider",
"slider",
Icon = "icon-navigation-horizontal")]
Icon = "icon-navigation-horizontal",
ValueEditorIsReusable = true)]
public class SliderPropertyEditor : DataEditor
{
private readonly IEditorConfigurationParser _editorConfigurationParser;

View File

@@ -23,7 +23,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
Constants.PropertyEditors.Aliases.Tags,
"Tags",
"tags",
Icon = "icon-tags")]
Icon = "icon-tags",
ValueEditorIsReusable = true)]
public class TagsPropertyEditor : DataEditor
{
private readonly IEditorConfigurationParser _editorConfigurationParser;

View File

@@ -18,7 +18,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
"Textarea",
"textarea",
ValueType = ValueTypes.Text,
Icon = "icon-application-window-alt")]
Icon = "icon-application-window-alt",
ValueEditorIsReusable = true)]
public class TextAreaPropertyEditor : DataEditor
{
private readonly IEditorConfigurationParser _editorConfigurationParser;

View File

@@ -17,7 +17,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
EditorType.PropertyValue | EditorType.MacroParameter,
"Textbox",
"textbox",
Group = Constants.PropertyEditors.Groups.Common)]
Group = Constants.PropertyEditors.Groups.Common,
ValueEditorIsReusable = true)]
public class TextboxPropertyEditor : DataEditor
{
private readonly IEditorConfigurationParser _editorConfigurationParser;

View File

@@ -18,7 +18,8 @@ namespace Umbraco.Cms.Core.PropertyEditors;
"boolean",
ValueType = ValueTypes.Integer,
Group = Constants.PropertyEditors.Groups.Common,
Icon = "icon-checkbox")]
Icon = "icon-checkbox",
ValueEditorIsReusable = true)]
public class TrueFalsePropertyEditor : DataEditor
{
private readonly IEditorConfigurationParser _editorConfigurationParser;

View File

@@ -0,0 +1,132 @@
using System.Linq;
using Microsoft.Extensions.Logging;
using Moq;
using NUnit.Framework;
using Umbraco.Cms.Core.IO;
using Umbraco.Cms.Core.PropertyEditors;
using Umbraco.Cms.Core.Serialization;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Strings;
namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.PropertyEditors;
[TestFixture]
public class DataValueEditorReuseTests
{
private Mock<IDataValueEditorFactory> _dataValueEditorFactoryMock;
private PropertyEditorCollection _propertyEditorCollection;
[SetUp]
public void SetUp()
{
_dataValueEditorFactoryMock = new Mock<IDataValueEditorFactory>();
_dataValueEditorFactoryMock
.Setup(m => m.Create<TextOnlyValueEditor>(It.IsAny<DataEditorAttribute>()))
.Returns(() => new TextOnlyValueEditor(
new DataEditorAttribute("a", "b", "c"),
Mock.Of<ILocalizedTextService>(),
Mock.Of<IShortStringHelper>(),
Mock.Of<IJsonSerializer>(),
Mock.Of<IIOHelper>()));
_propertyEditorCollection = new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty<IDataEditor>));
_dataValueEditorFactoryMock
.Setup(m =>
m.Create<BlockEditorPropertyEditor.BlockEditorPropertyValueEditor>(It.IsAny<DataEditorAttribute>()))
.Returns(() => new BlockEditorPropertyEditor.BlockEditorPropertyValueEditor(
new DataEditorAttribute("a", "b", "c"),
_propertyEditorCollection,
Mock.Of<IDataTypeService>(),
Mock.Of<IContentTypeService>(),
Mock.Of<ILocalizedTextService>(),
Mock.Of<ILogger<BlockEditorPropertyEditor.BlockEditorPropertyValueEditor>>(),
Mock.Of<IShortStringHelper>(),
Mock.Of<IJsonSerializer>(),
Mock.Of<IIOHelper>(),
Mock.Of<IPropertyValidationService>()));
}
[Test]
public void GetValueEditor_Reusable_Value_Editor_Is_Reused_When_Created_Without_Configuration()
{
var textboxPropertyEditor = new TextboxPropertyEditor(
_dataValueEditorFactoryMock.Object,
Mock.Of<IIOHelper>(),
Mock.Of<IEditorConfigurationParser>());
// textbox is set to reuse its data value editor when created *without* configuration
var dataValueEditor1 = textboxPropertyEditor.GetValueEditor();
Assert.NotNull(dataValueEditor1);
var dataValueEditor2 = textboxPropertyEditor.GetValueEditor();
Assert.NotNull(dataValueEditor2);
Assert.AreSame(dataValueEditor1, dataValueEditor2);
_dataValueEditorFactoryMock.Verify(
m => m.Create<TextOnlyValueEditor>(It.IsAny<DataEditorAttribute>()),
Times.Once);
}
[Test]
public void GetValueEditor_Reusable_Value_Editor_Is_Not_Reused_When_Created_With_Configuration()
{
var textboxPropertyEditor = new TextboxPropertyEditor(
_dataValueEditorFactoryMock.Object,
Mock.Of<IIOHelper>(),
Mock.Of<IEditorConfigurationParser>());
// no matter what, a property editor should never reuse its data value editor when created *with* configuration
var dataValueEditor1 = textboxPropertyEditor.GetValueEditor("config");
Assert.NotNull(dataValueEditor1);
Assert.AreEqual("config", ((DataValueEditor)dataValueEditor1).Configuration);
var dataValueEditor2 = textboxPropertyEditor.GetValueEditor("config");
Assert.NotNull(dataValueEditor2);
Assert.AreEqual("config", ((DataValueEditor)dataValueEditor2).Configuration);
Assert.AreNotSame(dataValueEditor1, dataValueEditor2);
_dataValueEditorFactoryMock.Verify(
m => m.Create<TextOnlyValueEditor>(It.IsAny<DataEditorAttribute>()),
Times.Exactly(2));
}
[Test]
public void GetValueEditor_Not_Reusable_Value_Editor_Is_Not_Reused_When_Created_Without_Configuration()
{
var blockListPropertyEditor = new BlockListPropertyEditor(
_dataValueEditorFactoryMock.Object,
new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty<IDataEditor>)),
Mock.Of<IIOHelper>(),
Mock.Of<IEditorConfigurationParser>());
// block list is *not* set to reuse its data value editor
var dataValueEditor1 = blockListPropertyEditor.GetValueEditor();
Assert.NotNull(dataValueEditor1);
var dataValueEditor2 = blockListPropertyEditor.GetValueEditor();
Assert.NotNull(dataValueEditor2);
Assert.AreNotSame(dataValueEditor1, dataValueEditor2);
_dataValueEditorFactoryMock.Verify(
m => m.Create<BlockEditorPropertyEditor.BlockEditorPropertyValueEditor>(It.IsAny<DataEditorAttribute>()),
Times.Exactly(2));
}
[Test]
public void GetValueEditor_Not_Reusable_Value_Editor_Is_Not_Reused_When_Created_With_Configuration()
{
var blockListPropertyEditor = new BlockListPropertyEditor(
_dataValueEditorFactoryMock.Object,
new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty<IDataEditor>)),
Mock.Of<IIOHelper>(),
Mock.Of<IEditorConfigurationParser>());
// no matter what, a property editor should never reuse its data value editor when created *with* configuration
var dataValueEditor1 = blockListPropertyEditor.GetValueEditor("config");
Assert.NotNull(dataValueEditor1);
Assert.AreEqual("config", ((DataValueEditor)dataValueEditor1).Configuration);
var dataValueEditor2 = blockListPropertyEditor.GetValueEditor("config");
Assert.NotNull(dataValueEditor2);
Assert.AreEqual("config", ((DataValueEditor)dataValueEditor2).Configuration);
Assert.AreNotSame(dataValueEditor1, dataValueEditor2);
_dataValueEditorFactoryMock.Verify(
m => m.Create<BlockEditorPropertyEditor.BlockEditorPropertyValueEditor>(It.IsAny<DataEditorAttribute>()),
Times.Exactly(2));
}
}