aspnet-api-versioning icon indicating copy to clipboard operation
aspnet-api-versioning copied to clipboard

API versioning OData doesn't works with custom ODataQueryOptions

Open vvaskiv-lohika-tix opened this issue 4 years ago • 8 comments
trafficstars

Framework: netcoreapp3.1

I'm having an issue with OData API versioning. When I don't use ODataQueryOptions in the action method, I received that the API version is not supported.

Here example of OData controller configuration. In this case implemented custom ODataQueryOptionsSlim, it's lightweight replacement of ODataQueryOptions.

    [ApiVersion("1.0")]
    [ApiVersion("2.0")]
    [Route("")]
    [ControllerName("orders")]
    public class OrdersController : ODataController
    {
        [HttpGet]
        public IActionResult Get(ODataQueryOptionsSlim<OrderTemp> options) =>
            Ok(new[] { new OrderTemp() { Id = 1, Customer = "Bill Mei" } });

        [HttpGet("{key:int}")]
        public IActionResult Get(int key, ODataQueryOptionsSlim<OrderTemp> options) =>
            Ok(new OrderTemp() { Id = key, Customer = "Bill Mei" });
    }

When I used the controller with ODataQueryOptionsSlim, I always received that API version not supported. See screenshot.

image

When the change from ODataQueryOptionsSlim to ODataQueryOptions, everything works.

    [ApiVersion("1.0")]
    [ApiVersion("2.0")]
    [Route("")]
    [ControllerName("orders")]
    public class OrdersController : ODataController
    {
        [HttpGet]
        public IActionResult Get(ODataQueryOptions<OrderTemp> options) =>
            Ok(new[] { new OrderTemp() { Id = 1, Customer = "Bill Mei" } });

        [HttpGet("{key:int}")]
        public IActionResult Get(int key, ODataQueryOptions<OrderTemp> options) =>
            Ok(new OrderTemp() { Id = key, Customer = "Bill Mei" });
    }

image

How to resolve this issue, in order to ODataQueryOptionsSlim also works with API versioning? Does API versioning depend on ODataQueryOptions? Maybe exist any attributes for mark action to supporting API versioning without ODataQueryOptions?

vvaskiv-lohika-tix avatar Sep 29 '21 17:09 vvaskiv-lohika-tix

I can't think of anything in API Versioning that would care about this. There are only a handful of places where the special built-in OData types such as ODataQueryOptions matter. In all such cases, type evaluation is based on the base or any derived typed. There's always a chance of a bug, but it's suspect.

The million dollar question is does it work without API Versioning? The special OData types such as:

  • ODataQueryOptions
  • ODataQueryOptions<T>
  • Delta
  • Delta<T>
  • ODataPath

Use a model binder provided by OData. It's similar to how the ApiVersion and CancellationToken action parameters work. I wouldn't be completely surprised if the OData implementation doesn't check/consider derived types. If it does, did you make sure that your ODataQueryOptionsSlim<T> inherits from ODataQueryOptions<T> or ODataQueryOptions? If not, I suspect it will not work. You can't just drop in your own, arbitrary type; that is without a model binder. If you create your own model binder to complement your type, it should work.

commonsensesoftware avatar Sep 29 '21 18:09 commonsensesoftware

ODataQueryOptionsSlim implements interface of IODataQueryOptions. Yes, it works without API Versioning. See screenshots.

image

image

image

Regarding model binder, we created it. And one thing which I noticed it's that model binder doesn't execute when route with API versioning, but when without API versioning it's executing. I'm a little bit confusing, why model binder doesn't invoke on route with the API version.

image

vvaskiv-lohika-tix avatar Sep 29 '21 19:09 vvaskiv-lohika-tix

I believe that you have something working. However, there is no such [official] interface as IODataQueryOptions as can be seen here and here. You also shouldn't need [FromQuery] because your own model binder would handle that.

Do you have the world's simplest repro with the necessary bits? It would make an investigation much faster.

commonsensesoftware avatar Sep 29 '21 19:09 commonsensesoftware

Ohhh. Regarding IODataQueryOptions, it's my mistake. Sorry. What do you mean by 'simplest repro with the necessary bits'? Are you about the sample repository?

vvaskiv-lohika-tix avatar Sep 29 '21 19:09 vvaskiv-lohika-tix

Do you have, or can you provide, a version of your code that demonstrates the problem and can be uploaded to this issue for further investigation?

commonsensesoftware avatar Sep 29 '21 19:09 commonsensesoftware

I created a simple example, to view the issue which I mentioned.

https://github.com/vaskiv99/odata-api-versioning

vvaskiv-lohika-tix avatar Sep 30 '21 16:09 vvaskiv-lohika-tix

I realize it's been quite some time. Thanks for the repo. I haven't run it - yet. API Versioning doesn't really care about this, but I seem to recall that the older OData routing logic may have been affected by it. That logic wasn't extensible so some of it had to be forked by API Versioning. It's entirely possible that something was missed or not updated.

You might want to bounce this against the forthcoming 6.0 release here. The whole implementation changes for OData 8.0 and a bunch of things API Versioning used to have to do in order to make it work has been removed. If that work for you, then this issue may not be worth pursuing further.

commonsensesoftware avatar Mar 21 '22 01:03 commonsensesoftware

6.0 Preview 3 is available with support for OData 8.0. I have a feeling this might work now.

commonsensesoftware avatar May 09 '22 19:05 commonsensesoftware

6.0 has officially been released. In terms of routing, if it works with vanilla OData 8.x+, then it should just work with API Versioning. 6.0 will still support .NET Core 3.1 as well as .NET 6.0. Thank you for the repro, it was useful. If this has somehow not been solved, I'm happy reopen the issue and investigate further. Thanks.

commonsensesoftware avatar Aug 24 '22 02:08 commonsensesoftware