Aspnet-api-versioning: Two-parameter GET in OData controller with multi-segment key item throws after update Microsoft.AspNetCore.OData up to 7.2.1

[brainboost]

Hello, this issue detected after taking current master on dotnet 2.2, updating SwaggerODataSample code with last stable Microsoft.AspNetCore.OData 7.2.1 nuget.

Exception

Microsoft.OData.ODataException: 'Bad Request - Error in query syntax.'
in Microsoft.AspNet.OData.Routing.VersionedODataPathRouteConstraint.
Stacktrace:
at Microsoft.OData.UriParser.ODataUriResolver.ResolveKeys(IEdmEntityType type, IDictionary2 namedValues, Func3 convertFunc)

To reproduce

Model with two fields

    public class Doublekey
    {
        public int Id { get; set; }
        public string Customer { get; set; }
    }

Controller:

    [ApiVersion( "1.0" )]
    [ODataRoutePrefix( "Doublekey" )]
    public class DoublekeyController : ODataController
    {
        [ODataRoute( "({key},{cust})" )]
        [Produces( "application/json" )]
        [ProducesResponseType( typeof( Doublekey ), StatusCodes.Status200OK )]
        [EnableQuery( AllowedQueryOptions = AllowedQueryOptions.Select )]
        public SingleResult<Doublekey> Get( int key, string cust )
            => SingleResult.Create( new[] {new Doublekey {Id = key, Customer = cust}}
               .AsQueryable() );
    }

Configuration:

 var doublekey = builder.EntitySet<Doublekey>( "Doublekey" ).EntityType
 .HasKey( o => new {o.Id, o.Customer} );

Request: http://localhost:59918/api/doublekey(key=1,cust='test')?api-version=1.0
Note, the plain non-versioned OData controller with Microsoft.AspNetCore.OData 7.2.1 with same params works well. Any ideas, how to use it with 7.2.1?

[commonsensesoftware]

Which version were you upgrading from? Have you tried applying [FromODataUri] to the action parameters. Ideally that _shouldn't_ matter, but I've been snagged by that many, many times. I usually add it now, even if it's not needed, just to be sure.

It's also worth noting that the VersionedODataPathRouteConstraint doesn't do anything with ODataUriResolver. The source of the exception appears to be down in the ODataPathRouteConstraint base class.

I can't say why this would suddenly stop working, but it wouldn't be the first time I've seen a behavioral change between package versions.

[brainboost]

Well, yes, I tried to put [FromODataUri], it did not change anything. It was working okay with OData 7.1.0, that was default version for versioning library. I just tried to build with 7.2.0 and got the same exception, so the breaking change was 7.1.0->7.2.0. The exception was in ODataPathRouteConstraint.Match( httpContext, route, routeKey, values, routeDirection ), route param is "{api/{*odataPath}}" in this call.

[commonsensesoftware]

That's odd. The path should be api/{*odataPath}. It would seem there are extra braces on the outside.

[brainboost]

Yes, it is api/{*odataPath}, I copied it from debugger viewer as object, not text, text was without braces. And routeKey value was "ODataConstraint"

[commonsensesoftware]

You said you're working from master. There's a chance that the recent change to key delimiters is causing this. The default delimiter in the OData library is now slash instead of parentheses. It has been that way for a while, but API Versioning wasn't aligned. The default, configured value for ODataUrlKeyDelimiter is null. API Versioning has no convenient way to maintain parity with the OData defaults other than to assume the same defaults.

OData itself doesn't perform any route template construction, but API Versioning does. This is used both for routing and API exploration. It's possible this behavior is causing an issue.

Try adding the following to your configuration before routeBuilder.MapVersionedODataRoutes:

c# routeBuilder.ServiceProvider .GetRequiredService<ODataOptions>() .UrlKeyDelimiter = ODataUrlKeyDelimiter.Parentheses;

The most recent versions of the sample projects are also setup this way. Even if this isn't the cause, it's a good idea to configure to ensure that the correct key delimiters are used.

NOTE: do not use:
routeBuilder.SetDefaultODataOptions( new ODataOptions() { UrlKeyDelimiter = ODataUrlKeyDelimiter.Parentheses } );
it has a bug and actually doesn't change any options at all.

