Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

By namespace + RoutePrefix + AssumeDefaultVersionWhenUnspecified not working #73

Closed
Danielku15 opened this issue Jan 10, 2017 · 7 comments
Assignees
Milestone

Comments

@Danielku15
Copy link

TL;DR: It seems the the AssumeDefaultVersionWhenUnspecified is not working with RoutePrefix and ByNamespace versioning. It might be too complex for the routing system to handle this combinations but would be definitly great if it is supported.

Long Version:
Consider the ByNamespaceWebApiSample project as a starting point and let us combine it with attribute routing:

namespace Microsoft.Examples.V1.Controllers
{
    [ApiVersion( "1.0" )]
    [RoutePrefix( "v{version:apiVersion}/agreements" )]
    public class AgreementsController : ApiController { [Route("{accountId}")] public IHttpActionResult Get( string accountId ) ... }
}
namespace Microsoft.Examples.V2.Controllers
{
    [ApiVersion( "2.0" )]
    [RoutePrefix( "v{version:apiVersion}/agreements" )]
    public class AgreementsController : ApiController { [Route("{accountId}")] public IHttpActionResult Get( string accountId ) ... }
}
namespace Microsoft.Examples.V3.Controllers
{
    [ApiVersion( "3.0" )]
    [RoutePrefix( "v{version:apiVersion}/agreements" )]
    public class AgreementsController : ApiController { [Route("{accountId}")] public IHttpActionResult Get( string accountId ) ... }
}

And we change the Startup do use the attributes instead of fixed routes:

var constraintResolver = new DefaultInlineConstraintResolver() { ConstraintMap = { ["apiVersion"] = typeof(ApiVersionRouteConstraint) } };
var configuration = new HttpConfiguration();
var httpServer = new HttpServer( configuration );
configuration.AddApiVersioning(o =>
{
    o.ReportApiVersions = true;
    o.DefaultApiVersion = new ApiVersion(3, 0);
    o.AssumeDefaultVersionWhenUnspecified = true;
} );
configuration.MapHttpAttributeRoutes(constraintResolver);
builder.UseWebApi( httpServer );

Then it is possible to call the different versions via the explicit version:

  • v1/agreements/4711 -> {"Controller":"Microsoft.Examples.V1.Controllers.AgreementsController","AccountId":"test","ApiVersion":"1"}
  • v1.0/agreements/4711 -> {"Controller":"Microsoft.Examples.V1.Controllers.AgreementsController","AccountId":"test","ApiVersion":"1.0"}
  • v2/agreements/4711 -> {"Controller":"Microsoft.Examples.V2.Controllers.AgreementsController","AccountId":"test","ApiVersion":"2"}
  • v2.0/agreements/4711 -> {"Controller":"Microsoft.Examples.V2.Controllers.AgreementsController","AccountId":"test","ApiVersion":"2.0"}
  • v3/agreements/4711 -> {"Controller":"Microsoft.Examples.V3.Controllers.AgreementsController","AccountId":"test","ApiVersion":"3"}
  • v3.0/agreements/4711 -> {"Controller":"Microsoft.Examples.V3.Controllers.AgreementsController","AccountId":"test","ApiVersion":"3.0"}

But if you try to call the latest version by removing the version, you only get an error 404:

  • agreements/4711 -> HTTP Error 404.0 - Not Found instead of {"Controller":"Microsoft.Examples.V3.Controllers.AgreementsController","AccountId":"test","ApiVersion":"3.0"}

Is there a way to have this combination? From code perspective the by namespace is simply the cleanest way to versionize your API. We also prefer the url segment based versioning due to
various reasons (documentation with swagger and swagger UI, browser/html-form compatibility,...). But still we want to provide also a default endpoint for the current version.

@commonsensesoftware
Copy link
Collaborator

Implicitly matching an API version in the URL path is not supported. There is really no way that this ever could be supported. The path segment of the URL identifies the resource. I know it's a heated debate with some folks, but that is one of the consequences of implementing versioning that way. This also one of the main reasons the default behavior is to use the query string. The query portion of a URL does not identify a resource.

