OData / AspNetCoreOData

ASP.NET Core OData: A server library built upon ODataLib and ASP.NET Core
Other
453 stars 159 forks source link

Package update to Microsoft.AspNetCore.OData 8.0.4 not allowing to have multiple EDM for different controllers #418

Open padhumailin opened 2 years ago

padhumailin commented 2 years ago

Earlier in OData v7, we have route mapped with multiple edm for different controller in startup using the method "MapODataServiceRoute" and ODataRoute attribute decorator is added in controller. like this :-

appBuilder.UseEndpoints(odata =>
{
    odata.MapControllers();
    // Enable the dependency injection support for all HTTP routes
    odata.EnableDependencyInjection();
    // Add querying options(Select, Expand, Filter, OrderBy, MaxTop)
    odata.Select().Expand().Filter().OrderBy().MaxTop(null).Count();

     odata.MapODataServiceRoute("Accounts", "api/Accounts",
                configureAction => configureAction.
                  AddService(Microsoft.OData.ServiceLifetime.Singleton, typeof(IEdmModel),
                      serviceProvider => edmModelAccounts()))

    odata.MapODataServiceRoute("Devices", "api/Devices",
                configureAction => configureAction.
                  AddService(Microsoft.OData.ServiceLifetime.Singleton, typeof(IEdmModel),
                      serviceProvider => edmModelDevices()))
}

AccountsController : ODataController

[HttpGet]
 [ODataRoute("", RouteName = "Accounts")]
 [EnableQuery]
  public async Task<IActionResult> GetAccounts()
    {
         return Ok(_db.Accounts);
    }

DevicesController : ODataController

[HttpGet]
 [ODataRoute("", RouteName = "Devices")]
 [EnableQuery]
  public async Task<IActionResult> GetDevices()
    {
         return Ok(_db.Devices);
    }

Endpoint URLS: http://localhost:52434/api/Accounts Response:

{
    "@odata.context": "http://localhost:52434/api/Accounts/$metadata",
    "value": [
        {}
    ]
}

http://localhost:52434/api/Devices

Response:

{
    "@odata.context": "http://localhost:52434/api/Devices/$metadata",
    "value": [
        {}
    ]
}

But after updating to OData ASP.NET Core OData 8.0.4, facing difficulty in mapping multiple EDM to different controller without changing the route URL because ODataRoute attribute is not exists.

Added the code below in startup:

services.AddControllers().AddOData(option => option.Select().Filter().Count().OrderBy().Expand().SetMaxTop(null)
    .AddRouteComponents("api/Accounts", ODataHelper.edmModelAccounts(),
            builder => builder.AddSingleton<IODataSerializerProvider, ODataSerializerProvider>())
    .AddRouteComponents("api/Devices", ODataHelper.edmModelDevices(),
            builder => builder.AddSingleton<IODataSerializerProvider, ODataSerializerProvider>())

and removed the odatroute attribute from controller action methods. but the endpoints mentioned above returns as not implemented. and it returns data when endoints are given as below:

http://localhost:52434/api/Accounts/**Accounts**
http://localhost:52434/api/Devices/**Devices**

it expects additional entity set to be added in route path. Accounts and Devices entity sets are different from each other and not related to each other, so we need to map these different EDM's to different controllers.

Is there anyway to make it work without changing the endpoint URL? Please help

Framework : .Net 6 Microsoft.AspNetCore.OData : 8.0.4

xuzhg commented 2 years ago

@padhumailin Since you are using attribute routing in 7 version, you can continue to use attribute routing in 8.

Attribute routing in 8 is changed to use [RouteAttribute], [HttpGetAttribute], [HttpPostAttribute]. You can refer to :https://devblogs.microsoft.com/odata/attribute-routing-in-asp-net-core-odata-8-0-rc/ for more details.

Let me know if it can't work.

padhumailin commented 2 years ago

@xuzhg i have tried attribute routing, but could not able to get it work.

  1. To have two different EDM in metadata URL for two different controller action menthods since we cannot have duplicate route prefix in AddRouteComponents, i need to specify route prefix including entityset as below:

.AddRouteComponents("api/Accounts", ODataHelper.edmModelAccounts()) .AddRouteComponents("api/Devices", ODataHelper.edmModelDevices())

  1. In route attribute, i have put empty path because i have already mentioned the complete URL in route prefix itself

`AccountsController : ODataController [HttpGet("")] [EnableQuery] public async Task GetAccounts() {}

    DevicesController : ODataController
    [HttpGet("")]
    [EnableQuery]
    public async Task<IActionResult> GetDevices() {}`

in Postman when executing the URL "http://localhost:52434/api/Accounts" throws below error:

` Microsoft.AspNetCore.Routing.Matching.AmbiguousMatchException HResult=0x80131500 Message=The request matched multiple endpoints. Matches:

API.Web.Controllers.AccountsController.Get (API.Web) Microsoft.AspNetCore.OData.Routing.Controllers.MetadataController.GetServiceDocument (Microsoft.AspNetCore.OData) Source=Microsoft.AspNetCore.Routing`

But, http://localhost:52434/api/Accounts/Accounts works well. earlier in v7 in ODataRoute attribute, routename parameter bridges the route registration and the action method. and this is missing in v8.

xuzhg commented 2 years ago

@xuzhg As i mentioned in the post, the attribute routing using the route template merged from [RouteAttribute](if have) and other HTTP verb attribute class. for example [HttpGetAttribute].

for your scenario, you can 1) merge your two models into one model, and use .AddRouteComponents("api", ODataHelper.edmModelAccounts()) 2) or customize your own routing.

