api-supported-versions and api-deprecated-versions Headers are incorrect in combination with convention: VersionByNamespaceConvention

[cert-jverdaasdonk]

Hi,

I'm using the version framework with the namespacing url convention.
To demonstrate a simple structure I have:
Controlers/Private/V1/ConnectionController decorated with:
[ApiVersion("1.0")] [RoutePrefix( "private/v{version:apiVersion}/connection" )]

Controlers/Public/V1/ConnectionController decorated with:
[ApiVersion("1.0", Deprecated = true)] [RoutePrefix( "public/v{version:apiVersion}/connection" )]

Controlers/Public/V2/ConnectionController decorated with:
[ApiVersion("2.0")] [RoutePrefix( "public/v{version:apiVersion}/connection" )]

But when I call request: ../private/v1.0/connection/1 I get these headers:
api-supported-versions: 1.0, 2.0
api-deprecated-versions: 1.0

Because namespace private has only one V1 version for the connectioncontroller I would expect only this header:
api-supported-versions: 1.0

So it looks like the implementation is scanning the assembly for usage of the ApiVersion attribute and use that as information?
I was expecting that if your using namespaces it would look for the attributes in the namespace they are defined?

Can someone comment what the expected behavior should be?
FYI all routes seam to work fine so it doesn't appear to be a configuration issue.

[commonsensesoftware]

I'll prefix the answer by first saying that API Versioning doesn't directly care about attributes. The built-in convention for namespaces will extract and parse the version to be added to the metadata.

Collation in API Versioning is done by controller name not route template. Route templates are quite complex and there's no guarantee they match up. This is the most complex and difficult part of making API versioning work. There really any good answers.

Ultimately, there is some type of issue here. As currently written, I would have expected one of the following outputs:

Merged

Some API supports 1.0 and 2.0

api-supported-versions: 1.0, 2.0

Split

api-supported-versions: 2.0
api-deprecated-versions: 1.0

Reporting versions is performed at the controller-level, not the action-level. If it was the action-level, there's nothing to aggregate and it's a lot more complex. The reported API versions do not imply which HTTP methods are supported. A practical usage of this information would be with tooling. For example:

OPTIONS public/v1/connection
Host: my.api.com

HTTP/2 200
Allow: GET, POST

api-supported-versions: 2.0
api-deprecated-versions: 1.0

It's all a simple rollup. Nothing in implied from headers alone as to which methods or API paths are supported. Deprecation times are also not conveyed. Those are policy decisions. Aside from tooling, a client should only be interested in these headers if they might be interested in moving to a newer version of the API or to detect that an API may be going away some time in the near future. Anything more complex than that would be better suited by something like Open API/Swagger.

In your particular case, both sets of controllers use the type name ConnectionController, but with different routes. These are logically grouped together. There's two possible ways to split these:

1. Rename the 2nd set of controllers to PrivateConnectionController
2. Apply [ControllerName("PrivateConnection")] to the controllers in the /private path. This attribute will supersede the default convention from the type name.

Yet another possible solution would be to use authorization. Your route templates seem to suggest that you have the same resource, but you are trying to split them apart using different route prefixes. I would say that's bad form in REST. A "connection" is a "connection" and using the prefix doesn't change that - logically. You may have some other reasons for this decision, but authorization could simplify your implementation while still denying access where there shouldn't be.

I'll look into why both sets are being reported; that feels wrong to me. I believe you've provided enough context for me to create repo. I hope the rest of this information is useful to you in the meantime.

[commonsensesoftware]

Apologies for taking so long to catch up on the backlog of issues. This is indeed a bug, but not for the reason you may think. Version aggregation and collation is done by controller name - as mentioned above. This because it is very difficult and brittle to group on route template. For example, you and I can tell that v{v:apiVersion}/connection/{id} and v{version:apiVersion}/connection/{id:int} are semantically equivalent, but it's a lot harder to do that in code. Route parameter names and constraints do not have to be consistent across versions. REST APIs should be resource-based and the controller name typically represents that resource. While possible, it is uncommon and somewhat unusual IMO that you'd have two different resources and meanings for the name logical resource name. While it might not be obvious, some basic renaming of controllers (but not necessarily their route templates) can address this. The next minor release will include a new feature that gives you more control over how the name is provided for the purposes of grouping.

I should point out that your example setup included attributes, but these are not necessary if you are using the VersionByNamespaceConvention. This might have just been a product of you trying to solve the problem. Your controllers should have looked like this:

namespace Example.Controllers
{
    using Microsoft.Web.Http;
    using System;
    using System.Web.Http;

    namespace Private.V1
    {
        [RoutePrefix( "private/v{version:apiVersion}/connection" )]
        public class ConnectionController : ApiController
        {
            [HttpGet]
            [Route]
            public IHttpActionResult Get( ApiVersion version ) => Ok( $"Private v{version:VVV}" );
        }
    }

    namespace Public.V1
    {
        [Obsolete] // ← this will indicate that 1.0 from the namespace V1 is deprecated
        [RoutePrefix( "public/v{version:apiVersion}/connection" )]
        public class ConnectionController : ApiController
        {
            [HttpGet]
            [Route]
            public IHttpActionResult Get( ApiVersion version ) => Ok( $"Public v{version:VVV}" );
        }
    }

    namespace Public.V2
    {
        [RoutePrefix( "public/v{version:apiVersion}/connection" )]
        public class ConnectionController : ApiController
        {
            [HttpGet]
            [Route]
            public IHttpActionResult Get( ApiVersion version ) => Ok( $"Public v{version:VVV}" );
        }
    }
}

This will produce the same results that you had. Unfortunately, the reported API versions are still wrong. This is where the bug actually is. Surprisingly, I have missed removing supported API versions from the list of deprecated versions during aggregation. I really can't believe this hasn't happened or been reported before. It's likely because it would probably only happen for two similarly named resources with completely different route templates. An API version cannot be both supported and deprecated at the same time. The term implemented is used to refer to both supported and deprecated APIs together.

The fix is incredibly simple and will go out in the next patch release. Interestingly enough, this normalization logic already exists in the API Explorer. If you need an immediate workaround you can replace the implementation of the IReportApiVersions service fix up the API versions before reporting them (e.g. remove supported versions from the deprecated list). You can call through to DefaultApiVersionReporter.Instance to use the built-in reporting implementation.

Thanks for your patience.