huahaofeng
/
aspnetcore
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Improve SetParametersAsync docs (#12826) 

* Improve SetParametersAsync docs

Fixes: #12820

* Clarify semantics of clashing cascading parameters

This change attempts to lock down the semantics of how cascading
parameters behave inside ParameterView.SetParameterProperties.

Previously a cascading value could be used to set a non-cascading
parameter, and vice-versa. We were just missing tests in general for how
cascading values are interpreted by the parameter set logic.
This commit is contained in:
Ryan Nowak 2019-08-06 07:37:49 -07:00 committed by GitHub
parent 31cfa2e305
commit 2b0a1686c3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 279 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
src/Components/Components/src/ComponentBase.cs
View File

@ -171,10 +171,25 @@ namespace Microsoft.AspNetCore.Components
_renderHandle = renderHandle;
}
/// <summary>
/// Method invoked to apply initial or updated parameters to the component.
/// Sets parameters supplied by the component's parent in the render tree.
/// </summary>
/// <param name="parameters">The parameters to apply.</param>
/// <param name="parameters">The parameters.</param>
/// <returns>A <see cref="Task"/> that completes when the component has finished updating and rendering itself.</returns>
/// <remarks>
/// <para>
/// The <see cref="SetParametersAsync(ParameterView)"/> method should be passed the entire set of parameter values each
/// time <see cref="SetParametersAsync(ParameterView)"/> is called. It not required that the caller supply a parameter
/// value for all parameters that are logically understood by the component.
/// </para>
/// <para>
/// The default implementation of <see cref="SetParametersAsync(ParameterView)"/> will set the value of each property
/// decorated with <see cref="ParameterAttribute" /> or <see cref="CascadingParameterAttribute" /> that has
/// a corresponding value in the <see cref="ParameterView" />. Parameters that do not have a corresponding value
/// will be unchanged.
/// </para>
/// </remarks>
public virtual Task SetParametersAsync(ParameterView parameters)
{
parameters.SetParameterProperties(this);

2
src/Components/Components/src/ComponentFactory.cs
View File

@ -61,7 +61,7 @@ namespace Microsoft.AspNetCore.Components
(
propertyName: property.Name,
propertyType: property.PropertyType,
setter: MemberAssignment.CreatePropertySetter(type, property)
setter: MemberAssignment.CreatePropertySetter(type, property, cascading: false)
)).ToArray();
return Initialize;

5
src/Components/Components/src/IComponent.cs
View File

@ -21,6 +21,11 @@ namespace Microsoft.AspNetCore.Components
/// </summary>
/// <param name="parameters">The parameters.</param>
/// <returns>A <see cref="Task"/> that completes when the component has finished updating and rendering itself.</returns>
/// <remarks>
/// The <see cref="SetParametersAsync(ParameterView)"/> method should be passed the entire set of parameter values each
/// time <see cref="SetParametersAsync(ParameterView)"/> is called. It not required that the caller supply a parameter
/// value for all parameters that are logically understood by the component.
/// </remarks>
Task SetParametersAsync(ParameterView parameters);
}
}

61
src/Components/Components/src/Reflection/ComponentProperties.cs
View File