Also keep in mind that these libraries do not change routing in any way (many people think it does). These libraries let routing do what it always did and then disambiguates matching routes using the API version. The default behavior would have treated multiple route candidates as an ambiguous error made by the service author. In the case of URL path segment versioning, the route constraint is only used to verify that the API version specified is well-formed and lets the platform extract the requested API version without relying on messy, brittle URL string manipulation like regular expressions. It also gives service authors a lot more flexibility in how they define their route templates (ex: whether to use the "v" prefix).

There are a couple possible solutions to this problem:

  • Don't allow implicit versioning. This is probably the simplest and safest approaches. Clients will always know what version they are bound to, which is great for the principle of least astonishment. Remember that unless the implicit version always points to the same version (usually V1), there is a potential to break clients when things change.
  • Use URL rewriting. Using patterns, things always map to some static version; either the initial, implicit version or whatever version you to advance things to. You could always write custom code to determine which API version to forward to, but I suspect that is pretty involved.
  • Use multiple, overlapping routes. Just because you want to route using URL path segments, doesn't mean you can't also route using the query string. This would allow routing by both methods and would allow the implicitly assumed version to get picked up. You can't prevent a client from using one method over another because it's really up to them. You can, however, choose to not to document other use cases of the query string method aside from the default version case.

A couple of people have asked about this in the past. I should update the wiki to indicate this won't work out-of-the-box.

@Danielku15
Copy link
Author

Danielku15 commented Jan 11, 2017

Thanks for the response.

In general I agree: There are really mixed opinions on the version in the URL and whether it's proper RESTful. But I rather see the versionized URL as an API endpoint/entry point than part of the resource identifier. My thinking is: I versionize the API, not the resources.

Don't allow implicit versioning.
The idea of the implicit versioning is very useful for clients. If you develop an API client, you only need to retest if your client is still working with the new version or if there have been breaking changes that break your app. If you point your API client to a fixed version you will not benefit from enhancements that were done to the API. If there are multiple installations of your API host in the market, your client might even not work anymore since you pointed it to V1 and the new API host has removed a deprecated API. You need to release a new version of your API client but customers might not be able to upgrade since they don't have a license for the new software. So basically your whole API infrastructure becomes unusable. Beside that it's considered as a good practice to provide a default version endpoint. That's also what you provide exactly with the AssumeDefaultVersionWhenUnspecified but just as query parameter.

multiple, overlapping routes
I think I easily was able to solve the issue by creating a new IDirectRouteProvider. On startup while all routes are created based on the attributes, I simply check for all actions whether they are part of the "default version". If that's the case, I register an additional route without the URL segment containing the version.

Maybe this solution would be feasable for you to integrate it directly as option which does this setup automatically.

// startup: 
var defaultApiVersion = new ApiVersion(3, 0);
configuration.MapHttpAttributeRoutes(constraintResolver, new VersionAwareDirectRouteProvider(defaultApiVersion));
configuration.AddApiVersioning(o =>
{
    o.ReportApiVersions = true;
    o.DefaultApiVersion = defaultApiVersion;
    o.AssumeDefaultVersionWhenUnspecified = true;
});


// routeprovider
public class VersionAwareDirectRouteProvider : DefaultDirectRouteProvider
{
    private readonly ApiVersion _defaultVersion;

    public VersionAwareDirectRouteProvider(ApiVersion defaultVersion)
    {
        _defaultVersion = defaultVersion;
    }

