From a956e479e9ef82ccd1cb22adece8df1aa31ece57 Mon Sep 17 00:00:00 2001 From: Julien Lebosquain Date: Wed, 17 Apr 2024 11:05:17 +0200 Subject: [PATCH] Made AvaloniaPropertyMetadata immutable after property initialization (#15384) * Made AvaloniaPropertyMetadata immutable after property initialization * Removed redundant throw --- src/Avalonia.Base/AvaloniaProperty.cs | 13 ++++++--- src/Avalonia.Base/AvaloniaPropertyMetadata.cs | 14 ++++++++++ src/Avalonia.Base/DirectProperty.cs | 1 + src/Avalonia.Base/DirectPropertyMetadata`1.cs | 8 +++++- src/Avalonia.Base/StyledPropertyMetadata`1.cs | 8 +++++- .../AvaloniaPropertyTests.cs | 21 ++++++++++++++ .../DirectPropertyTests.cs | 20 +++++++++++++ .../StyledPropertyTests.cs | 28 +++++++++++++++++++ 8 files changed, 107 insertions(+), 6 deletions(-) diff --git a/src/Avalonia.Base/AvaloniaProperty.cs b/src/Avalonia.Base/AvaloniaProperty.cs index a2005f0ab0..b096de26a1 100644 --- a/src/Avalonia.Base/AvaloniaProperty.cs +++ b/src/Avalonia.Base/AvaloniaProperty.cs @@ -50,7 +50,10 @@ namespace Avalonia AvaloniaPropertyMetadata metadata, Action? notifying = null) { - _ = name ?? throw new ArgumentNullException(nameof(name)); + ThrowHelper.ThrowIfNull(name, nameof(name)); + ThrowHelper.ThrowIfNull(valueType, nameof(valueType)); + ThrowHelper.ThrowIfNull(ownerType, nameof(ownerType)); + ThrowHelper.ThrowIfNull(metadata, nameof(metadata)); if (name.Contains('.')) { @@ -60,12 +63,13 @@ namespace Avalonia _metadata = new Dictionary(); Name = name; - PropertyType = valueType ?? throw new ArgumentNullException(nameof(valueType)); - OwnerType = ownerType ?? throw new ArgumentNullException(nameof(ownerType)); + PropertyType = valueType; + OwnerType = ownerType; Notifying = notifying; Id = s_nextId++; - _metadata.Add(hostType, metadata ?? throw new ArgumentNullException(nameof(metadata))); + metadata.Freeze(); + _metadata.Add(hostType, metadata); _defaultMetadata = metadata.GenerateTypeSafeMetadata(); _singleMetadata = new(hostType, metadata); } @@ -584,6 +588,7 @@ namespace Avalonia var baseMetadata = GetMetadata(type); metadata.Merge(baseMetadata, this); + metadata.Freeze(); _metadata.Add(type, metadata); _metadataCache.Clear(); diff --git a/src/Avalonia.Base/AvaloniaPropertyMetadata.cs b/src/Avalonia.Base/AvaloniaPropertyMetadata.cs index ec29d14693..2aec72aaf7 100644 --- a/src/Avalonia.Base/AvaloniaPropertyMetadata.cs +++ b/src/Avalonia.Base/AvaloniaPropertyMetadata.cs @@ -1,3 +1,4 @@ +using System; using Avalonia.Data; namespace Avalonia @@ -7,6 +8,7 @@ namespace Avalonia /// public abstract class AvaloniaPropertyMetadata { + private bool _isReadOnly; private BindingMode _defaultBindingMode; /// @@ -54,6 +56,11 @@ namespace Avalonia AvaloniaPropertyMetadata baseMetadata, AvaloniaProperty property) { + if (_isReadOnly) + { + throw new InvalidOperationException("The metadata is read-only."); + } + if (_defaultBindingMode == BindingMode.Default) { _defaultBindingMode = baseMetadata.DefaultBindingMode; @@ -62,6 +69,13 @@ namespace Avalonia EnableDataValidation ??= baseMetadata.EnableDataValidation; } + /// + /// Makes this instance read-only. + /// No further modifications are allowed after this call. + /// + public void Freeze() + => _isReadOnly = true; + /// /// Gets a copy of this object configured for use with any owner type. /// diff --git a/src/Avalonia.Base/DirectProperty.cs b/src/Avalonia.Base/DirectProperty.cs index ef1b67615f..720b21c484 100644 --- a/src/Avalonia.Base/DirectProperty.cs +++ b/src/Avalonia.Base/DirectProperty.cs @@ -94,6 +94,7 @@ namespace Avalonia enableDataValidation: enableDataValidation); metadata.Merge(GetMetadata(), this); + metadata.Freeze(); var result = new DirectProperty( (DirectPropertyBase)this, diff --git a/src/Avalonia.Base/DirectPropertyMetadata`1.cs b/src/Avalonia.Base/DirectPropertyMetadata`1.cs index 5471826f9f..2c653f7481 100644 --- a/src/Avalonia.Base/DirectPropertyMetadata`1.cs +++ b/src/Avalonia.Base/DirectPropertyMetadata`1.cs @@ -46,6 +46,12 @@ namespace Avalonia } } - public override AvaloniaPropertyMetadata GenerateTypeSafeMetadata() => new DirectPropertyMetadata(UnsetValue, DefaultBindingMode, EnableDataValidation); + /// + public override AvaloniaPropertyMetadata GenerateTypeSafeMetadata() + { + var copy = new DirectPropertyMetadata(UnsetValue, DefaultBindingMode, EnableDataValidation); + copy.Freeze(); + return copy; + } } } diff --git a/src/Avalonia.Base/StyledPropertyMetadata`1.cs b/src/Avalonia.Base/StyledPropertyMetadata`1.cs index 9db460dba3..57207e46a0 100644 --- a/src/Avalonia.Base/StyledPropertyMetadata`1.cs +++ b/src/Avalonia.Base/StyledPropertyMetadata`1.cs @@ -59,6 +59,12 @@ namespace Avalonia } } - public override AvaloniaPropertyMetadata GenerateTypeSafeMetadata() => new StyledPropertyMetadata(DefaultValue, DefaultBindingMode, enableDataValidation: EnableDataValidation ?? false); + /// + public override AvaloniaPropertyMetadata GenerateTypeSafeMetadata() + { + var copy = new StyledPropertyMetadata(DefaultValue, DefaultBindingMode, null, EnableDataValidation ?? false); + copy.Freeze(); + return copy; + } } } diff --git a/tests/Avalonia.Base.UnitTests/AvaloniaPropertyTests.cs b/tests/Avalonia.Base.UnitTests/AvaloniaPropertyTests.cs index d23151ca15..041cbfff85 100644 --- a/tests/Avalonia.Base.UnitTests/AvaloniaPropertyTests.cs +++ b/tests/Avalonia.Base.UnitTests/AvaloniaPropertyTests.cs @@ -82,6 +82,27 @@ namespace Avalonia.Base.UnitTests Assert.Equal(BindingMode.TwoWay, result.DefaultBindingMode); } + [Fact] + public void Default_Metadata_Cannot_Be_Changed_After_Property_Initialization() + { + var metadata = new TestMetadata(); + var property = new TestProperty("test", typeof(Class1), metadata); + + Assert.Throws(() => metadata.Merge(new TestMetadata(), property)); + } + + [Fact] + public void Overridden_Metadata_Cannot_Be_Changed_After_OverrideMetadata() + { + var metadata = new TestMetadata(BindingMode.TwoWay); + var overridden = new TestMetadata(); + var property = new TestProperty("test", typeof(Class1), metadata); + + property.OverrideMetadata(overridden); + + Assert.Throws(() => overridden.Merge(new TestMetadata(), property)); + } + [Fact] public void Changed_Observable_Fired() { diff --git a/tests/Avalonia.Base.UnitTests/DirectPropertyTests.cs b/tests/Avalonia.Base.UnitTests/DirectPropertyTests.cs index e7e3b5764f..3e41117788 100644 --- a/tests/Avalonia.Base.UnitTests/DirectPropertyTests.cs +++ b/tests/Avalonia.Base.UnitTests/DirectPropertyTests.cs @@ -1,3 +1,4 @@ +using System; using Xunit; namespace Avalonia.Base.UnitTests @@ -46,6 +47,25 @@ namespace Avalonia.Base.UnitTests Assert.Same(p1.Changed, p2.Changed); } + [Fact] + public void Default_GetMetadata_Cannot_Be_Changed() + { + var p1 = Class1.FooProperty; + var metadata = p1.GetMetadata(); + + Assert.Throws(() => metadata.Merge(new DirectPropertyMetadata(), p1)); + } + + [Fact] + public void AddOwnered_GetMetadata_Cannot_Be_Changed() + { + var p1 = Class1.FooProperty; + var p2 = p1.AddOwner(_ => null, (_, _) => { }); + var metadata = p2.GetMetadata(); + + Assert.Throws(() => metadata.Merge(new DirectPropertyMetadata(), p2)); + } + private class Class1 : AvaloniaObject { public static readonly DirectProperty FooProperty = diff --git a/tests/Avalonia.Base.UnitTests/StyledPropertyTests.cs b/tests/Avalonia.Base.UnitTests/StyledPropertyTests.cs index 5304c74c39..f4b07f58ae 100644 --- a/tests/Avalonia.Base.UnitTests/StyledPropertyTests.cs +++ b/tests/Avalonia.Base.UnitTests/StyledPropertyTests.cs @@ -1,3 +1,4 @@ +using System; using Xunit; namespace Avalonia.Base.UnitTests @@ -32,6 +33,33 @@ namespace Avalonia.Base.UnitTests Assert.Same(p1, p2); } + [Fact] + public void Default_GetMetadata_Cannot_Be_Changed() + { + var p1 = new StyledProperty( + "p1", + typeof(Class1), + typeof(Class1), + new StyledPropertyMetadata()); + var metadata = p1.GetMetadata(); + + Assert.Throws(() => metadata.Merge(new StyledPropertyMetadata(), p1)); + } + + [Fact] + public void AddOwnered_GetMetadata_Cannot_Be_Changed() + { + var p1 = new StyledProperty( + "p1", + typeof(Class1), + typeof(Class1), + new StyledPropertyMetadata()); + var p2 = p1.AddOwner(); + var metadata = p2.GetMetadata(); + + Assert.Throws(() => metadata.Merge(new StyledPropertyMetadata(), p2)); + } + private class Class1 : AvaloniaObject { }