Unwanted $count segment always present with EntitySetRoutingConvention

[kjkrum]

I created a very simple EDM and ODataController. My intended API works. But the Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionGroupCollectionProvider that gets injected into Swashbuckle's SwaggerGenerator contains an unexpected and unwanted ApiDescription for the $count query. That unwanted API doesn't even work unless I add Count() to the ODataOptions and [EnableQuery] to the controller action.

I created a repro project. In this commit I tried several things to explicitly disable $count, and it still shows up.

I'm using 8.0.10. This seems to affect all 8.x versions.

[julealgon]

You seem to be using conventional routes instead of explicit routes in your controller. Can you try switching to explicit and then avoid declaring the count route and see how that works?

I believe conventional routing will create the count route for you even if it is not possible to call it due to restrictions on the actions, and that may be why you are seeing one there.

[kjkrum]

That helps narrow it down. I found a blog post about 8.0 preview where it's mentioned that EntitySetRoutingConvention creates the $count path. It seems like a bug or misfeature that it does so even when count is disabled for the entity.

By explicit routes, I think you mean [HttpGet("/some/route")]. When I use this, I get the attribute route and the conventional routes. If the attribute route is the same as the conventional route, I still get both, and an error from Swagger/Swashbuckle.

The blog post mentions an ODataRoute attribute, but this no longer exists in 8.x. If you reference it, VS suggests adding Microsoft.AspNet.OData, which is a .NET Framework lib.

[kjkrum]

As a workaround, the conventional routes can be removed and selectively re-added like this:

builder.Services
	.AddControllers()
	.AddOData(options =>
	{
		options.Conventions.Clear();
		options.Conventions.Add(new MetadataRoutingConvention());
		options.AddRouteComponents("odata", EdmBuilder.Build());
	});

I'll dig into this and see if there's a less heavy-handed approach.

[julealgon]

By explicit routes, I think you mean [HttpGet("/some/route")].

Correct. Just attribute-based routing, which also respects the [Route] attribute on the controller.

When I use this, I get the attribute route and the conventional routes. If the attribute route is the same as the conventional route, I still get both, and an error from Swagger/Swashbuckle.

For testing purposes, you can just rename the controller action to something other than Get and it will stop matching the convention @kjkrum . Obviously not a sustainable solution, but should help to diagnose here.

The blog post mentions an ODataRoute attribute, but this no longer exists in 8.x. If you reference it, VS suggests adding Microsoft.AspNet.OData, which is a .NET Framework lib.

ODataRoute has since been deprecated in favor of the standard MVC attributes: no special OData route attributes anymore.

[kjkrum]

It looks like routing conventions are an area that still needs a lot of work in 8.x. The aforementioned blog post says,

We want to make developer to customize his own routing convention more easy. Developer can derive from any built-in convention to start his own convention.

But extending conventions is pointless since the methods that do the real work are private. Re-implementing them isn't straightforward, either, because they rely heavily on extensions from an internal class. Routing conventions were definitely not designed for customization and extension. And the more I study the code, the more I question everything about it.

[ElizabethOkerio]

$count is a segment and Count() is a query option. The two are different and the $count segment is not related to the Count() query option. For you to use the Count() query option, you have to add it to the ODataOptions and add the EnableQuery attribute to the controller action. $count segment is built by default in the convention routing.

You can although add a $count configuration in the OData routing convention configuration-- which is a new feature and we welcome any contribution on this.

The correct place to control how OData routing convention is applied is through the OData routing convention configuration.

[kjkrum]

@ElizabethOkerio The issue is that the functionality of $count segment seems to depend on the availability of the Count() query option. If the Count() query is disabled, the $count segment is still added, but it does not work.

IMO, the routing convention should not add the $count segment if the Count() query option is disabled. Alternatively, the functionality of the $count segment should not depend on the Count() query option.

What routing convention configuration is available other than adding or removing elements of ODataOptions.Conventions?

[jonmotos]

Is it possible to disable globally or selectively the $count endpoints, while still maintaining the default convention routing?

[julealgon]

Is it possible to disable globally or selectively the $count endpoints, while still maintaining the default convention routing?

Doesn't look like it @JonAdamsFromNC . The .../$count routes are generated by the EntitySetRoutingConvention convention and that also takes care of the standard entityset routes, so if you remove that, you remove a significant portion of the standard conventions with it.

https://github.com/OData/AspNetCoreOData/blob/f3fa98f939ab7df3ef27e33ec72c4e1ee834d58a/src/Microsoft.AspNetCore.OData/Routing/Conventions/EntitySetRoutingConvention.cs#L18-L29

I don't see any conditional logic around the count bits too.

https://github.com/OData/AspNetCoreOData/blob/f3fa98f939ab7df3ef27e33ec72c4e1ee834d58a/src/Microsoft.AspNetCore.OData/Routing/Conventions/EntitySetRoutingConvention.cs#L129-L140

Is there a specific reason you want to disable these routes?

[kjkrum]

@julealgon Any reason for which you'd want to disable the Count() query. The existence of a feature to disable that query implies the existence of a reason to do so. If the $count route is in fact unrelated to the Count() query as @ElizabethOkerio suggests, then the functionality of the $count route should not depend on the availability of the Count() query.

[jonmotos]

