Net core odata with versioning and template in url prefix

[rakkum]

Hello,
it might be a stupid question but I coudn't find anything in the documentation about it.
The idea is to pass a tenantId in the URL and process that in a ODataController to select the current tenant. It should not be part of the odata query so I wanted to have it in such a setup.

I have a route setup like this
routes.MapVersionedODataRoutes("odata-bypath", "api/v{version:apiVersion}/tenants/{tenantId}", models);
and a controller such as

[ODataRoutePrefix("WorkItemSearch")]
  public class ODataWorkItemSearchController :
      ODataController
  {
      public ODataWorkItemSearchController(MyDbContext context)
      {
           this.context = context;
      }

      private MyDbContext context;

      [EnableQuery]
      public IQueryable<ODataWorkItemSnapshot> Get(Guid tenantId)
      {
          return search.Snapshots.Where(x => x.TenantId == tenantId);
      }
}

When accessing api/v1/tenants/00000000-0000-0000-0000-000000000001 i get the odata context as expected.
However, when accessing api/v1/tenants/00000000-0000-0000-0000-000000000001/WorkItemSearch i get a 404.
I also tried /api/v1/tenants/00000000-0000-0000-0000-000000000001/WorkItemSearch/00000000-0000-0000-0000-000000000001 just to make sure but there I get the same 404 response.
Swagger reports api/v1/tenants/{tenantId} as the path to the controller so I don't know what is going wrong.

Any ideas?

Thanks,
Klemens

[commonsensesoftware]

Yes - this type of configuration will work. I suspect that this is purely a matter of configuration. I was able to get things working using the provided Basic OData Example with versioning.

First, change your route mapping to:

routeBuilder.MapVersionedODataRoutes(
    "odata",
    "api/v{version:apiVersion}/tenants/{tenantId:guid}",
    models );

Then, make sure that your controller has all of OData route attributes. I recommend going either full attribute-based or full convention-based. It's hard to rationalize things otherwise and the results will be unpredictable. Here's a modified version of the Orders controller that works:

[ApiVersion( "1.0" )]
[ODataRoutePrefix( "Orders" )]
public class OrdersController : ODataController
{
    // GET ~/api/tenants/{tenantId}/v1/orders
    [ODataRoute]
    public IActionResult Get( Guid tenantId ) =>
        Ok( new[] { new Order() { Id = 1, Customer = "Bill Mei" } } );

    // GET ~/api/tenants/{tenantId}/v1/orders/{id}
    [ODataRoute( "{id}" )]
    public IActionResult Get( Guid tenantId, [FromODataUri] int id ) =>
        Ok( new Order() { Id = id, Customer = "Bill Mei" } );
}

I hope that helps.

[rakkum]

Hello,

thanks for the fast answer! I retired the example with your suggestion but still cannot get it to work. The method gets indeed called but the tenantId will always be the default value (Guid.Empty).

I attached the small changed that i made to the example, maybe there really is something obvious that i am missing.

Thanks again for the help.

ODataBasicSample.zip

[commonsensesoftware]

Confirmed. I guess I blew right past that very small, but important detail. Model binding in OData can be infuriating at times. Sometimes you need attribution and sometimes you don't. In general, I've started always adding the attributes (ex: [FromODataUri]) so I never go crazy over these little issues.

In this case, you just need change the parameter to [FromRoute] Guid tenantId. I'm not sure why OData isn't picking it up otherwise because it will pick up query parameters. I suspect it has something do with the hand-off between the intrinsic routing infrastructure and when OData takes over. Regardless, I was able to get the request to succeed and get a non-empty GUID with this change.

I hope that helps.

[rakkum]

Thanks! With the extra attribute i got it to work! Next step for me was to try Swagger to get the parameter. Here its strange again. ODataQueryOptions seems to be expanded to more than 1700 query fields. I filtered it via an IOperationFilter with

foreach (var parameter in operation.Parameters.OfType<NonBodyParameter>().ToList())
            {
                var description = apiDescription.ParameterDescriptions.First(p => p.Name == parameter.Name);

                if (typeof(ODataQueryOptions).IsAssignableFrom(description.ParameterDescriptor?.ParameterType))
                {
                    operation.Parameters.Remove(parameter);
                }
            }

but the output still is not correct. TenantId is now present twice for some reason and there are not all fields that are allowed present. What do I need to do to get the ODataQueryOptions properly read? Do I need to do something special? My Odata.ApiExplorer nuget is at version 3.2.2.

[commonsensesoftware]