    protected override IReadOnlyList<RouteEntry> GetActionDirectRoutes(HttpActionDescriptor actionDescriptor, IReadOnlyList<IDirectRouteFactory> factories,
        IInlineConstraintResolver constraintResolver)
    {
        // let the DefaultDirectRouteProvider create the normal routes based on all factories
        var routes = new List<RouteEntry>(base.GetActionDirectRoutes(actionDescriptor, factories, constraintResolver));

        // check if the current action is part of the default API version by looking at the ApiVersionAttributes defined
        var isDefaultVersionRoute = actionDescriptor.ControllerDescriptor.GetCustomAttributes<ApiVersionAttribute>()
            .Any(versions => versions.Versions.Any(v => v == _defaultVersion));

        if (isDefaultVersionRoute)
        {
            // find all routes for the action that have a version constraint
            var versionizedRoutes = routes.Where(r => r.Route.Constraints.Any(c => c.Value is ApiVersionRouteConstraint));
            foreach (var versionizedRoute in versionizedRoutes)
            {
                // separate the URL template into the path segments
                var chunks = versionizedRoute.Route.RouteTemplate.Split('/');

                // build the {version} string based on the route constraint name
                var versionConstraint = "{" + versionizedRoute.Route.Constraints.Where(c => c.Value is ApiVersionRouteConstraint).Select(c => c.Key).First() + "}";

                // join the segments again together but remove the chunks containing the URL constraint
                var newRouteTemplate = string.Join("/", chunks.Where(c => !c.Contains(versionConstraint)));

                // build a new route for the new URL template
                var context = new DirectRouteFactoryContext(string.Empty, new[] { actionDescriptor }, constraintResolver, false);
                var builder = context.CreateBuilder(newRouteTemplate);
                if (!string.IsNullOrEmpty(versionizedRoute.Name))
                {
                    builder.Name = versionizedRoute.Name + "DefaultVersion";
                }
                routes.Add(builder.Build());
            }

        }


        return routes;
    }
}

@commonsensesoftware
Copy link
Collaborator

In the purest sense, versioning is really referring to the versioning of a resource. Many REST authorities would likely assert that the only method for any type of versioning should be achieved using media type negotiation. This is something I'm not opposed to and is supported by the libraries, but unfortunately there is standard format for specifying a version parameter in a media type; therefore, this support doesn't exist directly out-of-the-box. I certainly won't advocate that versioning using any part of the URL is better, but it's certainly easier to consume.

We all generally accept the term API Versioning, but really we are talking about resource versioning - if we're doing REST. HTTP is the API (aka the Uniform Interface). The URL identifies a resource in the HTTP API. At least for me, years and years of RPC and Messaging-style platforms and frameworks forced me to do a lot of re-thinking and re-learning to move toward resource-oriented architectures like REST. You can do REST without HTTP and you can use HTTP without being RESTful. These libraries don't care, which approach you use. That's also why it's not called the REST API Versioning library. ;) I tried really hard to strike the right balance to embrace REST principles as closely as possible for those that want to go that path, but still make things are super easy if you just want to throw any 'ol HTTP endpoint up that uses more of an RPC or Message-style API (which I see a lot of).

I won't question your desire for implicit versioning, but I've seen it go sideways on teams many, many times. The call is always down to the service author and how they want to interact with their clients. Obviously, you know best. The biggest issue I've seen is that service authors assume that their clients implement tolerant readers and can therefore handle scenarios where the service adds new data members to a resource. Sadly, this is not always so. If you have a small client base or provide a client library that provides this tolerant readers, then it's a non-issue. Versioning changes aren't always additive either. You can't safely take things away in new a version without potentially breaking a client. As a service author, you could use logging or some other facility that tracks the API version of incoming requests. When you see the volume for a deprecated version drop to zero, then and only then, you can completely drop support. If you track or audit these requests, you can potentially contact the client before you shut off support and cause them to break. Again, these are all policies that only you can determine.

Your solution for implicit versioning using URL path segments is clever. From my cursory examination, it seems possible that this could be made into something more general purpose. I'll look at queuing up some work for this in the next milestone. One thing that you might not be aware of is that, there are built-in extension methods for retrieving the defined API version information. The infrastructure doesn't rely on attributes, that just happens to be one of the mechanisms by which you can get the information.

For more complete access to the information, you can revise your current code like this:

var apiModel = actionDescriptor.ControllerDescriptor.GetApiVersionModel();
var isDefaultVersionRoute = apiModel.ImplementedApiVersions.Contains( defaultVersion );
// ... omitted for brevity

There's nothing wrong with passing in the default API version, but you can get this from the configuration later like so:

