From 9d697c8184626e294d5ae5040449ee83ed0efd99 Mon Sep 17 00:00:00 2001 From: Julien Lebosquain Date: Mon, 15 Apr 2024 17:02:50 +0200 Subject: [PATCH 1/2] Made AvaloniaPropertyMetadata immutable after property initialization --- src/Avalonia.Base/AvaloniaProperty.cs | 11 ++++++-- 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, 106 insertions(+), 5 deletions(-) diff --git a/src/Avalonia.Base/AvaloniaProperty.cs b/src/Avalonia.Base/AvaloniaProperty.cs index a2005f0ab01..da37ea92442 100644 --- a/src/Avalonia.Base/AvaloniaProperty.cs +++ b/src/Avalonia.Base/AvaloniaProperty.cs @@ -50,7 +50,10 @@ private protected AvaloniaProperty( 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,11 +63,12 @@ private protected AvaloniaProperty( _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.Freeze(); _metadata.Add(hostType, metadata ?? throw new ArgumentNullException(nameof(metadata))); _defaultMetadata = metadata.GenerateTypeSafeMetadata(); _singleMetadata = new(hostType, metadata); @@ -584,6 +588,7 @@ private protected void OverrideMetadata(Type type, AvaloniaPropertyMetadata meta 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 ec29d146931..2aec72aaf70 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 @@ public virtual void Merge( 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 @@ public virtual void Merge( 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 ef1b67615f5..720b21c4844 100644 --- a/src/Avalonia.Base/DirectProperty.cs +++ b/src/Avalonia.Base/DirectProperty.cs @@ -94,6 +94,7 @@ public DirectProperty AddOwner( 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 5471826f9fd..2c653f7481b 100644 --- a/src/Avalonia.Base/DirectPropertyMetadata`1.cs +++ b/src/Avalonia.Base/DirectPropertyMetadata`1.cs @@ -46,6 +46,12 @@ public override void Merge(AvaloniaPropertyMetadata baseMetadata, AvaloniaProper } } - 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 9db460dba30..57207e46a09 100644 --- a/src/Avalonia.Base/StyledPropertyMetadata`1.cs +++ b/src/Avalonia.Base/StyledPropertyMetadata`1.cs @@ -59,6 +59,12 @@ public override void Merge(AvaloniaPropertyMetadata baseMetadata, AvaloniaProper } } - 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 d23151ca15f..041cbfff851 100644 --- a/tests/Avalonia.Base.UnitTests/AvaloniaPropertyTests.cs +++ b/tests/Avalonia.Base.UnitTests/AvaloniaPropertyTests.cs @@ -82,6 +82,27 @@ public void OverrideMetadata_Should_Merge_Values() 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 e7e3b5764fe..3e41117788b 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 @@ public void AddOwnered_Properties_Should_Share_Observables() 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 5304c74c39b..f4b07f58ae1 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 @@ public void AddOwnered_Property_Should_Be_Same() 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 { } From 92b79dc7deeb73e1cb84db93c35019abb15ba70c Mon Sep 17 00:00:00 2001 From: Julien Lebosquain Date: Tue, 16 Apr 2024 10:54:13 +0200 Subject: [PATCH 2/2] Removed redundant throw --- src/Avalonia.Base/AvaloniaProperty.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Avalonia.Base/AvaloniaProperty.cs b/src/Avalonia.Base/AvaloniaProperty.cs index da37ea92442..b096de26a13 100644 --- a/src/Avalonia.Base/AvaloniaProperty.cs +++ b/src/Avalonia.Base/AvaloniaProperty.cs @@ -69,7 +69,7 @@ private protected AvaloniaProperty( Id = s_nextId++; metadata.Freeze(); - _metadata.Add(hostType, metadata ?? throw new ArgumentNullException(nameof(metadata))); + _metadata.Add(hostType, metadata); _defaultMetadata = metadata.GenerateTypeSafeMetadata(); _singleMetadata = new(hostType, metadata); }