If it can help, can you share with me your project then I can modify it?

padhumailin commented 2 years ago

@xuzhg please look at the project https://github.com/padhumailin/ODataV8Demo

padhumailin commented 2 years ago

@xuzhg Did you find any time to check the code?

xuzhg commented 2 years ago

@padhumailin see the PR for extensions.

padhumailin commented 2 years ago

@xuzhg Thanks. It works with expected route URL.

padhumailin commented 2 years ago

@xuzhg, When adding a custom serializer in the same project, the provider class (ETagSerializerProvider) and the overridden method GetEdmTypeSerializer is not working. Is there anything i'm missing?

.AddRouteComponents("api/accounts", edmModelAccounts(), builder => builder.AddSingleton<IODataSerializerProvider, ETagSerializerProvider>() .AddSingleton<IODataDeserializerProvider, ETagDeserializerProvider>()) .AddRouteComponents("api/devices", edmModelDevices(), builder => builder.AddSingleton<IODataSerializerProvider, ETagSerializerProvider>() .AddSingleton<IODataDeserializerProvider, ETagDeserializerProvider>())

https://github.com/padhumailin/ODataV8Demo/commit/a9875eaa6bdff16c7eaa6255ac6c596d524cf7d8

padhumailin commented 2 years ago

@xuzhg Custom serializer works good when added with fakemodel route components.

Thanks

padhumailin commented 2 years ago

@xuzhg in the sample project shared:- 404 error is thrown when accessing metadata url (odata context url) http://localhost:34987/api/$metadata#Accounts

please help to resolve this issue

julealgon commented 2 years ago

@padhumailin I know this doesn't solve your problem, but do you mind if I ask why you need to have split EDM models like this per controller? This seems fairly unusual.

If the APIs are so distinct that would require separate models, have you perhaps considered hosting 2 APIs, and having one controller in each?

padhumailin commented 2 years ago

@julealgon We have hosted multiple api's for different modules already... But within same api and different controller (for example : Reports)we have endpoints with different master EDM.

Since we can't change the url because which is already using by the end user.

julealgon commented 2 years ago

Since we can't change the url because which is already using by the end user.

This is just food for thought, but you might still be able to host multiple distinct APIs over the same base address and port using something like YARP @padhumailin .

This allows you to have 2 fully independent middleware pipelines which would make things easier on your side.

Of course, that is assuming you are not already using some sort of advanced API gateway already, like AWS Api Gateway. That would also allow you to expose your multiple APIs however you want.

Anyways, apologies for the derail. I just wanted to understand your scenario a little bit better.

padhumailin commented 2 years ago

@julealgon In my case base address itself vary for each hosted api's. We are hosted in azure as webapps.

