OData - Same Named Function in Two Configurations Causes System.InvalidOperationException on Startup

[benhysell]

tldr - In the OData example project I added a function with the same name to two different configurations. The examples can no longer run. I used to be able to add functions with the same name to two different OData endpoints without issue.

Example Repo - https://github.com/benhysell/aspnet-api-versioning
Example Project - https://github.com/benhysell/aspnet-api-versioning/tree/master/samples/aspnetcore/SwaggerODataSample

Using the examples I've duplicated the MostExpensive function from Order to Person via the steps below to trigger the exception.

Steps to Reproduce

• Take the Example project SwaggerODataSample
• Modify the configuration of PersonModelConfiguration.cs to add function
  person.Collection.Function( "MostExpensive" ).ReturnsFromEntitySet<Person>( "People" );
  https://github.com/benhysell/aspnet-api-versioning/blob/master/samples/aspnetcore/SwaggerODataSample/Configuration/PersonModelConfiguration.cs#L34
• Note this same function exists in the OrderModelConfiguration.cs -- https://github.com/benhysell/aspnet-api-versioning/blob/c29e95940cbaa66784b38f864ed51c60a9971d55/samples/aspnetcore/SwaggerODataSample/Configuration/OrderModelConfiguration.cs#L31
• Run the application
• Exception is thrown ODataRouteBuilderContext.cs ln 252

Is it allowable to have the same named function in two different models? And if not why not?

[commonsensesoftware]

It should be allowable. Resolving an operation by its qualified name was previously an issue. I could have sworn I had a test for that. I'll have to spelunk some more. This seems to be the issue. I distinctly recall using SingleOrDefault to make sure this exact failure would happen in the event the name is ambiguous (as FirstOrDefault would likely be harder to understand or track down).

I did notice you copied the MostExpensive method verbatim from OrdersController. The OrdersController intentionally uses attribute-routing and PeopleController intentionally uses convention-based routing to demonstrate the different styles. I doubt that's the cause this issue, but you'll want to be careful with mixing and matching as it's bound to cause issues.

Since you already have a repro setup with the source, you might break inside ODataRouteBuilderContext.ResolveOperation. You might find why the function is not being resolved.

[benhysell]

@commonsensesoftware My sloppy cut and paste job on the examples was the fastest way to show the error. I have a much larger application where I do not mix and match different methods/schemas and I tripped over this issue.

In the attached screenshot:

In EdmModel.SchemaElements the element Name:"MostExpensive" shows up twice. Not sure how it gets to this point...

[commonsensesoftware]

Thanks.

I kind of gathered that from since you indicated Line 252. ;) I'm wondering why it wasn't resolved by FindBoundOperations. It would seem, there is a resolution case for collections missing against the entity set itself. That might explain how things fall through to this case and work with a single operation, but not with multiple. I was suspecting this might be the issue, but then I couldn't figure out how it worked at all. This behavior would explain how a uniquely named operation would yield a false positive.

I don't have IntelliSense handy. Is there a way to find or resolve an operation by qualifiedName + EntitySet? You can probably use Quick Watch to poke around in the EDM to figure what element the function is associated with. I expect it to be the entity set itself. I would then expect an extension method overload for FindBoundOperations, FindDeclaredOperations, or some similarly named method that will accept an interface exposed by an entity set.

[vbray1979]

This is replicable on action as well. The following trigger the error message "Sequence contains more than one element" on "CreateWebHostBuilder(args).Build().Run();"

builder.EntityType<CsvTransactionModel>().Collection.Action("Upload");
builder.EntityType<CsvPriceModel>().Collection.Action("Upload");

This is working with Microsoft.AspNetCore.OData.Versioning v4.1.1

[benhysell]

Are all of the unit tests passing for everyone else? I was about to dig into this but kicked up three failed unit tests.

[benhysell]

I have a failing unit test in the example repo...I replicated my 'sloppy' cut and paste of MostExpensive to get odata_api_explorer_should_group_and_order_descriptions_on_providers_executed to fail. Latest code is pushed to the example repo.

Walking through the code, @commonsensesoftware, you are right, ln 244 var operation = EdmModel.FindBoundOperations( qualifiedName, entities[i] ).SingleOrDefault(); always falls through, throwing the exception on ln 252. Forgive my earlier post, still feeling my way around the project.

While fishing around the EdmModel I kick up two entries for MostExpensive, both with the same FullName field but differ by ReturnType. See attached screenshot.

