OData SDK v8.0.1 Creating Duplicate Routes

[donile]

When the IServiceCollection.AddOData() extension method is called, conventional and attribute routes are added. When the path template specified on an [HttpMethod] attribute is the same as the path template generated by conventional routing, two routes for the same HTTP method and path template combination are created.

Example

The example below will generate two GET odata/Items routes.

[Route("odata")]
public class ItemsController : ODataController
{
    ...
    [HttpGet("Items")]
    public IActionResult GetItems()
    {
        ...
        return Ok(items);
    }
}

Reproduction Steps

> git clone [email protected]:donile/odata-duplicate-routes-example.git

> cd ./odata-duplicate-routes-example

> dotnet run --project .\src\ODataDuplicateRoutesExample\ODataDuplicateRoutesExample.csproj

HTTP GET https://localhost:5001/swagger/v1/swagger.json with your http client of choice. =)

Upon attempting to retrieve the OpenAPI schema, an HTTP response with status code 500 is returned, including the following error:

Swashbuckle.AspNetCore.SwaggerGen.SwaggerGeneratorException: Conflicting method/path combination "GET odata/Items" for actions - ODataDuplicateRoutesExample.Controllers.ItemsController.GetItems (ODataDuplicateRoutesExample),ODataDuplicateRoutesExample.Controllers.ItemsController.GetItems (ODataDuplicateRoutesExample). Actions require a unique method/path combination for Swagger/OpenAPI 3.0. Use ConflictingActionsResolver as a workaround
   at Swashbuckle.AspNetCore.SwaggerGen.SwaggerGenerator.GenerateOperations(IEnumerable`1 apiDescriptions, SchemaRepository schemaRepository)
   at Swashbuckle.AspNetCore.SwaggerGen.SwaggerGenerator.GeneratePaths(IEnumerable`1 apiDescriptions, SchemaRepository schemaRepository)
   at Swashbuckle.AspNetCore.SwaggerGen.SwaggerGenerator.GetSwagger(String documentName, String host, String basePath)
   at Swashbuckle.AspNetCore.Swagger.SwaggerMiddleware.Invoke(HttpContext httpContext, ISwaggerProvider swaggerProvider)
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(HttpContext context)

Offer to Help

I would like to help resolve the issue and submit a PR, but before I investigate further, I was hoping I could receive some guidance regarding acceptance criteria for this feature. Is this a bug or is the observed behavior expected? If it's not the expected behavior, what is the expected behavior? The way I see it, there are three options.

1. This is the expected behavior.
2. When an attribute route matches the conventional route, ignore the conventional route.
3. When an action method is decorated with an [HttpMethod], ignore the conventional route. 3.1 When a controller is decorated with an [ODataAttributeRouting], do not generate conventional routes for any of the controller's action methods.

[xuzhg]

@donile That's by design.

GetItems() in ItemsController meets:

1. EntitySet Routing Convention:
   a) Controller Name "Items" is an EntitySet b) Action Name "GetItems()" is the valid the action name.
2. Attribute routing convention a) The route template from [Route] and [HttpGet] meets the OData attribute route template

Thanks for your suggestion in No 2 and No 3. I have a similar idea but haven't make a decision.

The reason is that: Why does a customer make an action like this? Besides, we have a lot of ways to suppress one of them:

1. We can remove the template in [HttpGet] to suppress the attribute routing for this action
2. We can use ODataOptions to suppress the whole OData attribute routing
3. We can remove the Attribute routing convention from "Conventions" in ODataOptions
4. We can rename the "GetItems()" action name to suppress the entity set routing conventions.
5. We can rename the Controller name to suppress the entity set routing conventions. etc.

Of course, Thanks for reporting this. I'd like to hear more feedback from community and looking forward to your PR to discuss.

[kcaswick]

Automagically generating a duplicate route is very confusing, especially when refactoring an existing web API codebase to use OData. I ran into both this and #154 due to the confusing conventional routing being applied on top of attribute routing. I would be in favor of 3.1, or at least some easy† way to disable all conventional routing.

† It's not clearly documented, but it is possible to disable all conventional routing across an entire project with the following in Startup.ConfigureServices:

            services.AddControllers()
                .AddOData(opt =>
                {
                    // Turn off all the magic routes based on method names
                    var keepConvention = opt.Conventions.Where(x => x is AttributeRoutingConvention).First();
                    opt.Conventions.Clear();
                    opt.Conventions.Add(new MetadataRoutingConvention());
                    opt.Conventions.Add(keepConvention);
                    // Rest of configuration...
                });

[enriqueraso]

I reproduced the same issue. I upgraded a WebAPI with OData 7.5.8 to 8.0.2. I guess the issue is in AddRouteComponents method. Now you don't need IServiceProvider to create an EdmModel.

Also I noticed OData is not creating the routes when you added a service as scope and it is used inside the controller. This it was working with OData 7.5.8 (I am using ITokenAdquisition)

[KamranMammadov]

Any updates? I also faced with this issue...

[julealgon]

@KamranMammadov check if these are related:

• https://github.com/OData/AspNetCoreOData/issues/428
• https://github.com/OData/AspNetCoreOData/issues/431

Also I noticed OData is not creating the routes when you added a service as scope and it is used inside the controller. This it was working with OData 7.5.8 (I am using ITokenAdquisition)

@enriqueraso as far as I know, OData doesn't care at all what you inject in your controller, so that should in no way be interfering with the routes you are seeing. Can you provide a sample that reproduces the issue you are seeing?

[sliekens]

This is quite annoying. I do not like conventions routing based on controller/method names. I have a strongly held opinion that API routes must be constant strings, never a computed property of the code. If you don't even own the code that does the computation, then you don't own the public API of your own code.

I always use a route template with constant strings in non-OData APIs.

[HttpGet("always-do-this/no-exceptions/ever")

I wanted to do the same in my OData controller, but it's breaking SwaggerGen with the same exception as the issue author. It also took me a long time to figure out why exactly, the conflicting behavior with conventions routing is not documented on Microsoft Learn.

I forgot what's the opposite of the pit of success, but this is it.

I'm experimenting with disabling the conventions:

services.AddControllers()
    .AddOData((odata, sp) =>
    {
        odata.EnableQueryFeatures(maxTopValue: 1000)
            .AddRouteComponents(
                routePrefix: "odata",
                model: GetEdmModel()
            );

        // Turn off routing conventions that generate routes from controller actions
        odata.Conventions.Clear();

        // Only keep the cool ones: /odata/$metadata and routes from attributes
        odata.Conventions.Add(ActivatorUtilities.CreateInstance<MetadataRoutingConvention>(sp));
        odata.Conventions.Add(ActivatorUtilities.CreateInstance<AttributeRoutingConvention>(sp));
    });

I support OP's suggestion to ignore conventional routes when a path conflict exists in routes from attributes.

[nmase88]

Is there any plans to resolve this problem? It's been outstanding now for nearly 2 years

[julealgon]

Hot take: conventional routing should be obsoleted and removed in future versions of OData. In the meantime, an option should be offered to easily disable it.

Please consider it @xuzhg / @mikepizzo / @habbes , especially now that .NET is moving fast towards minimal APIs as the new standard.

[sliekens]

I'm okay with having to mess with the conventions collections to disable the entityset routing. But it's really inconvenient that entityset routing is not overwritten when you use an attribute. Like trying to grab the wheel to steer away but the autopilot keeps fighting back.