diff --git a/src/Components/blazor/samples/StandaloneApp/Pages/FetchData.cshtml b/src/Components/blazor/samples/StandaloneApp/Pages/FetchData.cshtml index 06ae482c5544..2113b17af92e 100644 --- a/src/Components/blazor/samples/StandaloneApp/Pages/FetchData.cshtml +++ b/src/Components/blazor/samples/StandaloneApp/Pages/FetchData.cshtml @@ -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( $"sample-data/weather.json?date={StartDate.ToString("yyyy-MM-dd")}"); diff --git a/src/Components/samples/ComponentsApp.App/Pages/FetchData.cshtml b/src/Components/samples/ComponentsApp.App/Pages/FetchData.cshtml index b8c8244128da..30677449d683 100644 --- a/src/Components/samples/ComponentsApp.App/Pages/FetchData.cshtml +++ b/src/Components/samples/ComponentsApp.App/Pages/FetchData.cshtml @@ -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); } } diff --git a/src/Components/src/Microsoft.AspNetCore.Components/CascadingValue.cs b/src/Components/src/Microsoft.AspNetCore.Components/CascadingValue.cs index 006fca448f63..53e356ae2eee 100644 --- a/src/Components/src/Microsoft.AspNetCore.Components/CascadingValue.cs +++ b/src/Components/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/Components/src/Microsoft.AspNetCore.Components/ComponentBase.cs b/src/Components/src/Microsoft.AspNetCore.Components/ComponentBase.cs index 1ff2b60b7b96..58a23ee546da 100644 --- a/src/Components/src/Microsoft.AspNetCore.Components/ComponentBase.cs +++ b/src/Components/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/Components/src/Microsoft.AspNetCore.Components/Layouts/LayoutDisplay.cs b/src/Components/src/Microsoft.AspNetCore.Components/Layouts/LayoutDisplay.cs index c9b26ea897ca..afd205c40dc2 100644 --- a/src/Components/src/Microsoft.AspNetCore.Components/Layouts/LayoutDisplay.cs +++ b/src/Components/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/Components/src/Microsoft.AspNetCore.Components/Microsoft.AspNetCore.Components.csproj b/src/Components/src/Microsoft.AspNetCore.Components/Microsoft.AspNetCore.Components.csproj index 8c211bdda190..bf2746fc612d 100644 --- a/src/Components/src/Microsoft.AspNetCore.Components/Microsoft.AspNetCore.Components.csproj +++ b/src/Components/src/Microsoft.AspNetCore.Components/Microsoft.AspNetCore.Components.csproj @@ -1,8 +1,9 @@ - + netstandard2.0 Components feature for ASP.NET Core. + true diff --git a/src/Components/src/Microsoft.AspNetCore.Components/ParameterCollectionExtensions.cs b/src/Components/src/Microsoft.AspNetCore.Components/ParameterCollectionExtensions.cs index 2319708751c0..436f8f6d29dd 100644 --- a/src/Components/src/Microsoft.AspNetCore.Components/ParameterCollectionExtensions.cs +++ b/src/Components/src/Microsoft.AspNetCore.Components/ParameterCollectionExtensions.cs @@ -16,18 +16,16 @@ 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 ConcurrentDictionary _cachedWritersByType + = new ConcurrentDictionary(); /// - /// Iterates through the , assigning each parameter - /// to a property of the same name on . + /// For each parameter property on , updates its value to + /// match the corresponding entry in the . /// /// The . /// An object that has a public writable property matching each parameter's name and type. - public static void AssignToProperties( + public unsafe static void SetParameterProperties( in this ParameterCollection parameterCollection, object target) { @@ -37,23 +35,36 @@ public static void AssignToProperties( } 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 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 @@ public static void AssignToProperties( $"type '{target.GetType().FullName}'. The error was: {ex.Message}", ex); } } - } - - internal static IEnumerable GetCandidateBindableProperties(Type targetType) - => MemberAssignment.GetPropertiesIncludingInherited(targetType, _bindablePropertyFlags); - private static IDictionary CreateParameterWriters(Type targetType) - { - var result = new Dictionary(StringComparer.OrdinalIgnoreCase); - - foreach (var propertyInfo in GetCandidateBindableProperties(targetType)) + // 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++) { - var shouldCreateWriter = propertyInfo.IsDefined(typeof(ParameterAttribute)) - || propertyInfo.IsDefined(typeof(CascadingParameterAttribute)); - if (!shouldCreateWriter) + if (index >= numWriters) { - continue; + // This should not be possible + throw new InvalidOperationException("Ran out of writers before marking them all as used."); } - var propertySetter = MemberAssignment.CreatePropertySetter(targetType, propertyInfo); - - var propertyName = propertyInfo.Name; - if (result.ContainsKey(propertyName)) + if (!usageFlags[index]) { - 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."); + writers.WritersByIndex[index].SetDefaultValue(target); + numUsedWriters++; } - - result.Add(propertyName, (object target, object parameterValue) => - { - propertySetter.SetValue(target, parameterValue); - }); } - - return result; } + internal static IEnumerable GetCandidateBindableProperties(Type targetType) + => MemberAssignment.GetPropertiesIncludingInherited(targetType, _bindablePropertyFlags); + 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 @@ private static void ThrowForUnknownIncomingParameterName(Type targetType, string $"matching the name '{parameterName}'."); } } + + class WritersForType + { + public Dictionary WritersByName { get; } + public List WritersByIndex { get; } + + public WritersForType(Type targetType) + { + var propertySettersByName = new Dictionary(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(); + WritersByName = new Dictionary(StringComparer.OrdinalIgnoreCase); + foreach (var pair in propertySettersByName) + { + WritersByName.Add(pair.Key, (WritersByIndex.Count, pair.Value)); + WritersByIndex.Add(pair.Value); + } + } + } } } diff --git a/src/Components/src/Microsoft.AspNetCore.Components/Reflection/IPropertySetter.cs b/src/Components/src/Microsoft.AspNetCore.Components/Reflection/IPropertySetter.cs index 061580f2ca84..8cb862433f59 100644 --- a/src/Components/src/Microsoft.AspNetCore.Components/Reflection/IPropertySetter.cs +++ b/src/Components/src/Microsoft.AspNetCore.Components/Reflection/IPropertySetter.cs @@ -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); } } diff --git a/src/Components/src/Microsoft.AspNetCore.Components/Reflection/MemberAssignment.cs b/src/Components/src/Microsoft.AspNetCore.Components/Reflection/MemberAssignment.cs index eff055a1c7c8..ede570059599 100644 --- a/src/Components/src/Microsoft.AspNetCore.Components/Reflection/MemberAssignment.cs +++ b/src/Components/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; @@ -48,10 +48,14 @@ public PropertySetter(MethodInfo setMethod) { _setterDelegate = (Action)Delegate.CreateDelegate( typeof(Action), 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); } } } diff --git a/src/Components/src/Microsoft.AspNetCore.Components/Routing/Router.cs b/src/Components/src/Microsoft.AspNetCore.Components/Routing/Router.cs index 8ccbfc119bc0..82b15251c8b6 100644 --- a/src/Components/src/Microsoft.AspNetCore.Components/Routing/Router.cs +++ b/src/Components/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/src/Components/test/Microsoft.AspNetCore.Components.E2ETest/Tests/RoutingTest.cs b/src/Components/test/Microsoft.AspNetCore.Components.E2ETest/Tests/RoutingTest.cs index f7d0f69e633c..146b629f9be3 100644 --- a/src/Components/test/Microsoft.AspNetCore.Components.E2ETest/Tests/RoutingTest.cs +++ b/src/Components/test/Microsoft.AspNetCore.Components.E2ETest/Tests/RoutingTest.cs @@ -203,7 +203,17 @@ public void CanFollowLinkToPageWithParameters() var app = MountTestComponent(); 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"); } diff --git a/src/Components/test/Microsoft.AspNetCore.Components.Test/ParameterCollectionAssignmentExtensionsTest.cs b/src/Components/test/Microsoft.AspNetCore.Components.Test/ParameterCollectionAssignmentExtensionsTest.cs index cc8d62191113..a3c4ba3d767d 100644 --- a/src/Components/test/Microsoft.AspNetCore.Components.Test/ParameterCollectionAssignmentExtensionsTest.cs +++ b/src/Components/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/src/Components/test/Microsoft.AspNetCore.Components.Test/RenderTreeDiffBuilderTest.cs b/src/Components/test/Microsoft.AspNetCore.Components.Test/RenderTreeDiffBuilderTest.cs index 53dd093061dc..4866be353f56 100644 --- a/src/Components/test/Microsoft.AspNetCore.Components.Test/RenderTreeDiffBuilderTest.cs +++ b/src/Components/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/src/Components/test/Microsoft.AspNetCore.Components.Test/RendererTest.cs b/src/Components/test/Microsoft.AspNetCore.Components.Test/RendererTest.cs index 6d84c590cb58..5fe8b7909753 100644 --- a/src/Components/test/Microsoft.AspNetCore.Components.Test/RendererTest.cs +++ b/src/Components/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(); } diff --git a/src/Components/test/shared/AutoRenderComponent.cs b/src/Components/test/shared/AutoRenderComponent.cs index b28d045a8565..05e7ad289575 100644 --- a/src/Components/test/shared/AutoRenderComponent.cs +++ b/src/Components/test/shared/AutoRenderComponent.cs @@ -17,7 +17,7 @@ public void Init(RenderHandle renderHandle) public virtual void SetParameters(ParameterCollection parameters) { - parameters.AssignToProperties(this); + parameters.SetParameterProperties(this); TriggerRender(); } diff --git a/src/Components/test/testapps/BasicTestApp/RouterTest/Links.cshtml b/src/Components/test/testapps/BasicTestApp/RouterTest/Links.cshtml index 10c401898f76..e849ba75f7f0 100644 --- a/src/Components/test/testapps/BasicTestApp/RouterTest/Links.cshtml +++ b/src/Components/test/testapps/BasicTestApp/RouterTest/Links.cshtml @@ -10,7 +10,8 @@
  • Other with base-relative URL (matches all)
  • Other with query
  • Other with hash
    • -
  • With parameters
    • +
  • With parameters
    • +
  • With more parameters