I'm not able to step into EdmModel.FindBoundOperations, so I'm not sure what it is trying to do / how it is trying to resolve what it is looking for. The qaualifiedName is Default.MostExpensive

Any suggestions on how to further track this down?

[hasandogu]

We have updated OData versioning support to 5.0.0 and we seem to have an issue related with this. We have functions of the same name in a few of our entities and none of the paths were recognized (except one) for each function. All the other implementations using the same function name are ignored in Swagger and they return 404 when called. For instance, we have the following model and we can only call accounts/categories after upgrading. The others seem to be ignored

            builder.EntityType<AccountDto>().Collection
                .Function("Categories")
                .ReturnsCollection<AccountCategoryGroupingDto>();

            builder.EntityType<ProductDto>().Collection.Function("Categories")
                .ReturnsCollection<ProductCategoryGroupingDto>();

            builder.EntityType<SubscriptionDto>().Collection.Function("Categories")
                .ReturnsCollection<SubscriptionCategoryGroupingDto>();

They are however listed in metadata as bound functions. For instance:

            <Function Name="Categories" IsBound="true">
                <Parameter Name="bindingParameter" Type="Collection(Api.v1.Models.Dto.ProductDto)" />
                <ReturnType Type="Collection(Api.v1.Models.Dto.ProductCategoryGroupingDto)" />
            </Function>

Do you have any insight as to why this might be the case?

[cyril-iselin]

we are running into the same issue.

Also when I create a method like this:

[HttpGet] [ODataRoute( "GetAbc(Id={id},IncludeSubHistories={includeSubHistories})" )] [Produces( "application/json" )] [ProducesResponseType( typeof( Order ), Status200OK )] [ProducesResponseType( Status404NotFound )] [EnableQuery( AllowedQueryOptions = Select )] public Order GetAbc( [FromODataUri] int id, [FromODataUri] bool includeSubHistories ) { return new Order(); }

var config = builder.EntityType<Order>().Collection.Function( "GetAbc" ).Returns<Order>(); config.Parameter<int>( "Id" ); config.Parameter<bool>( "IncludeSubHistories" );