@ -14,6 +14,9 @@ namespace Microsoft.AspNetCore.Components.Reflection
{
private const BindingFlags _bindablePropertyFlags = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.IgnoreCase;
// Right now it's not possible for a component to define a Parameter and a Cascading Parameter with
// the same name. We don't give you a way to express this in code (would create duplicate properties),
// and we don't have the ability to represent it in our data structures.
private readonly static ConcurrentDictionary<Type, WritersForType> _cachedWritersByType
= new ConcurrentDictionary<Type, WritersForType>();
@ -44,6 +47,24 @@ namespace Microsoft.AspNetCore.Components.Reflection
ThrowForUnknownIncomingParameterName(targetType, parameterName);
throw null; // Unreachable
}
else if (writer.Cascading && !parameter.Cascading)
{
// We don't allow you to set a cascading parameter with a non-cascading value. Put another way:
// cascading parameters are not part of the public API of a component, so it's not reasonable
// for someone to set it directly.
//
// If we find a strong reason for this to work in the future we can reverse our decision since
// this throws today.
ThrowForSettingCascadingParameterWithNonCascadingValue(targetType, parameterName);
throw null; // Unreachable
}
else if (!writer.Cascading && parameter.Cascading)
{
// We're giving a more specific error here because trying to set a non-cascading parameter
// with a cascading value is likely deliberate (but not supported), or is a bug in our code.
ThrowForSettingParameterWithCascadingValue(targetType, parameterName);
throw null; // Unreachable
}
SetProperty(target, writer, parameterName, parameter.Value);
}
@ -62,7 +83,24 @@ namespace Microsoft.AspNetCore.Components.Reflection
}
var isUnmatchedValue = !writers.WritersByName.TryGetValue(parameterName, out var writer);
if (isUnmatchedValue)
if ((isUnmatchedValue && parameter.Cascading) || (writer != null && !writer.Cascading && parameter.Cascading))
{
// Don't allow an "extra" cascading value to be collected - or don't allow a non-cascading
// parameter to be set with a cascading value.
//
// This is likely a bug in our infrastructure or an attempt to deliberately do something unsupported.
ThrowForSettingParameterWithCascadingValue(targetType, parameterName);
throw null; // Unreachable
}
else if (isUnmatchedValue ||
// Allow unmatched parameters to collide with the names of cascading parameters. This is
// valid because cascading parameter names are not part of the public API. There's no
// way for the user of a component to know what the names of cascading parameters
// are.
(writer.Cascading && !parameter.Cascading))
{
unmatched ??= new Dictionary<string, object>(StringComparer.OrdinalIgnoreCase);
unmatched[parameterName] = parameter.Value;
@ -138,6 +176,20 @@ namespace Microsoft.AspNetCore.Components.Reflection
}
}
private static void ThrowForSettingCascadingParameterWithNonCascadingValue(Type targetType, string parameterName)
{
throw new InvalidOperationException(
$"Object of type '{targetType.FullName}' has a property matching the name '{parameterName}', " +
$"but it does not have [{nameof(ParameterAttribute)}] applied.");
}
private static void ThrowForSettingParameterWithCascadingValue(Type targetType, string parameterName)
{
throw new InvalidOperationException(
$"The property '{parameterName}' on component type '{targetType.FullName}' cannot be set " +
$"using a cascading value.");
}
private static void ThrowForCaptureUnmatchedValuesConflict(Type targetType, string parameterName, Dictionary<string, object> unmatched)
{
throw new InvalidOperationException(
@ -179,13 +231,14 @@ namespace Microsoft.AspNetCore.Components.Reflection
foreach (var propertyInfo in GetCandidateBindableProperties(targetType))
{
var parameterAttribute = propertyInfo.GetCustomAttribute<ParameterAttribute>();
var isParameter = parameterAttribute != null || propertyInfo.IsDefined(typeof(CascadingParameterAttribute));
var cascadingParameterAttribute = propertyInfo.GetCustomAttribute<CascadingParameterAttribute>();
var isParameter = parameterAttribute != null || cascadingParameterAttribute != null;
if (!isParameter)
{
continue;
}
var propertySetter = MemberAssignment.CreatePropertySetter(targetType, propertyInfo);
var propertySetter = MemberAssignment.CreatePropertySetter(targetType, propertyInfo, cascading: cascadingParameterAttribute != null);
var propertyName = propertyInfo.Name;
if (WritersByName.ContainsKey(propertyName))
@ -213,7 +266,7 @@ namespace Microsoft.AspNetCore.Components.Reflection
ThrowForInvalidCaptureUnmatchedValuesParameterType(targetType, propertyInfo);
}
CaptureUnmatchedValuesWriter = MemberAssignment.CreatePropertySetter(targetType, propertyInfo);
CaptureUnmatchedValuesWriter = MemberAssignment.CreatePropertySetter(targetType, propertyInfo, cascading: false);
CaptureUnmatchedValuesPropertyName = propertyInfo.Name;
}
}

2
src/Components/Components/src/Reflection/IPropertySetter.cs
View File

@ -5,6 +5,8 @@ namespace Microsoft.AspNetCore.Components.Reflection
{
internal interface IPropertySetter
{
bool Cascading { get; }
void SetValue(object target, object value);
}
}

10
src/Components/Components/src/Reflection/MemberAssignment.cs
View File