1700?! That definitely doesn't sound right. ODataQueryOptions and ODataQueryOptions<T> should be ignored completely. They have a special parameter source that should result in them being ignored. What is the method signature of the action that causes this?

The duplicate tenantId is almost certainly a bug. The API Explorer considers any parameters not satisfied by the EDM to be query parameters. It's clearly not checking to see whether the parameter has already been added from another source, such as the path. That should be a straight forward fix.

[rakkum]

I would assume that for the same reason that the FromRoute attribute is required the tenantId is not properly matched.
The method signature is nothing special

 [ODataRoute]
        [ProducesResponseType(typeof(ODataValue<ODataWorkItemSnapshot>), StatusCodes.Status200OK)]
        [EnableQuery(MaxTop = 100, AllowedQueryOptions = AllowedQueryOptions.Select | 
                                                         AllowedQueryOptions.Top | 
                                                         AllowedQueryOptions.Skip | 
                                                         AllowedQueryOptions.Count)]
        public async Task<ActionResult> Get([FromRoute] Guid tenantId, ODataQueryOptions<ODataWorkItemSnapshot> options)

for some reason i don't see the same behaviour in the example but cant see any difference in the config. For me it seems that the ODataQueryOptions class is not being recognized as a special type. There are no errors in the log so its very hard for me to pinpoint. So a pointer on what i can look for is highly appreciated.

[commonsensesoftware]

Hmmm... I know for sure that ODataQueryOptions, Delta, and a few other types have special handling by the OData model binder. This results in them not requiring attribution for them to resolve. They report themselves to the API Explorer as Custom or Special. This is similar to the behavior of CancellationToken and, now, the ApiVersion if defined as a parameter in your action.

Do you have a partial sample (or repro) that demonstrates the options exploding? Do you have an example of one or more parameters that should not be there? From your description, it sounds like you're getting a bunch of junk. I do know, and recently faced a somewhat similar situation, that ASP.NET Core intrinsically handles complex model binding from a URL. This means that if you forget to use [FromBody] on a POST or PUT, it might shread the object into constituent pieces where each property is a query string parameter. That doesn't seem to the case in your example, but something like that could account for a huge number of parameters.

[commonsensesoftware]

I believe the latest patch should fix the duplicate route template parameter issue. Were you able to confirm that? Were you also able to solve the query option explosion issue?

[rakkum]

Ok, i will try the new patch, thanks a lot! This week I could not really spend a lot of time to reproduce it but it behaved strange in general. Once I added AllowedQueryOptions the tenantid was still only taken from the querystring and not the route, when I only had the EnableQuery attribute it started working again. Then I did a request via swagger and subsequent calls always returned the default value.
As soon as i have more info i will update you.

[rakkum]

I spent a bit more time with it and worked from another angle. I created a new project with the same template as before (with angular), added an odata controller to it and can get the same behaviour. Unfortunately the project is not that small but it might still help to pinpoint the issue with swagger.
I removed all angular references from the solution that are not necessary for reproduction. If you start the project and go to /swagger there is a single People controller entry that will show the many parameters once expanded.

quickapp.zip

[Diraekt]

I run into the same issue. With many controllers this results in a OutOfMemoryException :-)

if you have a controller method with a ODataQueryOptions parameter, it will result in many parameters:

I also have the same with the Delta parameter:

So yes, I also think, the Odata Parameters (OdataQueryOptions and Delta) are not well handled.
Here my signature of the patch method:

Here my signature from the get method:

Also I noticed that some OdataQueryOptions are recognized as BodyParameter.

I tested it with the current release and also with the preview release.

Thanks for helping us out :-)

[rakkum]

As a workaround I am using this to filter all ODataQueryOptions fields in a IOperationFilter

foreach (var parameter in operation.Parameters.OfType<NonBodyParameter>().ToList())
            {
                var description = apiDescription.ParameterDescriptions.First(p => p.Name == parameter.Name);

                if (typeof(ODataQueryOptions).IsAssignableFrom(description.ParameterDescriptor?.ParameterType))
                {
                    operation.Parameters.Remove(parameter);
                }
            }

[orphisAG]

I found out the following:

• The PseudoModelBindingVisitor has source.IsGreed = false, that's why the parameters will get documented.
• When I remove from my Test-Controller the [ODataRoutePrefix("test")] attribute, the OdataQueryOptions and Delta parameters don't get documented (right!)

News:

• When the OdataRoutePrefix Attribute has the same name as the controller, we get this unexpected behaivor. When I change the ODataRoutePrefix to "tests" from the TestController it start working.