var defaultApiVersion = actionDescriptor.Configuration.GetApiVersioningOptions().DefaultApiVersion;

It's also worth mentioning that any controller that has no versioning attribution and has not explicitly opted-out of API versioning using [ApiVersionNeutral] will also be assumed to be this default API version. I mention that because your current logic would miss this case. Of course for you, if you explicitly add versioning attributes to all your controllers (which I recommend), you never have this case or problem.

@Danielku15
Copy link
Author

I understand your opinion on RESTful versioning and that you rather version the resources than the API. It's a tricky (and strongly opinion based) topic. That's where you find that many discussions about that in the internet (https://www.troyhunt.com/your-api-versioning-is-wrong-which-is/ is a nice blog post with references). I believe that in long term this library can provide all concepts in parallel.

The tricky part will be to combine my proposed solution with a header based versioning #70 . If this ambiguity can also be solved, the client can really decide how he'd prefers to select the version: query string, url segment, header or default version.

We annotate all our controllers with an API version that's likely why I missed that controllers not having a version attribute automatically count to the CurrentVersion. It would be great if you can really adapt this solution into the library, otherwise we somehow need to maintain a mix of our custom versioning and aspnet-api-versioning within our applications.

@commonsensesoftware commonsensesoftware added this to the 1.1.0 milestone Feb 22, 2017
@commonsensesoftware
Copy link
Collaborator

I've been re-evaluating this issue. While your solution is elegant, I can't seem to replicate feature parity in ASP.NET Core. Having the same OOB capabilities in both implementations is important. I suspect this is likely due to the changes in routing.

I was, however, able to create an alternate solution that seems to get you what you want. Let's assume you are using the same configuration as above. You can then have a controller defined like this:

namespace Microsoft.Examples.V3.Controllers
{
    [ApiVersion( "3.0" )]
    public class AgreementsController : ApiController
    {
      [Route( "agreements" )]
      [Route( "v{version:apiVersion}/agreements" )]
      public IHttpActionResult Get() => Ok();

      [Route( "agreements/{accountId}" )]
      [Route( "v{version:apiVersion}/agreements/{accountId}" )]
      public IHttpActionResult Get( string accountId ) => Ok( accountId );
    }
}

Unfortunately, in Web API the RoutePrefixAttribute can only be applied once, which means in order to achieve the route setup, you would have to apply the route template multiple times. You only need to do this on the current version controller. Since the default, configured API version is 3.0, the following routes will all match the version 3.0 routes:

  • /v3/agreements
  • /v3/agreements/42
  • /agreements
  • /agreements/42

In this particular case, it might be easier to manage the route definitions using conventions instead because every time you create a new version, you need to move the current version routes to the new controller class. Mixing and matching conventions and direct routes (aka attribute routing) may work, but due to limitations in the Web API infrastructure that I can't control or fix, there are some scenarios were it won't work. I can't thing of the exact setup that will cause this, but I've encountered it numerous times; particularly in supporting OData (which only uses conventions). Regardless of which one of these variants you choose, both will give you stronger alignment to ASP.NET Core whenever you make the switch.

If you're happy with your current approach, there's nothing wrong with it. I just don't see a way to carry it forward to ASP.NET Core. I want to do everything possible to keep the two platforms aligned so that devs do not have to relearn everything.

@Danielku15
Copy link
Author

I stumbled over a similar problem when I was preparing a pull request with my requested feature. The APIs in this area are a bit inconsistent (or follow different concepts). This is why I implemented a custom nuget package on top of this framework to get those missing features in while focusing only on traditional Web API.

https://github.com/Danielku15/aspnet-url-versioning

We are using it since a while and it works perfectly for our case (including ApiExplorer/Swagger/Swashbuckle support).

@commonsensesoftware
Copy link
Collaborator

Sounds like you're set on this issue, so I'm going to close it out. Thanks for all the feedback and in-depth discussion. I'm close to having the 1.1.0 release ready. Once that's out, I'll have some bandwidth freed up to look back at various documentation support. I'll definitely be giving your work a look to see what bits may be able to be incorporated.

Let me know if you need anything else. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants