Replace AssignToProperties with SetParameterProperties, which also clears unspecified parameter properties (imported from Blazor PR 1108) (#4797)

This commit is contained in:
Steve Sanderson 2018-12-14 17:07:07 +00:00 committed by GitHub
parent 3757908b14
commit 343208331d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
17 changed files with 143 additions and 88 deletions

View File

@ -1,4 +1,4 @@
@page "/fetchdata"
@page "/fetchdata"
@page "/fetchdata/{StartDate:datetime}"
@inject HttpClient Http
@ -48,14 +48,14 @@ else
WeatherForecast[] forecasts;
public override void SetParameters(ParameterCollection parameters)
{
StartDate = DateTime.Now;
base.SetParameters(parameters);
}
protected override async Task OnParametersSetAsync()
{
// If no value was given in the URL for StartDate, apply a default
if (StartDate == default)
{
StartDate = DateTime.Now;
}
forecasts = await Http.GetJsonAsync<WeatherForecast[]>(
$"sample-data/weather.json?date={StartDate.ToString("yyyy-MM-dd")}");

View File

@ -48,14 +48,14 @@ else
WeatherForecast[] forecasts;
public override void SetParameters(ParameterCollection parameters)
{
StartDate = DateTime.Now;
base.SetParameters(parameters);
}
protected override async Task OnParametersSetAsync()
{
// If no value was given in the URL for StartDate, apply a default
if (StartDate == default)
{
StartDate = DateTime.Now;
}
forecasts = await ForecastService.GetForecastAsync(StartDate);
}
}

View File

@ -58,7 +58,7 @@ namespace Microsoft.AspNetCore.Components
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;

View File

@ -151,7 +151,7 @@ namespace Microsoft.AspNetCore.Components
/// <param name="parameters">The parameters to apply.</param>
public virtual void SetParameters(ParameterCollection parameters)
{
parameters.AssignToProperties(this);
parameters.SetParameterProperties(this);
if (!_hasCalledInit)
{

View File

@ -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 @@ namespace Microsoft.AspNetCore.Components.Layouts
/// <inheritdoc />
public void SetParameters(ParameterCollection parameters)
{
parameters.AssignToProperties(this);
parameters.SetParameterProperties(this);
Render();
}

View File

@ -1,8 +1,9 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
<Description>Components feature for ASP.NET Core.</Description>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
</PropertyGroup>
<ItemGroup>

View File

@ -16,18 +16,16 @@ namespace Microsoft.AspNetCore.Components
{
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 ConcurrentDictionary<Type, WritersForType> _cachedWritersByType
= new ConcurrentDictionary<Type, WritersForType>();
/// <summary>
/// Iterates through the <see cref="ParameterCollection"/>, assigning each parameter
/// to a property of the same name on <paramref name="target"/>.
/// For each parameter property on <paramref name="target"/>, updates its value to
/// match the corresponding entry in the <see cref="ParameterCollection"/>.
/// </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 unsafe static void SetParameterProperties(
in this ParameterCollection parameterCollection,
object target)
{
@ -37,23 +35,36 @@ namespace Microsoft.AspNetCore.Components
}
var targetType = target.GetType();
if (!_cachedParameterWriters.TryGetValue(targetType, out var parameterWriters))
if (!_cachedWritersByType.TryGetValue(targetType, out var writers))
{
parameterWriters = CreateParameterWriters(targetType);
_cachedParameterWriters[targetType] = parameterWriters;
writers = new WritersForType(targetType);
_cachedWritersByType[targetType] = writers;
}
// We only want to iterate through the parameterCollection once, and by the end of it,
// need to have tracked which of the parameter properties haven't yet been written.
// To avoid allocating any list/dictionary to track that, here we stackalloc an array
// of flags and set them based on the indices of the writers we use.
var numWriters = writers.WritersByIndex.Count;
var numUsedWriters = 0;
// TODO: Once we're able to move to netstandard2.1, this can be changed to be
// a Span<bool> and then the enclosing method no longer needs to be 'unsafe'
bool* usageFlags = stackalloc bool[numWriters];
foreach (var parameter in parameterCollection)
{
var parameterName = parameter.Name;
if (!parameterWriters.TryGetValue(parameterName, out var parameterWriter))
if (!writers.WritersByName.TryGetValue(parameterName, out var writerWithIndex))
{
ThrowForUnknownIncomingParameterName(targetType, parameterName);
}
try
{
parameterWriter(target, parameter.Value);
writerWithIndex.Writer.SetValue(target, parameter.Value);
usageFlags[writerWithIndex.Index] = true;
numUsedWriters++;
}
catch (Exception ex)
{
@ -62,43 +73,28 @@ namespace Microsoft.AspNetCore.Components
$"type '{target.GetType().FullName}'. The error was: {ex.Message}", ex);
}
}
// Now we can determine whether any writers have not been used, and if there are
// some unused ones, find them.
for (var index = 0; numUsedWriters < numWriters; index++)
{
if (index >= numWriters)
{
// This should not be possible
throw new InvalidOperationException("Ran out of writers before marking them all as used.");
}
if (!usageFlags[index])
{
writers.WritersByIndex[index].SetDefaultValue(target);
numUsedWriters++;
}
}
}
internal static IEnumerable<PropertyInfo> GetCandidateBindableProperties(Type targetType)
=> MemberAssignment.GetPropertiesIncludingInherited(targetType, _bindablePropertyFlags);
private static IDictionary<string, WriteParameterAction> CreateParameterWriters(Type targetType)
{
var result = new Dictionary<string, WriteParameterAction>(StringComparer.OrdinalIgnoreCase);
foreach (var propertyInfo in GetCandidateBindableProperties(targetType))
{
var shouldCreateWriter = propertyInfo.IsDefined(typeof(ParameterAttribute))
|| propertyInfo.IsDefined(typeof(CascadingParameterAttribute));
if (!shouldCreateWriter)
{
continue;
}
var propertySetter = MemberAssignment.CreatePropertySetter(targetType, propertyInfo);
var propertyName = propertyInfo.Name;
if (result.ContainsKey(propertyName))
{
throw new InvalidOperationException(
$"The type '{targetType.FullName}' declares more than one parameter matching the " +
$"name '{propertyName.ToLowerInvariant()}'. Parameter names are case-insensitive and must be unique.");
}
result.Add(propertyName, (object target, object parameterValue) =>
{
propertySetter.SetValue(target, parameterValue);
});
}
return result;
}
private static void ThrowForUnknownIncomingParameterName(Type targetType, string parameterName)
{
// We know we're going to throw by this stage, so it doesn't matter that the following
@ -126,5 +122,47 @@ namespace Microsoft.AspNetCore.Components
$"matching the name '{parameterName}'.");
}
}
class WritersForType
{
public Dictionary<string, (int Index, IPropertySetter Writer)> WritersByName { get; }
public List<IPropertySetter> WritersByIndex { get; }
public WritersForType(Type targetType)
{
var propertySettersByName = new Dictionary<string, IPropertySetter>(StringComparer.OrdinalIgnoreCase);
foreach (var propertyInfo in GetCandidateBindableProperties(targetType))
{
var shouldCreateWriter = propertyInfo.IsDefined(typeof(ParameterAttribute))
|| propertyInfo.IsDefined(typeof(CascadingParameterAttribute));
if (!shouldCreateWriter)
{
continue;
}
var propertySetter = MemberAssignment.CreatePropertySetter(targetType, propertyInfo);
var propertyName = propertyInfo.Name;
if (propertySettersByName.ContainsKey(propertyName))
{
throw new InvalidOperationException(
$"The type '{targetType.FullName}' declares more than one parameter matching the " +
$"name '{propertyName.ToLowerInvariant()}'. Parameter names are case-insensitive and must be unique.");
}
propertySettersByName.Add(propertyName, propertySetter);
}
// Now we know all the entries, construct the resulting list/dictionary
// with well-defined indices
WritersByIndex = new List<IPropertySetter>();
WritersByName = new Dictionary<string, (int, IPropertySetter)>(StringComparer.OrdinalIgnoreCase);
foreach (var pair in propertySettersByName)
{
WritersByName.Add(pair.Key, (WritersByIndex.Count, pair.Value));
WritersByIndex.Add(pair.Value);
}
}
}
}
}

