Support API Version Interleaving Across Types

[commonsensesoftware]

Overview

Support API versioning where the API versions can interleave across implemented types - namely controllers. The ability to interleave API versions across types would simplify inheritance and other partial implementation scenarios.

Challenges

Supporting API version interleaving across types would require a higher level of aggregation in order to disambiguate route candidates. Ideally, all API versions would be pre-aggregated at startup to optimize lookup performance as evaluating the information only at runtime could be expensive. This new type of aggregation would likely require a significant change to the current implementation.

Possible impacted features:

• Controller and action selection
• API version reporting
• API Explorer and Swagger integration

[dawaji]

In order to make the attribute [MapToApiVersion] work across types, we've changed the way of seaching the actions and the controllers.
We look first at the actions that have the attribute MapApiVersion matching with the requested version and the actions that have no attribute but the matched version on the controller.

I've created a branch from my fork : https://github.com/dawaji/aspnet-api-versioning/pulls

Could you take a look, and tell me what do you think ?

Thanks!

[vzwick]

Hey @commonsensesoftware,

would you be open to tackling this issue by accepting an IControllerSelectorPredicateProvider on the ApiVersioningOptions object, with a signature along the lines of

public interface IControllerSelectorPredicateProvider
{
    public Func<HttpControllerDescriptor, bool> CreateControllerSelectionPredicate( ApiVersion version );
    public HttpControllerDescriptor             HandleAmbiguousMatches( ApiVersion version, IEnumerable<HttpControllerDescriptor> ambiguousMatches, Func<IEnumerable<HttpControllerDescriptor>, Exception> exceptionProvider );
}

?

The value provided by CreateControllerSelectionPredicate would essentially become a drop-in replacement for the hard-coded predicates in the current ControllerSelector implementations.

An adapted DirectRouteControllerSelector implementation could be

static HttpControllerDescriptor ResolveController( CandidateAction[] directRouteCandidates, ApiVersion requestedVersion, ApiVersioningOptions options )
{
    Contract.Requires( directRouteCandidates != null );
    Contract.Requires( directRouteCandidates.Length > 0 );
    Contract.Requires( requestedVersion != null );
    Contract.Requires( options != null );

    var controllerDescriptor = default( HttpControllerDescriptor );
    var predicate = options.ControllerSelectorPredicateProvider.CreateControllerSelectionPredicate( requestedVersion );
    var matches = directRouteCandidates
        .Select( candidate => candidate.ActionDescriptor.ControllerDescriptor )
        .Where( predicate )
        .Distinct();

    if ( matches.Count <= 1 )
    {
        return matches.FirstOrDefault();
    }

    return options.HandleAmbiguousMatches( requestedVersion, matches, CreateAmbiguousControllerException );
}

The default IControllerSelectorPredicateProvider implementation could look like

class ExactVersionMatchControllerSelectorPredicateProvider : IControllerSelectorPredicateProvider
{
    public Func<HttpControllerDescriptor, bool> CreateControllerSelectionPredicate( ApiVersion version ) 
    {
        return ( controller ) => controller.GetDeclaredApiVersions().Contains( requestedVersion );
    }

    public HttpControllerDescriptor HandleAmbiguousMatches( ApiVersion version,
        IEnumerable<HttpControllerDescriptor> ambiguousMatches,
        Func<IEnumerable<HttpControllerDescriptor>, Exception> exceptionProvider )
    {
        throw exceptionProvider( ambiguousMatches );
    }
}

Background: we have a rather large API (not versioned as of yet, would become v1) that we would like to incrementally update towards v2, while redirecting all requests where no v2 endpoint is found to the corresponding v1 endpoint.

So, our IControllerSelectorPredicateProvider implementation might look something like this:

