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

Author: SteveSandersonMS

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<WeatherForecast[]>(
             $"sample-data/weather.json?date={StartDate.ToString("yyyy-MM-dd")}");
 

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);
     }
 }

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;

src/Components/src/Microsoft.AspNetCore.Components/ComponentBase.cs

@@ -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)
             {

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)
         /// <inheritdoc />
         public void SetParameters(ParameterCollection parameters)
         {
-            parameters.AssignToProperties(this);
+            parameters.SetParameterProperties(this);
             Render();
         }
 

src/Components/src/Microsoft.AspNetCore.Components/Microsoft.AspNetCore.Components.csproj

@@ -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>

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<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 @@ 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<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 @@ public static void AssignToProperties(
                         $"type '{target.GetType().FullName}'. The error was: {ex.Message}", ex);
                 }
             }
-        }
-
-        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))
+            // 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<PropertyInfo> 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<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);
+                }
+            }
+        }
     }
 }

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);
     }
 }

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<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);
         }
     }
 }

src/Components/src/Microsoft.AspNetCore.Components/Routing/Router.cs

@@ -49,7 +49,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();

src/Components/test/Microsoft.AspNetCore.Components.E2ETest/Tests/RoutingTest.cs

@@ -212,7 +212,17 @@ public void CanFollowLinkToPageWithParameters()
 
             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");
         }