OData / WebApi

OData Web API: A server library built upon ODataLib and WebApi
https://docs.microsoft.com/odata
Other
855 stars 473 forks source link

Routing to controllers with the same name #1494

Open andrew-laughlin opened 6 years ago

andrew-laughlin commented 6 years ago

I have 2 controllers with the same name, but in different namespaces. Both have GET actions with differing parameter lists. I'm using convention-based routing.

// v1
namespace v1
{
    public class SensorsController : ODataController 
    {
        [HttpGet]
        public IActionResult Get(ODataQueryOptions<v1.Sensor> query, ODataQuerySettings querySettings) {}
     }
}

// v2
namespace v2
{
    public class SensorsController : ODataController 
    {
        [HttpGet]
        public IActionResult Get(ODataQueryOptions<v2.Sensor> query, ODataQuerySettings querySettings, [FromODataUri] Guid tid){}
     }
}

// Startup.cs
app.UseMvc(routeBuilder =>
{
    IEdmModel model = BuildEdmModelV1(app.ApplicationServices); // v1 models
    routeBuilder.MapODataServiceRoute("ODataRouteV1", "odata", model);

    IEdmModel model = BuildEdmModelV2(app.ApplicationServices); // v2 models
    routeBuilder.MapODataServiceRoute("ODataRouteV2", "v2/odata/tenants/{tid}", model);
});

This is successful: http://localhost:3000/v2/odata/tenants/myTidGuid/Sensors

This results in the exception below: http://localhost:3000/odata/Sensors

[11:21:06 DBG] Request successfully matched the route with name 'ODataRouteV1' and template 'odata/{*odataPath}'. [11:21:06 DBG] Executing action v2.SensorsController.Get [11:21:07 INF] Executed action v2.SensorsController.Get in 337.9326ms [11:21:07 ERR] Connection id "0HLEMR6DSEVVE", Request id "0HLEMR6DSEVVE:00000003": An unhandled exception was thrown by the application. System.ArgumentException: The given model does not contain the type 'v1.Sensor'. Parameter name: elementClrType at Microsoft.AspNet.OData.ODataQueryContext..ctor(IEdmModel model, Type elementClrType, ODataPath path) in C:\Repos\WebApi\src\Microsoft.AspNet.OData.Shared\ODataQueryContext.cs:line 55 at Microsoft.AspNet.OData.ODataQueryParameterBindingAttribute.ODataQueryParameterBinding.BindModelAsync(ModelBindingContext bindingContext) in C:\Repos\WebApi\src\Microsoft.AspNetCore.OData\ODataQueryParameterBindingAttribute.cs:line 89 at Microsoft.AspNetCore.Mvc.ModelBinding.Binders.BinderTypeModelBinder.d__2.MoveNext()

As shown above, the v2 controller is selected for the v1 route. In the code below (from ODataActionSelector.cs) considerCandidatescontains GET actions from both v1 and v2 controllers. However it's not able to disambiguate and just returns FirstOrDefault(), which happens to always be the v2 controller. So the v2 route works, but v1 doesn't.

var matchedCandidates = considerCandidates
                    .Where(c => !c.FilteredParameters.Any() || c.FilteredParameters.All(p => availableKeys.Contains(p.Name.ToLowerInvariant())))
                    .OrderByDescending(c => c.FilteredParameters.Count)
                    .ThenByDescending(c => c.TotalParameterCount);

Assemblies affected

Microsoft.AspNetCore.OData 7.0.0.B4

Reproduce steps

The simplest set of steps to reproduce the issue. If possible, reference a commit that demonstrates the issue.

Expected result

The correct controller should be selected based on the registered route.

Actual result

The correct controller is not selected.

Additional detail

I've tried various forms of attribute routing without success. It does work with MVC routing, however the entities are not returned in an OData JSON envelope, which will break clients.

// v1
[HttpGet("odata/sensors")] // MVC route. This works
public IActionResult Get(ODataQueryOptions<Sensor> query, ODataQuerySettings querySettings)

// Startup.cs
routeBuilder.MapODataServiceRoute("ODataRouteV1", "odata", model);
routeBuilder.EnableDependencyInjection();

Any idea what I'm doing wrong?

xuzhg commented 6 years ago

@andrew-laughlin I am afraid that the Web API OData routing doesn't support your scenario now, because just you found, it's using the controller name to match.

For the attribute routing, you can add as follows, but at the end, it's return to compare the controller name.

[ODataRoute("Sensors", RouteName = "ODataRouteV1")]
public IActionResult Get()

[ODataRoute("Sensors", RouteName = "ODataRouteV2")]
public IActionResult Get()

I mark it as feature and it's better to find a solution in the next release.

andrew-laughlin commented 6 years ago

@xuzhg thanks very much for the info. I'll look for an alternate solution. Should this be closed?

xuzhg commented 6 years ago

@andrew-laughlin I'd love to keep it open because I think it's a nice to have feature in the next version.

andrew-laughlin commented 6 years ago

In case anyone is interested, to work around this issue in the interim, we've implement a custom routing solution based on info here --> http://odata.github.io/WebApi/#03-04-custom-routing-convention

Essentially the code implements IODataRoutingConvention and inserts itself as the first routing convention. It then processes all existing conventions and, if necessary disambiguates the results (read: list of actions) returned by any existing routing convention handler. Our disambiguation is based on version, but any criteria could be used.

albertoVieiraNeto commented 6 years ago

Is there a plan to implement this? currently i have my api separated in areas of concern, i was able to separate almost everything but the same happens to me, the router selects any of the controllers disregarding namespace and pretty much everything.

Angelinsky7 commented 4 years ago

@andrew-laughlin your link is not working anymore. Where can i find the documenation about the workaround now ? Thanks

icnocop commented 4 years ago

@Angelinsky7 try https://docs.microsoft.com/en-us/odata/webapi/custom-routing-convention

Angelinsky7 commented 4 years ago

@icnocop thanks a lot !