class HighestImplementedVersionMatchControllerSelectorPredicateProvider : IControllerSelectorPredicateProvider
{
    public Func<HttpControllerDescriptor, bool> CreateControllerSelectionPredicate( ApiVersion version ) 
    {
        return (controller) => controller
            .GetDeclaredApiVersions()
            .Any( version => version <= requestedVersion && version.Status == null );
    }

    public HttpControllerDescriptor HandleAmbiguousMatches( ApiVersion version,
        IEnumerable<HttpControllerDescriptor> ambiguousMatches,
        Func<IEnumerable<HttpControllerDescriptor>, Exception> exceptionProvider )
    {
        return ambiguousMatches
            .OrderByDescending( controller
                => controller
                    .GetDeclaredApiVersions()
                    .Where( version => version.Status == null )
                    .OrderByDescending( version => version.MajorVersion )
                    .ThenByDescending( version => version.MinorVersion )
                    .FirstOrDefault() )
            .FirstOrDefault()
    }
}

If this is a route that you are open to pursuing, ping me and I'll throw together a PR.

[commonsensesoftware]

While I welcome and appreciate the help to drive this enhancement forward, there a couple of fundamental things that are being glossed over.

1. Web API uses a two-phase selection process: controllers and actions. There's no way around that so we shouldn't fight it. Conceptually, it seems that the discovery of API versions for a controller needs to expand to include API versions on actions. This is a fundamental behavior change that I suspect will need quite a bit of new test cases.
2. Web API needs to also perform both route selection models. It first needs to try the convention-based route and then the direct (aka attribute) route methods.
3. An API version is not guaranteed to be numeric. It can be a date, a number, both, and/or with a text status. The ApiVersion class supports all the necessary things for comparisons and sorting. However, it is not safe to increment or otherwise mutate the API version value as some have proposed.
4. The API version metadata must be appropriately lifted out of the matching process. This is critical for things like Swagger integration to continue to work as expected. Furthermore, correctly enhancing the metadata discovery process should negate the need to for things like a custom selection predicate. The goal is to enable the ability to interleave with little or no configuration, which I believe is possible. I'm very wary of lighting this feature up too quickly without verification of the Swagger behavior because if they aren't congruent, then the bugs wills stockpile and the new design might even need another reboot.

Any acceptable PR needs to have:

1. A Web API version
2. An OData version (this may require nothing since it piggybacks on Web API)
3. An ASP.NET Core version
4. Work with Swagger integration
5. Include new unit and acceptance tests

That might all sound like I'm resistant to the change, but I'm not. I do know that there is lot of devil in the details. The relative simplicity of consuming these libraries is largely due to keeping the devil at bay. ;)

[commonsensesoftware]

@vzwick you can address your scenario without this feature. The design is already meant to solve the exact scenario you are describing (e.g. transitioning from no versioning to formal versioning). Your v1 always had a version, you just never assigned it a name or value until now (because you need a v2).

A lot of people use it for other purposes, but the primary use case for setting options.AssumeDefaultVersionWhenUnspecified = true is meant to solve your scenario. How to do enable versioning without breaking all your existing clients? By configuring this option to true, all your existing clients continue to work without any breaking changes. The default value for options.DefaultApiVersion is 1.0, but you can change it if your initial API version is something else. Once you lock that in, I do not recommend changing it - ever.

I see that you're using attribute routing and your opting to use URL segment versioning. Since you're not locked in yet, I'd urge you to reconsider. I know the URL segment method is popular, but it actually causes a lot of grief (for you) when the API version is embedded in the URL. There's a number of things I could list off (as I have in many other threads), but let's keep it relevant to just URL management for now.

Assuming you have route api/resources. By default, if you enable implicit API versioning a la options.AssumeDefaultVersionWhenUnspecified, all of your existing routes will now support api/resources and api/resources?api-version=1.0 without any modification at all. Your controllers don't even need the [ApiVersion] attribute (or conventions) because the value of options.DefaultApiVersion is used when no other information is provided. This will then allow you to start progressively fanning out your v2 APIs incrementally.

