Skip to content

Commit 2c3a443

Browse files
SteveSandersonMSSteveSandersonMS
authored
Clear unused routing params. Fixes #7419 (#12407)
1 parent 6a24db5 commit 2c3a443

File tree

9 files changed

+150
-48
lines changed

9 files changed

+150
-48
lines changed

‎src/Components/Blazor/testassets/StandaloneApp/Pages/FetchData.razor

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -34,36 +34,32 @@ else
3434
</tbody>
3535
</table>
3636
<p>
37-
<a href="fetchdata/@StartDate.AddDays(-5).ToString("yyyy-MM-dd")" class="btn btn-secondary float-left">
37+
<a href="fetchdata/@startDate.AddDays(-5).ToString("yyyy-MM-dd")" class="btn btn-secondary float-left">
3838
◀ Previous
3939
</a>
40-
<a href="fetchdata/@StartDate.AddDays(5).ToString("yyyy-MM-dd")" class="btn btn-secondary float-right">
40+
<a href="fetchdata/@startDate.AddDays(5).ToString("yyyy-MM-dd")" class="btn btn-secondary float-right">
4141
Next ▶
4242
</a>
4343
</p>
4444
}
4545

4646
@code {
47-
[Parameter] public DateTime StartDate { get; set; }
47+
[Parameter] public DateTime? StartDate { get; set; }
4848

4949
WeatherForecast[] forecasts;
50-
51-
public override Task SetParametersAsync(ParameterCollection parameters)
52-
{
53-
StartDate = DateTime.Now;
54-
return base.SetParametersAsync(parameters);
55-
}
50+
DateTime startDate;
5651

5752
protected override async Task OnParametersSetAsync()
5853
{
54+
startDate = StartDate.GetValueOrDefault(DateTime.Now);
5955
forecasts = await Http.GetJsonAsync<WeatherForecast[]>(
60-
$"sample-data/weather.json?date={StartDate.ToString("yyyy-MM-dd")}");
56+
$"sample-data/weather.json?date={startDate.ToString("yyyy-MM-dd")}");
6157

6258
// Because StandaloneApp doesn't really have a server endpoint to get dynamic data from,
6359
// fake the DateFormatted values here. This would not apply in a real app.
6460
for (var i = 0; i < forecasts.Length; i++)
6561
{
66-
forecasts[i].DateFormatted = StartDate.AddDays(i).ToShortDateString();
62+
forecasts[i].DateFormatted = startDate.AddDays(i).ToShortDateString();
6763
}
6864
}
6965

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,16 @@ public PropertySetter(MethodInfo setMethod)
5151
}
5252

5353
public void SetValue(object target, object value)
54-
=> _setterDelegate((TTarget)target, (TValue)value);
54+
{
55+
if (value == null)
56+
{
57+
_setterDelegate((TTarget)target, default);
58+
}
59+
else
60+
{
61+
_setterDelegate((TTarget)target, (TValue)value);
62+
}
63+
}
5564
}
5665
}
5766
}

‎src/Components/Components/src/Routing/RouteEntry.cs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,17 @@ namespace Microsoft.AspNetCore.Components.Routing
88
{
99
internal class RouteEntry
1010
{
11-
public RouteEntry(RouteTemplate template, Type handler)
11+
public RouteEntry(RouteTemplate template, Type handler, string[] unusedRouteParameterNames)
1212
{
1313
Template = template;
14+
UnusedRouteParameterNames = unusedRouteParameterNames;
1415
Handler = handler;
1516
}
1617

1718
public RouteTemplate Template { get; }
1819

20+
public string[] UnusedRouteParameterNames { get; }
21+
1922
public Type Handler { get; }
2023

2124
internal void Match(RouteContext context)
@@ -45,6 +48,18 @@ internal void Match(RouteContext context)
4548
}
4649
}
4750

51+
// In addition to extracting parameter values from the URL, each route entry
52+
// also knows which other parameters should be supplied with null values. These
53+
// are parameters supplied by other route entries matching the same handler.
54+
if (UnusedRouteParameterNames.Length > 0)
55+
{
56+
parameters ??= new Dictionary<string, object>(StringComparer.Ordinal);
57+
for (var i = 0; i < UnusedRouteParameterNames.Length; i++)
58+
{
59+
parameters[UnusedRouteParameterNames[i]] = null;
60+
}
61+
}
62+
4863
context.Parameters = parameters;
4964
context.Handler = Handler;
5065
}

‎src/Components/Components/src/Routing/RouteTableFactory.cs

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,26 +34,53 @@ public static RouteTable Create(Assembly appAssembly)
3434

