Fix #6140 - Allow linking between controller and page

The issue here is that route values used for action selection are
'global'. That means that pages need to have a 'null' route value for
'action' and controllers need to have a 'null' route value for pages. This
is the same way that areas work.

The fix is to move the 'merge' of route values up to a level where pages
and controllers can work together. Since ADPs use the russian-doll
pattern, the fix is to run this 'merge' in the controller ADP, but after
all of the ADs have been created.
This commit is contained in:
Ryan Nowak 2017-04-18 23:39:13 -07:00
parent 6cd487e15f
commit 4bf518b09b
7 changed files with 124 additions and 43 deletions

View File

@ -34,8 +34,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal
{
var actions = new List<ControllerActionDescriptor>();
var routeValueKeys = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
var methodInfoMap = new MethodToActionMap();
var routeTemplateErrors = new List<string>();
@ -64,7 +62,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal
actionDescriptor.ControllerTypeInfo = controller.ControllerType;
AddApiExplorerInfo(actionDescriptor, application, controller, action);
AddRouteValues(routeValueKeys, actionDescriptor, controller, action);
AddRouteValues(actionDescriptor, controller, action);
AddProperties(actionDescriptor, action, controller, application);
actionDescriptor.BoundProperties = controllerPropertyDescriptors;
@ -108,12 +106,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal
// attribute routes with a given name have the same template.
AddActionToNamedGroup(actionsByRouteName, attributeRouteInfo.Name, actionDescriptor);
}
// Add a route value with 'null' for each user-defined route value in the set to all the
// actions that don't have that value. For example, if a controller defines
// an area, all actions that don't belong to an area must have a route
// value that prevents them from matching an incoming request when area is specified.
AddGlobalRouteValues(actionDescriptor, routeValueKeys);
}
if (attributeRoutingConfigurationErrors.Any())
@ -409,7 +401,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal
}
public static void AddRouteValues(
ISet<string> keys,
ControllerActionDescriptor actionDescriptor,
ControllerModel controller,
ActionModel action)
@ -421,8 +412,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal
// generating a link.
foreach (var kvp in action.RouteValues)
{
keys.Add(kvp.Key);
// Skip duplicates
if (!actionDescriptor.RouteValues.ContainsKey(kvp.Key))
{
@ -432,8 +421,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal
foreach (var kvp in controller.RouteValues)
{
keys.Add(kvp.Key);
// Skip duplicates - this also means that a value on the action will take precedence
if (!actionDescriptor.RouteValues.ContainsKey(kvp.Key))
{
@ -483,19 +470,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal
}
}
private static void AddGlobalRouteValues(
ControllerActionDescriptor actionDescriptor,
ISet<string> removalConstraints)
{
foreach (var key in removalConstraints)
{
if (!actionDescriptor.RouteValues.ContainsKey(key))
{
actionDescriptor.RouteValues.Add(key, string.Empty);
}
}
}
private static void AddActionToNamedGroup(
IDictionary<string, IList<ActionDescriptor>> actionsByRouteName,
string routeName,

View File

@ -66,6 +66,36 @@ namespace Microsoft.AspNetCore.Mvc.Internal
/// <inheritdoc />
public void OnProvidersExecuted(ActionDescriptorProviderContext context)
{
// After all of the providers have run, we need to provide a 'null' for each all of route values that
// participate in action selection.
//
// This is important for scenarios like Razor Pages, that use the 'page' route value. An action that
// uses 'page' shouldn't match when 'action' is set, and an action that uses 'action' shouldn't match when
// 'page is specified.
//
// Or for another example, consider areas. A controller that's not in an area needs a 'null' value for
// area so it can't match when the route produces an 'area' value.
var keys = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
for (var i = 0; i < context.Results.Count; i++)
{
var action = context.Results[i];
foreach (var key in action.RouteValues.Keys)
{
keys.Add(key);
}
}
for (var i = 0; i < context.Results.Count; i++)
{
var action = context.Results[i];
foreach (var key in keys)
{
if (!action.RouteValues.ContainsKey(key))
{
action.RouteValues.Add(key, null);
}
}
}
}
internal protected IEnumerable<ControllerActionDescriptor> GetDescriptors()

View File

@ -28,7 +28,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure
_pagesOptions = pagesOptionsAccessor.Value;
}
public int Order { get; set; }
public int Order { get; set; } = -900; // Run after the default MVC provider, but before others.
public void OnProvidersExecuting(ActionDescriptorProviderContext context)
{

View File

@ -277,11 +277,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal
var descriptorWithoutValue = Assert.Single(
descriptors,
ad => ad.RouteValues.Any(kvp => kvp.Key == "key" && string.IsNullOrEmpty(kvp.Value)));
ad => !ad.RouteValues.ContainsKey("key"));
var descriptorWithValue = Assert.Single(
descriptors,
ad => ad.RouteValues.Any(kvp => kvp.Key == "key" && kvp.Value == "value"));
ad => ad.RouteValues.ContainsKey("key"));
// Assert
Assert.Equal(2, descriptors.Length);
@ -303,7 +303,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal
c.Key == "key" &&
c.Value == "value");
Assert.Equal(3, descriptorWithoutValue.RouteValues.Count);
Assert.Equal(2, descriptorWithoutValue.RouteValues.Count);
Assert.Single(
descriptorWithoutValue.RouteValues,
c =>
@ -314,11 +314,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal
c =>
c.Key == "action" &&
c.Value == "OnlyPost");
Assert.Single(
descriptorWithoutValue.RouteValues,
c =>
c.Key == "key" &&
c.Value == string.Empty);
}
[Fact]
@ -932,7 +927,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal
var indexAction = Assert.Single(actionDescriptors, ad => ad.ActionName.Equals("Index"));
Assert.Equal(5, indexAction.RouteValues.Count);
Assert.Equal(3, indexAction.RouteValues.Count);
var controllerDefault = Assert.Single(indexAction.RouteValues, rd => rd.Key.Equals("controller", StringComparison.OrdinalIgnoreCase));
Assert.Equal("ConventionalAndAttributeRoutedActionsWithArea", controllerDefault.Value);
@ -942,12 +937,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal
var areaDefault = Assert.Single(indexAction.RouteValues, rd => rd.Key.Equals("area", StringComparison.OrdinalIgnoreCase));
Assert.Equal("Home", areaDefault.Value);
var mvRouteValueDefault = Assert.Single(indexAction.RouteValues, rd => rd.Key.Equals("key", StringComparison.OrdinalIgnoreCase));
Assert.Equal(string.Empty, mvRouteValueDefault.Value);
var anotherRouteValue = Assert.Single(indexAction.RouteValues, rd => rd.Key.Equals("second", StringComparison.OrdinalIgnoreCase));
Assert.Equal(string.Empty, anotherRouteValue.Value);
}
[Fact]
@ -1355,6 +1344,42 @@ namespace Microsoft.AspNetCore.Mvc.Internal
Assert.Single(action.ActionConstraints, a => a is ConstraintAttribute);
}
[Fact]
public void OnProviderExecuted_AddsGlobalRouteValues()
{
// Arrange
var context = new ActionDescriptorProviderContext();
context.Results.Add(new ActionDescriptor()
{
RouteValues = new Dictionary<string, string>()
{
{ "controller", "Home" },
{ "action", "Index" },
}
});
context.Results.Add(new ActionDescriptor()
{
RouteValues = new Dictionary<string, string>()
{
{ "page", "/Some/Page" }
}
});
var provider = GetProvider();
// Act
provider.OnProvidersExecuted(context);
// Assert
Assert.True(context.Results[0].RouteValues.ContainsKey("page"));
Assert.Null(context.Results[0].RouteValues["page"]);
Assert.True(context.Results[1].RouteValues.ContainsKey("controller"));
Assert.Null(context.Results[1].RouteValues["controller"]);
Assert.True(context.Results[1].RouteValues.ContainsKey("action"));
Assert.Null(context.Results[1].RouteValues["action"]);
}
private ControllerActionDescriptorProvider GetProvider(
TypeInfo controllerTypeInfo,
IEnumerable<IFilterMetadata> filters = null)

