OData / OData/AspNetCoreOData

ODataRoutingMatcherPolicy Does Not Consider All Routing Metadata

Open
#753 3 comments 2 reactions 1 assignee View on GitHub

@xuzhg is already working on this.

Since Dec 6, 2022.

feature
Dominant language
C#
Stars
505
Forks
186
PR merge metrics
No merged PRs in 30d

Description

Assemblies affected
ASP.NET Core OData 8.x

Describe the bug
Various paths through ODataRoutingApplicationModelProvider can add multiple IODataRoutingMetadata to an Endpoint, but ODataRoutingMatcherPolicy will only ever consider the first one:

https://github.com/OData/AspNetCoreOData/blob/61ae3232d4ad687bc552fc583b32103f1aeff41a/src/Microsoft.AspNetCore.OData/Routing/ODataRoutingMatcherPolicy.cs#L86

This can result in some endpoints to not match when they should. There doesn't appear to be any documentation that indicates this is the expected behavior.

Reproduce steps
Consider a versioned API, which defines:

  • 1.0
  • 2.0
  • 3.0
  • 0.9 (deprecated)

Each version has its own EDM and ultimately adds 4 ODataRoutingMetadata instances to the Endpoint. The appropriate EDM is matched by the applied ApiVersionAnnotation to the incoming request. Consider the following controller.

[ApiVersionNeutral]
public class FunctionsController : ODataController
{
    [HttpGet( "api/GetSalesTaxRate(PostalCode={postalCode})" )]
    [ProducesResponseType( typeof( double ), 200 )]
    public IActionResult GetSalesTaxRate( int postalCode ) => Ok( 5.6 );
}

A version-neutral controller can match any API version, including none at all. OData, however, must have an EDM. In this scenario, a developer is expected to use the same function definition for each version, but that's their discretion. This configuration yields the following results:

Request Result
api/GetSalesTaxRate(PostalCode=98052) 200
api/GetSalesTaxRate(PostalCode=98052)?api-version=1.0 200
api/GetSalesTaxRate(PostalCode=98052)?api-version=2.0 404
api/GetSalesTaxRate(PostalCode=98052)?api-version=3.0 404

This happens because once an explicit version is specified, it cannot match up to the correct EDM as only the first set of metadata is considered. This is only one example, but there are any number of other cases where this could happen.

Expected behavior
ODataRoutingMatcherPolicy.ApplyAsync should consider all IODataRoutingMetadata before invalidating a candidate.

- IODataRoutingMetadata metadata = candidate.Endpoint.Metadata.OfType<IODataRoutingMetadata>().FirstOrDefault();
+ IODataRoutingMetadata[] metadata = candidate.Endpoint.Metadata.OfType<IODataRoutingMetadata>().ToArray();
- if (metadata == null)
+ if (metadata.Length == 0)
{
    continue;
}

if (odataFeature.Path != null)
{
    // If it's odata endpoint, and we have a path set, let other odata endpoints invalid.
    candidates.SetValidity(i, false);
    continue;
}

- ODataTemplateTranslateContext translatorContext =
-     new ODataTemplateTranslateContext(httpContext, candidate.Endpoint, candidate.Values, metadata.Model);

- ODataPath odataPath = _translator.Translate(metadata.Template, translatorContext);

+ ODataPath odataPath = null;

+ for (var j = 0; odataPath == null && j < metadata.Length; i++)
+ {
+     ODataTemplateTranslateContext translatorContext =
+         new ODataTemplateTranslateContext(httpContext, candidate.Endpoint, candidate.Values, metadata[j].Model);
+     odataPath = _translator.Translate(metadata[j].Template, translatorContext);
+ }

if (odataPath != null)
{
    odataFeature.RoutePrefix = metadata.Prefix;
    odataFeature.Model = metadata.Model;
    odataFeature.Path = odataPath;

    MergeRouteValues(translatorContext.UpdatedValues, candidate.Values);

    // Shall we break the remaining candidates?
    // So far the answer is no. Because we can use this matcher to obsolete the unmatched endpoint.
    // break;
}
else
{
    candidates.SetValidity(i, false);
}

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.