In contrast, if you choose the URL segment versioning method, you'll have to update every controller to have [Route("api/resources")] and [Route("api/{version:apiVersion}/resources")]. Alternatively, you can swap out the IDirectRouteProvider service and generate this in a single place for all v1 controllers. I do not recommend changing the API version matched to implicit routes because it's unstable for clients, but if you choose to enable that, you'll also have to move [Route("api/resources")] to whichever controller should currently match it. There is no way to implicitly match a route parameter value in the middle of a route template. You mentioned you have a large API surface, so you can imagine how untenable this can become from a management perspective.

Also keep in mind that there is not an equivalent of IDirectRouteProvider in ASP.NET Core. I imagine that you'll move to that platform at some point. By relying on Web API-specific features, it may make your transition less smooth.

Regardless of the path you choose, you can meet your goals to today without this change. Hopefully that provides you some insight on how to achieve and perhaps even a little advice making the evolution your API easier on you.

[vzwick]

Hey @commonsensesoftware,

thanks for your feedback and recommendations. We are aware of AssumeDefaultVersionWhenUnspecified and use it exactly the way you described.

Our actual issue is this:

namespace FooProject.Controllers.V1
{
    public class FooController : ApiController
    {
        public int Get()
        {
            return 1;
        }

        public int Post()
        {
            return 1;
        }
    }
}

namespace FooProject.Controllers.V2
{
    [ApiVersion("2.0")]
    public class FooController : ApiController
    {
        public int Get()
        {
            return 2;
        }
    }
}

GET /api/Foo => 1
GET /api/Foo?api-version=1 => 1
GET /api/Foo?api-version=2 => 2
POST /api/Foo => 1
POST /api/Foo?api-version=1 => 1
POST /api/Foo?api-version=2 => Exception, no matching route for API version 2.

The latter is what we would like to address by the approach I outlined above. Unless there is some other way to express "use the requested implementation where available, otherwise, fall back to v1", we would end up blatantly copying every single v1 controller to the v2 namespace otherwise - which, as I'm sure you agree, is a terrible solution.

[commonsensesoftware]

Ah ... I see what you're trying to do now. Personally, I'm not a fan of things falling back. It might seem convenient as a service author, but it's unpredictable for a client. The server should only match what the client asks for, save for the one exception where both the server and client have a single, implied default version which is not formally named nor has a version number.

I can see how inheritance and/or type interleaving can make service authoring easier, which is why this issue now formally exists. I've have seen teams blatantly copy all the code to new spaces actually. I call it the Copy-Paste-Replace (CPR) method. If done correctly, it's not the worst idea ever, but we can do better.

I like my controller types to be completely independent per API version without inheritance. It makes the routing and wire protocol components crystal clear. If a lot of logic is put into the controller, this clearly begs to for reuse. My approach and suggestion for remedy that is to push these things to actual business layer components that more naturally support inheritance, dependency injection, and the likes. By organizing things that way, the CPR method isn't such a big deal. The only things that should really be changing in a new API version the wire protocol stuff anyway. For very small changes, interleaving actions is feasible.

One of the few cases (IMHO) where an action can reasonably be inherited is something like DELETE, which will never change in behavior unless you remove the API. You're not using inheritance, but that appears a lot on these threads. Remember that you can't uninherit an inherited action method that should no longer be used. Most service taxonomies have a support policy like n-2 API versions. If you keep this policy under say five API versions, the CPR method may not be all that bad. More than that and it can be a pain. You also have to question why so many parallel API versions are needed.

That leads me to the final note of caution. While you can fallback, make sure you consider the consequences of removing an API version. Deprecating an API version doesn't remove it. The only way to remove it is - well - actually remove it. This very difficult to rationalize with fallbacks. API version ranges are not supported out-of-the-box for the same reason. Things need to be deterministic.

All that said, I think there definitely an opportunity to improve the service authoring experience.