Skip to content
This repository was archived by the owner on Feb 25, 2021. It is now read-only.

trying to fix 1009 by changing the AssignToProperties method behavior #1108

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Microsoft.AspNetCore.Components/CascadingValue.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.AspNetCore.Components/ComponentBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ void IComponent.Init(RenderHandle renderHandle)
/// <param name="parameters">The parameters to apply.</param>
public virtual void SetParameters(ParameterCollection parameters)
{
parameters.AssignToProperties(this);
parameters.SetParameterProperties(this);

if (!_hasCalledInit)
{
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.AspNetCore.Components/Layouts/LayoutDisplay.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -42,7 +42,7 @@ public void Init(RenderHandle renderHandle)
/// <inheritdoc />
public void SetParameters(ParameterCollection parameters)
{
parameters.AssignToProperties(this);
parameters.SetParameterProperties(this);
Render();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;

namespace Microsoft.AspNetCore.Components
Expand All @@ -16,18 +17,15 @@ 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<Type, IDictionary<string, WriteParameterAction>> _cachedParameterWriters
= new ConcurrentDictionary<Type, IDictionary<string, WriteParameterAction>>();
private readonly static IDictionary<Type, IDictionary<string, IPropertySetter>> _cachedParameterWriters = new ConcurrentDictionary<Type, IDictionary<string, IPropertySetter>>();

/// <summary>
/// Iterates through the <see cref="ParameterCollection"/>, assigning each parameter
/// to a property of the same name on <paramref name="target"/>.
/// </summary>
/// <param name="parameterCollection">The <see cref="ParameterCollection"/>.</param>
/// <param name="target">An object that has a public writable property matching each parameter's name and type.</param>
public static void AssignToProperties(
public static void SetParameterProperties(
in this ParameterCollection parameterCollection,
object target)
{
Expand All @@ -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;
Expand All @@ -53,7 +53,8 @@ public static void AssignToProperties(

try
{
parameterWriter(target, parameter.Value);
parameterWriter.SetValue(target, parameter.Value);
localParameterWriter.Remove(parameterWriter);
}
catch (Exception ex)
{
Expand All @@ -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<PropertyInfo> GetCandidateBindableProperties(Type targetType)
=> MemberAssignment.GetPropertiesIncludingInherited(targetType, _bindablePropertyFlags);

private static IDictionary<string, WriteParameterAction> CreateParameterWriters(Type targetType)
private static IDictionary<string, IPropertySetter> CreateParameterWriters(Type targetType)
{
var result = new Dictionary<string, WriteParameterAction>(StringComparer.OrdinalIgnoreCase);
var result = new Dictionary<string, IPropertySetter>(StringComparer.OrdinalIgnoreCase);

foreach (var propertyInfo in GetCandidateBindableProperties(targetType))
{
Expand All @@ -90,10 +96,7 @@ private static IDictionary<string, WriteParameterAction> 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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -8,5 +8,7 @@ namespace Microsoft.AspNetCore.Components.Reflection
internal interface IPropertySetter
{
void SetValue(object target, object value);

object GetDefaultValue();
}
}
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -43,15 +43,23 @@ public static IPropertySetter CreatePropertySetter(Type targetType, PropertyInfo
class PropertySetter<TTarget, TValue> : IPropertySetter
{
private readonly Action<TTarget, TValue> _setterDelegate;
private object _defaultValue;

public PropertySetter(MethodInfo setMethod)
{
_setterDelegate = (Action<TTarget, TValue>)Delegate.CreateDelegate(
typeof(Action<TTarget, TValue>), 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;
}
}
}
}
2 changes: 1 addition & 1 deletion src/Microsoft.AspNetCore.Components/Routing/Router.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public void Init(RenderHandle renderHandle)
/// <inheritdoc />
public void SetParameters(ParameterCollection parameters)
{
parameters.AssignToProperties(this);
parameters.SetParameterProperties(this);
var types = ComponentResolver.ResolveComponents(AppAssembly);
Routes = RouteTable.Create(types);
Refresh();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -64,15 +64,15 @@ public void IncomingParameterMatchesInheritedDeclaredParameter_SetsValue()
var target = new HasInheritedProperties();

// Act
parameterCollection.AssignToProperties(target);
parameterCollection.SetParameterProperties(target);

// Assert
Assert.Equal(123, target.IntProp);
Assert.Equal(456, target.DerivedClassIntProp);
}

[Fact]
public void NoIncomingParameterMatchesDeclaredParameter_LeavesValueUnchanged()
public void NoIncomingParameterMatchesDeclaredParameter_SetValuesDefault()
{
// Arrange
var existingObjectValue = new object();
Expand All @@ -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]
Expand All @@ -106,7 +106,7 @@ public void IncomingParameterMatchesNoDeclaredParameter_Throws()

// Act
var ex = Assert.Throws<InvalidOperationException>(
() => parameterCollection.AssignToProperties(target));
() => parameterCollection.SetParameterProperties(target));

// Assert
Assert.Equal(
Expand All @@ -127,7 +127,7 @@ public void IncomingParameterMatchesPropertyNotDeclaredAsParameter_Throws()

// Act
var ex = Assert.Throws<InvalidOperationException>(
() => parameterCollection.AssignToProperties(target));
() => parameterCollection.SetParameterProperties(target));

// Assert
Assert.Equal(default, target.IntProp);
Expand All @@ -150,7 +150,7 @@ public void IncomingParameterValueMismatchesDeclaredParameterType_Throws()

// Act
var ex = Assert.Throws<InvalidOperationException>(
() => parameterCollection.AssignToProperties(target));
() => parameterCollection.SetParameterProperties(target));

// Assert
Assert.Equal(
Expand All @@ -171,7 +171,7 @@ public void PropertyExplicitSetterException_Throws()

// Act
var ex = Assert.Throws<InvalidOperationException>(
() => parameterCollection.AssignToProperties(target));
() => parameterCollection.SetParameterProperties(target));

// Assert
Assert.Equal(
Expand All @@ -189,7 +189,7 @@ public void DeclaredParametersVaryOnlyByCase_Throws()

// Act
var ex = Assert.Throws<InvalidOperationException>(() =>
parameterCollection.AssignToProperties(target));
parameterCollection.SetParameterProperties(target));

// Assert
Assert.Equal(
Expand All @@ -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<InvalidOperationException>(() =>
parameterCollection.AssignToProperties(target));
parameterCollection.SetParameterProperties(target));

// Assert
Assert.Equal(
Expand All @@ -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; }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1557,7 +1557,7 @@ private class FakeComponent : IComponent
public void Init(RenderHandle renderHandle) { }
public void SetParameters(ParameterCollection parameters)
{
parameters.AssignToProperties(this);
parameters.SetParameterProperties(this);
}
}

Expand Down
4 changes: 2 additions & 2 deletions test/Microsoft.AspNetCore.Components.Test/RendererTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1310,7 +1310,7 @@ public void Init(RenderHandle renderHandle)

public void SetParameters(ParameterCollection parameters)
{
parameters.AssignToProperties(this);
parameters.SetParameterProperties(this);
Render();
}

Expand Down
2 changes: 1 addition & 1 deletion test/shared/AutoRenderComponent.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public void Init(RenderHandle renderHandle)

public virtual void SetParameters(ParameterCollection parameters)
{
parameters.AssignToProperties(this);
parameters.SetParameterProperties(this);
TriggerRender();
}

Expand Down