Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RedirectToAction not working with endpoint routing #415

Closed
jasonycw opened this issue Dec 18, 2018 · 10 comments
Closed

RedirectToAction not working with endpoint routing #415

jasonycw opened this issue Dec 18, 2018 · 10 comments

Comments

@jasonycw
Copy link

I came across some issue when I was trying RedirectToAction

With .NET Core 2.2 and Versioning 3.0.0, RedirectToAction cannot properly redirect my call to another controller's action.

 return RedirectToAction("Search", "Users", new { query, token });

It throws the following error message

No route matches the supplied values.

Turn out it is again caused by the new endpoint routing and setting the following will fix the issue.

.AddMvc(opt => opt.EnableEndpointRouting = false)

Reference: #363 (comment)
Related: #18 #131

@commonsensesoftware
Copy link
Collaborator

Correct. 3.0 doesn't support Endpoint Routing - yet. This was mentioned, albeit casually, in the release notes. I'm currently working on the support for it. It will be available in 3.1 and is being tracked by #413.

In case you're wondering, the main reason for not immediately supporting it at the release of 3.0 is because Endpoint Routing will require using ASP.NET Core 2.2+. The 3.0 release still allows you to go back to ASP.NET Core 1.0. The planned release is by the end of January, but things are progressing very well and I expect it to be much earlier than that. I'm nearly down to just testing, but there is a large test matrix to replicate for Endpoint Routing because the legacy routing system is still supported.

@jasonycw
Copy link
Author

So, with 3.0, if we are on .NET Core 2.2, it will be better adding EnableEndpointRouting = false as most stuffs are broken because of this Endpoint Routing.

@commonsensesoftware
Copy link
Collaborator

If you want to use other 2.2 specific features (a la the compatibility switch), then - yes.

There's obviously a lot of interest in the new 2.2 features, but I was already in the middle of flushing out the 3.0 release. Unfortunately, I just need have the capacity to get everything done ahead of time. In addition, the requirement to update to 2.2 drove me to have a 3.0 release that still uses 2.0 which is closely followed up by 3.1 which does.

I'll notify all open issues once the beta is ready. Thanks for your patience.

@commonsensesoftware
Copy link
Collaborator

3.1 has been published and includes support for Endpoint Routing. All of the sample applications have been updated to use Endpoint Routing by default as well. Thanks.

@jasonycw
Copy link
Author

@commonsensesoftware Seems like 3.1 doesn't fix this issue.
I upgraded from NuGet to <PackageReference Include="Microsoft.AspNetCore.Mvc.Versioning" Version="3.1.0" />
Turn back Endpoint Routing on again by removing opt => opt.EnableEndpointRouting = false
RedirectToAction throw No route matches the supplied values. again.

Disable Endpoint Routing will again fix this issue.

@commonsensesoftware
Copy link
Collaborator

Can you share your setup and/or repro? The sample apps and tests have been updated to use CreatAtAction and they all work fine.

@jasonycw
Copy link
Author

jasonycw commented Dec 24, 2018

The problem with my case is with RedirectToAction, not sure if it is related to CreatAtAction

Here is a quick repo I just created with .NET Core 2.2 API template and Versioning 3.1.0
https://github.com/jasonycw/TestRedirectToAction

I added a TestController to redirect to another API in ValueController

using Microsoft.AspNetCore.Mvc;
using System.Collections.Generic;

namespace TestApi.Controllers
{
    [Route("api/v{version:apiVersion}/[controller]")]
    [ApiController]
    [ApiVersion("1.0")]
    public class TestController : Controller
    {
        [HttpGet]
        public ActionResult<IEnumerable<string>> Get() 
            => RedirectToAction("Get", "Values");
    }
}

With the following line, RedirectToAction will throw InvalidOperationException: No route matches the supplied values.
https://github.com/jasonycw/TestRedirectToAction/blob/master/TestApi/Startup.cs#L18

If I change that to

.AddMvc(opt => opt.EnableEndpointRouting = false)

/api/v1/test will redirect to /api/v1/value

@commonsensesoftware
Copy link
Collaborator

I suspect this is because the version route parameter has not been specified. The legacy routing system used existing, matching route parameters, but Endpoint Routing doesn't. Something like:

[HttpGet]
public ActionResult<IEnumerable<string>> Get(ApiVersion apiVersion)
            => RedirectToAction("Get", "Values", new { version = apiVersion.ToString() });

3.0 introduced model binding support for the ApiVersion. Be aware that cross-controller link generation like this needs to have congruent API version numbers.

@jasonycw
Copy link
Author

jasonycw commented Dec 24, 2018

Oh, so specifying the version do make it work, thanks!

[HttpGet]
public ActionResult<IEnumerable<string>> Get() 
    => RedirectToAction("Get", "Values", new { version = "1.0" });

But with this, that means I cannot reuse the route query values directly

[HttpGet("redirect")]
public ActionResult<IEnumerable<string>> Redirect([FromQuery]Query q) 
    => RedirectToAction("Test", "Values", q);

and need to layout every parameter

[HttpGet("redirect")]
public ActionResult<IEnumerable<string>> Redirect([FromQuery]Query q) 
     => RedirectToAction("Test", "Values", new { version = "1.0", id = q.Id, ... });

Is it possible to add another attribute for specify the version instead of binding it to the routeValues, like

RedirectToAction("Search", "Users", query, version: "1.0");

If not, we will need to think of how to pass our query model and handle versioning properly 🤔

@commonsensesoftware
Copy link
Collaborator

The ApiVersion is special as it relates to model binding because it can come from multiple places, even within the same request. The value is ultimately provided by the IApiVersioningFeature. There are a couple of other ways you can get at this value too.

  • Request.HttpContext.GetRequestedApiVersion()
  • HttpContext.Features.Get<IApiVersioningFeature>().RequestedApiVersion

The following union will work for your scenario:

[HttpGet("redirect")]
public ActionResult<IEnumerable<string>> Redirect([FromQuery]Query q, ApiVersion apiVersion) 
{
  var routeValues = new RouteValueDictionary(q) { ["version"] = apiVersion.ToString() };
  return RedirectToAction("Test", "Values", routeValues);
}

There might be a way to hook into or otherwise extend the URL generation mechanism to automatically inject the route value when available. Keep in mind the following:

  1. The version of requested API and the linked API need to match
  2. The route parameter names always need to be the same (e.g. version)

If those are wrong, link generation may fail or produce the incorrect result.

I hope that helps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants