Microsoft.AspNetCore.Mvc.Versioning 5.0.0 removes number from end of controller name

[kimsey0]

Microsoft.AspNetCore.Mvc.Versioning 5.0.0 seems to introduce a breaking change from 4.2.0 where it removes numbers at the end of controller names when generating API descriptions and paths for them.

Reproduction steps:

1. Create a new solution in Visual Studio using the ASP.NET Core Web API template, leaving all settings at their default values (including keeping Enable OpenAPI support checked).
2. Rename the created WeatherForecastController class to WeatherForecastV1Controller.
3. Install the Microsoft.AspNetCore.Mvc.Versioning NuGet package in version 5.0.0.
4. Add services.AddApiVersioning(); after services.AddControllers(); in Startup.cs.
5. Start debugging using the default IIS Express launch profile.

Expected results:

The Swagger UI shows one path, WeatherForecastV1, and allows calling it at `/WeatherForecastV1, which is correctly routed to the controller by ASP.NET Core (though returning a 400 status code).

This behavior is seen if you install version 4.2.0 of the NuGet package above.

Actual results:

The Swagger UI shows one path, WeatherForecastV, and calls it at /WeatherForecastV, which is not correctly routed to WeatherForecastV1Controller and therefore returns a 404 status code.

[commonsensesoftware]

In short, this has always been the case since 1.0. APIs are grouped by their corresponding controller name. There's really no other, reliable information to group on. A lot of people think you can use route template, but it isn't that simple. Consider that forecast/{city} and forecast/{cityid:int} are 100% semantically equivalent from an API perspective, but are different templates.

ASP.NET has a built-in convention to strip off the trailing Controller part of the name by default. Similarly, API Versioning does the same thing for any trailing number; e.g. <Name>[Number]Controller. In this case, WeatherForecastV1Controller becomes WeatherForecastV. The reason for this behavior has to do with easing the limitation of unique type names in .NET. As an example, you might have WeatherForecast1Controller, WeatherForecast2Controller, and so on. If this wasn't the case, then a developer would be forced to use distinct namespaces for every controller or decorate each controller with a unique name. Workable solutions, but a bit painful.

It should be noted that route template and controller name are distinct things. Regardless of version, all related controllers should have the same logical name or they cannot be grouped together. This may not necessarily flow into your route template. You did not show the code or route template, but I'm guessing it is defined as [Route("[controller]")], which will produce /WeatherForecastV. You should instead chose one of the following alternatives:

1. Decorate the controller with [ControllerName("WeatherForecast")]
2. Use a route template without the [controller] token; e.g. [Route("WeatherForecast")]

All versions of the Weather Forecast service should have the same path regardless of version.

[kimsey0]

In short, this has always been the case since 1.0.

That doesn't seem to be the case. Going through the reproduction steps and installing Microsoft.AspNetCore.Mvc.Versioning version 4.2.0 does not strip the 1 from the end of the controller name. However, installing version 5.0.0 does strip it. I'd call that a breaking change which is not described in the release notes.

Note that I'm not arguing that the new behavior is incorrect, just that it's breaking and should be documented as such.

You did not show the code or route template, but I'm guessing it is defined as [Route("[controller]")], which will produce /WeatherForecastV.

Yes, that is what the default template included with Visual Studio uses.

(For completeness, we did just remove the [controller] token from the few controllers that used it, but it surprised us when our tests broke after a supposedly non-breaking package upgrade.)

[gokhanabatay]

I agree with @kimsey0 it' breaking change

[commonsensesoftware]

This is a regression so I'll accept it as a bug. The behavior that I've already described is what has always happened and will not be changing. What changed is from some recent performance optimizations. The trimming was happening on every use of ControllerModel.ControllerName and was changed to update the property with the trimmed name rather than repeat that over and over again. I'll have to come up with a way to separate the two.

This setup is very strange to me. Why would anyone want to do this? It effectively adds its own pseudo mechanism for versioning in the path. If you're going to do it that way, then you probably don't need API Versioning. Controllers will still be grouped by WeatherForecastV and that won't change (anytime soon). This will be fixed at some point, but you'll need to convince me why this should be prioritized. It would seem you've found an edge case that should work, but doesn't; however, the configuration also doesn't conform to any known method of versioning.

You didn't show the controller or any code for that matter. Based on the repro steps, you haven't added any attributes or conventions and used all the default settings. This means that WeatherForecastV1Controller has 1.0 applied to it. 1.0 is the default API version. Once you opt it, there is no such thing as a controller without a version unless you are version-neutral (which this is not).

As a workaround, I would expect the following route template to work:

[Route("WeatherForecastV{version:apiVersion}")]

This can be made to pair with the API Explorer extensions to produce the correct URL.

[kimsey0]

you'll need to convince me why this should be prioritized.

I'm not going to try. We have fixed the one weird case where it affected us. I was just surprised that having a number at the end of a controller name led to ApiExplorer (or the interaction with Swashbuckle?) generating incorrect API paths. I could imagine the same surprise if the number had nothing to do with versioning. (Maybe ConfigureHTTP3Controller or ConvertIPv4ToIPv6Controller?)

You didn't show the controller or any code for that matter. Based on the repro steps, you haven't added any attributes or conventions and used all the default settings.

Yes. The reproduction steps in the report produce a full and working example that demonstrates the regression. I did not need to add any additional attributes or conventions. If you don't have access to an environment with Visual Studio, or find that there's too much code in the template solution for it to be a minimal working example, I can try and create a smaller one?

[commonsensesoftware]

ConvertIPv4ToIPv6Controller seems plausible. I wouldn't use the controller name to produce the resource path /ConvertIPv4ToIPv6, but to each their own. I'm sure there is some variant that would eventually make sense. The behavior should comply with the Principle of Least Astonishment. 😉

I'm going to have to think about what the long-term solution for this is. The original intent was to ease the requirements for code organization. .NET types must be unique per namespace. The original behavior addresses the common scenario if you chose to have all of your controllers in a single namespace. Consider the following:

• ValuesController
• Values2Controller
• Values3Controller

All 3 of these should have the controller name and route parameter value values. The Controller suffix is automatically stripped before API Versioning gets involved. While it's true that Values2Controller and Values3Controller could explicitly define their own controller name explicitly, it's highly inconvenient and not necessarily obvious that you need to do so. This is the conflicting case where you want to have ControllerModel.ControllerName with the text Values2 and Values3 simply become Values or the resultant URL is again wrong.

It hasn't happened often, but this is not the first time that people have been hung up by this magical convention. People expect:

• ValuesController
• ValuesController2
• ValuesController3

To work by default, but it doesn't. ASP.NET will not strip the suffix because it is not exactly Controller.

I can certainly add the hook with built-in behaviors to control how this is processed, but I'm pretty sure most will not understand this is what is causing them grief and that they need to change things. Discoverability of this nuance is difficult no matter which direction you come at it from.

I'm open to suggestions.

[kimsey0]

I'm not sure that anything needs to change. I understand the reasoning behind stripping the numbers which you describe above. If I actually wanted my (fictional) IPv6Controller to map from /IPv6, I'd specify that in the route template.

But I would at least document the change in behavior. The release may have been out for a couple of months now, but there are probably going to be developers upgrading for a long time. (They may be staying on .NET Core 3.1 LTS until .NET 6.) Maybe they can search up this issue, but it might be easier to find if it was linked from https://github.com/microsoft/aspnet-api-versioning/releases/tag/v5.0.0

[commonsensesoftware]

Fair enough. As I seem to recall now, I realized things (from the other side) didn't work properly if you didn't explicitly add the controller name or route template. Defining the route template with the expected path or ControllerNameAttribute will take precedence over the convention.

I'll add a section to the wiki about this and link it back to the release notes.

[commonsensesoftware]

A small update on this issue. I've decided to lift this behavior out into the new:

interface IControllerNameConvention
{
    string NormalizeName(string controllerName);
    string GroupName(string controllerName);
}

There will be 3 out-of-the-box variants:

1. default - The default behavior which will trim off trailing numbers
2. original - No modifications, but this may produce unexpected results
3. grouped - Names are only normalized for the purposes of grouping; ControllerModel.ControllerName is unchanged

Changing the behavior (like you want) will look like:

options.ControllerNameConvention = ControllerNameConvention.Grouped; // instead of '.Default'

For any other scenario or edge case, people can roll their own implementation.

No commitment on a specific release date, but I've got things put together and it will be available in the next iteration

[gt-hans]

I have a controller for AWS S3 and the controller name ending with S3, suddenly I found all my request returned 404, it wasted me a few hours before I found this thread...
Hm... I am really surprised if breaking something previously working is acceptable in dotnet core...