Skip to content

Is there any plan to support OData Endpoint routing? #616

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
xuzhg opened this issue Mar 31, 2020 · 11 comments
Closed

Is there any plan to support OData Endpoint routing? #616

xuzhg opened this issue Mar 31, 2020 · 11 comments

Comments

@xuzhg
Copy link

xuzhg commented Mar 31, 2020

See detail at this issue:

OData/WebApi#2103

Any feedback, please let me know.
THanks

@commonsensesoftware
Copy link
Collaborator

Thanks @xuzhg (Sam). This is essentially a duplicate of #608. I've been participating on the threads on the OData repo for some time. I internally met with Saurabh and Mike several months back. I'm not one to chase pre-releases because I am the team here and I just don't have capacity to deal with a moving target. It's not really clear to me what the release schedule is or how many more changes may be coming. The beta seems to have been stable for a month or so now.

Sadly, it appears I may have to rewrite the OData routing infrastructure - again. I've had to do so in every flavor of OData since 5.8.x. The fact that the API Versioning matcher policy doesn't work as is is an indicator without even looking at the code. API Versioning doesn't do anything expect disambiguate duplicate routes by API Version. OData, in particular, always assumes there's one, and only one, possible match or fails. In Endpoint Routing, the matcher policy should only decide the possible matches. The base infrastructure decides on a 404 if there are no matches or 500 if there are duplicate matches. I've worked with the ASP.NET team since the public creation of the project and API Versioning was one of the scenarios they used to evaluate the Endpoint Routing design.

Despite my best efforts to urge this not to be the case in OData, the behavior always seems to remain the same. I'm hoping this time that may be just be the default behavior and easy to change without reimplementing the whole thing. Better still, since things are still in the beta phase, perhaps there will be a chance to make things play nicer together before the final release. I'm happy to have a conversation with you internally if the will help things become more synergistic. :)

Cheers!

@xuzhg
Copy link
Author

xuzhg commented Mar 31, 2020

@commonsensesoftware Thanks for sharing your thoughts. We (OData) schedule a regular sync meeting with .NET Team (ASP.NET) to figure out the routing stuffs. If you are interested in, i can forward that meeting to you so we can have a same platform to discuss the routing.

@dxynnez
Copy link

dxynnez commented Jul 10, 2020

Any update on this? Is the current workaround to not use endpoint routing?

@cvraman
Copy link

cvraman commented Jul 16, 2020

Any update on this ?

@commonsensesoftware
Copy link
Collaborator

There's currently nothing new to report at this time. Officially, it's not supported - yet. I've heard it may just work, but I haven't tried it myself. Your mileage may vary.

@commonsensesoftware
Copy link
Collaborator

For those following, this will now be the primary issue (since Sam is attached). I have an initial implementation working for, at least, the simple Endpoint Routing scenarios. No commitment on a release date, but I expect things to start progressing to a releasable state in the coming weeks.

@commonsensesoftware
Copy link
Collaborator

commonsensesoftware commented Sep 23, 2020

@xuzhg as feared, OData continues to certain design decisions that make it hard to extend. I get that some things cannot (or will not) be changed, so I'll focus on the items that can be fixed:

  1. ODataRouteBuilderExtensions.ConfigureDefaultServices and ODataEndpointBuilderExtensions.ConfigureDefaultServices are both internal, but should be public. This requires duplicating the code or using Reflection. I've historically done both. I'm back to Reflection because OData sometimes makes changes that are not duplicated between versions and then things break.
  2. ODataBatchPathMapping.IsEndpointRouting should be public when it's internal. This makes it difficult for perform alternate setups. I have to use Reflection to set it.
  3. BUG: ODataInputFormatter and ODataOutputFormatter Does Not Override Expected Behavior OData/WebApi#1750 is still not fixed. When will this ever happen? I've outlined everything, including the 5 lines of code to fix it. I need to commitment that PR will be accepted and release before I put in the effort. The list of old PRs do not instill confidence it would be worth my time, despite the community wanting it. @hassanhabib is even working around it in his demo. This is really needed for the API Explorer, which affects OpenAPI/Swagger support.

