From 3b57b3496a12c619958f6afc7dbb66a42d353be7 Mon Sep 17 00:00:00 2001 From: Tom Edwards Date: Mon, 6 Mar 2023 21:02:27 +0100 Subject: [PATCH] Added GenerateTypeSafeMetadata to property metadata This method is used to strip coercion methods out of StyledPropertyMetadata when applying it to a new owner. Also fixed AvaloniaProperty.(IPropertyInfo.CanSet) returning true for read-only properties --- Avalonia.Desktop.slnf | 5 +- src/Avalonia.Base/AvaloniaProperty.cs | 65 ++++++++----------- src/Avalonia.Base/AvaloniaPropertyMetadata.cs | 10 ++- src/Avalonia.Base/DirectPropertyMetadata`1.cs | 2 + src/Avalonia.Base/StyledProperty.cs | 7 +- src/Avalonia.Base/StyledPropertyMetadata`1.cs | 2 + .../AvaloniaPropertyTests.cs | 46 +++++++++---- 7 files changed, 80 insertions(+), 57 deletions(-) diff --git a/Avalonia.Desktop.slnf b/Avalonia.Desktop.slnf index 477aaec6a8..d4cde99240 100644 --- a/Avalonia.Desktop.slnf +++ b/Avalonia.Desktop.slnf @@ -8,9 +8,9 @@ "samples\\GpuInterop\\GpuInterop.csproj", "samples\\IntegrationTestApp\\IntegrationTestApp.csproj", "samples\\MiniMvvm\\MiniMvvm.csproj", + "samples\\ReactiveUIDemo\\ReactiveUIDemo.csproj", "samples\\SampleControls\\ControlSamples.csproj", "samples\\Sandbox\\Sandbox.csproj", - "samples\\ReactiveUIDemo\\ReactiveUIDemo.csproj", "src\\Avalonia.Base\\Avalonia.Base.csproj", "src\\Avalonia.Build.Tasks\\Avalonia.Build.Tasks.csproj", "src\\Avalonia.Controls.ColorPicker\\Avalonia.Controls.ColorPicker.csproj", @@ -41,6 +41,7 @@ "src\\Windows\\Avalonia.Direct2D1\\Avalonia.Direct2D1.csproj", "src\\Windows\\Avalonia.Win32.Interop\\Avalonia.Win32.Interop.csproj", "src\\Windows\\Avalonia.Win32\\Avalonia.Win32.csproj", + "src\\tools\\Avalonia.Generators\\Avalonia.Generators.csproj", "src\\tools\\DevAnalyzers\\DevAnalyzers.csproj", "src\\tools\\DevGenerators\\DevGenerators.csproj", "src\\tools\\PublicAnalyzers\\Avalonia.Analyzers.csproj", @@ -63,4 +64,4 @@ "tests\\Avalonia.UnitTests\\Avalonia.UnitTests.csproj" ] } -} +} \ No newline at end of file diff --git a/src/Avalonia.Base/AvaloniaProperty.cs b/src/Avalonia.Base/AvaloniaProperty.cs index 24244c5068..9efbf678fe 100644 --- a/src/Avalonia.Base/AvaloniaProperty.cs +++ b/src/Avalonia.Base/AvaloniaProperty.cs @@ -4,7 +4,6 @@ using System.Diagnostics.CodeAnalysis; using Avalonia.Data; using Avalonia.Data.Core; using Avalonia.PropertyStore; -using Avalonia.Styling; using Avalonia.Utilities; namespace Avalonia @@ -20,12 +19,20 @@ namespace Avalonia public static readonly object UnsetValue = new UnsetValueType(); private static int s_nextId; + + /// + /// Provides a metadata object for types which have no metadata of their own. + /// private readonly AvaloniaPropertyMetadata _defaultMetadata; + + /// + /// Provides a fast path when the property has no metadata overrides. + /// + private KeyValuePair? _singleMetadata; + private readonly Dictionary _metadata; private readonly Dictionary _metadataCache = new Dictionary(); - private bool _hasMetadataOverrides; - /// /// Initializes a new instance of the class. /// @@ -57,7 +64,8 @@ namespace Avalonia Id = s_nextId++; _metadata.Add(ownerType, metadata ?? throw new ArgumentNullException(nameof(metadata))); - _defaultMetadata = metadata; + _defaultMetadata = metadata.GenerateTypeSafeMetadata(); + _singleMetadata = new(ownerType, metadata); } /// @@ -80,9 +88,6 @@ namespace Avalonia Id = source.Id; _defaultMetadata = source._defaultMetadata; - // Properties that have different owner can't use fast path for metadata. - _hasMetadataOverrides = true; - if (metadata != null) { _metadata.Add(ownerType, metadata); @@ -442,33 +447,14 @@ namespace Avalonia } /// - /// Gets the property metadata for the specified type. + /// Gets the which applies to this property when it is used with the specified type. /// - /// The type. - /// - /// The property metadata. - /// - public AvaloniaPropertyMetadata GetMetadata() where T : AvaloniaObject - { - return GetMetadata(typeof(T)); - } + /// The type for which to retrieve metadata. + public AvaloniaPropertyMetadata GetMetadata() where T : AvaloniaObject => GetMetadata(typeof(T)); - /// - /// Gets the property metadata for the specified type. - /// - /// The type. - /// - /// The property metadata. - /// - public AvaloniaPropertyMetadata GetMetadata(Type type) - { - if (!_hasMetadataOverrides) - { - return _defaultMetadata; - } - - return GetMetadataWithOverrides(type); - } + /// + /// The type for which to retrieve metadata. + public AvaloniaPropertyMetadata GetMetadata(Type type) => GetMetadataWithOverrides(type); /// /// Checks whether the is valid for the property. @@ -567,7 +553,7 @@ namespace Avalonia _metadata.Add(type, metadata); _metadataCache.Clear(); - _hasMetadataOverrides = true; + _singleMetadata = null; } protected abstract IObservable GetChanged(); @@ -584,7 +570,12 @@ namespace Avalonia return result; } - Type? currentType = type; + if (_singleMetadata is { } singleMetadata) + { + return _metadataCache[type] = singleMetadata.Key.IsAssignableFrom(type) ? singleMetadata.Value : _defaultMetadata; + } + + var currentType = type; while (currentType != null) { @@ -598,13 +589,11 @@ namespace Avalonia currentType = currentType.BaseType; } - _metadataCache[type] = _defaultMetadata; - - return _defaultMetadata; + return _metadataCache[type] = _defaultMetadata; } bool IPropertyInfo.CanGet => true; - bool IPropertyInfo.CanSet => true; + bool IPropertyInfo.CanSet => !IsReadOnly; object? IPropertyInfo.Get(object target) => ((AvaloniaObject)target).GetValue(this); void IPropertyInfo.Set(object target, object? value) => ((AvaloniaObject)target).SetValue(this, value); } diff --git a/src/Avalonia.Base/AvaloniaPropertyMetadata.cs b/src/Avalonia.Base/AvaloniaPropertyMetadata.cs index 62bb65351f..ec29d14693 100644 --- a/src/Avalonia.Base/AvaloniaPropertyMetadata.cs +++ b/src/Avalonia.Base/AvaloniaPropertyMetadata.cs @@ -5,7 +5,7 @@ namespace Avalonia /// /// Base class for avalonia property metadata. /// - public class AvaloniaPropertyMetadata + public abstract class AvaloniaPropertyMetadata { private BindingMode _defaultBindingMode; @@ -61,5 +61,13 @@ namespace Avalonia EnableDataValidation ??= baseMetadata.EnableDataValidation; } + + /// + /// Gets a copy of this object configured for use with any owner type. + /// + /// + /// For example, delegates which receive the owner object should be removed. + /// + public abstract AvaloniaPropertyMetadata GenerateTypeSafeMetadata(); } } diff --git a/src/Avalonia.Base/DirectPropertyMetadata`1.cs b/src/Avalonia.Base/DirectPropertyMetadata`1.cs index 451ff6ce00..5471826f9f 100644 --- a/src/Avalonia.Base/DirectPropertyMetadata`1.cs +++ b/src/Avalonia.Base/DirectPropertyMetadata`1.cs @@ -45,5 +45,7 @@ namespace Avalonia UnsetValue ??= src.UnsetValue; } } + + public override AvaloniaPropertyMetadata GenerateTypeSafeMetadata() => new DirectPropertyMetadata(UnsetValue, DefaultBindingMode, EnableDataValidation); } } diff --git a/src/Avalonia.Base/StyledProperty.cs b/src/Avalonia.Base/StyledProperty.cs index 5052840013..dfe5a44f1b 100644 --- a/src/Avalonia.Base/StyledProperty.cs +++ b/src/Avalonia.Base/StyledProperty.cs @@ -18,7 +18,10 @@ namespace Avalonia /// The type of the class that registers the property. /// The property metadata. /// Whether the property inherits its value. - /// A value validation callback. + /// + /// A method which returns "false" for values that are never valid for this property. + /// This method is not part of the property's metadata and so cannot be changed after registration. + /// /// A callback. public StyledProperty( string name, @@ -41,7 +44,7 @@ namespace Avalonia } /// - /// Gets the value validation callback for the property. + /// A method which returns "false" for values that are never valid for this property. /// public Func? ValidateValue { get; } diff --git a/src/Avalonia.Base/StyledPropertyMetadata`1.cs b/src/Avalonia.Base/StyledPropertyMetadata`1.cs index 6f10de3651..9db460dba3 100644 --- a/src/Avalonia.Base/StyledPropertyMetadata`1.cs +++ b/src/Avalonia.Base/StyledPropertyMetadata`1.cs @@ -58,5 +58,7 @@ namespace Avalonia } } } + + public override AvaloniaPropertyMetadata GenerateTypeSafeMetadata() => new StyledPropertyMetadata(DefaultValue, DefaultBindingMode, enableDataValidation: EnableDataValidation ?? false); } } diff --git a/tests/Avalonia.Base.UnitTests/AvaloniaPropertyTests.cs b/tests/Avalonia.Base.UnitTests/AvaloniaPropertyTests.cs index 181596a681..bde750efdc 100644 --- a/tests/Avalonia.Base.UnitTests/AvaloniaPropertyTests.cs +++ b/tests/Avalonia.Base.UnitTests/AvaloniaPropertyTests.cs @@ -2,8 +2,6 @@ using System; using System.Collections.Generic; using Avalonia.Data; using Avalonia.PropertyStore; -using Avalonia.Styling; -using Avalonia.Utilities; using Xunit; namespace Avalonia.Base.UnitTests @@ -29,7 +27,7 @@ namespace Avalonia.Base.UnitTests [Fact] public void GetMetadata_Returns_Supplied_Value() { - var metadata = new AvaloniaPropertyMetadata(); + var metadata = new TestMetadata(); var target = new TestProperty("test", typeof(Class1), metadata); Assert.Same(metadata, target.GetMetadata()); @@ -38,26 +36,30 @@ namespace Avalonia.Base.UnitTests [Fact] public void GetMetadata_Returns_Supplied_Value_For_Derived_Class() { - var metadata = new AvaloniaPropertyMetadata(); + var metadata = new TestMetadata(); var target = new TestProperty("test", typeof(Class1), metadata); Assert.Same(metadata, target.GetMetadata()); } [Fact] - public void GetMetadata_Returns_Supplied_Value_For_Unrelated_Class() + public void GetMetadata_Returns_TypeSafe_Metadata_For_Unrelated_Class() { - var metadata = new AvaloniaPropertyMetadata(); + var metadata = new TestMetadata(BindingMode.OneWayToSource, true, x => { _ = (StyledElement)x; }); var target = new TestProperty("test", typeof(Class3), metadata); - Assert.Same(metadata, target.GetMetadata()); + var targetMetadata = (TestMetadata)target.GetMetadata(); + + Assert.Equal(metadata.DefaultBindingMode, targetMetadata.DefaultBindingMode); + Assert.Equal(metadata.EnableDataValidation, targetMetadata.EnableDataValidation); + Assert.Equal(null, targetMetadata.OwnerSpecificAction); } [Fact] public void GetMetadata_Returns_Overridden_Value() { - var metadata = new AvaloniaPropertyMetadata(); - var overridden = new AvaloniaPropertyMetadata(); + var metadata = new TestMetadata(); + var overridden = new TestMetadata(); var target = new TestProperty("test", typeof(Class1), metadata); target.OverrideMetadata(overridden); @@ -68,9 +70,9 @@ namespace Avalonia.Base.UnitTests [Fact] public void OverrideMetadata_Should_Merge_Values() { - var metadata = new AvaloniaPropertyMetadata(BindingMode.TwoWay); + var metadata = new TestMetadata(BindingMode.TwoWay); var notify = (Action)((a, b) => { }); - var overridden = new AvaloniaPropertyMetadata(); + var overridden = new TestMetadata(); var target = new TestProperty("test", typeof(Class1), metadata); target.OverrideMetadata(overridden); @@ -131,15 +133,31 @@ namespace Avalonia.Base.UnitTests [Fact] public void PropertyMetadata_BindingMode_Default_Returns_OneWay() { - var data = new AvaloniaPropertyMetadata(defaultBindingMode: BindingMode.Default); + var data = new TestMetadata(defaultBindingMode: BindingMode.Default); Assert.Equal(BindingMode.OneWay, data.DefaultBindingMode); } + private class TestMetadata : AvaloniaPropertyMetadata + { + public Action OwnerSpecificAction { get; } + + public TestMetadata(BindingMode defaultBindingMode = BindingMode.Default, + bool? enableDataValidation = null, + Action ownerSpecificAction = null) + : base(defaultBindingMode, enableDataValidation) + { + OwnerSpecificAction = ownerSpecificAction; + } + + public override AvaloniaPropertyMetadata GenerateTypeSafeMetadata() => + new TestMetadata(DefaultBindingMode, EnableDataValidation, null); + } + private class TestProperty : AvaloniaProperty { - public TestProperty(string name, Type ownerType, AvaloniaPropertyMetadata metadata = null) - : base(name, ownerType, metadata ?? new AvaloniaPropertyMetadata()) + public TestProperty(string name, Type ownerType, TestMetadata metadata = null) + : base(name, ownerType, metadata ?? new TestMetadata()) { }