-> Swagger does not replace the parameters, it will get appended as querystring.
-> When I call the method manually, then I recive an odataexception :-(

[nokternol]

Subscribed to this issue as I am currently migrating an API from .net framework to .net5 and have hit this problem.
In our situation it is bound collection actions with the same name that is causing the issue.
Would appreciate any insight into a workaround why this is being resolved.

[engenb]

I've run into this issue as well.

This appears to be caused by a bug way down in the depths of Microsoft.OData.Edm, right here.

For some context, I created a quick sample to isolate this issue with two edm entity types Foo and Bar, each having a function Bulk

The line of code linked above is ultimately called by ODataRouteBuilderContext, from here

When we hit line 2528 of HasEquivalentBindingType, parameterType.TypeKind is EdmTypeKind.Collection while bindingType.TypeKind is EdmTypeKind.Entity. This results in this method immediately returning false on line 2530.

I do not know, but it looks like the special collection handling on line 2533 might need to be run first such as

if (parameterType.TypeKind == EdmTypeKind.Collection)
{
    // covariance applies here, so IEnumerable<A> is applicable to IEnumerable<B> where B:A
    IEdmCollectionType parameterCollectionType = (IEdmCollectionType)parameterType;
    IEdmCollectionType bindingCollectionType = (IEdmCollectionType)bindingType;

    return bindingCollectionType.ElementType.Definition.IsOrInheritsFrom(parameterCollectionType.ElementType.Definition);
}
else if (parameterType.TypeKind != bindingType.TypeKind)
{
    return false;
}
else
{
    return bindingType.IsOrInheritsFrom(parameterType);
}

or something along those lines. I should point out that the above code doesn't actually work because parameterType.TypeKind is Collection and bindingType does not implement IEdmCollectionType

@commonsensesoftware I don't know if it'd be more productive for you to communicate this issue with whoever maintains Microsoft.OData.Edm or if I should write up an issue against that repo.

Sample project can be found here

[engenb]

did some further testing. the following resolves the issue. I will write up a PR against Microsoft.OData.Edm.

if (parameterType.TypeKind == EdmTypeKind.Collection && bindingType.TypeKind == EdmTypeKind.Entity)
{
    var parameterCollectionType = (IEdmCollectionType)parameterType;
    return bindingType.IsOrInheritsFrom(parameterCollectionType.ElementType.Definition);
}

if (parameterType.TypeKind != bindingType.TypeKind)
{
    return false;
}

if (parameterType.TypeKind == EdmTypeKind.Collection)
{
    // covariance applies here, so IEnumerable<A> is applicable to IEnumerable<B> where B:A
    IEdmCollectionType parameterCollectionType = (IEdmCollectionType)parameterType;
    IEdmCollectionType bindingCollectionType = (IEdmCollectionType)bindingType;

    return bindingCollectionType.ElementType.Definition.IsOrInheritsFrom(parameterCollectionType.ElementType.Definition);
}
else
{
    return bindingType.IsOrInheritsFrom(parameterType);
}

@commonsensesoftware if you've got any pull with to grease the wheels to get this resolved, that'd be excellent. Hoping to not have to design around this bug.

[engenb]

even further testing! (sorry to spam)

The bug may actually be in ODataRouteBuilderContext.

Functions may be bound to either a type or a collection as such:

foo.Collection.Function("Bulk")
    .ReturnsFromEntitySet<Foo>("Foos");

foo.Function("Add")
    .ReturnsFromEntitySet<Foo>("Foos");

ODataRouteBuilderContext is always assuming the function is bound to an entity type, and not to an entity type collection, right here - the function is actually bound to Collection<Foo>, not Foo itself. When ODataRouteBuilderContext later performs a lookup for the function by name and bound type Foo, the lookup fails because the function is actually bound to Collection<Foo>.

if ( EntitySet != null )
{
    var entity = EntitySet.Type;

    if ( entities.Count == 0 || !entities[0].Equals( entity ) )
    {
        entities.Add( entity );
    }
}

the above change resolves the issue, but then creates a new issue for same-named functions bound directly to the entity type.

One workaround could have this add both EntitySet.Type and EntitySet.EntityType() to entities to handle if the function is bound to an entity or a collection. However, I can see this also causing an issue if you have two functions with the same name, one bound to the entity and one bound to the collection.

// I would expect this be bound to /Foos/Audit
foo.Collection.Function("Audit")
    .ReturnsFromEntitySet<Foo>("Foos");

// not sure about routing conventions for functions on entity types - /Foos({id})/Audit?
foo.Function("Audit")
    .ReturnsFromEntitySet<Foo>("Foos");

In this scenario, the ODataRouteBuilderContext will not know which controller action to bind to which edm function. I don't know if there is any convention or context that could allow ODataRouteBuilderContext to determine that an action should be mapped to a function that is entity-bound vs collection-bound.

[cyril-iselin]

engenb very interessting 👍 . This issue also prevent us from upgrading. What solution you use, if any ? :-)

[strandman]

I got the same exception in "Microsoft.AspNet.OData.Routing.ODataRouteBuilderContext.ResolveOperation" when trying to add a function overload:

builder.EntityType<Foo>().Collection.Function("Bar")
.ReturnsCollectionFromEntitySet<Foo>("Foos");

builder.EntityType<Foo>().Collection.Function("Bar")
.ReturnsCollectionFromEntitySet<Foo>("Foos")
.Parameter<T>("param");

Edit:
Never mind, I solved my issue by using optional parameter:

builder.EntityType<Foo>().Collection.Function("Bar")
.ReturnsCollectionFromEntitySet<Foo>("Foos")
.Parameter<T>("param").Optional();

[adrien-constant]

I also have the issue with version 5.0.0. The following configuration code raises the exception :

builder.EntityType<EntityA>().Collection.Action("BatchCreate") 
  .Returns<BatchCreateResultA>().CollectionParameter<EntityA>("entities");
builder.EntityType<EntityB>().Collection.Action("BatchCreate")
   .Returns<BatchCreateResultB>().CollectionParameter<EntityB>("entities");

I didn't find a way to bypass it, even by adding an optional parameter. The only way is to use two different method names BatchCreateA and BatchCreateB instead of BatchCreate for both.

Swagger doesn't like it either but it can be fixed with a custom configuration code :

            services.AddSwaggerGen(options =>
            {
                options.CustomSchemaIds(type => type.ToString());
            });

Do you know when the fix is going to be realeased ? Thanks.

[CSharpFiasco]

@engenb were you able to open a pull request for this issue?

[nokternol]

@engenb It feels (to me at least) that the newly introduced issue of same name method bound to both the collection and Foo is far more of an edge case for odata than the name named action across many entity collection.