View File

@ -1,12 +1,12 @@
// 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;
namespace Microsoft.AspNetCore.Components.Reflection
{
internal interface IPropertySetter
{
void SetValue(object target, object value);
void SetDefaultValue(object target);
}
}

View File

@ -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;
@ -48,10 +48,14 @@ namespace Microsoft.AspNetCore.Components.Reflection
{
_setterDelegate = (Action<TTarget, TValue>)Delegate.CreateDelegate(
typeof(Action<TTarget, TValue>), setMethod);
var propertyType = typeof(TValue);
}
public void SetValue(object target, object value)
=> _setterDelegate((TTarget)target, (TValue)value);
public void SetDefaultValue(object target)
=> _setterDelegate((TTarget)target, default);
}
}
}

View File

@ -49,7 +49,7 @@ namespace Microsoft.AspNetCore.Components.Routing
/// <inheritdoc />
public void SetParameters(ParameterCollection parameters)
{
parameters.AssignToProperties(this);
parameters.SetParameterProperties(this);
var types = ComponentResolver.ResolveComponents(AppAssembly);
Routes = RouteTable.Create(types);
Refresh();

View File

@ -212,7 +212,17 @@ namespace Microsoft.AspNetCore.Components.E2ETest.Tests
var app = MountTestComponent<TestRouter>();
app.FindElement(By.LinkText("With parameters")).Click();
WaitAssert.Equal("Your full name is Abc .", () => app.FindElement(By.Id("test-info")).Text);
AssertHighlightedLinks("With parameters");
// Can add more parameters while remaining on same page
app.FindElement(By.LinkText("With more parameters")).Click();
WaitAssert.Equal("Your full name is Abc McDef.", () => app.FindElement(By.Id("test-info")).Text);
AssertHighlightedLinks("With parameters", "With more parameters");
// Can remove parameters while remaining on same page
app.FindElement(By.LinkText("With parameters")).Click();
WaitAssert.Equal("Your full name is Abc .", () => app.FindElement(By.Id("test-info")).Text);
AssertHighlightedLinks("With parameters");
}

View File

@ -27,7 +27,7 @@ namespace Microsoft.AspNetCore.Components.Test
var target = new HasInstanceProperties();
// Act
parameterCollection.AssignToProperties(target);
parameterCollection.SetParameterProperties(target);
// Assert
Assert.Equal(123, target.IntProp);
@ -46,7 +46,7 @@ namespace Microsoft.AspNetCore.Components.Test
var target = new HasInstanceProperties();
// Act
parameterCollection.AssignToProperties(target);
parameterCollection.SetParameterProperties(target);
// Assert
Assert.Equal(123, target.IntProp);
@ -64,7 +64,7 @@ namespace Microsoft.AspNetCore.Components.Test
var target = new HasInheritedProperties();
// Act
parameterCollection.AssignToProperties(target);
parameterCollection.SetParameterProperties(target);
// Assert
Assert.Equal(123, target.IntProp);
@ -72,7 +72,7 @@ namespace Microsoft.AspNetCore.Components.Test
}
[Fact]
public void NoIncomingParameterMatchesDeclaredParameter_LeavesValueUnchanged()
public void NoIncomingParameterMatchesDeclaredParameter_SetValuesDefault()
{
// Arrange
var existingObjectValue = new object();
@ -86,12 +86,12 @@ namespace Microsoft.AspNetCore.Components.Test
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 @@ namespace Microsoft.AspNetCore.Components.Test
// Act
var ex = Assert.Throws<InvalidOperationException>(
() => parameterCollection.AssignToProperties(target));
() => parameterCollection.SetParameterProperties(target));
// Assert
Assert.Equal(
@ -127,7 +127,7 @@ namespace Microsoft.AspNetCore.Components.Test
// Act
var ex = Assert.Throws<InvalidOperationException>(
() => parameterCollection.AssignToProperties(target));
() => parameterCollection.SetParameterProperties(target));
// Assert
Assert.Equal(default, target.IntProp);
@ -150,7 +150,7 @@ namespace Microsoft.AspNetCore.Components.Test
// Act
var ex = Assert.Throws<InvalidOperationException>(
() => parameterCollection.AssignToProperties(target));
() => parameterCollection.SetParameterProperties(target));
// Assert
Assert.Equal(
@ -171,7 +171,7 @@ namespace Microsoft.AspNetCore.Components.Test
// Act
var ex = Assert.Throws<InvalidOperationException>(
() => parameterCollection.AssignToProperties(target));
() => parameterCollection.SetParameterProperties(target));
// Assert
Assert.Equal(
@ -189,7 +189,7 @@ namespace Microsoft.AspNetCore.Components.Test
// Act
var ex = Assert.Throws<InvalidOperationException>(() =>
parameterCollection.AssignToProperties(target));
parameterCollection.SetParameterProperties(target));
// Assert
Assert.Equal(
@ -205,14 +205,14 @@ namespace Microsoft.AspNetCore.Components.Test
// 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(
@ -226,7 +226,7 @@ namespace Microsoft.AspNetCore.Components.Test
{
// "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; }

View File

@ -1557,7 +1557,7 @@ namespace Microsoft.AspNetCore.Components.Test
public void Init(RenderHandle renderHandle) { }
public void SetParameters(ParameterCollection parameters)
{
parameters.AssignToProperties(this);
parameters.SetParameterProperties(this);
}
}

View File

@ -1202,7 +1202,7 @@ namespace Microsoft.AspNetCore.Components.Test
=> RenderHandle = renderHandle;
public void SetParameters(ParameterCollection parameters)
=> parameters.AssignToProperties(this);
=> parameters.SetParameterProperties(this);
}
private class EventComponent : AutoRenderComponent, IComponent, IHandleEvent
@ -1310,7 +1310,7 @@ namespace Microsoft.AspNetCore.Components.Test
public void SetParameters(ParameterCollection parameters)
{
parameters.AssignToProperties(this);
parameters.SetParameterProperties(this);
Render();
}