How you can reproduce that:

• In your SwaggerODataSample (asp.net core) change the following:
  --> OrderModelConfiguration : builder.EntitySet("Orders") to builder.EntitySet("Order") (removing the s)
  --> Inside the OrderControllers (v1,v2,v3) change OdataRoutePrefix from Orders to Order
  --> Navigate to swagger, change Version to v3 -> you will have the same issue, as we have.

May that will help you to help us to solve the problem :-)

[commonsensesoftware]

Thanks @orphisAG. This will be very useful is the investigation.

[orphisAG]

Another information I found out:

ModelBuilder:
builder.EntitySet<ActivityDtoV3>("Activity").EntityType.HasKey(o => o.Id);

Controller:
[ODataRoutePrefix("Activity")]
public class ActivityController : OdataController

My attribute name was "activity" on the controller and ODataQueryOptions was not ignored in the documentation (Swagger). Then I renamed it to "Activity" and then it starts working.

[Gigabyte0x1337]

I got the same problem so i did some digging and found out why it marks it as query instead of body.

src/Microsoft.AspNetCore.OData.Versioning.ApiExplorer/AspNetCore.Mvc.ApiExplorer/ODataApiDescriptionProvider.cs

`static void UpdateBindingInfo( ApiParameterContext context, ParameterDescriptor parameter, ModelMetadata metadata ) {
...
static bool IsSpecialBindingSource( BindingInfo info, Type type ) {
...
if ( ( type.IsODataQueryOptions() || type.IsODataPath() ) && info.BindingSource == Custom )
{
info.BindingSource = Special;
return true;
}
...
}

if ( IsSpecialBindingSource( bindingInfo, parameterType ) )
{
	return;
}
...
switch ( context.RouteContext.ActionType ) {
		...
		default:
			source = Query;
            break;
}
...

}`
In the case of the controllers / actions above the BindingSource is Body and the type is delta so it wil not return and by default it will be marked as Query if the actionType is unknown.

So IsSpecialBindingSource needs to be changed to allow delta and body i think.

[commonsensesoftware]

Finally, circling back around...

@GerjanVlot are you saying that the BindingSource for Delta<T> isn't already marked as Body? That seems odd, but is possible. Interesting that the OData+Swagger example has a controller with Delta<T> in it and it works just fine. I'm wondering there's a little more to the story.

@ALL the controller name must match the entity set name. The ODataRoutePrefix does not influence the name that is used. I can't say why a name mismatch would cause this parameter explosion. Perhaps it's something of a union between multiple versions.

Resuming investigation...

[Gigabyte0x1337]

@commonsensesoftware
[ApiVersion(Constants.Api.V1)]
[ODataRoutePrefix(Constants.Api.Routes.Activities)]
public class ActivityController : ODataController
{
[HttpPatch]
[ODataRoute("{activityId}")]
[Consumes(Constants.Api.ApplicationJson)]
[Produces(Constants.Api.ApplicationJson)]
[ProducesResponseType(200, Type = typeof(Activity))]
[HasPermission(Constants.Api.Permissions.Activities.Update)]
public async Task PatchActivity([FromODataUri] int activityId, [FromBody] Delta activityDelta)
{
}
}
ModelBuilder:
var configuration = builder.AddEntityType(typeof(Activity));
builder.AddEntitySet(Constants.Api.Routes.Activities, configuration);

https://1drv.ms/v/s!Alu0uzi9pf9UhYEOVTxLQSqr7qQkkQ?e=2BOqjL

Does this give you more context?

[commonsensesoftware]

@GerjanVlot a little. With the [FromBody] annotation, I would expect the parameter to explicitly be BindingSource.Body; however, this might vary by how things are built up. It's not immediately clear to me if this would override the parameter descriptor. This annotation is not needed because the OData model binding support for Delta already defines it. You have to spelunk for it, but it's provided by NonValidatingParameterBindingAttribute, which is decorated on Delta and Delta<T>.

The video helps quite a bit. It's strange that the operation would not be for an entity set. That may be the culprit. This appears to be a vanilla PATCH on the Activity entity set. I've been trying to avoid special exceptions for specific types, but perhaps adding for IDelta makes sense. If any of the Delta or ODataQueryOptions end up being sourced as a query string, I see how that would cause a 💥.

BTW: I noticed that it looks like you are decompiling the libraries to debug. You shouldn't have to do that. All API Versioning libraries support SourceLink. If that isn't working, please file a separate issue and I'll investigate. From my experience, testing, and feedback, it does work.