Skip to content

Api Versioning break CreatedAt on MVC Core 2.2 #409

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

Closed
vanbukin opened this issue Dec 7, 2018 · 22 comments
Closed

Api Versioning break CreatedAt on MVC Core 2.2 #409

vanbukin opened this issue Dec 7, 2018 · 22 comments

Comments

@vanbukin
Copy link

vanbukin commented Dec 7, 2018

If I make GET request on http://localhost:5000/api/v1/values/42, then I've got the following result

{
    "value": "model 42"
}

and it's ok. Bui if I make a POST on http://localhost:5000/api/v1/values
with following body

{
    "value": 42
}

I've got the following stacktrace

fail: Microsoft.AspNetCore.Server.Kestrel[13]
      Connection id "0HLIS5H63UVVC", Request id "0HLIS5H63UVVC:00000003": An unhandled exception was thrown by the application.
System.NullReferenceException: Object reference not set to an instance of an object.
   at Microsoft.AspNetCore.Mvc.Routing.ApiVersionRouteConstraint.Match(HttpContext httpContext, IRouter route, String routeKey, RouteValueDictionary values, RouteDirection routeDirection)
   at Microsoft.AspNetCore.Routing.Template.TemplateBinder.TryProcessConstraints(HttpContext httpContext, RouteValueDictionary combinedValues, String& parameterName, IRouteConstraint& constraint)
   at Microsoft.AspNetCore.Routing.DefaultLinkGenerator.TryProcessTemplate(HttpContext httpContext, RouteEndpoint endpoint, RouteValueDictionary values, RouteValueDictionary ambientValues, LinkOptions options, ValueTuple`2& result)
   at Microsoft.AspNetCore.Routing.DefaultLinkGenerator.GetPathByEndpoints(List`1 endpoints, RouteValueDictionary values, RouteValueDictionary ambientValues, PathString pathBase, FragmentString fragment, LinkOptions options)
   at Microsoft.AspNetCore.Routing.DefaultLinkGenerator.GetPathByAddress[TAddress](HttpContext httpContext, TAddress address, RouteValueDictionary values, RouteValueDictionary ambientValues, Nullable`1 pathBase, FragmentString fragment, LinkOptions options)
   at Microsoft.AspNetCore.Routing.LinkGeneratorRouteValuesAddressExtensions.GetPathByRouteValues(LinkGenerator generator, HttpContext httpContext, String routeName, Object values, Nullable`1 pathBase, FragmentString fragment, LinkOptions options)
   at Microsoft.AspNetCore.Mvc.Routing.EndpointRoutingUrlHelper.Action(UrlActionContext urlActionContext)
   at Microsoft.AspNetCore.Mvc.UrlHelperExtensions.Action(IUrlHelper helper, String action, String controller, Object values, String protocol, String host, String fragment)
   at Microsoft.AspNetCore.Mvc.UrlHelperExtensions.Action(IUrlHelper helper, String action, String controller, Object values, String protocol, String host)
   at Microsoft.AspNetCore.Mvc.CreatedAtActionResult.OnFormatting(ActionContext context)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ObjectResultExecutor.ExecuteAsync(ActionContext context, ObjectResult result)
   at Microsoft.AspNetCore.Mvc.ObjectResult.ExecuteResultAsync(ActionContext context)
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeResultAsync(IActionResult result)
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeNextResultFilterAsync[TFilter,TFilterAsync]()
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.Rethrow(ResultExecutedContext context)
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.ResultNext[TFilter,TFilterAsync](State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeResultFilters()
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeNextResourceFilter()
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.Rethrow(ResourceExecutedContext context)
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeFilterPipelineAsync()
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeAsync()
   at Microsoft.AspNetCore.Routing.EndpointMiddleware.Invoke(HttpContext httpContext)
   at Microsoft.AspNetCore.Routing.EndpointRoutingMiddleware.Invoke(HttpContext httpContext)
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequests[TContext](IHttpApplication`1 application)
info: Microsoft.AspNetCore.Hosting.Internal.WebHost[2]
      Request finished in 191.4428ms 500

I can put breakpoint inside Post action and see that it calls correctly. The problem is somewhere inside CreatedAtAction.

Here is minimal repro
.csproj

<Project Sdk="Microsoft.NET.Sdk.Web">

  <PropertyGroup>
    <TargetFramework>netcoreapp2.2</TargetFramework>
    <AspNetCoreHostingModel>OutOfProcess</AspNetCoreHostingModel>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.AspNetCore.App" />
    <PackageReference Include="Microsoft.AspNetCore.Razor.Design" Version="2.2.0" PrivateAssets="All" />
    <PackageReference Include="Microsoft.AspNetCore.Mvc.Versioning" Version="3.0.0-beta2" />
    <PackageReference Include="Microsoft.AspNetCore.Mvc.Versioning.ApiExplorer" Version="3.0.0-beta2" />
  </ItemGroup>

</Project>

and Program.cs

using Microsoft.AspNetCore;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.DependencyInjection;

namespace ApiVersionTrouble
{
    public static class Program
    {
        public static void Main(string[] args)
        {
            CreateWebHostBuilder(args).Build().Run();
        }

        public static IWebHostBuilder CreateWebHostBuilder(string[] args)
        {
            return WebHost.CreateDefaultBuilder(args)
                .UseStartup<Startup>();
        }
    }

    public class Startup
    {
        public void ConfigureServices(IServiceCollection services)
        {
            var builder = services.AddMvc();
            builder.SetCompatibilityVersion(CompatibilityVersion.Version_2_2);
            services.AddApiVersioning();
            services.AddVersionedApiExplorer();
        }

        public void Configure(IApplicationBuilder app, IHostingEnvironment env)
        {
            app.UseMvc();
        }
    }

    [ApiController]
    [ApiVersion("1.0")]
    [Route("api/v{version:apiVersion}/values")]
    [Produces("application/json")]
    public class ValuesController : ControllerBase
    {
        [HttpGet("{settingsId:long}")]
        public ValueResponse Get([FromRoute] long settingsId)
        {
            return new ValueResponse(settingsId);
        }

        [HttpPost]
        public ActionResult Post([FromBody] ValueInput model)
        {
            var response = new ValueResponse(model.Value);
            return CreatedAtAction(
                nameof(Get),
                new {version = "1", settingsId = model.Value},
                response);
        }
    }

    public class ValueInput
    {
        public long Value { get; set; }
    }

    public class ValueResponse
    {
        public ValueResponse(long id)
        {
            Value = "model " + id.ToString("D");
        }

        public string Value { get; }
    }
}
@AdamWillden
Copy link

@vanbukin I had same thing and am raising issue on aspnetcore. you can get behaviour back if you set EnableEndpointRouting flag false in MvcOptions within AddMvc(options => ...)

@AdamWillden
Copy link

I'll link to this issue just in case the issue is here but I think it's the new Endpoint Routing

@AdamWillden
Copy link

dotnet/aspnetcore#4502

@commonsensesoftware
Copy link
Collaborator

@vanbukin, @AdamWillden is correct. 2.2 introduces a new type of routing system called Endpoint Routing, which is the new default. You can use the legacy routing mechanism by disabling this in the options. I've been aware of this change for some time and, in fact, some of those design driving the routing changes come from my feedback. Unfortunately, the ASP.NET has an entire platform team and I'm just one guy. I'm not even part of the ASP.NET team.

You can kvetch to the ASP.NET team, but we already have fairly regular discussions.

That said, I'm working toward supporting this as quickly as possible. The largest barrier isn't the changes in code, but the amount of testing and coordinating that the API Versioning 3.0 release. The 3.0 release brings a lot of fixes and other great features that do not depend on 2.2. If I jump the gun and force that immediately, all library consumers have to move up to 2.2, which is almost certainly not viable for everyone.

I had a recent chat with the ASP.NET team and their LTS story will be on 2.1 and that isn't expected to change again until 3.1. My current plan is to release 3.0, which only depends on 2.1. That will have a good story for existing adopters for the foreseeable future. I don't have the capacity to maintain a ton of version variations, so I definitely won't backport features and bug fixes will have to absolutely critical for me to consider them on older versions. The reported issues have dropped significantly since the Beta 2 release, which indicates stability and has given me some bandwidth. I anticipate that the official release will be later this month. The changes to support Endpoint Routing should be mostly internal shuffling and minor refactoring. I don't have a firm date, but I anticipate that it will be sometime in January.

I know the community is champing at the bit for this. I'm going as fast as my fingers and physics allow, but - again - I'm just one guy. I still have to do my day job on top of it. Thanks for you patience.

@xakep139
Copy link

Not only CreatedAtAction() returns null, Url.AbsoluteAction() returns null too

@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.

@vanbukin
Copy link
Author

@commonsensesoftware You are awesome! Thank you!

@AdamWillden
Copy link

@vanbukin let me know if you're still experiencing issues, I'm still seeing same issue with new 3.1 package.

/cc @commonsensesoftware

@commonsensesoftware
Copy link
Collaborator

I can confirm that there was indeed a bug in the ApiVersionRouteConstraint with Endpoint Routing. The new URL generation mechanism can call IRouteConstraint instances with a null HttpContext. This was fixed and is included in 3.1.

The example here is similar to the scenario you are describing. The only difference is that it uses CreatedAtRoute.

I tweaked it ever so slightly to repro your exact scenario:

[ApiController]
[ApiVersion( "1.0" )]
[Route( "api/v{version:apiVersion}/[controller]" )]
public class HelloWorldController : ControllerBase
{
  [HttpGet]
  public IActionResult Get( ApiVersion apiVersion ) =>
    Ok( new { Controller = GetType().Name, Version = apiVersion.ToString() } );

  [HttpGet( "{id:int}" )]
  public IActionResult Get( int id, ApiVersion apiVersion ) =>
    Ok( new { Controller = GetType().Name, Id = id, Version = apiVersion.ToString() } );

  [HttpPost]
  public IActionResult Post( ApiVersion apiVersion ) =>
    CreatedAtAction( nameof( Get ), new { id = 42, version = apiVersion.ToString() }, null );
}

Here's the results of calling POST:

POST /api/v1/helloworld
Host: localhost:5000
HTTP/1.1 201
Content-Length: 0
Location: http://localhost:5000/api/v1/HelloWorld/42
api-supported-versions: 1.0

This demonstrates that CreatedAtAction is working.

You might want to make sure you've done a restore and clean build with the latest update. You might also check and/or report the stack trace to confirm it's happening within API versioning. Given the information you've provided, I'm not repro'ing your scenario (anymore).

I'm happy to investigate further with some more info.

@commonsensesoftware
Copy link
Collaborator

BTW: support for the ApiVersion as a model-bound action parameter is a new feature 3.0. This way you don't have to hardcode the values nor use Request.HttpContext.GetRequestedApiVersion() (which is still available).

@commonsensesoftware
Copy link
Collaborator

I went one step further. CreatedAtRoute is lame and its only usefulness for me was parity with Web API, which doesn't always make sense. I've updated all samples and tests for ASP.NET Core to use CreatedAtAction instead. The acceptance tests use both legacy and Endpoint Routing so it definitely works with API versioning. If you're continuing to have trouble, you might consider comparing to any number of samples or even the tests. If you're stuck, I'll do what I can to help.

@dnutiu
Copy link

dnutiu commented May 7, 2019

I've tried your example from above and I got an error saying that the API version is not specified. Had to use the following options to get it to work:

services.AddApiVersioning(options => { options.AssumeDefaultVersionWhenUnspecified = true; });

@commonsensesoftware
Copy link
Collaborator

Are you trying to use CreatedAtRoute or CreatedAtAction? The AssumeDefaultVersionWhenUnspecified option doesn't play a factor into building route templates. I would need more information to address your specific issue.

@dnutiu
Copy link

dnutiu commented May 7, 2019

Hello,

Sorry for that. I'm using the CreatedAtAction. The project I'm working on is just a toy project that I'm playing with in order to learn ASP.Net Core and C#. It's hosted on my Github: https://github.com/dnutiu/NucuPaste/blob/master/NucuPaste/Controllers/PastesController.cs#L102

@commonsensesoftware
Copy link
Collaborator

No problem. It appears the primary reason it's not working for you is that you have not applied the ApiVersionRouteConstraint. This is required when versioning by URL segment.

Your route template is:

[Route( "api/v{version}/[controller]" )]

But it should be:

[Route( "api/v{version:apiVersion}/[controller]" )]

apiVersion is the default name associated with the route constraint. It's possible to change this name in the options if you really want to. The name version is your user-defined route parameter name. This can be named whatever you want, but I usually use version in the examples.

I hope that helps.

@dnutiu
Copy link

dnutiu commented May 7, 2019

Aha, it makes sense now! Thank you!

@elonmallin
Copy link

Hey @commonsensesoftware , I also got this problem using CreatedAtAction(nameof(GetUser), new { id = user.Id }, await GetUser(user.Id)). I had to set the EnableEndpointRouting = false to get it to work like so:

services.AddMvc(options =>
{
    options.EnableEndpointRouting = false;
});

I understood the conversation above as EnableEndpointRouting doesn't have to be set to false anymore? It's hard to know that it needs to be false in order to work. Also I didn't look up what it does yet so don't know if I am giving something up by setting it or not.

I'm using Microsoft.AspNetCore.App (2.2.0) and Microsoft.AspNetCore.Mvc.Versioning (3.1.6).

@commonsensesoftware
Copy link
Collaborator

@elonmallin, you didn't provide information about your controller or action, but I'm willing to guess that you're versioning by URL segment as in the examples above. Based on what you've provided, I surmise that you are simply not including the API version as one of the route parameters for URL generation. I can't say why the legacy routing system automatically has the route parameter populated, but Endpoint Routing doesn't.

You should be able to resolve things using one of the following:

Option 1

Explicitly retrieve the requested API version and specify it in the route generation process.

public async Task<IActionResult> Post([FromBody] User user)
{
  var version = Request.HttpContext.GetRequestedApiVersion().ToString();
  return CreatedAtAction(nameof(GetUser), new { id = user.Id, version }, await GetUser(user.Id));
}

Option 2

Implicitly retrieve the requested API version via model binding and specify it in the route generation process.

public async Task<IActionResult> Post([FromBody] User user, ApiVersion apiVersion)
{
  var version = apiVersion.ToString();
  return CreatedAtAction(nameof(GetUser), new { id = user.Id, version }, await GetUser(user.Id));
}

@elonmallin
Copy link

Okey, thanks! I'll try Option 1 since I'm setting the version in the route as you suspected.

Do you know if this will be made to work without any of these workarounds in the future?

@commonsensesoftware
Copy link
Collaborator

Doubtful. I wouldn't consider this a workaround. You should specify the version route parameter using this method IMO. The current route and target route do not have to have the same API version. This is a consequence of versioning by URL segment (the least RESTful method). All other built-in versioning methods do not have this problem.

@elonmallin
Copy link

Okey, yea that's true. However the current and target route doesn't have to be on the same controller either I guess. But it's the default.

I thought the whole controller path including version would be the default when using the CreatedAtAction function.

If other built in versioning methods doesn't have this problem it would be more consistent too I guess if this method didn't have this "problem" either.

But I guess it has to do with how query params are passed along by default and the [controller] is also deeply ingrained in the routing system or something.

@commonsensesoftware
Copy link
Collaborator

Agreed; they don't have to be same controller. Be aware that the route tables are not API version aware. This may require you to create a route naming convention which bakes in the API version; for example, "GetUserV1" or "GetUserV2".

The reason other versioning methods do not suffer from this problem is because the API version is not in the route template (e.g. the path). Query parameters, headers, etc are specified by the client - as they should be. The server's job is not to tell the client which version they want. By definition of the Uniform Interface constraint, the path api/v1/orders/123 and api/v2/orders/123 implies two completely different resources, but that's almost certainly not true; they are different representations - at best. The path is the resource identifier. Other methods use a stable path and identifier. The client should indicate which representation they want by specifying the appropriate media type, query parameter, header, etc.

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