View File

@ -17,7 +17,7 @@ namespace Microsoft.AspNetCore.Components.Test.Helpers
public virtual void SetParameters(ParameterCollection parameters)
{
parameters.AssignToProperties(this);
parameters.SetParameterProperties(this);
TriggerRender();
}

View File

@ -10,7 +10,8 @@
<li><NavLink href="Other" Match=NavLinkMatch.All>Other with base-relative URL (matches all)</NavLink></li>
<li><NavLink href="/subdir/Other?abc=123">Other with query</NavLink></li>
<li><NavLink href="/subdir/Other#blah">Other with hash</NavLink></li>
<li><NavLink href="/subdir/WithParameters/Name/Abc/LastName/McDef">With parameters</NavLink></li>
<li><NavLink href="/subdir/WithParameters/Name/Abc">With parameters</NavLink></li>
<li><NavLink href="/subdir/WithParameters/Name/Abc/LastName/McDef">With more parameters</NavLink></li>
</ul>
<button id="do-navigation" onclick=@(x => uriHelper.NavigateTo("Other"))>

View File

@ -1,3 +1,4 @@
@page "/WithParameters/Name/{firstName}"
@page "/WithParameters/Name/{firstName}/LastName/{lastName}"
<div id="test-info">Your full name is @FirstName @LastName.</div>
<Links />