These are the most significant issues. I'll surface any others I find.

@commonsensesoftware
Copy link
Collaborator

+@NetTecture

The next release to support this will contain a considerable refactoring to the way routes are configured. The primary objective is to address the existing inability to customize/extend the routing configuration without reimplementing everything. A key problem is that it's all done with extension methods, which are static and can, therefore, not be overriden. To address this, things will move to extension methods that return public, extendable builder objects.

For example, the new configuration will look like:

app.UseEndpoints( endpoints =>
{
    endpoints.MapControllers();
    endpoints.UseApiVersioning().MapODataRoutes( "odata", "api", modelBuilder.GetEdmModels() );
} );

This setup will also apply to the legacy IRouter setup for parity. The next release will keep the existing routeBuilder.MapVersionedODataRoute and routeBuilder.MapVersionedODataRoutes variants, but they will be marked obsolete and call routeBuilder.UseApiVersioning().MapODataRoute and routeBuilder.UseApiVersioning().MapODataRoutes respectively. I expect to completely remove the old extension methods in the next major version release.

I'm happy to discuss or debate if there is any feedback.

@xuzhg
Copy link
Author

xuzhg commented Sep 23, 2020

@commonsensesoftware

Just for your information. We are investigating the next Web API OData.
The configuration for the OData service will be different between the old and the new.
You can find it here: https://github.com/OData/AspNetCoreOData/blob/master/sample/ODataRoutingSample/Startup.cs#L68-L72
The above line will be the only configure line. There will be no "MapOData....Routes".
I'd wondering whether it can easy the api-version configuration or not?

@commonsensesoftware
Copy link
Collaborator

@xuzhg looking good. If you're interested and willing, I'm happy to sync up internally.

I'd be interested to know how .AddModel will be mapped. API Versioning, in its purest form, is very simple. It adds a route matching policy that disambiguates duplicate routes by API version configured as pure metadata. I have stressed it many times before that OData should allow duplicate matches and then let the policy infrastructure decide what to do. By default, a client receives a 500 when there are duplicates, indicating the service author made a mistake. API Versioning adds a policy that will resolve the ambiguity if there is an appropriate match or pass through, resulting in the same 500. This is very simple with Endpoint Routing, but has continued to be difficult with all forms of routing in OData. I would be interested in any changes or thoughts being put into how OData routing will be realized.

I understand why the prefix exists, but I really think it should go away - for good. This continues to be incongruent with the rest of the routing system. Since it's not otherwise easy to understand the URL, it would be better suited by standard route constraints. For example:

  • api/[controller]/{property:edmProperty}
  • api/v{version:apiVersion}/[controller]/{action:edmAction}

This is obviously hypothetical and would probably negate the need for the ODataPathParser. While it's true that authors have to know about OData routing conventions, this is already a problem in all forms of OData routing (convention or attribute). Things bleed through one away or another. This type of approach would allow for strict and loose conformance to OData URL conventions, but allow developers to easily be in the driver's seat.

The 2nd part has to do with discovery, particularly for OpenAPI/Swagger. With respect to OData, this has an affinity to one or more EDM models. Today, API versioning has to collate these, which is a bit difficult with the prefix stuff. API Versioning knows how to pair up an EDM model because it annotates the EDM model with its own ApiVersionAnnotation that was mapped through VersionedODataModelBuilder.GetEdmModels(). Ultimately, I would ❤️, and believe, the OData team should own the core of API Explorer support for OData. The current support is virtually non-existent. I've had to create all of it for OData, even though a bunch of it is not specific to API Versioning. Many developers adopt OData + API Versioning just so they get the rich API Explorer support.

@commonsensesoftware
Copy link
Collaborator

5.0.0-RC.1 is now available. It includes support for Endpoint Routing. While minor on the surface, there are significant changes under the hood. Be sure to review the release notes and sample applications. The official release will likely occur after 2 weeks of burn-in. Please report any issues you encounter in a separate issue. Thanks.

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

4 participants