Revert feature that resets unspecified parameters to default(type). Fixes #6864 (#6931)

Author: SteveSandersonMS

src/Components/Blazor/testassets/StandaloneApp/Pages/FetchData.cshtml

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

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

@@ -3,7 +3,6 @@
   <PropertyGroup>
     <TargetFramework>netstandard2.0</TargetFramework>
     <Description>Components feature for ASP.NET Core.</Description>
-    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
     <IsShippingPackage>true</IsShippingPackage>
   </PropertyGroup>
 

src/Components/Components/src/ParameterCollectionExtensions.cs

@@ -25,7 +25,7 @@ private readonly static ConcurrentDictionary<Type, WritersForType> _cachedWriter
         /// </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 unsafe static void SetParameterProperties(
+        public static void SetParameterProperties(
             in this ParameterCollection parameterCollection,
             object target)
         {
@@ -41,17 +41,6 @@ public unsafe static void SetParameterProperties(
                 _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;
@@ -62,9 +51,7 @@ public unsafe static void SetParameterProperties(
 
                 try
                 {
-                    writerWithIndex.Writer.SetValue(target, parameter.Value);
-                    usageFlags[writerWithIndex.Index] = true;
-                    numUsedWriters++;
+                    writerWithIndex.SetValue(target, parameter.Value);
                 }
                 catch (Exception ex)
                 {
@@ -73,23 +60,6 @@ public unsafe static void SetParameterProperties(
                         $"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)
@@ -125,12 +95,11 @@ private static void ThrowForUnknownIncomingParameterName(Type targetType, string
 
         class WritersForType
         {
-            public Dictionary<string, (int Index, IPropertySetter Writer)> WritersByName { get; }
-            public List<IPropertySetter> WritersByIndex { get; }
+            public Dictionary<string, IPropertySetter> WritersByName { get; }
 
             public WritersForType(Type targetType)
             {
-                var propertySettersByName = new Dictionary<string, IPropertySetter>(StringComparer.OrdinalIgnoreCase);
+                WritersByName = new Dictionary<string, IPropertySetter>(StringComparer.OrdinalIgnoreCase);
                 foreach (var propertyInfo in GetCandidateBindableProperties(targetType))
                 {
                     var shouldCreateWriter = propertyInfo.IsDefined(typeof(ParameterAttribute))
@@ -143,24 +112,14 @@ public WritersForType(Type targetType)
                     var propertySetter = MemberAssignment.CreatePropertySetter(targetType, propertyInfo);
 
                     var propertyName = propertyInfo.Name;
-                    if (propertySettersByName.ContainsKey(propertyName))
+                    if (WritersByName.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);
+                    WritersByName.Add(propertyName, propertySetter);
                 }
             }
         }

src/Components/Components/src/Reflection/IPropertySetter.cs

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

src/Components/Components/src/Reflection/MemberAssignment.cs

@@ -48,14 +48,10 @@ 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/Components/test/ParameterCollectionAssignmentExtensionsTest.cs

@@ -73,7 +73,7 @@ public void IncomingParameterMatchesInheritedDeclaredParameter_SetsValue()
         }
 
         [Fact]
-        public void NoIncomingParameterMatchesDeclaredParameter_SetValuesDefault()
+        public void NoIncomingParameterMatchesDeclaredParameter_LeavesValueUnchanged()
         {
             // Arrange
             var existingObjectValue = new object();
@@ -90,9 +90,9 @@ public void NoIncomingParameterMatchesDeclaredParameter_SetValuesDefault()
             parameterCollection.SetParameterProperties(target);
 
             // Assert
-            Assert.Equal(0, target.IntProp);
-            Assert.Null(target.StringProp);
-            Assert.Null(target.ObjectPropCurrentValue);
+            Assert.Equal(456, target.IntProp);
+            Assert.Equal("Existing value", target.StringProp);
+            Assert.Same(existingObjectValue, target.ObjectPropCurrentValue);
         }
 
         [Fact]

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

@@ -221,6 +221,10 @@ public void CanFollowLinkToPageWithParameters()
             AssertHighlightedLinks("With parameters", "With more parameters");
 
             // Can remove parameters while remaining on same page
+            // WARNING: This only works because the WithParameters component overrides SetParametersAsync
+            // and explicitly resets its parameters to default when each new set of parameters arrives.
+            // Without that, the page would retain the old value.
+            // See https://github.com/aspnet/AspNetCore/issues/6864 where we reverted the logic to auto-reset.
             app.FindElement(By.LinkText("With parameters")).Click();
             WaitAssert.Equal("Your full name is Abc .", () => app.FindElement(By.Id("test-info")).Text);
             AssertHighlightedLinks("With parameters");

src/Components/test/testassets/BasicTestApp/RouterTest/WithParameters.cshtml

@@ -5,9 +5,15 @@
 
 @functions
 {
-    [Parameter]
-    string FirstName { get; set; }
+    [Parameter] string FirstName { get; set; }
 
-    [Parameter]
-    string LastName { get ; set; }
+    [Parameter] string LastName { get ; set; }
+
+    public override Task SetParametersAsync(ParameterCollection parameters)
+    {
+        // Manually reset parameters to defaults so we don't retain any from an earlier URL
+        FirstName = default;
+        LastName = default;
+        return base.SetParametersAsync(parameters);
+    }
 }

src/Components/test/testassets/ComponentsApp.App/Pages/FetchData.cshtml

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