Skip to content

Multiple versions on the same controller #821

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
Mhirji opened this issue Apr 22, 2022 · 6 comments
Closed

Multiple versions on the same controller #821

Mhirji opened this issue Apr 22, 2022 · 6 comments

Comments

@Mhirji
Copy link

Mhirji commented Apr 22, 2022

Hi,

I'm not sure if you are seeing the same thing (and I don't suspect it is normal behavior) but when using the ODataOpenApiExample project, calling version v0.9 ~/api/Orders/1 returns a 404 and v1 ~/api/Orders/1 returns the order as expected. v2 and v3 both work as expected, but they each have their own controller.

On my own project, when using multiple versions on the same controller, one version will throw a 404 where the other returns the data - the same behavior. I should note in my case neither version is deprecated - thought that might be a clue but nope. I thought it may be due to the order, but changing it does not seem to make a difference either on the ODataOpenApiExample project or mine - which to be honest I found strange :) - it appears to always take the lowest whole number. So in my case I had v3 and v4, v3 would return data and v4 would return a 404.

Perhaps I am missing something or misunderstanding something somewhere?

@commonsensesoftware
Copy link
Collaborator

This is very strange indeed. I can confirm that I can reproduce the behavior.

Order does not, nor ever, matters. The API version information is purely metadata and you can annotate it in any way you like. The collated lists are often sorted, but that is purely for sanity and optimizations when they are accessed later (ex: reporting API versions). Deprecated also does not matter. Deprecated or not, it's a matchable candidate. Deprecated just means the API version will go away.

The stranger thing is that a similar configuration is used for People and both api/people/42?api-version=0.9 and api/people/42?api-version=1.0 work. The ODataBasicExample has a similar setup and it also works.

Debugging, I see the route is mapped and resolves to the correct endpoint. It's not clear how we get to 404? 😕 Continuing to research. There's a chance it's a bug, but I haven't been able to prove it - yet. Thanks for reporting the issue.

@Mhirji
Copy link
Author

Mhirji commented May 6, 2022

Hmmmm, a mystery 🤔. Have you tried with preview three? If not, let me know and I will to see if some of the changes fixed the underlying issue. It would be good to get to the root cause, but if it's working with the next release...

@commonsensesoftware
Copy link
Collaborator

I'm working directly from the Preview 3 build and it's still there - unfortunately. I can reproduce the problem without any OpenAPI stuff. It's very strange that it only happens sometimes. Stranger still, I've noticed that adding the explicit route template (ex: [HttpGet("api/[controller]/{key}")]) fixes the problem in many cases. That should not be required. That might be a short-term stopgap for you. The issue seems to be somewhere down were OData is not recognizing or otherwise building out the implicit conventions. There is likely a mistake - somewhere. I haven't found it, but that's my hunch.

@commonsensesoftware
Copy link
Collaborator

Ultimately, this was a bug, which has now been fixed. It was the last hurdle I wanted to iron out before publishing Preview 3 with all the other changes and fixes. I've also gone through each route in the example to make sure it actually works.

TL;DR

The cause comes down to the combination of a few things. OData adds multiple SelectorModel instances to ActionModel. ASP.NET Core will create an ActionDescriptor for each ActionModel.Selectors it finds. The problem is that API Versioning has already run and applied ApiVersionMetadata to the ActionModel.Selectors[0] (in most scenarios there is only one). By default, there is no way that OData could know this exists and there is no way for API Versioning (core) to know this will happen. This is also only problem when there multiple API versions, and hence EDMs, pointing to the same action (e.g. ActionModel == ActionDescriptor). That is why some routes work just fine, while others don't.

The fix was simple that the API Versioning extensions for OData need to realize that if there is more than one EDM created and there are more than one selectors per action (e.g. ActionModel.Selectors.Count > 1), then any ApiVersionMetadata needs to be copied from ActionModel.Selectors[0].EndpointMetadata to ActionModel.Selectors[i].EndpointMetadata, where i > 1.

I had actually forgotten and vaguely remembered that I had previously run into this issue. There was some code in place to handle it, but it was doing it wrong. There's some internal shuffling to put things in the right spot and optimize whether processing is even required. Fortunately, any work performed is fast and cheap. ApiVersionMetadata is immutable and doesn't need to be recalculated or cloned; it just needs a shallow copy to be added to other selectors.

@commonsensesoftware
Copy link
Collaborator

Preview 3 is now available and contains the fix for this issue.

@Mhirji
Copy link
Author

Mhirji commented May 11, 2022

Thanks @commonsensesoftware!!

@Mhirji Mhirji closed this as completed May 11, 2022
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