3535
internal static RouteTable Create(IEnumerable<Type> componentTypes)
3636
{
37-
var routes = new List<RouteEntry>();
38-
foreach (var type in componentTypes)
37+
var templatesByHandler = new Dictionary<Type, string[]>();
38+
foreach (var componentType in componentTypes)
3939
{
4040
// We're deliberately using inherit = false here.
4141
//
4242
// RouteAttribute is defined as non-inherited, because inheriting a route attribute always causes an
4343
// ambiguity. You end up with two components (base class and derived class) with the same route.
44-
var routeAttributes = type.GetCustomAttributes<RouteAttribute>(inherit: false);
44+
var routeAttributes = componentType.GetCustomAttributes<RouteAttribute>(inherit: false);
45+
46+
var templates = routeAttributes.Select(t => t.Template).ToArray();
47+
templatesByHandler.Add(componentType, templates);
48+
}
49+
return Create(templatesByHandler);
50+
}
4551

46-
foreach (var routeAttribute in routeAttributes)
52+
internal static RouteTable Create(Dictionary<Type, string[]> templatesByHandler)
53+
{
54+
var routes = new List<RouteEntry>();
55+
foreach (var keyValuePair in templatesByHandler)
56+
{
57+
var parsedTemplates = keyValuePair.Value.Select(v => TemplateParser.ParseTemplate(v)).ToArray();
58+
var allRouteParameterNames = parsedTemplates
59+
.SelectMany(GetParameterNames)
60+
.Distinct(StringComparer.OrdinalIgnoreCase)
61+
.ToArray();
62+
63+
foreach (var parsedTemplate in parsedTemplates)
4764
{
48-
var template = TemplateParser.ParseTemplate(routeAttribute.Template);
49-
var entry = new RouteEntry(template, type);
65+
var unusedRouteParameterNames = allRouteParameterNames
66+
.Except(GetParameterNames(parsedTemplate), StringComparer.OrdinalIgnoreCase)
67+
.ToArray();
68+
var entry = new RouteEntry(parsedTemplate, keyValuePair.Key, unusedRouteParameterNames);
5069
routes.Add(entry);
5170
}
5271
}
5372

5473
return new RouteTable(routes.OrderBy(id => id, RoutePrecedence).ToArray());
5574
}
5675

76+
private static string[] GetParameterNames(RouteTemplate routeTemplate)
77+
{
78+
return routeTemplate.Segments
79+
.Where(s => s.IsParameter)
80+
.Select(s => s.Value)
81+
.ToArray();
82+
}
83+
5784
/// <summary>
5885
/// Route precedence algorithm.
5986
/// We collect all the routes and sort them from most specific to

‎src/Components/Components/test/ParameterCollectionAssignmentExtensionsTest.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,24 @@ public void DeclaredParameterClashesWithInheritedParameter_Throws()
358358
ex.Message);
359359
}
360360

361+
[Fact]
362+
public void SupplyingNullWritesDefaultForType()
363+
{
364+
// Arrange
365+
var parameterCollection = new ParameterCollectionBuilder
366+
{
367+
{ nameof(HasInstanceProperties.IntProp), null },
368+
{ nameof(HasInstanceProperties.StringProp), null },
369+
}.Build();
370+
var target = new HasInstanceProperties { IntProp = 123, StringProp = "Hello" };
371+
372+
// Act
373+
parameterCollection.SetParameterProperties(target);
374+
375+
// Assert
376+
Assert.Equal(0, target.IntProp);
377+
Assert.Null(target.StringProp);
378+
}
361379

