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

ASP.Net Core: internal routing fails with CreatedAtRoute #18

Closed
bschaeublin opened this issue Sep 14, 2016 · 11 comments
Closed

ASP.Net Core: internal routing fails with CreatedAtRoute #18

bschaeublin opened this issue Sep 14, 2016 · 11 comments
Assignees

Comments

@bschaeublin
Copy link

bschaeublin commented Sep 14, 2016

I detected a problem with the routing, if you want to create an object via post and then return a CreatedAtRoute result. It always returns an error 500: "No route matches the supplied values"

If i remove the package (aspnet-api-versioning), and rename the route of the controller to /api/v1 in a fixed way, it works.

The provided samples in this repository are not (unfortunately ) helping in this case, because there are or only 'GET' request samples.

Can you help me? If you need more informations, i would be happy to send you some more.

Here's my code:

[ApiVersion("1.0")]
[Route("api/v{version:apiVersion}/[Controller]")]
public class DocumentController : Controller
{

[HttpGet("{guid}", Name = "GetDocument")]
public IActionResult GetByGuid(string guid)
{
    var doc = DocumentDataProvider.Find(guid);
    if (doc == null)
        return NotFound();

    return new ObjectResult(doc) {StatusCode = 200};
}


[HttpPost]
public IActionResult Create([FromBody] Document doc)
{
    //... Creating Doc

     var newguid = newDoc.Guid.ToString("N");

    //Does not work
   val = CreatedAtRoute("GetDocument", new{ controller =  "document", guid = newguid, version = HttpContext.GetRequestedApiVersion().ToString() }}, newDoc);

    return val;
}
}
@commonsensesoftware
Copy link
Collaborator

Coincidentally, I discovered this problem yesterday. I've captured the description and cause of the problem in Issue #19. The issue is specific the route constraint used for URL segment API versioning. This problem only occurs when you use that style of versioning. I've already fixed the issue and it will go out in an official release as soon as possible.

@commonsensesoftware
Copy link
Collaborator

It might also be worth mentioning that you don't have to re-specify route parameters that are already provided by the current route.

For example, the following will work just fine (after the fix):

CreatedAtRoute( "GetDocument", new { guid = newguid }, newDoc );

The controller and version route parameters are already provided so you don't need to specify them again. There's nothing wrong with your approach, but you can make things more succinct if you want. :)

@bschaeublin
Copy link
Author

Thank you for your fast reply and analysis!
I'll wait for the new release then :-).

For example, the following will work just fine (after the fix):
CreatedAtRoute( "GetDocument", new { guid = newguid }, newDoc );

Thank you, i had that on my first try but it was just another desperate attempt to make it work ;)

@R4VANG3R
Copy link

I'm having this problem on version 3.1.6 on Dotnet Core 2.2. The version parameter is provided by the controller route but when I return:

CreatedAtAction("Get", new { cId, aId, pId}, result);

I still get the error No route matches the supplied values.

However when I add the version it does work

CreatedAtAction("Get", new { version = HttpContext.GetRequestedApiVersion().ToString(), cId, aId, pId}, result);

@commonsensesoftware
Copy link
Collaborator

You must be versioning by URL segment. CreatedAtAction requires all of the route parameters. Since the API version is special, the ASP.NET doesn't know where it comes from. The ApiVersion type now supports model binding so you don't have to use the extension method anymore, unless you want to.

For example:

public IActionResult Get(int cId, int aId, int pId, ApiVersion version) => Ok();

public IActionResult Post([FromBody] Result result, ApiVersion version)
{
   var cId = result.cId;
   var aId = result.aId;
   var pId = result.pId;
   var apiVersion = version.ToString();
   return CreatedAtAction(nameof(Get), new {cId, aId, pId, apiVersion}, result);
}

Ultimately, this is another consequence of versioning by URL segment. While it might seem strange, it's completely valid for POST to be v1 and GET to be V2 or vice versa.

I hope that helps clear things up.

@DavidRogersDev
Copy link

This is a bit disappointing.
So, I have to hard code the location for the location header.

Nah, I'd rather leave it as an empty string. My client app will know where to get it from.

@commonsensesoftware
Copy link
Collaborator

@DavidRogersDev what is disappointing? Including the necessary route parameters? ASP.NET Core will only consider a very few, well-known intrinsic ambient values such as controller and action. It's taken quite some time to figure exactly when/where/how this might be addressed. Explicitly providing the value will still work as expected, but for those that feel it's too much effort, it has been resolved by #663 once and for all and is available in 5.0.0-RC.1+.

It's also worth mentioning that you should prefer CreatedAtAction over CreatedAtRoute. The route name specified via CreatedAtRoute must be unique and is not API version aware. The simplest away to get around that is to use a name such as "GetV1", "GetV2", etc, but that gets messy over time. I've yet to find a flexible solution without changing some of the standard routing setup. Not changing routing has always been a standing design principle. CreatedAtAction doesn't have this problem and doesn't require a name. Unless you are trying to build URLs across controllers, that would be the recommended way to go.

@SouhaibAkrouchi
Copy link

No route matches the supplied values.

Can't get why my post-method doesn't work. I think I have the same issue as the people above..
Any idea how I can fix this?

[HttpGet("{variable}/{value}", Name = "GetProblem")]
        public IActionResult GetTheProblem(string variable, string value)
        {
            if (variable == "id")
            {
                Problem problem = _problemRepository.GetById(int.Parse(value));
                if (problem == null) return Ok(_problemRepository.GetAll());
                return Ok(problem);
            }
            else if( variable == "title")
            {
                if (string.IsNullOrEmpty(value))
                    return Ok(_problemRepository.GetAll());
                return Ok(_problemRepository.GetByTitle(value));
            }
            return NotFound();
        }

[HttpPost] 
        public ActionResult<Problem> PostProblem(ProblemDTO problem)
        {
            Problem problemToCreate = new Problem() { Title = problem.Tittle, Description = problem.Description };
            foreach (var p in problem.Solutions)
                problemToCreate.AddSolution(new Solution(p.Answer));
            _problemRepository.Add(problemToCreate);
            _problemRepository.SaveChanges();

            return CreatedAtAction("GetProblem", new { id = problemToCreate.Id }, problemToCreate);
        }

@commonsensesoftware
Copy link
Collaborator

@SouhaibAkrouchi the issue is that you've crossed the streams. In CreateAtAction you've specified "GetProblem"; however, that's the name of a route. The name of the action (e.g. the method name) is GetTheProblem. Routing tables are not API version aware so I don't recommend using route names. If you do, you need to define your own scheme for naming such as GetProblemV1, GetProblemV2, and so on. CreateAtAction is scoped to methods/actions defined within the defining controller. Using nameof(GetTheProblem) for the key is a good way to avoid magic strings and ensure things stay in-sync if you refactor the method names. Since CreateAtAction is scoped to the defining controller, you don't have to worry about a version-aware naming scheme. That approach is only limiting if you try to resolve routes from outside of the controller.

@SouhaibAkrouchi
Copy link

Aight, thx man! You're the best.
It worked for me :)

@pasindumadushan
Copy link

app.MapControllerRoute(
name: "default",
pattern: "{controller}/{action=Index}/{id?}");

apply this program.cs

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

6 participants