Nswag: When using inheritance api versions are not correctly versioned

[giggio]

In ASP.NET Core, if you have a base controller with operations, and an inherited controller, and you use versioning, the version is not correctly discovered.

[RicoSuter]

v11.17.2

[steji113]

This still seems to happen in 11.17.7 as far as I can tell.
I have a base controller like this:

[ApiVersion("1.0")]
[ApiController]
public class ApiV1BaseController : Controller
{
}

and an inherited controller:

[Route("api/v{version:apiVersion}/audio-devices")]
public class AudioDevicesController : ApiV1BaseController
{
}

[RicoSuter]

I hope the commit fixes this... what do you think?

[steji113]

It looks sensible to me! I would be happy to try it out if there was a way for me to get a build of the package?

[RicoSuter]

There is a CI nuget feed on MyGet but there will be a release soon.

[steji113]

Great, thanks!

[RicoSuter]

Will release a new version in 15 min

[RicoSuter]

v11.17.10

[steji113]

Thanks for turning this around so quickly. I am sorry to report it still does not seem to be working :( If I figure out how to get NSwag's code up and running, I may be able to try debugging it further to contribute a fix.

[RicoSuter]

You can clone the project and start/test with one of the sample apps (installer can be unloaded)

[RicoSuter]

And sorry, didnt test it... another option is to copy the processor class code into your project, remove the existing one from the settings and add your own with a fix... then post here the fixes :-)

[RicoSuter]

https://github.com/RSuter/NSwag/blob/master/src/NSwag.SwaggerGeneration/Processors/ApiVersionProcessor.cs

Easy changeable in the middleware settings (GeneratorSettings property).

[steji113]

Outstanding, that sounds like the best option. I will give that a try.

[RicoSuter]

Are you using ASP.NET or ASP.NET Core? Just for your background: The processor should be working correctly for both as it is used for both...

[steji113]

It is ASP.NET Core. Last night when playing around with ApiVersionProcessor, I found that the GetCustomAttributes(inherit: true) was not working as we expected. I was able to get my scenario to work by doing this:

var controllerAttributes =
        Attribute.GetCustomAttributes(context.ControllerType.GetTypeInfo(), inherit: true);
var controllerBaseAttributes =
        Attribute.GetCustomAttributes(context.ControllerType.BaseType.GetTypeInfo(), inherit: true);

var versionAttributes = context.MethodInfo.GetCustomAttributes()
        .Concat(controllerAttributes)
        .Concat(controllerBaseAttributes)
        .Where(a => a.GetType().IsAssignableTo("MapToApiVersionAttribute", TypeNameStyle.Name) ||
                a.GetType().IsAssignableTo("ApiVersionAttribute", TypeNameStyle.Name))
        .Select(a => (dynamic)a)
        .ToArray();

However, this solution isn't very scalable because it directly inspect's the Controller's base type. I could have inherited from a deeper hierarchy and it still may not have picked up the version attribute. Also, another side note, it did seem that in the current code, context.MethodInfo.DeclaringType and context.ControllerType evaluated to the same thing. So the two calls were redundant there.

I am not sure what the right solution is to handle all of the desired cases. We may need to write some unit tests that exercise these various scenarios and see if we can come up with a robust solution.

[RicoSuter]

However, this solution isn't very scalable because it directly inspect's the Controller's base type. I could have inherited from a deeper hierarchy and it still may not have picked up the version attribute.

Maybe with a loop which loops through the BaseTypes until BaseType is null...

context.MethodInfo.DeclaringType and context.ControllerType evaluated to the same thing. So the two calls were redundant there.

Yep, when I looked at the code I also wondered if we can just remove this...