dotnet / aspnet-api-versioning

Provides a set of libraries which add service API versioning to ASP.NET Web API, OData with ASP.NET Web API, and ASP.NET Core.
MIT License
3.04k stars 703 forks source link

HttpContext.GetRequestedApiVersion is null on Asp.Net Core 2.2 #445

Closed mdmoura closed 5 years ago

mdmoura commented 5 years ago

I have the following Asp.Net core 2.2 controller:

[ApiVersion("1.0", Deprecated = false), Route("v{apiVersion}")]
public class FileController : Controller {

    protected ApiVersion RequestedApiVersion => HttpContext.GetRequestedApiVersion();

   [HttpGet("files/{fileId}")]
   public IActionResult GetContentByFileId2(Int32 fileId) {

    } 

}

And on Startup I have:

  services.AddApiVersioning(x => {
    x.ApiVersionSelector = new CurrentImplementationApiVersionSelector(x);
    x.AssumeDefaultVersionWhenUnspecified = true;
    x.DefaultApiVersion = new ApiVersion(1, 0);
    x.ReportApiVersions = true;                 
  });    

When I access the path "v1.0/files/1" the RequestedApiVersion is null.

Any idea why?

commonsensesoftware commented 5 years ago

Yes - actually I do! ;)

In your route template, you are missing the route constraint token. It should be:

[ApiVersion("1.0", Deprecated = false), Route("v{apiVersion:apiVersion}")]

The left-hand part of the route parameter is always the user-defined name of the parameter. This can be whatever you want. The name immediately proceeding the : character is the name of the registered route constraint. This has the default name of apiVersion, but can now be changed via options.RouteConstraintName. Regardless, the registered route constraint name must match the configuration and the name specified your route templates. After this small change, I expect things will begin working for you.

It might be worth mentioning that starting in 3.0+, you can now use Model Binding for the API version instead of HttpContext.GetRequestedApiVersion(). For example:

[HttpGet("files/{fileId}")]
public IActionResult GetContentByFileId2(Int32 fileId, ApiVersion requestedApiVersion) {
} 

The two values are always the same, but you might find using Model Binding more natural.

I hope that helps.

mdmoura commented 5 years ago

@commonsensesoftware Thank you. It helped. It is working now.

cvjlooc commented 2 years ago

@commonsensesoftware I have a similar issue where inside OnActionExecuted(ActionExecutedContext context) of a custom ActionFilterAttribute, context.HttpContext.GetRequestedApiVersion() seems to be null.

Do you know what is missing? Thank you.

commonsensesoftware commented 2 years ago

@cvjlooc I'm afraid I need some more context (setup, config, etc). GetRequestedApiVersion() will return null if no API version was requested or the value is invalid. It would be unexpected in user code, but it is also possible to simply, explicitly set it to null as well (which I don't think you did).

If OnActionExecuted is called, that would suggest that the action was matched and called. The only scenario I can think of where this would happen is if the action is version-neutral and no API version was specified. Even if you allowed a version to be assumed for backward compatibility, it would be explicitly set to that version by this point in the pipeline.

If you can share some additional details or a repro, I can provide more context. GetRequestedApiVersion() is just a shortcut for HttpContext.Features.Get<IApiVersioningFeature>().RequestedApiVersion. There is also RawRequestedApiVersion (e.g. string) that will indicate if anything was picked up. That might be useful.

Which library version is this? Is this still ASP.NET Core 2.2? I might need to jump start the Delorean to remember what I was doing 😆.

cvjlooc commented 2 years ago

@commonsensesoftware

Oh I'm sorry I didn't say that I'm using .NET 5! I'm assuming that OnActionExecuted is being called and that GetRequestedApiVersion() is returning null since I'm not seeing the intended effects of the code inside OnActionExecuted. I still didn't figure how to add a logger inside OnActionExcecuted.

The custom ActionFilterAttribute is applied to a number of Controller classes.

This is what I have in Startup.cs: services.AddApiVersioning( options => options.ApiVersionReader = new QueryStringApiVersionReader("api-version"));

What kind of other details are you looking for?

I read your code here about RawRequestedApiVersion but my gut feeling is that it might not work too :(

commonsensesoftware commented 2 years ago

Using a logger depends on how your attribute is instantiated. If you're applying as a normal attribute, you can resolve it from the action context. Something like context.HttpContext.RequestServices.GetRequiredService<ILogger<MyAttribute>>().

Based on this configuration, I would expect a request such as values?api-version=1.0. That should parse just fine. It sounds like you're not 100% sure if GetRequestedApiVersion() is returning null or not. You should be able to test by setting a breakpoint in OnActionExecuted or within the controller by calling GetRequestedApiVersion(). ApiVersion also has a model binder so you can add a parameter for it in your controller action as well.

cvjlooc commented 2 years ago

I did set a breakpoint in onActionExecuted in the past but context.HttpContext.GetRequestedApiVersion() returned null but that could be because when I ran things, our tests don't build the full context. In any case, I added HttpContext.GetRequestedApiVersion() to a controller and ran a local integration test and it still returned null probably because of the same reason as the above.

commonsensesoftware commented 2 years ago

Do you have something else in your setup you can share or a repro? There's something more going on here that I'm not seeing.

watchloop commented 2 years ago

@commonsensesoftware I'm facing a similar issue (HttpContext.GetRequestedApiVersion() returns null) but in testing. Here's my question with more details on SO.

Do you know why GetRequestedApiVersion() returns null? Thank you

commonsensesoftware commented 2 years ago

@watchloop There is several possibilities. The most obvious would be that the provided API version string is invalid. If that value fails to parse, then the result will yield null. It will not otherwise fail or throw an exception - by design. This is almost certainly the cause as I would missing things, such as not calling AddApiVersioning(), would result in a runtime exception from some something not being configured. In most cases, there should be a graceful fallback path that should always work when setting things up to version by query string parameter since it's the default.