362380
class HasInstanceProperties
363381
{

‎src/Components/Components/test/Routing/RouteTableFactoryTests.cs

Lines changed: 60 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -268,8 +268,9 @@ public void PrefersLiteralTemplateOverTemplateWithParameters()
268268
{
269269
// Arrange
270270
var routeTable = new TestRouteTableBuilder()
271-
.AddRoute("/an/awesome/path")
272-
.AddRoute("/{some}/awesome/{route}/").Build();
271+
.AddRoute("/an/awesome/path", typeof(TestHandler1))
272+
.AddRoute("/{some}/awesome/{route}/", typeof(TestHandler2))
273+
.Build();
273274
var context = new RouteContext("/an/awesome/path");
274275

275276
// Act
@@ -346,9 +347,58 @@ public void DetectsAmbiguousRoutes(string left, string right)
346347
Assert.Equal(expectedMessage, exception.Message);
347348
}
348349

350+
[Fact]
351+
public void SuppliesNullForUnusedHandlerParameters()
352+
{
353+
// Arrange
354+
var routeTable = new TestRouteTableBuilder()
355+
.AddRoute("/", typeof(TestHandler1))
356+
.AddRoute("/products/{param1:int}", typeof(TestHandler1))
357+
.AddRoute("/products/{param2}/{PaRam1}", typeof(TestHandler1))
358+
.AddRoute("/{unrelated}", typeof(TestHandler2))
359+
.Build();
360+
var context = new RouteContext("/products/456");
361+
362+
// Act
363+
routeTable.Route(context);
364+
365+
// Assert
366+
Assert.Collection(routeTable.Routes,
367+
route =>
368+
{
369+
Assert.Same(typeof(TestHandler1), route.Handler);
370+
Assert.Equal("/", route.Template.TemplateText);
371+
Assert.Equal(new[] { "param1", "param2" }, route.UnusedRouteParameterNames);
372+
},
373+
route =>
374+
{
375+
Assert.Same(typeof(TestHandler2), route.Handler);
376+
Assert.Equal("{unrelated}", route.Template.TemplateText);
377+
Assert.Equal(Array.Empty<string>(), route.UnusedRouteParameterNames);
378+
},
379+
route =>
380+
{
381+
Assert.Same(typeof(TestHandler1), route.Handler);
382+
Assert.Equal("products/{param1:int}", route.Template.TemplateText);
383+
Assert.Equal(new[] { "param2" }, route.UnusedRouteParameterNames);
384+
},
385+
route =>
386+
{
387+
Assert.Same(typeof(TestHandler1), route.Handler);
388+
Assert.Equal("products/{param2}/{PaRam1}", route.Template.TemplateText);
389+
Assert.Equal(Array.Empty<string>(), route.UnusedRouteParameterNames);
390+
});
391+
Assert.Same(typeof(TestHandler1), context.Handler);
392+
Assert.Equal(new Dictionary<string, object>
393+
{
394+
{ "param1", 456 },
395+
{ "param2", null },
396+
}, context.Parameters);
397+
}
398+
349399
private class TestRouteTableBuilder
350400
{
351-
IList<(string, Type)> _routeTemplates = new List<(string, Type)>();
401+
IList<(string Template, Type Handler)> _routeTemplates = new List<(string, Type)>();
352402
Type _handler = typeof(object);
353403

354404
public TestRouteTableBuilder AddRoute(string template, Type handler = null)
@@ -361,10 +411,10 @@ public RouteTable Build()
361411
{
362412
try
363413
{
364-
return new RouteTable(_routeTemplates
365-
.Select(rt => new RouteEntry(TemplateParser.ParseTemplate(rt.Item1), rt.Item2))
366-
.OrderBy(id => id, RouteTableFactory.RoutePrecedence)
367-
.ToArray());
414+
var templatesByHandler = _routeTemplates
415+
.GroupBy(rt => rt.Handler)
416+
.ToDictionary(group => group.Key, group => group.Select(g => g.Template).ToArray());
417+
return RouteTableFactory.Create(templatesByHandler);
368418
}
369419
catch (InvalidOperationException ex) when (ex.InnerException is InvalidOperationException)
370420
{
@@ -373,5 +423,8 @@ public RouteTable Build()
373423
}
374424
}
375425
}
426+
427+
class TestHandler1 { }
428+
class TestHandler2 { }
376429
}
377430
}

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -226,10 +226,6 @@ public void CanFollowLinkToPageWithParameters()
226226
AssertHighlightedLinks("With parameters", "With more parameters");
227227

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

‎src/Components/test/testassets/BasicTestApp/RouterTest/WithParameters.razor

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,4 @@
88
[Parameter] public string FirstName { get; set; }
99

1010
[Parameter] public string LastName { get ; set; }
11-
12-
public override Task SetParametersAsync(ParameterCollection parameters)
13-
{
14-
// Manually reset parameters to defaults so we don't retain any from an earlier URL
15-
FirstName = default;
16-
LastName = default;
17-
return base.SetParametersAsync(parameters);
18-
}
1911
}

‎src/Components/test/testassets/ComponentsApp.App/Pages/FetchData.razor

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,28 +34,24 @@ else
3434
</tbody>
3535
</table>
3636
<p>
37-
<a href="fetchdata/@StartDate.AddDays(-5).ToString("yyyy-MM-dd")" class="btn btn-secondary float-left">
37+
<a href="fetchdata/@startDate.AddDays(-5).ToString("yyyy-MM-dd")" class="btn btn-secondary float-left">
3838
◀ Previous
3939
</a>
40-
<a href="fetchdata/@StartDate.AddDays(5).ToString("yyyy-MM-dd")" class="btn btn-secondary float-right">
40+
<a href="fetchdata/@startDate.AddDays(5).ToString("yyyy-MM-dd")" class="btn btn-secondary float-right">
4141
Next ▶
4242
</a>
4343
</p>
4444
}
4545

4646
@code {
47-
[Parameter] public DateTime StartDate { get; set; }
47+
[Parameter] public DateTime? StartDate { get; set; }
4848

4949
WeatherForecast[] forecasts;
50-
51-
public override Task SetParametersAsync(ParameterCollection parameters)
52-
{
53-
StartDate = DateTime.Now;
54-
return base.SetParametersAsync(parameters);
55-
}
50+
DateTime startDate;
5651

5752
protected override async Task OnParametersSetAsync()
5853
{
59-
forecasts = await ForecastService.GetForecastAsync(StartDate);
54+
startDate = StartDate.GetValueOrDefault(DateTime.Now);
55+
forecasts = await ForecastService.GetForecastAsync(startDate);
6056
}
6157
}

0 commit comments

Comments
 (0)