Please look at the sample webapps and endpoints hosted.

Webapp 1: HttpGet OData endpoints Https://abc_reports.azurewebsites.net/reportsapi/accounts

Https://abc_reports.azurewebsites.net/reportsapi/devices

Webapp 2: HttpPost (not using OData) Https://abc_accounts.azurewebsites.net/accountsapi/accounts

Webapp 3: HttpPost (not using OData) Https://abc_devices.azurewebsites.net/devicesapi/devices

padhumailin commented 2 years ago

@julealgon @xuzhg

In response the odata context URL shown http://localhost:34987/api/$metadata#Accounts, an empty edm model is shown Not Working Metadata URL: http://localhost:34987/api/$metadata#Accounts

response: `

` But when following URL is requested correct edm is shown in metadata. The path template in context url is not as expected. Working Metadata URL: http://localhost:34987/api/Accounts/$metadata ` ` How can I customize the OData context url in response with correct path template? please advise
julealgon commented 2 years ago

How can I customize the OData context url in response with correct path template?

@padhumailin I'm sorry but I don't quite understand what you are asking there. What is exactly that you want to change? What is the current response and what is the response you want customized?

padhumailin commented 2 years ago

@julealgon

please look at the project https://github.com/padhumailin/ODataV8Demo

the odata context URL in response is not working. means metadata EDM model failed to return

julealgon commented 2 years ago

Ok I think I understand what you are getting at now @padhumailin . I see you have this "fake" EDM in the root of the system, and then you have 2 "actual" EDM models branching out of that. When accessing one of the branches, OData is thinking that the EDM model for that branch is the root one and generating the wrong metadata URL. For instance:

"@odata.context": "http://localhost:34987/api/$metadata#Accounts",

Instead of:

"@odata.context": "http://localhost:34987/api/Accounts/$metadata",

image

Just out of curiosity, did this same setup work fine in v7-? It seems weird to have an EDM in a root folder, and then have separate EDMs in branches of the same folder. I actually don't know if that's supposed to work or not to be honest.

EDIT: I see you already covered this in the OP. My bad, should've checked that as well.

I see that removing the fake OData component causes an exception to be thrown:

System.ArgumentNullException: Value cannot be null. (Parameter 'provider') at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetRequiredService[T](IServiceProvider provider) at Microsoft.AspNetCore.OData.Extensions.HttpRequestExtensions.CreateRequestScope(HttpRequest request, String routePrefix) at Microsoft.AspNetCore.OData.Extensions.HttpRequestExtensions.CreateRouteServices(HttpRequest request, String routePrefix) at Microsoft.AspNetCore.OData.Extensions.HttpRequestExtensions.GetRouteServices(HttpRequest request) at Microsoft.AspNetCore.OData.Query.ODataQueryOptions..ctor(ODataQueryContext context, HttpRequest request) at Microsoft.AspNetCore.OData.Query.EnableQueryAttribute.OnActionExecuting(ActionExecutingContext actionExecutingContext) at Microsoft.AspNetCore.Mvc.Filters.ActionFilterAttribute.OnActionExecutionAsync(ActionExecutingContext context, ActionExecutionDelegate next) at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.gAwaited|10_0(ControllerActionInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted) at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Rethrow(ActionExecutedContextSealed context) at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted) at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.InvokeInnerFilterAsync() --- End of stack trace from previous location --- at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.g__Awaited|20_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted) at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.gLogged|17_1(ResourceInvoker invoker) at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.g__Logged|17_1(ResourceInvoker invoker) at Microsoft.AspNetCore.Routing.EndpointMiddleware.g__AwaitRequestTask|6_0(Endpoint endpoint, Task requestTask, ILogger logger) at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context) at Microsoft.AspNetCore.OData.Routing.ODataRouteDebugMiddleware.Invoke(HttpContext context) at Microsoft.AspNetCore.OData.Routing.ODataRouteDebugMiddleware.Invoke(HttpContext context) at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(HttpContext context)

I'll leave this one to someone else to answer though. Clearly, this exception shouldn't be happening, so I think at the very least that part is a bug. If the configuration you are using is not allowed for some reason, that should be surfaced in a proper error message.