@ -26,7 +26,7 @@ namespace Microsoft.AspNetCore.Components.Reflection
}
}
public static IPropertySetter CreatePropertySetter(Type targetType, PropertyInfo property)
public static IPropertySetter CreatePropertySetter(Type targetType, PropertyInfo property, bool cascading)
{
if (property.SetMethod == null)
{
@ -37,19 +37,23 @@ namespace Microsoft.AspNetCore.Components.Reflection
return (IPropertySetter)Activator.CreateInstance(
typeof(PropertySetter<,>).MakeGenericType(targetType, property.PropertyType),
property.SetMethod);
property.SetMethod,
cascading);
}
class PropertySetter<TTarget, TValue> : IPropertySetter
{
private readonly Action<TTarget, TValue> _setterDelegate;
public PropertySetter(MethodInfo setMethod)
public PropertySetter(MethodInfo setMethod, bool cascading)
{
_setterDelegate = (Action<TTarget, TValue>)Delegate.CreateDelegate(
typeof(Action<TTarget, TValue>), setMethod);
Cascading = cascading;
}
public bool Cascading { get; }
public void SetValue(object target, object value)
{
if (value == null)

208
src/Components/Components/test/ParameterViewTest.Assignment.cs
View File

@ -5,9 +5,8 @@ using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Components.Rendering;
using Microsoft.AspNetCore.Components.RenderTree;
using Microsoft.AspNetCore.Components.Test.Helpers;
using Xunit;
namespace Microsoft.AspNetCore.Components
@ -96,23 +95,58 @@ namespace Microsoft.AspNetCore.Components
}
[Fact]
public void IncomingParameterMatchesNoDeclaredParameter_Throws()
public void IncomingCascadingValueMatchesCascadingParameter_SetsValue()
{
// Arrange
var target = new HasPropertyWithoutParameterAttribute();
var parameters = new ParameterViewBuilder
{
{ "AnyOtherKey", 123 },
}.Build();
var builder = new ParameterViewBuilder();
builder.Add(nameof(HasCascadingParameter.Cascading), "hi", cascading: true);
var parameters = builder.Build();
var target = new HasCascadingParameter();
// Act
var ex = Assert.Throws<InvalidOperationException>(
() => parameters.SetParameterProperties(target));
parameters.SetParameterProperties(target);
// Assert
Assert.Equal("hi", target.Cascading);
}
[Fact]
public void NoIncomingCascadingValueMatchesDeclaredCascadingParameter_LeavesValueUnchanged()
{
// Arrange
var builder = new ParameterViewBuilder();
var parameters = builder.Build();
var target = new HasCascadingParameter()
{
Cascading = "bye",
};
// Act
parameters.SetParameterProperties(target);
// Assert
Assert.Equal("bye", target.Cascading);
}
[Fact]
public void IncomingCascadingValueMatchesNoDeclaredParameter_Throws()
{
// Arrange
var builder = new ParameterViewBuilder();
builder.Add("SomethingElse", "hi", cascading: true);
var parameters = builder.Build();
var target = new HasCascadingParameter();
// Act
var ex = Assert.Throws<InvalidOperationException>(() => parameters.SetParameterProperties(target));
// Assert
Assert.Equal(
$"Object of type '{typeof(HasPropertyWithoutParameterAttribute).FullName}' does not have a property " +
$"matching the name 'AnyOtherKey'.",
$"Object of type '{typeof(HasCascadingParameter).FullName}' does not have a property " +
$"matching the name 'SomethingElse'.",
ex.Message);
}
@ -138,6 +172,45 @@ namespace Microsoft.AspNetCore.Components
ex.Message);
}
[Fact]
public void IncomingNonCascadingValueMatchesCascadingParameter_Throws()
{
// Arrange
var target = new HasCascadingParameter();
var parameters = new ParameterViewBuilder
{
{ nameof(HasCascadingParameter.Cascading), 123 },
}.Build();
// Act
var ex = Assert.Throws<InvalidOperationException>(() => parameters.SetParameterProperties(target));
// Assert
Assert.Equal(
$"Object of type '{typeof(HasCascadingParameter).FullName}' has a property matching the name '{nameof(HasCascadingParameter.Cascading)}', " +
$"but it does not have [{nameof(ParameterAttribute)}] applied.",
ex.Message);
}
[Fact]
public void IncomingCascadingValueMatchesNonCascadingParameter_Throws()
{
// Arrange
var target = new HasInstanceProperties();
var builder = new ParameterViewBuilder();
builder.Add(nameof(HasInstanceProperties.IntProp), 16, cascading: true);
var parameters = builder.Build();
// Act
var ex = Assert.Throws<InvalidOperationException>(() => parameters.SetParameterProperties(target));
// Assert
Assert.Equal(
$"The property '{nameof(HasInstanceProperties.IntProp)}' on component type '{typeof(HasInstanceProperties).FullName}' " +
$"cannot be set using a cascading value.",
ex.Message);
}
[Fact]
public void SettingCaptureUnmatchedValuesParameterExplicitlyWorks()
{
@ -274,6 +347,51 @@ namespace Microsoft.AspNetCore.Components
ex.Message);
}
[Fact]
public void IncomingNonCascadingValueMatchesCascadingParameter_WithCaptureUnmatchedValues_DoesNotThrow()
{
// Arrange
var target = new HasCaptureUnmatchedValuesPropertyAndCascadingParameter()
{
Cascading = "bye",
};
var parameters = new ParameterViewBuilder
{
{ nameof(HasCaptureUnmatchedValuesPropertyAndCascadingParameter.Cascading), "hi" },
}.Build();
// Act
parameters.SetParameterProperties(target);
Assert.Collection(
target.CaptureUnmatchedValues,
kvp =>
{
Assert.Equal(nameof(HasCaptureUnmatchedValuesPropertyAndCascadingParameter.Cascading), kvp.Key);
Assert.Equal("hi", kvp.Value);
});
Assert.Equal("bye", target.Cascading);
}
[Fact]
public void IncomingCascadingValueMatchesNonCascadingParameter_WithCaptureUnmatchedValues_Throws()
{
// Arrange
var target = new HasCaptureUnmatchedValuesProperty();
var builder = new ParameterViewBuilder();
builder.Add(nameof(HasInstanceProperties.IntProp), 16, cascading: true);
var parameters = builder.Build();
// Act
var ex = Assert.Throws<InvalidOperationException>(() => parameters.SetParameterProperties(target));
// Assert
Assert.Equal(
$"The property '{nameof(HasCaptureUnmatchedValuesProperty.IntProp)}' on component type '{typeof(HasCaptureUnmatchedValuesProperty).FullName}' " +
$"cannot be set using a cascading value.",
ex.Message);
}
[Fact]
public void IncomingParameterValueMismatchesDeclaredParameterType_Throws()
{
@ -396,6 +514,11 @@ namespace Microsoft.AspNetCore.Components
}
}
class HasCascadingParameter
{
[CascadingParameter] public string Cascading { get; set; }
}
class HasPropertyWithoutParameterAttribute
{
internal int IntProp { get; set; }
@ -435,6 +558,12 @@ namespace Microsoft.AspNetCore.Components
[Parameter(CaptureUnmatchedValues = true)] public IReadOnlyDictionary<string, object> CaptureUnmatchedValues { get; set; }
}
class HasCaptureUnmatchedValuesPropertyAndCascadingParameter
{
[CascadingParameter] public string Cascading { get; set; }
[Parameter(CaptureUnmatchedValues = true)] public IReadOnlyDictionary<string, object> CaptureUnmatchedValues { get; set; }
}
class HasDupliateCaptureUnmatchedValuesProperty
{
[Parameter(CaptureUnmatchedValues = true)] public Dictionary<string, object> CaptureUnmatchedValuesProp1 { get; set; }
@ -448,11 +577,11 @@ namespace Microsoft.AspNetCore.Components
class ParameterViewBuilder : IEnumerable
{
private readonly List<(string Name, object Value)> _keyValuePairs
= new List<(string, object)>();
private readonly List<(string Name, object Value, bool Cascading)> _keyValuePairs
= new List<(string, object, bool)>();
public void Add(string name, object value)
=> _keyValuePairs.Add((name, value));
public void Add(string name, object value, bool cascading = false)
=> _keyValuePairs.Add((name, value, cascading));
public IEnumerator GetEnumerator()
=> throw new NotImplementedException();
@ -460,13 +589,56 @@ namespace Microsoft.AspNetCore.Components
public ParameterView Build()
{
var builder = new RenderTreeBuilder();
builder.OpenComponent<FakeComponent>(0);
foreach (var kvp in _keyValuePairs)
{
builder.AddAttribute(1, kvp.Name, kvp.Value);
if (!kvp.Cascading)
{
builder.AddAttribute(1, kvp.Name, kvp.Value);
}
}
builder.CloseComponent();
return new ParameterView(builder.GetFrames().Array, ownerIndex: 0);
var view = new ParameterView(builder.GetFrames().Array, ownerIndex: 0);
var cascadingParameters = new List<CascadingParameterState>();
foreach (var kvp in _keyValuePairs)
{
if (kvp.Cascading)
{
cascadingParameters.Add(new CascadingParameterState(kvp.Name, new TestCascadingValueProvider(kvp.Value)));
}
}
return view.WithCascadingParameters(cascadingParameters);
}
}
private class TestCascadingValueProvider : ICascadingValueComponent
{
public TestCascadingValueProvider(object value)
{
CurrentValue = value;
}
public object CurrentValue { get; }
public bool CurrentValueIsFixed => throw new NotImplementedException();
public bool CanSupplyValue(Type valueType, string valueName)
{
throw new NotImplementedException();
}
public void Subscribe(ComponentState subscriber)
{
throw new NotImplementedException();
}
public void Unsubscribe(ComponentState subscriber)
{
throw new NotImplementedException();
}
}
}