Skip to content

Version Selector ignored for OData Controllers #471

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
n-junge opened this issue Mar 6, 2019 · 8 comments
Closed

Version Selector ignored for OData Controllers #471

n-junge opened this issue Mar 6, 2019 · 8 comments

Comments

@n-junge
Copy link

n-junge commented Mar 6, 2019

I cannot get the api version selector to work when using OData.

I used the samples (ODataBasicSample, BasicSample) to verify that.

Added the following into the respective startup.cs

services.AddApiVersioning(
    options =>
    {
        // reporting api versions will return the headers "api-supported-versions" and "api-deprecated-versions"
        options.ReportApiVersions = true;
        options.AssumeDefaultVersionWhenUnspecified = true;
        options.ApiVersionSelector = new CurrentImplementationApiVersionSelector( options );
    } );

When omitting the api version (query parameter) in the BasicSample (without OData) the latest version is used.
For the ODataBasicSample the default version "1.0" is used.

I replaced the ApiVersionSelector with a custom one. It appears the SelectVersion Method is never called.

@commonsensesoftware
Copy link
Collaborator

Looks like your setup is correct; however, in order for the IApiVersionSelector to come into play (currently), no API version must be requested by the client. When a client specifies an API version, that value takes precedence and is always honored first. The server should always honor information provided by the client. There is currently no hook that would allow you to easily change this behavior. This behavior isn't limited to OData.

The only feasible workaround, which I don't recommend, would be to use a query parameter that isn't known to API versioning so that it will have the appearance that no API version was specified. This can be achieved by passing an empty list to the QueryStringApiVersionReader or creating and replacing the configured IApiVersionReader with an implementation that always returns null for IApiVersionReader.Read.

DISCLAIMER: Beware - there be dragons!

I hope that helps.

@n-junge
Copy link
Author

n-junge commented Mar 6, 2019

Looks like your setup is correct; however, in order for the IApiVersionSelector to come into play (currently), no API version must be requested by the client.

I understand that. But i'm not requesting a specific version via the client. I simple call '/api/People'. No header, no query, no api version in url, no version anywhere else. Still i'm getting the default version.

@commonsensesoftware
Copy link
Collaborator

Inferring some more information. I see that you are using ASP.NET Core as well as mixing OData and non-Data route. Which routing method is configured? OData doesn't work with Endpoint Routing.

@n-junge
Copy link
Author

n-junge commented Mar 7, 2019

As I said, I'm using the samples in this repository:
Everything things works fine as long as I specify the api version (?api-version=2.0). This is true for the ODataBasicSample Project (in this repo) as well: OData works perfect (btw. EndpointRouting is disabled there)

The only problem is when I omit the api version. Thus I request the latest implement version (after adding code snippet above). However I get the default 1.0, because apparently the VersionSelector is never called.

You can (or should be able to) reproduce that easily:

  1. Open this repo

  2. Add above code snippet into the startup.cs of both the BasicSample and the ODataBasicSample Project.

  3. Start BasicSample:

  • Call /api/values?api-version=1.0 => you get version 1.0
  • Call /api/values => you get version 2.0
  1. Start ODataBasicSample:
  • Call /api/People?api-version=1.0 => you get version 1.0
  • Call /api/People?api-version=2.0 => you get version 2.0 (now with email property)
  • Call /api/People => you get 1.0 (without email property), the latest version is 3.0

@n-junge
Copy link
Author

n-junge commented Mar 27, 2019

Are there still question or is the problem not reproducible?

@commonsensesoftware
Copy link
Collaborator

Apologies for the lapse in response. Been busy with the j-o-b recently.

Your information and repro was perfect and got right to the crux of the problem. I can confirm that this particular configuration yields a bug. >_< The root of the problem is in the VersionedODataPathRouteConstraint; mainly here.

I love OData as a protocol, but I really loathe the implementation sometimes. Rather than conform (e.g. rethink) to the routing in ASP.NET Core, the OData team has continued down the path of doing their own thing; specifically, the ODataPathRouteConstraint. API versioning and, hence, the IApiVersionSelector needs all candidates before it can make a decision, which isn't available at this point in the pipeline. The OData implementation always assumes there will only be one possible match (I guess based on the assumption of a single EDM).

I'm about to head out for vacation so it will probably be close to two weeks before I can commit serious time to resolving this. In the interim, there may be a possible workaround. If you set ApiVersioningOptions.DefaultApiVersion to the current API version, then things appear to work as expected (api/orders and api/people worked from the sample). Since you're always pushing clients forward to the current implementation, this appears to do the trick - for now. You'll have to bump this every time you change the current version, but that should be minimal maintenance. Hopefully, that will unblock you while I try to come up with a stable, lasting solution.

@Mhirji
Copy link

Mhirji commented Mar 27, 2019

On a side note, I wanted to set the default version to the latest for other reasons. The implementation is based on the SwaggerODataSample and I added
internal static readonly List<ApiVersion> Versions = new List<ApiVersion>() { V1, V2, V21 }; to the ApiVersions.cs in the Configuation. Then in startup, always set the default version to the last ApiVersion in the list. Maintenance wise it might be one less thing to think of as there is only one file to update (if you are using the ApiVersions.cs class.

@commonsensesoftware
Copy link
Collaborator

Just a quick update. I had to do some refactoring, but I have a reliable, long-term solution for this. I'm working on knocking out as many issues in the next release as possible. The fix for this will be included. Stay tuned. Thanks for your patience.

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

3 participants