View File

@ -791,6 +791,34 @@ Microsoft.AspNetCore.Mvc.ViewFeatures.ViewDataDictionary`1[AspNetCore._InjectedP
Assert.Equal(expected, response.Headers.Location.ToString());
}
[Fact]
public async Task Controller_RedirectToPage()
{
// Arrange
var expected = "/RedirectToController?param=17";
// Act
var response = await Client.GetAsync("/RedirectToPage");
// Assert
Assert.Equal(HttpStatusCode.Redirect, response.StatusCode);
Assert.Equal(expected, response.Headers.Location.ToString());
}
[Fact]
public async Task Page_RedirectToController()
{
// Arrange
var expected = "/RedirectToPage?param=92";
// Act
var response = await Client.GetAsync("/RedirectToController");
// Assert
Assert.Equal(HttpStatusCode.Redirect, response.StatusCode);
Assert.Equal(expected, response.Headers.Location.ToString());
}
private async Task AddAntiforgeryHeaders(HttpRequestMessage request)
{
var getResponse = await Client.GetAsync(request.RequestUri);

View File

@ -0,0 +1,16 @@
// 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 Microsoft.AspNetCore.Mvc;
namespace RazorPagesWebSite
{
public class RedirectController : Controller
{
[HttpGet("/RedirectToPage")]
public IActionResult RedirectToPage()
{
return RedirectToRoute(new { page = "/RedirectToController", param = 17 });
}
}
}

View File

@ -0,0 +1,8 @@
@page
@functions {
public IActionResult OnGet()
{
return new RedirectToRouteResult(new { controller = "Redirect", action = "RedirectToPage", param = 92 });
}
}