-
Notifications
You must be signed in to change notification settings - Fork 714
Need help with creating new tests #1119
-
Hello! I just finished adding a new API version to the eShop demo app, and in the process I learned a ton about how to implement versioning with this package. This is a great package!
But I did notice a few places where I think some small improvements would be useful. One example is that the "api-supported-versions" header is currently not included in the error response for an unspecified API version.
I can open issues for these, but was hoping to do more and actually propose a fix (this seems like it would be a relatively easy fix). But I'd like to start by adding a test that demonstrates the problem, and I can't figure out where such a test would go or how it would be written.
I believe the fix needs to go in
src\AspNetCore\WebApi\src\Asp.Versioning.Http\Routing\UnspecifiedApiVersionEndpoint.cs
but I don't see any unit tests for this class and the class is internal so not currently visible to the tests. I suppose I could use an attribute to make it visible, but I don't see that being done anywhere else in the repo so I'm reluctant to go that route.
Any guidance on how to proceed would be appreciated.
Beta Was this translation helpful? Give feedback.
All reactions
Replies: 2 comments 1 reply
-
Thank you for the kind words and interest. This particular case seems easy and straight forward, but it's actually more complex than it appears on the surface. The reason that you may not see api-supported-versions in this scenario has more to do with how things connect to routing. The long and short of it is that the API version comes into play before the route is evaluated. This means that api-supported-versions can't really be reported because we don't know which route is going to be matched - yet.
I don't have visualization offhand, however, if you run the acceptance tests, most of them call through to:
Line 41 in 3fc0719
This will output a link that you can open in a browser, which shows the directed graph of the route table. Hopefully, you see that without a version, you don't know which API (e.g. endpoint) is to be matched. The routing table can tell there is a candidate that is versioned, which is how you land on the error endpoint. Without knowing which endpoint (or set of endpoints) you'll land on, there's a chance you'd report the wrong versions.
Hopefully that makes sense. I'm happy to answer more questions. I suspect the test-generated graphs will provide a useful visual. If you have other ideas or suggestions, I'm open to them.
Beta Was this translation helpful? Give feedback.
All reactions
-
Is this situation specific to ASP.NET Core? I ask because it appears that for ASP.NET, the api-supported-versions header is included in a 400 response for Unspecified API version. The code that does this is in the HttpResponseExceptionFactory.cs.
And I confirmed that this does actually make it into the response by adding an assert in this test :
image
This test passes with my modification.
So given this seems to work in ASP.NET, shouldn't it be possible to also make it work in ASP.NET Core?
Beta Was this translation helpful? Give feedback.
All reactions
-
Yes, it is - now - specific to ASP.NET Core. Prior to 6.0, the both behaved the same way. Other original high-level concept was to allow the routing system to do its magic and then disambiguate the results by API version. In many cases, if an API version wasn't specified, you still had the candidates whittled down and you could return a sensible set of supported API versions. Unfortunately, this approach had a number of problems with the legacy routing system (a la IRouter) and, even with Endpoint Routing, resulted in the wrong behavior for certain scenarios; specifically around 405, 406, and 415. In 6.0, this was changed so that API version itself is part of the decision process in the route tree. API Versioning does not know if your APIs have symmetric version numbers. If they don't, reporting the aggregation of API versions could easily return a lie.
This highlights where the problem occurs:
Line 334 in 3fc0719
In theory, it might be possible for an unspecified API version to follow the same approach as an unsupported API version. I can't recall 100% off the top of my head.
This shows how the destination is selected:
Line 44 in 3fc0719
Here's an example of one of the directed graphs that is generated by the acceptance tests. Hopefully, that will provide some visualization to the routing tree and shed some light on where the challenges are.
API Versioning doesn't know the target endpoint yet so the challenge is accurately reporting the associated versions. These are stored on the matching endpoint. What it does know is that an API Version, any version, was expected to be specified, but wasn't.
May that give you some pointers and you'll find some additional information and/or results.
Beta Was this translation helpful? Give feedback.