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

This commit is contained in:
Steve Sanderson 2019-01-23 00:16:07 +00:00 committed by Nate McMaster
parent 1fe7f5e02a
commit a1d49e19b5
9 changed files with 36 additions and 74 deletions

View File

@ -48,14 +48,14 @@ else
WeatherForecast[] forecasts;
public override Task SetParametersAsync(ParameterCollection parameters)
{
StartDate = DateTime.Now;
return base.SetParametersAsync(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

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

View File

@ -25,7 +25,7 @@ namespace Microsoft.AspNetCore.Components
/// </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 @@ namespace Microsoft.AspNetCore.Components
_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 @@ namespace Microsoft.AspNetCore.Components
try
{
writerWithIndex.Writer.SetValue(target, parameter.Value);
usageFlags[writerWithIndex.Index] = true;
numUsedWriters++;
writerWithIndex.SetValue(target, parameter.Value);
}
catch (Exception ex)
{
@ -73,23 +60,6 @@ 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)
@ -125,12 +95,11 @@ namespace Microsoft.AspNetCore.Components
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 @@ namespace Microsoft.AspNetCore.Components
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);
}
}
}

View File

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

View File

@ -48,14 +48,10 @@ 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

@ -73,7 +73,7 @@ namespace Microsoft.AspNetCore.Components.Test
}
[Fact]
public void NoIncomingParameterMatchesDeclaredParameter_SetValuesDefault()
public void NoIncomingParameterMatchesDeclaredParameter_LeavesValueUnchanged()
{
// Arrange
var existingObjectValue = new object();
@ -90,9 +90,9 @@ namespace Microsoft.AspNetCore.Components.Test
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]

View File

@ -221,6 +221,10 @@ namespace Microsoft.AspNetCore.Components.E2ETest.Tests
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");

View File

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

View File

@ -48,14 +48,14 @@ else
WeatherForecast[] forecasts;
public override Task SetParametersAsync(ParameterCollection parameters)
{
StartDate = DateTime.Now;
return base.SetParametersAsync(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);
}
}