Skip to content

ODataRoutingMatcherPolicy Does Not Consider All Routing Metadata #753

Description

@commonsensesoftware

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:

IODataRoutingMetadata metadata = candidate.Endpoint.Metadata.OfType<IODataRoutingMetadata>().FirstOrDefault();

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);
}

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions