Skip to content

Commit 5f21e7c

Browse files
Copilotilonatommyjaviercn
authored
Add validation to Router component to prevent both NotFound and NotFoundPage being set (#62625)
* Add validation to prevent `NotFound` and `NotFoundPage` being set simultaneously * Add unit test for throwing when both `NotFound` and `NotFoundPage` are set * Remove NotFound fragment from templates - we have NotFoundPage instead. --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: ilonatommy <[email protected]> Co-authored-by: javiercn <[email protected]> Co-authored-by: Ilona Tomkowicz <[email protected]>
1 parent 3e86bc0 commit 5f21e7c

File tree

6 files changed

+91
-28
lines changed

6 files changed

+91
-28
lines changed

‎src/Components/Components/src/PublicAPI.Unshipped.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#nullable enable
22
*REMOVED*Microsoft.AspNetCore.Components.ResourceAsset.ResourceAsset(string! url, System.Collections.Generic.IReadOnlyList<Microsoft.AspNetCore.Components.ResourceAssetProperty!>? properties) -> void
33
Microsoft.AspNetCore.Components.ResourceAsset.ResourceAsset(string! url, System.Collections.Generic.IReadOnlyList<Microsoft.AspNetCore.Components.ResourceAssetProperty!>? properties = null) -> void
4-
Microsoft.AspNetCore.Components.Routing.Router.NotFoundPage.get -> System.Type!
4+
Microsoft.AspNetCore.Components.Routing.Router.NotFoundPage.get -> System.Type?
55
Microsoft.AspNetCore.Components.Routing.Router.NotFoundPage.set -> void
66
Microsoft.AspNetCore.Components.Infrastructure.ComponentsMetricsServiceCollectionExtensions
77
Microsoft.AspNetCore.Components.NavigationManager.OnNotFound -> System.EventHandler<Microsoft.AspNetCore.Components.Routing.NotFoundEventArgs!>!

‎src/Components/Components/src/Routing/Router.cs‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,14 +70,15 @@ static readonly IReadOnlyDictionary<string, object> _emptyParametersDictionary
7070
/// Gets or sets the content to display when no match is found for the requested route.
7171
/// </summary>
7272
[Parameter]
73+
[Obsolete("NotFound is deprecated. Use NotFoundPage instead.")]
7374
public RenderFragment NotFound { get; set; }
7475

7576
/// <summary>
7677
/// Gets or sets the page content to display when no match is found for the requested route.
7778
/// </summary>
7879
[Parameter]
7980
[DynamicallyAccessedMembers(LinkerFlags.Component)]
80-
public Type NotFoundPage { get; set; } = default!;
81+
public Type? NotFoundPage { get; set; }
8182

8283
/// <summary>
8384
/// Gets or sets the content to display when a match is found for the requested route.
@@ -143,6 +144,12 @@ public async Task SetParametersAsync(ParameterView parameters)
143144

144145
if (NotFoundPage != null)
145146
{
147+
#pragma warning disable CS0618 // Type or member is obsolete
148+
if (NotFound != null)
149+
{
150+
throw new InvalidOperationException($"Setting {nameof(NotFound)} and {nameof(NotFoundPage)} properties simultaneously is not supported. Use either {nameof(NotFound)} or {nameof(NotFoundPage)}.");
151+
}
152+
#pragma warning restore CS0618 // Type or member is obsolete
146153
if (!typeof(IComponent).IsAssignableFrom(NotFoundPage))
147154
{
148155
throw new InvalidOperationException($"The type {NotFoundPage.FullName} " +
@@ -401,10 +408,12 @@ private void RenderNotFound()
401408
new RouteData(NotFoundPage, _emptyParametersDictionary));
402409
builder.CloseComponent();
403410
}
411+
#pragma warning disable CS0618 // Type or member is obsolete
404412
else if (NotFound != null)
405413
{
406414
NotFound(builder);
407415
}
416+
#pragma warning restore CS0618 // Type or member is obsolete
408417
else
409418
{
410419
DefaultNotFoundContent(builder);
@@ -429,6 +438,7 @@ async Task IHandleAfterRender.OnAfterRenderAsync()
429438

430439
private static partial class Log
431440
{
441+
#pragma warning disable CS0618 // Type or member is obsolete
432442
[LoggerMessage(1, LogLevel.Debug, $"Displaying {nameof(NotFound)} because path '{{Path}}' with base URI '{{BaseUri}}' does not match any component route", EventName = "DisplayingNotFound")]
433443
internal static partial void DisplayingNotFound(ILogger logger, string path, string baseUri);
434444

@@ -440,5 +450,6 @@ private static partial class Log
440450

441451
[LoggerMessage(4, LogLevel.Debug, $"Displaying {nameof(NotFound)} on request", EventName = "DisplayingNotFoundOnRequest")]
442452
internal static partial void DisplayingNotFound(ILogger logger);
453+
#pragma warning restore CS0618 // Type or member is obsolete
443454
}
444455
}

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
#pragma warning disable CS0618 // Type or member is obsolete
5+
46
using System.Reflection;
57
using Microsoft.AspNetCore.Components.RenderTree;
68
using Microsoft.AspNetCore.Components.Test.Helpers;
@@ -265,6 +267,40 @@ await _renderer.Dispatcher.InvokeAsync(() =>
265267
Assert.Equal("Not found", renderedFrame.TextContent);
266268
}
267269

270+
[Fact]
271+
public async Task ThrowsExceptionWhenBothNotFoundAndNotFoundPageAreSet()
272+
{
273+
// Arrange
274+
var services = new ServiceCollection();
275+
services.AddSingleton<ILoggerFactory>(NullLoggerFactory.Instance);
276+
services.AddSingleton<NavigationManager>(_navigationManager);
277+
services.AddSingleton<INavigationInterception, TestNavigationInterception>();
278+
services.AddSingleton<IScrollToLocationHash, TestScrollToLocationHash>();
279+
var serviceProvider = services.BuildServiceProvider();
280+
281+
var renderer = new TestRenderer(serviceProvider);
282+
renderer.ShouldHandleExceptions = true;
283+
var router = (Router)renderer.InstantiateComponent<Router>();
284+
router.AppAssembly = Assembly.GetExecutingAssembly();
285+
router.Found = routeData => (builder) => builder.AddContent(0, $"Rendering route matching {routeData.PageType}");
286+
renderer.AssignRootComponentId(router);
287+
288+
var parameters = new Dictionary<string, object>
289+
{
290+
{ nameof(Router.AppAssembly), typeof(RouterTest).Assembly },
291+
{ nameof(Router.NotFound), (RenderFragment)(builder => builder.AddContent(0, "Custom not found")) },
292+
{ nameof(Router.NotFoundPage), typeof(NotFoundTestComponent) }
293+
};
294+
295+
// Act & Assert
296+
var exception = await Assert.ThrowsAsync<InvalidOperationException>(async () =>
297+
await renderer.Dispatcher.InvokeAsync(() =>
298+
router.SetParametersAsync(ParameterView.FromDictionary(parameters))));
299+
300+
Assert.Contains("Setting NotFound and NotFoundPage properties simultaneously is not supported", exception.Message);
301+
Assert.Contains("Use either NotFound or NotFoundPage", exception.Message);
302+
}
303+
268304
internal class TestNavigationManager : NavigationManager
269305
{
270306
public TestNavigationManager() =>
@@ -306,4 +342,8 @@ public class MatchAnythingComponent : ComponentBase { }
306342

307343
[Route("a/b/c")]
308344
public class MultiSegmentRouteComponent : ComponentBase { }
345+
346+
[Route("not-found")]
347+
public class NotFoundTestComponent : ComponentBase { }
348+
309349
}

‎src/Components/test/testassets/Components.TestServer/RazorComponents/App.razor‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,25 @@
3030
<HeadOutlet />
3131
</head>
3232
<body>
33-
<Router AppAssembly="@typeof(App).Assembly" AdditionalAssemblies="new[] { typeof(TestContentPackage.NotFound.NotFoundPage).Assembly }" NotFoundPage="NotFoundPageType">
34-
<Found Context="routeData">
35-
<RouteView RouteData="@routeData" />
36-
<FocusOnNavigate RouteData="@routeData" Selector="[data-focus-on-navigate]" />
37-
</Found>
38-
<NotFound><p id="not-found-fragment">There's nothing here</p></NotFound>
39-
</Router>
33+
@if (NotFoundPageType != null)
34+
{
35+
<Router AppAssembly="@typeof(App).Assembly" AdditionalAssemblies="new[] { typeof(TestContentPackage.NotFound.NotFoundPage).Assembly }" NotFoundPage="NotFoundPageType">
36+
<Found Context="routeData">
37+
<RouteView RouteData="@routeData" />
38+
<FocusOnNavigate RouteData="@routeData" Selector="[data-focus-on-navigate]" />
39+
</Found>
40+
</Router>
41+
}
42+
else
43+
{
44+
<Router AppAssembly="@typeof(App).Assembly" AdditionalAssemblies="new[] { typeof(TestContentPackage.NotFound.NotFoundPage).Assembly }">
45+
<Found Context="routeData">
46+
<RouteView RouteData="@routeData" />
47+
<FocusOnNavigate RouteData="@routeData" Selector="[data-focus-on-navigate]" />
48+
</Found>
49+
<NotFound><p id="not-found-fragment">There's nothing here</p></NotFound>
50+
</Router>
51+
}
4052
<script>
4153
// This script must come before blazor.web.js to test that
4254
// the framework does the right thing when an element is already focused.

‎src/Components/test/testassets/Components.WasmMinimal/Routes.razor‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,22 @@
2323
}
2424
}
2525