Sorry, I'm slightly worried I've misunderstood now. Do you mean functions bound with the same name to multiple entities or functions with the same name on the same entity?
The latter being far smaller than the original issue but the former I would agree is just as bad.

[commonsensesoftware]

The issue 🔥 down has begun! 😉. Apologies for the long overdue follow-up.

I believe I've been able to solve the problem. Thank you all for a lot of input, configurations, and variations for a repro. I've tried several combinations. Surprisingly, I couldn't reproduce the original problem. This could be due to any of the following:

• Unmerged WIP fixes
• Updating the OData MVC package
• Updating the OData EDM package

It did, however, find that although the exceptions went away, new, subtle issues crept in.

Here's the steps I used in an attempt to repro the problem:

1. Add function with duplicate name for an entity set and type
2. Replicate that across two different entities in the same API version

The Configuration

if ( apiVersion == ApiVersions.V1 )
{
    order.Function( "MostExpensive" ).ReturnsFromEntitySet<Order>( "Orders" );
}

if ( apiVersion >= ApiVersions.V1 )
{
    order.Collection.Function( "MostExpensive" ).ReturnsFromEntitySet<Order>( "Orders" );
}

Configuration/OrderModelConfiguration.cs

if ( apiVersion == ApiVersions.V1 )
{
    person.Function( "MostExpensive" ).ReturnsFromEntitySet<Person>( "People" );
    person.Collection.Function( "MostExpensive" ).ReturnsFromEntitySet<Person>( "People" );
}

Configuration/PersonModelConfiguration.cs

The Code

/// <summary>
/// Gets the most expensive order.
/// </summary>
/// <returns>The most expensive order.</returns>
/// <response code="200">The order was successfully retrieved.</response>
/// <response code="404">The no orders exist.</response>
[ODataRoute( "MostExpensive" )]
[MapToApiVersion( "1.0" )]
[Produces( "application/json" )]
[ProducesResponseType( typeof( Order ), Status200OK )]
[ProducesResponseType( Status404NotFound )]
[EnableQuery( AllowedQueryOptions = Select )]
public SingleResult<Order> MostExpensive() =>
    SingleResult.Create( new[] { new Order() { Id = 42, Customer = "Bill Mei" } }
                .AsQueryable() );

/// <summary>
/// Gets the most expensive order.
/// </summary>
/// <param name="key">The order identifier.</param>
/// <returns>The most expensive order.</returns>
/// <response code="200">The order was successfully retrieved.</response>
/// <response code="404">The no orders exist.</response>
[ODataRoute( "{key}/MostExpensive" )]
[MapToApiVersion( "1.0" )]
[Produces( "application/json" )]
[ProducesResponseType( typeof( Order ), Status200OK )]
[ProducesResponseType( Status404NotFound )]
[EnableQuery( AllowedQueryOptions = Select )]
public SingleResult<Order> MostExpensive( int key ) =>
    SingleResult.Create( new[] { new Order() { Id = key, Customer = "Bill Mei" } }
                .AsQueryable() );

Controllers/V1/OrdersController.cs Using Attribute-Based Routing

/// <summary>
/// Gets the most expensive person.
/// </summary>
/// <returns>The most expensive person.</returns>
/// <response code="200">The person was successfully retrieved.</response>
/// <response code="404">No people exist.</response>
[MapToApiVersion( "1.0" )]
[Produces( "application/json" )]
[ProducesResponseType( typeof( Order ), Status200OK )]
[ProducesResponseType( Status404NotFound )]
[EnableQuery( AllowedQueryOptions = Select )]
public SingleResult<Person> MostExpensive() =>
    SingleResult.Create( new[] { new Person() { Id = 42, FirstName = "Elon", LastName = "Musk" } }
                .AsQueryable() );

/// <summary>
/// Gets the most expensive person.
/// </summary>
/// <returns>The most expensive person.</returns>
/// <response code="200">The person was successfully retrieved.</response>
/// <response code="404">The person does not exist.</response>
[MapToApiVersion( "1.0" )]
[Produces( "application/json" )]
[ProducesResponseType( typeof( Order ), Status200OK )]
[ProducesResponseType( Status404NotFound )]
[EnableQuery( AllowedQueryOptions = Select )]
public SingleResult<Person> MostExpensive( int key ) =>
    SingleResult.Create( new[] { new Person() { Id = key, FirstName = "John", LastName = "Doe" } }
                .AsQueryable() );

Controllers/V1/PeopleController.cs Using Convention-Based Routing

