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

ApiVersionLinkGenerator methods that don't take HttpContext argument don't generate links #753

Closed
dbroudy opened this issue Jul 1, 2021 · 3 comments

Comments

@dbroudy
Copy link

dbroudy commented Jul 1, 2021

AddApiVersionRouteValueIfNecessary requires an HttpContext and is only called in methods that have one as an argument. It looks like this is intentional, but the result is that LinkGenerator doesn't return a value for calls that don't provide either:

  • HttpContext as an argument
  • Or the route value that was used as a version if URL Path Versioning is being used.

My specific case is ApiControllers that are calling GetPathByRouteValues(routeName, values), but I think it applies more broadly.

Versions
Microsoft.AspNetCore.Mvc.Versioning, 5.0.0
Microsoft.AspNetCore.App, 5.0.0

I'm working around this by using IHttpContextAccessor to get and pass the HttpContext, but that may not work for everyone (if services.AddHttpContextAccessor isn't called). I can create a minimal project that reproduces this if that's helpful.

@commonsensesoftware
Copy link
Collaborator

You'd have to share some additional details and perhaps a code snippet, but I suspect that this is - in fact - the expected behavior. This capability was added to address the expectation of auto-magically filling the API version route parameter when versioning by URL segment. Only the [controller] and [action] tokens are supported by default. In a similar fashion, value/{id}/subvalues will not generate a link if you don't provide {id}.

I'm going to assume you must be attempting this outside of a HTTP request. In order for API versioning to provide the route parameter value on your behalf, it must occur within the context of HTTP request. There is no other information where the value can be derived from. The interesting part is that you mention you can make it work with IHttpContextAccessor. That seems to suggest that you are within a HTTP request, but you're trying to generate the link in a place where HttpContext is no longer available.

The simplest way to make things work with the HttpContext is to provide the API version route parameter yourself. This was always required before this capability was introduced. For example, if you template were api/v{version:apiVersion}/value/{id}, then you need only provide:

var url = Url.Action( nameof( Get ), new { id = 42, version = "1" } );

You'd either have to know the API version for the path your traversing or capture it via model binding (e.g. action parameter) or the HttpContext.GetRequestedApiVersion() extension method. Another option would be to generate the link where HttpContext is easily available and then pass the generated link.

You should be cognizant that route names are not API version-aware. If you're going to use this approach, then you'll need some type of naming convention such as "GetValueById", "GetValueByIdV2", and so on. This approach would suggest that you already know the API version so hard-coding it might be ok. You can also pass it down or potentially extract it from the route name.

This issue illustrates one more complication from versioning by URL segment. It's the least RESTful method. It's probably too late to change methods, but it's something to think about in the future. No other method of versioning suffers from this issue.

@dbroudy
Copy link
Author

dbroudy commented Jul 1, 2021

Thanks for the quick and very helpful reply. As you guessed, it's too late to avoid URL segment versioning, I need it to maintain compatibility with current applications. I plan to migrate the API and use a Mime Type, but will need to make sure all clients are upgraded first.

I am attempting to generate the route inside the context of an http request, so I can pass the http context explicitly for my work around. All I did was change:

            return _linkGenerator.GetPathByRouteValues(routeName, routeValues);

to

            var httpContext = _httpContextAccessor.HttpContext;
            return _linkGenerator.GetPathByRouteValues(httpContext, routeName, routeValues);

routeValues is just a simple anonymous object: new { id = ... }.

Would it make any sense to have ApiVersionLinkGenerator implicitly try to get an http context for the methods (GetPathByAddress and GetUriByAddress)
where HttpContext is not an argument so that AddApiVersionRouteValueIfNecessary could still be called in those cases? Of course this would have to deal with being called outside of the context of an http request, and it appear to be inconsistent with how DefaultLinkGenerator is working, so it's probably a bad idea.

Thanks again for the detailed explanation. I think the MSDN docs are a little vague on the fact that ambient values are only available when an HttpContext is explicitly passed to LinkGenerator, but that's clearly the intent and you're following that model here, which makes perfect sense.

@commonsensesoftware
Copy link
Collaborator

Glad that solved, or at least provided a path to a solution, for your issue.

The ApiVersionLinkGenerator is only meant to be a decorator over the DefaultLinkGenerator. It is technically possible to inject the HttpContext via IHttpContextAccessor, but it would force the service registration and potentially breaks the expected behavior. As I understand it, the overloads that do not take HttpContext are meant to be generatable without it (presumably because all values are provided). For consistency and predictability, it's probably best to leave it.

I agree the documentation is light on this stuff. Since everything is open source, I tend to spelunk the source over MSDN anymore. The source can't lie. 😉 I was wondering how {controller} and {action} were working. It took me quite a bit of searching, but I eventually found the answer in UrlHelperBase.cs and EndpointRoutingUrlHelper.cs that they are honored as Ambient Values. The ApiVersionLinkGenerator effectively treats the API version route parameter as an ambient value too.

One additional side note is that the route parameter names in most cases are user-defined. controller and action are special, which is likely why they are the only out-of-the-box supported ambient values. In order for API Versioning to do the right thing, you would have also had to reach a point in the pipeline where the ApiVersionRouteConstraint has been evaluated. This is when/where the user-defined route parameter name is captured in the request. It's used for other purposes beyond URL generation. This enables you to call the API version route parameter whatever you want. For example, {version:apiVersion}, {ver:apiVersion}, and {v:apiVersion} will be version, ver, and v respectively. I doubt you'll hit that issue, but just FYI.

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