26-
<Router AppAssembly="@typeof(Program).Assembly" AdditionalAssemblies="new[] { typeof(TestContentPackage.NotFound.NotFoundPage).Assembly }" NotFoundPage="NotFoundPageType">
27-
<Found Context="routeData">
28-
<RouteView RouteData="@routeData" />
29-
<FocusOnNavigate RouteData="@routeData" Selector="h1" />
30-
</Found>
31-
<NotFound><p id="not-found-fragment">There's nothing here</p></NotFound>
32-
</Router>
26+
@if (NotFoundPageType != null)
27+
{
28+
<Router AppAssembly="@typeof(Program).Assembly" AdditionalAssemblies="new[] { typeof(TestContentPackage.NotFound.NotFoundPage).Assembly }" NotFoundPage="NotFoundPageType">
29+
<Found Context="routeData">
30+
<RouteView RouteData="@routeData" />
31+
<FocusOnNavigate RouteData="@routeData" Selector="h1" />
32+
</Found>
33+
</Router>
34+
}
35+
else
36+
{
37+
<Router AppAssembly="@typeof(Program).Assembly" AdditionalAssemblies="new[] { typeof(TestContentPackage.NotFound.NotFoundPage).Assembly }">
38+
<Found Context="routeData">
39+
<RouteView RouteData="@routeData" />
40+
<FocusOnNavigate RouteData="@routeData" Selector="h1" />
41+
</Found>
42+
<NotFound><p id="not-found-fragment">There's nothing here</p></NotFound>
43+
</Router>
44+
}

‎src/ProjectTemplates/Web.ProjectTemplates/content/ComponentsWebAssembly-CSharp/App.razor‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,6 @@
44
<RouteView RouteData="@routeData" DefaultLayout="@typeof(MainLayout)"/>
55
<FocusOnNavigate RouteData="@routeData" Selector="h1" />
66
</Found>
7-
<NotFound>
8-
<PageTitle>Not found</PageTitle>
9-
<LayoutView Layout="@typeof(MainLayout)">
10-
<p role="alert">Sorry, there's nothing at this address.</p>
11-
</LayoutView>
12-
</NotFound>
137
</Router>
148
#else
159
<CascadingAuthenticationState>
@@ -29,12 +23,6 @@
2923
</AuthorizeRouteView>
3024
<FocusOnNavigate RouteData="@routeData" Selector="h1" />
3125
</Found>
32-
<NotFound>
33-
<PageTitle>Not found</PageTitle>
34-
<LayoutView Layout="@typeof(MainLayout)">
35-
<p role="alert">Sorry, there's nothing at this address.</p>
36-
</LayoutView>
37-
</NotFound>
3826
</Router>
3927
</CascadingAuthenticationState>
4028
#endif*@

0 commit comments

Comments
 (0)