Open API / Swagger Definitions

This abridged list is for V1:

Orders

• GET /api/Orders/{key}/MostExpensive
• GET /api/Orders/MostExpensive

People

• GET /api/People/{key}/MostExpensive
• GET /api/People/MostExpensive

Requests

This worked in the UI and the HTTP REPL. Here is the REPL console output for simplicity.

http://localhost:5000/api> get orders/mostexpensive?api-version=1.0

HTTP/1.1 200 OK
api-deprecated-versions: 0.9
api-supported-versions: 1.0, 2.0, 3.0
Content-Type: application/json; odata.metadata=minimal; odata.streaming=true
Date: Thu, 24 Jun 2021 20:35:40 GMT
OData-Version: 4.0
Server: Kestrel
Transfer-Encoding: chunked

{
  "@odata.context": "http://localhost:5000/api/$metadata#Orders/$entity",
  "id": 42,
  "createdDate": 6/24/2021 1:35:41 PM,
  "customer": "Bill Mei"
}

http://localhost:5000/api> get orders/1/mostexpensive?api-version=1.0

HTTP/1.1 200 OK
api-deprecated-versions: 0.9
api-supported-versions: 1.0, 2.0, 3.0
Content-Type: application/json; odata.metadata=minimal; odata.streaming=true
Date: Thu, 24 Jun 2021 20:37:54 GMT
OData-Version: 4.0
Server: Kestrel
Transfer-Encoding: chunked

{
  "@odata.context": "http://localhost:5000/api/$metadata#Orders/$entity",
  "id": 1,
  "createdDate": 6/24/2021 1:37:55 PM,
  "customer": "Bill Mei"
}

http://localhost:5000/api> get people/mostexpensive?api-version=1.0

HTTP/1.1 200 OK
api-deprecated-versions: 0.9
api-supported-versions: 1.0, 2.0, 3.0
Content-Type: application/json; odata.metadata=minimal; odata.streaming=true
Date: Thu, 24 Jun 2021 20:39:42 GMT
OData-Version: 4.0
Server: Kestrel
Transfer-Encoding: chunked

{
  "@odata.context": "http://localhost:5000/api/$metadata#People/$entity",
  "id": 42,
  "firstName": "Elon",
  "lastName": "Musk"
}

http://localhost:5000/api> get people/1/mostexpensive?api-version=1.0

HTTP/1.1 200 OK
api-deprecated-versions: 0.9
api-supported-versions: 1.0, 2.0, 3.0
Content-Type: application/json; odata.metadata=minimal; odata.streaming=true
Date: Thu, 24 Jun 2021 20:39:57 GMT
OData-Version: 4.0
Server: Kestrel
Transfer-Encoding: chunked

{
  "@odata.context": "http://localhost:5000/api/$metadata#People/$entity",
  "id": 1,
  "firstName": "John",
  "lastName": "Doe"
}

Conclusion

Understanding the OData vernacular is a bit challenging. As I recall, IEdmImportOperation maps to an arbitrary OData function or action. This was previously called a Service Action. Interestingly, this seems to mean that FindDeclaredOperations is worthless because it will find any bound IEdmOperation by name, which can easily be ambiguous.

The only other possibilities are that an IEdmOperation is bound to a collection (e.g. entity set) or an entity. This can take one of the following forms:

• IEdmEntitySet.Type - The entity set collection
• IEdmEntitySet.EntityType() (extension) - The entity type in the collection
• IEdmSingleton - A singleton entity

As I understand it, you can can't have an operation that only appears on a singleton because it's just a special mapping to a specific entity. This list is, therefore, the order in which FindBoundOperations should be evaluated. There's no longer an need or situtation to fallback to using FindDeclaredOperations.

Things then get tricky. It's possible that a collection and entity both have the same operation name and the same number of parameters. In fact, I noticed that if these things line up, it will appear to have matched correctly (by happenstance). The only way to make sure the correct operation is selected is by matching up the parameters, which isn't so easy. A bound entity function must have a key parameter for the entity. If all other parameters match, but there are still some action parameters left, it is assumed that the operation is bound to the entity not the collection. As far as I can tell, this does result in the correct action-to-operation match every time.

This fix will be released in the next patch. Can't wait? The fixes are in this branch if you want to play around with them.

[chu-tianshu]

@commonsensesoftware thanks for investigating and fixing the issue. I'm hitting this too, and was wondering what version of he nuget package will have the fix, and when will it likely be available?