@julealgon I would like to disable the routes because they are unused in my context - and in fact, I am generating Swagger clients based on the OData OpenAPI specification; and the $count endpoints produce both inaccurate OpenAPI metadata, and unusable client code! I understand this is not a typical use-case, but it does seem odd that especially in light of the odd behavior in combination with disabling the Count() query option, that the option does not exist to remove the endpoints. In my case I need the Count() query option to stay enabled, but the $count endpoints to be disabled.

[julealgon]

That's totally fair. I tend to agree with most if not all of your points @JonAdamsFromNC .

Hopefully the team will step in and provide a solution to toggle these off without having to resort to fully manual route definitions.

I guess I could potentially follow-up with a question on "how important is it to you to keep using the conventional routes" and if you'd be willing to drop them in favor of manual ones, but I do understand this is a hassle.

This is why I usually don't love the kind of magic that OData does with the automatic routing... it is hard to have a fully automatic system that is also fully flexible at the same time, and once you introduce it, it is hard to remove it... if this was up to me (and it isn't, to be clear), I'd propose deprecating the automatic routes entirely and just have people define the routes manually honestly. Yes it is more upfront work and it is less newbie-friendly, but the benefits in terms of control and reduced maintenance on OData itself are just too big to ignore.

[awbacker]

I think having things explicit is MORE newbie friendly than convention (magic) based. This comes from a newbie trying to understand these "conventions" which are really only such a thing if you you already know them - the opposite of a newbie.

I am here, reading yet another bug report, because this thing is so horribly defined and documented that I can't even find a single example that works. OData team, please organize this sock drawer!

[HuntJason]

@julealgon I would like to disable the routes because they are unused in my context - and in fact, I am generating Swagger clients based on the OData OpenAPI specification; and the $count endpoints produce both inaccurate OpenAPI metadata, and unusable client code! I understand this is not a typical use-case, but it does seem odd that especially in light of the odd behavior in combination with disabling the Count() query option, that the option does not exist to remove the endpoints. In my case I need the Count() query option to stay enabled, but the $count endpoints to be disabled.

We are having the same issue. "How important is it to you to keep using the conventional routes" seems an odd question. They're called the conventional routes. They should be the most used convention and supported, no? Breaking this in an iteration without supporting "breaking change" documentation seems odd. Unintentional breaking of this seems lack of planning/testing.

I agree with @awbacker ... "OData team, please organize this sock drawer". Better testing, documentation, and support are necessary. This issue and lack of coordination with the OpenAPI/Swagger team (Thanks Chris, you have been the only help in this area) has caused me WEEKS of issues trying to migrate my OData services from .NET 6 to .NET 7; including a STILL outstanding support request with Microsoft on this issue where I was basically told to ask on GitHub.

[julealgon]

"How important is it to you to keep using the conventional routes" seems an odd question. They're called the conventional routes. They should be the most used convention and supported, no? Breaking this in an iteration without supporting "breaking change" documentation seems odd. Unintentional breaking of this seems lack of planning/testing.

Oh please don't take me wrong there: I totally agree with you guys 😆 Was just asking because shifting into attribute routing could be a workaround for you if you are not invested in having to use conventional routing.

I tend to favor attribute routing now if only because of the standardization factor in relation to the rest of AspNetCore which ditched these kinds of conventions long ago.

And to be 100% transparent here, I'm not part of the OData team so please don't take any of my suggestions as recommendations from Microsoft or anything of the sort.

[awbacker]

@julealgon

Was just asking because shifting into attribute routing could be a workaround for you if you are not invested in having to use conventional routing.

I've moved to fully attribute based routing, and it works a charm. I even followed the example here and removed everything except Metadata. The Object({key}) route was crashing anyway :) I am a bit worried about the metadata endpoint, not sure what happens now.

From various other places I learned:

• Use DbSet<Product> Get() for the list
• Use SingleResult<Product> Get(int id) for the individual object

Not sure if those are "the best", of course , but things seem to work properly now!

[julealgon]

I am a bit worried about the metadata endpoint, not sure what happens now.

Not entirely sure what you mean here. If you keep the metadata convention in place, the $metadata endpoint will work just fine. Feel free to elaborate your concerns.

From various other places I learned:

• Use DbSet<Product> Get() for the list
• Use SingleResult<Product> Get(int id) for the individual object

Not sure if those are "the best", of course , but things seem to work properly now!

SingleResult is definitely the way to go for OData actions with a single element as it takes care of transforming into a 404 for you but still allowing all filters to be applied automatically.

As for DbSet, I'd recommend returning just IQueryable<T> instead. This would decouple your controller a bit from the database (keep in mind OData doesn't need databases/EF to work at all so it's best to not introduce this kind of coupling if at all possible (even if what you are returning is a DbSet<T>, it can be exposed as IQueryable<T> without any loss of functionality.

[awbacker]

Thanks for the pointer about IQueryable, I'll consider doing that.

On metadata - I was not sure what might change, now that the EntitySetConvention and others are not involved now. That convention does a lot of work that I can't figure out.

I compared the metadata with and w/o the conventions and it seems there is no difference. I used only a simple EDM model though.

[julealgon]

On metadata - I was not sure what might change, now that the EntitySetConvention and others are not involved now. That convention does a lot of work that I can't figure out.

I compared the metadata with and w/o the conventions and it seems there is no difference. I used only a simple EDM model though.

There shouldn't be any difference. The entity type conventions are specifically for discovering/matching routes and shouldn't impact the metadata at all.