[brainboost]

It doesn't work neither with Parenthesis nor Slash, it has the same 404 - NotFound or exception in debugger.

[SiberaIndustries]

It is a long time ago, but we had the same issue. We read the official abnf construction doc and found this part: compoundKey = OPEN keyValuePair *( COMMA keyValuePair ) CLOSE

It means you should use comma separated KeyValuePairs, in your case e.g.:

GET Doublekey(Id=0,Customer=bob)

So have u tried [ODataRoute("(Id={key},Customer={cust})")]?

I guess we were able to transform the KeyValue pattern into a value only pattern, but we had to map the property names correctly, in your case:

[ODataRoute( "{keyId},{keyCustomer}" )]`
public SingleResult<Doublekey> Get([FromODataUri] int keyId, [FromODataUri] string keyCustomer)

I am not sure if there is still the possibility to run into a separate issue because of the new default ODataUrlKeyDelimiter behaviour as @commonsensesoftware mentioned, so you should first try this out with the old behaviour.

[commonsensesoftware]

Good call @Discjoggy! I totally missed that. You definitely have to express it as a key/value pair. I seem to also remember that the case of the keys have to match what the EDM says. I don't recall if case-insensitivity is allowed.

You do, however, get a little more flexibility with the parameter names when you use OData's attribute routing. With pure convention-based routing, the names must have the key prefix using the form shown. When you use attribute routing, the parameter names just have to make the route template tokens. Yeah _consistency_!

I would expect this to work:

c# [ODataRoute( "(Id={key},Customer={cust})" )] public SingleResult<Doublekey> Get([FromODataUri] int key, [FromODataUri] string cust) { }

with the request:

http://localhost:59918/api/doublekey(Id=1,Customer='test')?api-version=1.0

[SiberaIndustries]

I would expect this to work

Check: I locally tried the well reproducable example of @brainboost, and I was able to trigger the action method with this uri /api/doublekey(Id=1,Customer='test')?api-version=1.0. Would you agree @brainboost?

But I have no idea of how this should look like without Parentheses:

• /api/doublekey/Id=1,Customer='test'
• /api/doublekey/Id=1/Customer='test'

Both is not working (even if I change the ODataRoute tempate to the specific equivalent).

[commonsensesoftware]

@Discjoggy I've had similar issues with using the slash method. I've found zero documentation or examples that indicate whether that's even supported with the slash method. There's a chance that it's not. If you ever find it, please share. I've been guessing on the conventions for the API Explorer.

[brainboost]

Yes, I agree, the approach with parameter names works. Thanks a lot, @commonsensesoftware and @Discjoggy! It works with any actual param names, whether they start with 'key' or not. And only with parenthesis notation, with an equal sign.
Regarding other request formats, it would be much appreciated even in forms like /api/doublekey/Id(1)/Customer('test') or even /api/Id(1)/Customer('test')/doublekey, but I think it won't work even with custom routing convention now.

[commonsensesoftware]

The OData libraries support non-standard OData protocol features. I believe using a slash instead of parentheses is one of them. I skimmed through the specs and ABNF again too and I see no mention of being allowed to use a slash. I know part of the goal of the OData protocol was to be able to have standardized URL conventions. This is why the went with parentheses in the first place. Without all of the keys being in the same segment, it would very difficult, if not impossible, to figure out where the keys are. Consider a scenario where entity set has a composite key and it has a child navigation property with composite keys too >_<. I recently regained ties with the OData team (they _recycled_ numerous times). I'll reach out and see if I can get some concrete answers.

@brainboost are you satisfied with this solution are you are you looking for something more? This ultimately seems to be more of an issue with OData libraries and protocol than it does API versioning. Thanks.

[brainboost]

Thank you, @commonsensesoftware, for awesome answers! I'm going to close this issue, there's no more questions for this particular versioning library.

[commonsensesoftware]

FYI, I got confirmation from the OData team that while 4.01 of the protocol does support composite keys using the _Key as Segment_ method, it is not supported in the OData implementation libraries yet. Using parentheses is currently the only option to make it work.