From 0c2fa0bb3ae214d91a5775b21b4c04ffd842f8f6 Mon Sep 17 00:00:00 2001 From: remi_bourgarel Date: Mon, 9 Jul 2018 20:14:58 +0200 Subject: [PATCH 1/5] trying to fix 1009 by changing the AssignToProperties method behavior --- .../ParameterCollectionExtensions.cs | 34 +++++++++++++++---- ...meterCollectionAssignmentExtensionsTest.cs | 11 +++--- 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/src/Microsoft.AspNetCore.Blazor/Components/ParameterCollectionExtensions.cs b/src/Microsoft.AspNetCore.Blazor/Components/ParameterCollectionExtensions.cs index a0e85defb..9c9e63969 100644 --- a/src/Microsoft.AspNetCore.Blazor/Components/ParameterCollectionExtensions.cs +++ b/src/Microsoft.AspNetCore.Blazor/Components/ParameterCollectionExtensions.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using Microsoft.AspNetCore.Blazor.Reflection; @@ -18,8 +18,6 @@ public static class ParameterCollectionExtensions { private const BindingFlags _bindablePropertyFlags = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.IgnoreCase; - private delegate void WriteParameterAction(ref RenderTreeFrame frame, object target); - private readonly static IDictionary> _cachedParameterWriters = new ConcurrentDictionary>(); @@ -44,6 +42,7 @@ public static void AssignToProperties( parameterWriters = CreateParameterWriters(targetType); _cachedParameterWriters[targetType] = parameterWriters; } + var usedWriters = new List(); foreach (var parameter in parameterCollection) { @@ -55,7 +54,8 @@ public static void AssignToProperties( try { - parameterWriter(ref parameter.Frame, target); + parameterWriter.Setter(ref parameter.Frame, target); + usedWriters.Add(parameterWriter); } catch (Exception ex) { @@ -64,6 +64,12 @@ public static void AssignToProperties( $"type '{target.GetType().FullName}'. The error was: {ex.Message}", ex); } } + foreach (var nonUsedParameter in parameterWriters.Values.Except(usedWriters)) + { + var type = nonUsedParameter.PropertyType; + var nullValueTreeFrame = RenderTreeFrame.Attribute(0, null, type.IsValueType ? Activator.CreateInstance(type) : null); + nonUsedParameter.Setter(ref nullValueTreeFrame, target); + } } private static IDictionary CreateParameterWriters(Type targetType) @@ -82,10 +88,11 @@ private static IDictionary CreateParameterWriters( $"name '{propertyName.ToLowerInvariant()}'. Parameter names are case-insensitive and must be unique."); } - result.Add(propertyName, (ref RenderTreeFrame frame, object target) => + result.Add(propertyName, new WriteParameterAction((ref RenderTreeFrame frame, object target) => { propertySetter.SetValue(target, frame.AttributeValue); - }); + }, + propertyInfo.PropertyType)); } return result; @@ -122,5 +129,20 @@ private static void ThrowForUnknownIncomingParameterName(Type targetType, string $"matching the name '{parameterName}'."); } } + + + private class WriteParameterAction + { + public WriteParameterAction(WriteParameterActionSetter setter, Type propertyType) + { + Setter = setter; + PropertyType = propertyType; + } + + public delegate void WriteParameterActionSetter(ref RenderTreeFrame frame, object target); + public WriteParameterActionSetter Setter { get; set; } + + public Type PropertyType { get; set; } + } } } diff --git a/test/Microsoft.AspNetCore.Blazor.Test/ParameterCollectionAssignmentExtensionsTest.cs b/test/Microsoft.AspNetCore.Blazor.Test/ParameterCollectionAssignmentExtensionsTest.cs index b99c42993..90262b630 100644 --- a/test/Microsoft.AspNetCore.Blazor.Test/ParameterCollectionAssignmentExtensionsTest.cs +++ b/test/Microsoft.AspNetCore.Blazor.Test/ParameterCollectionAssignmentExtensionsTest.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -72,7 +72,7 @@ public void IncomingParameterMatchesInheritedDeclaredParameter_SetsValue() } [Fact] - public void NoIncomingParameterMatchesDeclaredParameter_LeavesValueUnchanged() + public void NoIncomingParameterMatchesDeclaredParameter_SetValuesDefault() { // Arrange var existingObjectValue = new object(); @@ -89,11 +89,10 @@ public void NoIncomingParameterMatchesDeclaredParameter_LeavesValueUnchanged() parameterCollection.AssignToProperties(target); // Assert - Assert.Equal(456, target.IntProp); - Assert.Equal("Existing value", target.StringProp); - Assert.Same(existingObjectValue, target.ObjectPropCurrentValue); + Assert.Equal(0, target.IntProp); + Assert.Null(target.StringProp); + Assert.Null(target.ObjectPropCurrentValue); } - [Fact] public void IncomingParameterMatchesNoDeclaredParameter_Throws() { From 1cb7b91c4a1a05cd080a46b8847caafe653bbdfd Mon Sep 17 00:00:00 2001 From: remi_bourgarel Date: Fri, 12 Oct 2018 22:46:43 +0200 Subject: [PATCH 2/5] Remove allocation and class generation, rename method to suit PR review --- .../Components/BlazorComponent.cs | 2 +- .../ParameterCollectionExtensions.cs | 49 ++++++------------- .../Layouts/LayoutDisplay.cs | 2 +- .../Reflection/IPropertySetter.cs | 4 +- .../Reflection/MemberAssignment.cs | 9 +++- .../Routing/Router.cs | 2 +- ...meterCollectionAssignmentExtensionsTest.cs | 20 ++++---- .../RenderTreeDiffBuilderTest.cs | 2 +- .../RendererTest.cs | 4 +- test/shared/AutoRenderComponent.cs | 2 +- 10 files changed, 42 insertions(+), 54 deletions(-) diff --git a/src/Microsoft.AspNetCore.Blazor/Components/BlazorComponent.cs b/src/Microsoft.AspNetCore.Blazor/Components/BlazorComponent.cs index a6b787af0..ae17abdf3 100644 --- a/src/Microsoft.AspNetCore.Blazor/Components/BlazorComponent.cs +++ b/src/Microsoft.AspNetCore.Blazor/Components/BlazorComponent.cs @@ -151,7 +151,7 @@ void IComponent.Init(RenderHandle renderHandle) /// The parameters to apply. public virtual void SetParameters(ParameterCollection parameters) { - parameters.AssignToProperties(this); + parameters.SetParameterProperties(this); if (!_hasCalledInit) { diff --git a/src/Microsoft.AspNetCore.Blazor/Components/ParameterCollectionExtensions.cs b/src/Microsoft.AspNetCore.Blazor/Components/ParameterCollectionExtensions.cs index 9c9e63969..0b0d26069 100644 --- a/src/Microsoft.AspNetCore.Blazor/Components/ParameterCollectionExtensions.cs +++ b/src/Microsoft.AspNetCore.Blazor/Components/ParameterCollectionExtensions.cs @@ -12,14 +12,14 @@ namespace Microsoft.AspNetCore.Blazor.Components { /// - /// Extension methods for the type. + /// Extsion methods for the type. /// public static class ParameterCollectionExtensions { private const BindingFlags _bindablePropertyFlags = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.IgnoreCase; - private readonly static IDictionary> _cachedParameterWriters - = new ConcurrentDictionary>(); + private readonly static IDictionary> _cachedParameterWriters + = new ConcurrentDictionary>(); /// /// Iterates through the , assigning each parameter @@ -27,8 +27,8 @@ private readonly static IDictionary /// The . /// An object that has a public writable property matching each parameter's name and type. - public static void AssignToProperties( - in this ParameterCollection parameterCollection, + public static void SetParameterProperties( + this ParameterCollection parameterCollection, object target) { if (target == null) @@ -42,7 +42,7 @@ public static void AssignToProperties( parameterWriters = CreateParameterWriters(targetType); _cachedParameterWriters[targetType] = parameterWriters; } - var usedWriters = new List(); + var localParameterWriter = parameterWriters.Values.ToList(); foreach (var parameter in parameterCollection) { @@ -54,8 +54,8 @@ public static void AssignToProperties( try { - parameterWriter.Setter(ref parameter.Frame, target); - usedWriters.Add(parameterWriter); + parameterWriter.SetValue(target, parameter.Frame.AttributeValue); + localParameterWriter.Remove(parameterWriter); } catch (Exception ex) { @@ -64,21 +64,19 @@ public static void AssignToProperties( $"type '{target.GetType().FullName}'. The error was: {ex.Message}", ex); } } - foreach (var nonUsedParameter in parameterWriters.Values.Except(usedWriters)) + foreach (var nonUsedParameter in localParameterWriter) { - var type = nonUsedParameter.PropertyType; - var nullValueTreeFrame = RenderTreeFrame.Attribute(0, null, type.IsValueType ? Activator.CreateInstance(type) : null); - nonUsedParameter.Setter(ref nullValueTreeFrame, target); + nonUsedParameter.SetValue(target, nonUsedParameter.GetDefaultValue()); } } - private static IDictionary CreateParameterWriters(Type targetType) + private static IDictionary CreateParameterWriters(Type targetType) { - var result = new Dictionary(StringComparer.OrdinalIgnoreCase); + var result = new Dictionary(StringComparer.OrdinalIgnoreCase); foreach (var propertyInfo in GetBindableProperties(targetType)) { - var propertySetter = MemberAssignment.CreatePropertySetter(targetType, propertyInfo); + IPropertySetter propertySetter = MemberAssignment.CreatePropertySetter(targetType, propertyInfo); var propertyName = propertyInfo.Name; if (result.ContainsKey(propertyName)) @@ -88,11 +86,7 @@ private static IDictionary CreateParameterWriters( $"name '{propertyName.ToLowerInvariant()}'. Parameter names are case-insensitive and must be unique."); } - result.Add(propertyName, new WriteParameterAction((ref RenderTreeFrame frame, object target) => - { - propertySetter.SetValue(target, frame.AttributeValue); - }, - propertyInfo.PropertyType)); + result.Add(propertyName, propertySetter); } return result; @@ -129,20 +123,5 @@ private static void ThrowForUnknownIncomingParameterName(Type targetType, string $"matching the name '{parameterName}'."); } } - - - private class WriteParameterAction - { - public WriteParameterAction(WriteParameterActionSetter setter, Type propertyType) - { - Setter = setter; - PropertyType = propertyType; - } - - public delegate void WriteParameterActionSetter(ref RenderTreeFrame frame, object target); - public WriteParameterActionSetter Setter { get; set; } - - public Type PropertyType { get; set; } - } } } diff --git a/src/Microsoft.AspNetCore.Blazor/Layouts/LayoutDisplay.cs b/src/Microsoft.AspNetCore.Blazor/Layouts/LayoutDisplay.cs index c37404d6f..bbe9b5a71 100644 --- a/src/Microsoft.AspNetCore.Blazor/Layouts/LayoutDisplay.cs +++ b/src/Microsoft.AspNetCore.Blazor/Layouts/LayoutDisplay.cs @@ -42,7 +42,7 @@ public void Init(RenderHandle renderHandle) /// public void SetParameters(ParameterCollection parameters) { - parameters.AssignToProperties(this); + parameters.SetParameterProperties(this); Render(); } diff --git a/src/Microsoft.AspNetCore.Blazor/Reflection/IPropertySetter.cs b/src/Microsoft.AspNetCore.Blazor/Reflection/IPropertySetter.cs index a106a4d7d..2e15f7ad6 100644 --- a/src/Microsoft.AspNetCore.Blazor/Reflection/IPropertySetter.cs +++ b/src/Microsoft.AspNetCore.Blazor/Reflection/IPropertySetter.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -8,5 +8,7 @@ namespace Microsoft.AspNetCore.Blazor.Reflection internal interface IPropertySetter { void SetValue(object target, object value); + + object GetDefaultValue(); } } diff --git a/src/Microsoft.AspNetCore.Blazor/Reflection/MemberAssignment.cs b/src/Microsoft.AspNetCore.Blazor/Reflection/MemberAssignment.cs index 10fdb47bd..436c39549 100644 --- a/src/Microsoft.AspNetCore.Blazor/Reflection/MemberAssignment.cs +++ b/src/Microsoft.AspNetCore.Blazor/Reflection/MemberAssignment.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -43,11 +43,18 @@ public static IPropertySetter CreatePropertySetter(Type targetType, PropertyInfo class PropertySetter : IPropertySetter { private readonly Action _setterDelegate; + private Type _propertyType; public PropertySetter(MethodInfo setMethod) { _setterDelegate = (Action)Delegate.CreateDelegate( typeof(Action), setMethod); + _propertyType = typeof(TValue); + } + + public object GetDefaultValue() + { + return typeof(TValue).IsValueType ? Activator.CreateInstance(typeof(TValue)) : null; } public void SetValue(object target, object value) diff --git a/src/Microsoft.AspNetCore.Blazor/Routing/Router.cs b/src/Microsoft.AspNetCore.Blazor/Routing/Router.cs index 1925bba6f..96adfbe4c 100644 --- a/src/Microsoft.AspNetCore.Blazor/Routing/Router.cs +++ b/src/Microsoft.AspNetCore.Blazor/Routing/Router.cs @@ -45,7 +45,7 @@ public void Init(RenderHandle renderHandle) /// public void SetParameters(ParameterCollection parameters) { - parameters.AssignToProperties(this); + parameters.SetParameterProperties(this); var types = ComponentResolver.ResolveComponents(AppAssembly); Routes = RouteTable.Create(types); Refresh(); diff --git a/test/Microsoft.AspNetCore.Blazor.Test/ParameterCollectionAssignmentExtensionsTest.cs b/test/Microsoft.AspNetCore.Blazor.Test/ParameterCollectionAssignmentExtensionsTest.cs index 90262b630..9f0688c3e 100644 --- a/test/Microsoft.AspNetCore.Blazor.Test/ParameterCollectionAssignmentExtensionsTest.cs +++ b/test/Microsoft.AspNetCore.Blazor.Test/ParameterCollectionAssignmentExtensionsTest.cs @@ -27,7 +27,7 @@ public void IncomingParameterMatchesAnnotatedPrivateProperty_SetsValue() var target = new HasInstanceProperties(); // Act - parameterCollection.AssignToProperties(target); + parameterCollection.SetParameterProperties(target); // Assert Assert.Equal(123, target.IntProp); @@ -46,7 +46,7 @@ public void IncomingParameterMatchesDeclaredParameterCaseInsensitively_SetsValue var target = new HasInstanceProperties(); // Act - parameterCollection.AssignToProperties(target); + parameterCollection.SetParameterProperties(target); // Assert Assert.Equal(123, target.IntProp); @@ -64,7 +64,7 @@ public void IncomingParameterMatchesInheritedDeclaredParameter_SetsValue() var target = new HasInheritedProperties(); // Act - parameterCollection.AssignToProperties(target); + parameterCollection.SetParameterProperties(target); // Assert Assert.Equal(123, target.IntProp); @@ -86,7 +86,7 @@ public void NoIncomingParameterMatchesDeclaredParameter_SetValuesDefault() var parameterCollection = new ParameterCollectionBuilder().Build(); // Act - parameterCollection.AssignToProperties(target); + parameterCollection.SetParameterProperties(target); // Assert Assert.Equal(0, target.IntProp); @@ -105,7 +105,7 @@ public void IncomingParameterMatchesNoDeclaredParameter_Throws() // Act var ex = Assert.Throws( - () => parameterCollection.AssignToProperties(target)); + () => parameterCollection.SetParameterProperties(target)); // Assert Assert.Equal( @@ -126,7 +126,7 @@ public void IncomingParameterMatchesPropertyNotDeclaredAsParameter_Throws() // Act var ex = Assert.Throws( - () => parameterCollection.AssignToProperties(target)); + () => parameterCollection.SetParameterProperties(target)); // Assert Assert.Equal(default, target.IntProp); @@ -149,7 +149,7 @@ public void IncomingParameterValueMismatchesDeclaredParameterType_Throws() // Act var ex = Assert.Throws( - () => parameterCollection.AssignToProperties(target)); + () => parameterCollection.SetParameterProperties(target)); // Assert Assert.Equal( @@ -170,7 +170,7 @@ public void PropertyExplicitSetterException_Throws() // Act var ex = Assert.Throws( - () => parameterCollection.AssignToProperties(target)); + () => parameterCollection.SetParameterProperties(target)); // Assert Assert.Equal( @@ -188,7 +188,7 @@ public void DeclaredParametersVaryOnlyByCase_Throws() // Act var ex = Assert.Throws(() => - parameterCollection.AssignToProperties(target)); + parameterCollection.SetParameterProperties(target)); // Assert Assert.Equal( @@ -211,7 +211,7 @@ public void DeclaredParameterClashesWithInheritedParameter_Throws() // Act var ex = Assert.Throws(() => - parameterCollection.AssignToProperties(target)); + parameterCollection.SetParameterProperties(target)); // Assert Assert.Equal( diff --git a/test/Microsoft.AspNetCore.Blazor.Test/RenderTreeDiffBuilderTest.cs b/test/Microsoft.AspNetCore.Blazor.Test/RenderTreeDiffBuilderTest.cs index fa9a43234..935076ba1 100644 --- a/test/Microsoft.AspNetCore.Blazor.Test/RenderTreeDiffBuilderTest.cs +++ b/test/Microsoft.AspNetCore.Blazor.Test/RenderTreeDiffBuilderTest.cs @@ -1523,7 +1523,7 @@ private class FakeComponent : IComponent public void Init(RenderHandle renderHandle) { } public void SetParameters(ParameterCollection parameters) { - parameters.AssignToProperties(this); + parameters.SetParameterProperties(this); } } diff --git a/test/Microsoft.AspNetCore.Blazor.Test/RendererTest.cs b/test/Microsoft.AspNetCore.Blazor.Test/RendererTest.cs index 26f8d39f5..fc3255155 100644 --- a/test/Microsoft.AspNetCore.Blazor.Test/RendererTest.cs +++ b/test/Microsoft.AspNetCore.Blazor.Test/RendererTest.cs @@ -1135,7 +1135,7 @@ public void Init(RenderHandle renderHandle) => RenderHandle = renderHandle; public void SetParameters(ParameterCollection parameters) - => parameters.AssignToProperties(this); + => parameters.SetParameterProperties(this); } private class EventComponent : AutoRenderComponent, IComponent, IHandleEvent @@ -1243,7 +1243,7 @@ public void Init(RenderHandle renderHandle) public void SetParameters(ParameterCollection parameters) { - parameters.AssignToProperties(this); + parameters.SetParameterProperties(this); Render(); } diff --git a/test/shared/AutoRenderComponent.cs b/test/shared/AutoRenderComponent.cs index f30e450e1..e0bf7c348 100644 --- a/test/shared/AutoRenderComponent.cs +++ b/test/shared/AutoRenderComponent.cs @@ -17,7 +17,7 @@ public void Init(RenderHandle renderHandle) public void SetParameters(ParameterCollection parameters) { - parameters.AssignToProperties(this); + parameters.SetParameterProperties(this); TriggerRender(); } From e7b7b76015fabaf26321bba38de59953824275f5 Mon Sep 17 00:00:00 2001 From: remi_bourgarel Date: Fri, 12 Oct 2018 22:48:51 +0200 Subject: [PATCH 3/5] Add local default value --- .../Reflection/MemberAssignment.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.AspNetCore.Blazor/Reflection/MemberAssignment.cs b/src/Microsoft.AspNetCore.Blazor/Reflection/MemberAssignment.cs index 436c39549..f15c9e02f 100644 --- a/src/Microsoft.AspNetCore.Blazor/Reflection/MemberAssignment.cs +++ b/src/Microsoft.AspNetCore.Blazor/Reflection/MemberAssignment.cs @@ -43,18 +43,19 @@ public static IPropertySetter CreatePropertySetter(Type targetType, PropertyInfo class PropertySetter : IPropertySetter { private readonly Action _setterDelegate; - private Type _propertyType; + private object _defaultValue; public PropertySetter(MethodInfo setMethod) { _setterDelegate = (Action)Delegate.CreateDelegate( typeof(Action), setMethod); - _propertyType = typeof(TValue); + var propertyType = typeof(TValue); + _defaultValue = propertyType.IsValueType ? Activator.CreateInstance(propertyType) : null; } public object GetDefaultValue() { - return typeof(TValue).IsValueType ? Activator.CreateInstance(typeof(TValue)) : null; + return _defaultValue; } public void SetValue(object target, object value) From bb07cc275abfcfb0b5a3d67f3139fd19f746ffad Mon Sep 17 00:00:00 2001 From: remi_bourgarel Date: Wed, 17 Oct 2018 22:29:31 +0200 Subject: [PATCH 4/5] fix build after PR --- .../Components/ParameterCollectionExtensions.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.AspNetCore.Blazor/Components/ParameterCollectionExtensions.cs b/src/Microsoft.AspNetCore.Blazor/Components/ParameterCollectionExtensions.cs index 6a3fce2cd..53d0c1433 100644 --- a/src/Microsoft.AspNetCore.Blazor/Components/ParameterCollectionExtensions.cs +++ b/src/Microsoft.AspNetCore.Blazor/Components/ParameterCollectionExtensions.cs @@ -6,6 +6,7 @@ using System.Collections.Concurrent; using System.Collections.Generic; using System.Reflection; +using System.Linq; namespace Microsoft.AspNetCore.Blazor.Components { @@ -52,7 +53,7 @@ public static void SetParameterProperties( try { - parameterWriter.SetValue(target, parameter.Frame.AttributeValue); + parameterWriter.SetValue(target, parameter.Value); localParameterWriter.Remove(parameterWriter); } catch (Exception ex) From c54dac63756b81736bd7c7b52caf614c3fc66155 Mon Sep 17 00:00:00 2001 From: remi_bourgarel Date: Sun, 25 Nov 2018 19:12:33 +0100 Subject: [PATCH 5/5] move changes to renamed folders --- .../CascadingValue.cs | 2 +- .../ComponentBase.cs | 2 +- .../Layouts/LayoutDisplay.cs | 4 +-- .../ParameterCollectionExtensions.cs | 27 +++++++++------- .../Reflection/IPropertySetter.cs | 4 ++- .../Reflection/MemberAssignment.cs | 10 +++++- .../Routing/Router.cs | 2 +- ...meterCollectionAssignmentExtensionsTest.cs | 32 +++++++++---------- .../RenderTreeDiffBuilderTest.cs | 2 +- .../RendererTest.cs | 4 +-- 10 files changed, 51 insertions(+), 38 deletions(-) diff --git a/src/Microsoft.AspNetCore.Components/CascadingValue.cs b/src/Microsoft.AspNetCore.Components/CascadingValue.cs index 006fca448..53e356ae2 100644 --- a/src/Microsoft.AspNetCore.Components/CascadingValue.cs +++ b/src/Microsoft.AspNetCore.Components/CascadingValue.cs @@ -58,7 +58,7 @@ public void Init(RenderHandle renderHandle) public void SetParameters(ParameterCollection parameters) { // Implementing the parameter binding manually, instead of just calling - // parameters.AssignToProperties(this), is just a very slight perf optimization + // parameters.SetParameterProperties(this), is just a very slight perf optimization // and makes it simpler impose rules about the params being required or not. var hasSuppliedValue = false; diff --git a/src/Microsoft.AspNetCore.Components/ComponentBase.cs b/src/Microsoft.AspNetCore.Components/ComponentBase.cs index 1ff2b60b7..58a23ee54 100644 --- a/src/Microsoft.AspNetCore.Components/ComponentBase.cs +++ b/src/Microsoft.AspNetCore.Components/ComponentBase.cs @@ -151,7 +151,7 @@ void IComponent.Init(RenderHandle renderHandle) /// The parameters to apply. public virtual void SetParameters(ParameterCollection parameters) { - parameters.AssignToProperties(this); + parameters.SetParameterProperties(this); if (!_hasCalledInit) { diff --git a/src/Microsoft.AspNetCore.Components/Layouts/LayoutDisplay.cs b/src/Microsoft.AspNetCore.Components/Layouts/LayoutDisplay.cs index c9b26ea89..afd205c40 100644 --- a/src/Microsoft.AspNetCore.Components/Layouts/LayoutDisplay.cs +++ b/src/Microsoft.AspNetCore.Components/Layouts/LayoutDisplay.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -42,7 +42,7 @@ public void Init(RenderHandle renderHandle) /// public void SetParameters(ParameterCollection parameters) { - parameters.AssignToProperties(this); + parameters.SetParameterProperties(this); Render(); } diff --git a/src/Microsoft.AspNetCore.Components/ParameterCollectionExtensions.cs b/src/Microsoft.AspNetCore.Components/ParameterCollectionExtensions.cs index 231970875..eb255223e 100644 --- a/src/Microsoft.AspNetCore.Components/ParameterCollectionExtensions.cs +++ b/src/Microsoft.AspNetCore.Components/ParameterCollectionExtensions.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Concurrent; using System.Collections.Generic; +using System.Linq; using System.Reflection; namespace Microsoft.AspNetCore.Components @@ -16,10 +17,7 @@ public static class ParameterCollectionExtensions { private const BindingFlags _bindablePropertyFlags = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.IgnoreCase; - private delegate void WriteParameterAction(object target, object parameterValue); - - private readonly static IDictionary> _cachedParameterWriters - = new ConcurrentDictionary>(); + private readonly static IDictionary> _cachedParameterWriters = new ConcurrentDictionary>(); /// /// Iterates through the , assigning each parameter @@ -27,7 +25,7 @@ private readonly static IDictionary /// The . /// An object that has a public writable property matching each parameter's name and type. - public static void AssignToProperties( + public static void SetParameterProperties( in this ParameterCollection parameterCollection, object target) { @@ -43,6 +41,8 @@ public static void AssignToProperties( _cachedParameterWriters[targetType] = parameterWriters; } + var localParameterWriter = parameterWriters.Values.ToList(); + foreach (var parameter in parameterCollection) { var parameterName = parameter.Name; @@ -53,7 +53,8 @@ public static void AssignToProperties( try { - parameterWriter(target, parameter.Value); + parameterWriter.SetValue(target, parameter.Value); + localParameterWriter.Remove(parameterWriter); } catch (Exception ex) { @@ -62,14 +63,19 @@ public static void AssignToProperties( $"type '{target.GetType().FullName}'. The error was: {ex.Message}", ex); } } + + foreach (var nonUsedParameter in localParameterWriter) + { + nonUsedParameter.SetValue(target, nonUsedParameter.GetDefaultValue()); + } } internal static IEnumerable GetCandidateBindableProperties(Type targetType) => MemberAssignment.GetPropertiesIncludingInherited(targetType, _bindablePropertyFlags); - private static IDictionary CreateParameterWriters(Type targetType) + private static IDictionary CreateParameterWriters(Type targetType) { - var result = new Dictionary(StringComparer.OrdinalIgnoreCase); + var result = new Dictionary(StringComparer.OrdinalIgnoreCase); foreach (var propertyInfo in GetCandidateBindableProperties(targetType)) { @@ -90,10 +96,7 @@ private static IDictionary CreateParameterWriters( $"name '{propertyName.ToLowerInvariant()}'. Parameter names are case-insensitive and must be unique."); } - result.Add(propertyName, (object target, object parameterValue) => - { - propertySetter.SetValue(target, parameterValue); - }); + result.Add(propertyName, propertySetter); } return result; diff --git a/src/Microsoft.AspNetCore.Components/Reflection/IPropertySetter.cs b/src/Microsoft.AspNetCore.Components/Reflection/IPropertySetter.cs index 061580f2c..62bab3467 100644 --- a/src/Microsoft.AspNetCore.Components/Reflection/IPropertySetter.cs +++ b/src/Microsoft.AspNetCore.Components/Reflection/IPropertySetter.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -8,5 +8,7 @@ namespace Microsoft.AspNetCore.Components.Reflection internal interface IPropertySetter { void SetValue(object target, object value); + + object GetDefaultValue(); } } diff --git a/src/Microsoft.AspNetCore.Components/Reflection/MemberAssignment.cs b/src/Microsoft.AspNetCore.Components/Reflection/MemberAssignment.cs index eff055a1c..5c9ed16e2 100644 --- a/src/Microsoft.AspNetCore.Components/Reflection/MemberAssignment.cs +++ b/src/Microsoft.AspNetCore.Components/Reflection/MemberAssignment.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -43,15 +43,23 @@ public static IPropertySetter CreatePropertySetter(Type targetType, PropertyInfo class PropertySetter : IPropertySetter { private readonly Action _setterDelegate; + private object _defaultValue; public PropertySetter(MethodInfo setMethod) { _setterDelegate = (Action)Delegate.CreateDelegate( typeof(Action), setMethod); + var propertyType = typeof(TValue); + _defaultValue = propertyType.IsValueType ? Activator.CreateInstance(propertyType) : null; } public void SetValue(object target, object value) => _setterDelegate((TTarget)target, (TValue)value); + + public object GetDefaultValue() + { + return _defaultValue; + } } } } diff --git a/src/Microsoft.AspNetCore.Components/Routing/Router.cs b/src/Microsoft.AspNetCore.Components/Routing/Router.cs index 8ccbfc119..82b15251c 100644 --- a/src/Microsoft.AspNetCore.Components/Routing/Router.cs +++ b/src/Microsoft.AspNetCore.Components/Routing/Router.cs @@ -45,7 +45,7 @@ public void Init(RenderHandle renderHandle) /// public void SetParameters(ParameterCollection parameters) { - parameters.AssignToProperties(this); + parameters.SetParameterProperties(this); var types = ComponentResolver.ResolveComponents(AppAssembly); Routes = RouteTable.Create(types); Refresh(); diff --git a/test/Microsoft.AspNetCore.Components.Test/ParameterCollectionAssignmentExtensionsTest.cs b/test/Microsoft.AspNetCore.Components.Test/ParameterCollectionAssignmentExtensionsTest.cs index cc8d62191..a3c4ba3d7 100644 --- a/test/Microsoft.AspNetCore.Components.Test/ParameterCollectionAssignmentExtensionsTest.cs +++ b/test/Microsoft.AspNetCore.Components.Test/ParameterCollectionAssignmentExtensionsTest.cs @@ -27,7 +27,7 @@ public void IncomingParameterMatchesAnnotatedPrivateProperty_SetsValue() var target = new HasInstanceProperties(); // Act - parameterCollection.AssignToProperties(target); + parameterCollection.SetParameterProperties(target); // Assert Assert.Equal(123, target.IntProp); @@ -46,7 +46,7 @@ public void IncomingParameterMatchesDeclaredParameterCaseInsensitively_SetsValue var target = new HasInstanceProperties(); // Act - parameterCollection.AssignToProperties(target); + parameterCollection.SetParameterProperties(target); // Assert Assert.Equal(123, target.IntProp); @@ -64,7 +64,7 @@ public void IncomingParameterMatchesInheritedDeclaredParameter_SetsValue() var target = new HasInheritedProperties(); // Act - parameterCollection.AssignToProperties(target); + parameterCollection.SetParameterProperties(target); // Assert Assert.Equal(123, target.IntProp); @@ -72,7 +72,7 @@ public void IncomingParameterMatchesInheritedDeclaredParameter_SetsValue() } [Fact] - public void NoIncomingParameterMatchesDeclaredParameter_LeavesValueUnchanged() + public void NoIncomingParameterMatchesDeclaredParameter_SetValuesDefault() { // Arrange var existingObjectValue = new object(); @@ -86,12 +86,12 @@ public void NoIncomingParameterMatchesDeclaredParameter_LeavesValueUnchanged() var parameterCollection = new ParameterCollectionBuilder().Build(); // Act - parameterCollection.AssignToProperties(target); + parameterCollection.SetParameterProperties(target); // Assert - Assert.Equal(456, target.IntProp); - Assert.Equal("Existing value", target.StringProp); - Assert.Same(existingObjectValue, target.ObjectPropCurrentValue); + Assert.Equal(0, target.IntProp); + Assert.Null(target.StringProp); + Assert.Null(target.ObjectPropCurrentValue); } [Fact] @@ -106,7 +106,7 @@ public void IncomingParameterMatchesNoDeclaredParameter_Throws() // Act var ex = Assert.Throws( - () => parameterCollection.AssignToProperties(target)); + () => parameterCollection.SetParameterProperties(target)); // Assert Assert.Equal( @@ -127,7 +127,7 @@ public void IncomingParameterMatchesPropertyNotDeclaredAsParameter_Throws() // Act var ex = Assert.Throws( - () => parameterCollection.AssignToProperties(target)); + () => parameterCollection.SetParameterProperties(target)); // Assert Assert.Equal(default, target.IntProp); @@ -150,7 +150,7 @@ public void IncomingParameterValueMismatchesDeclaredParameterType_Throws() // Act var ex = Assert.Throws( - () => parameterCollection.AssignToProperties(target)); + () => parameterCollection.SetParameterProperties(target)); // Assert Assert.Equal( @@ -171,7 +171,7 @@ public void PropertyExplicitSetterException_Throws() // Act var ex = Assert.Throws( - () => parameterCollection.AssignToProperties(target)); + () => parameterCollection.SetParameterProperties(target)); // Assert Assert.Equal( @@ -189,7 +189,7 @@ public void DeclaredParametersVaryOnlyByCase_Throws() // Act var ex = Assert.Throws(() => - parameterCollection.AssignToProperties(target)); + parameterCollection.SetParameterProperties(target)); // Assert Assert.Equal( @@ -205,14 +205,14 @@ public void DeclaredParameterClashesWithInheritedParameter_Throws() // an allowed scenario because there would be no way for the consumer to specify // both property values, and it's no good leaving the shadowed one unset because the // base class can legitimately depend on it for correct functioning. - + // Arrange var parameterCollection = new ParameterCollectionBuilder().Build(); var target = new HasParameterClashingWithInherited(); // Act var ex = Assert.Throws(() => - parameterCollection.AssignToProperties(target)); + parameterCollection.SetParameterProperties(target)); // Assert Assert.Equal( @@ -226,7 +226,7 @@ class HasInstanceProperties { // "internal" to show we're not requiring public accessors, but also // to keep the assertions simple in the tests - + [Parameter] internal int IntProp { get; set; } [Parameter] internal string StringProp { get; set; } diff --git a/test/Microsoft.AspNetCore.Components.Test/RenderTreeDiffBuilderTest.cs b/test/Microsoft.AspNetCore.Components.Test/RenderTreeDiffBuilderTest.cs index 53dd09306..4866be353 100644 --- a/test/Microsoft.AspNetCore.Components.Test/RenderTreeDiffBuilderTest.cs +++ b/test/Microsoft.AspNetCore.Components.Test/RenderTreeDiffBuilderTest.cs @@ -1557,7 +1557,7 @@ private class FakeComponent : IComponent public void Init(RenderHandle renderHandle) { } public void SetParameters(ParameterCollection parameters) { - parameters.AssignToProperties(this); + parameters.SetParameterProperties(this); } } diff --git a/test/Microsoft.AspNetCore.Components.Test/RendererTest.cs b/test/Microsoft.AspNetCore.Components.Test/RendererTest.cs index 6d84c590c..5fe8b7909 100644 --- a/test/Microsoft.AspNetCore.Components.Test/RendererTest.cs +++ b/test/Microsoft.AspNetCore.Components.Test/RendererTest.cs @@ -1202,7 +1202,7 @@ public void Init(RenderHandle renderHandle) => RenderHandle = renderHandle; public void SetParameters(ParameterCollection parameters) - => parameters.AssignToProperties(this); + => parameters.SetParameterProperties(this); } private class EventComponent : AutoRenderComponent, IComponent, IHandleEvent @@ -1310,7 +1310,7 @@ public void Init(RenderHandle renderHandle) public void SetParameters(ParameterCollection parameters) { - parameters.AssignToProperties(this); + parameters.SetParameterProperties(this); Render(); }