User-defined and out-of-the-box OData controllers now return 400 instead of 404 when a route could match, but doesn't for a specific API version. Fixes #17 (#21)

Author: commonsensesoftware

src/Microsoft.AspNet.OData.Versioning/System.Web.OData/HttpConfigurationExtensions.cs renamed to src/Microsoft.AspNet.OData.Versioning/System.Web.Http/HttpConfigurationExtensions.cs

@@ -8,20 +8,27 @@
     using Microsoft;
     using Microsoft.OData.Edm;
     using Microsoft.Web.Http;
+    using Microsoft.Web.Http.Routing;
     using Microsoft.Web.OData.Builder;
     using Microsoft.Web.OData.Routing;
     using OData.Batch;
     using OData.Extensions;
     using OData.Routing;
     using OData.Routing.Conventions;
+    using Routing;
     using static Linq.Expressions.Expression;
+    using static System.String;
+    using static System.StringComparison;
 
     /// <summary>
     /// Provides extension methods for the <see cref="HttpConfiguration"/> class.
     /// </summary>
     public static class HttpConfigurationExtensions
     {
         private const string ResolverSettingsKey = "System.Web.OData.ResolverSettingsKey";
+        private const string UnversionedRouteSuffix = "-Unversioned";
+        private const string ApiVersionConstraintName = "apiVersion";
+        private const string ApiVersionConstraint = "{" + ApiVersionConstraintName + "}";
         private static readonly Lazy<Action<DefaultODataPathHandler, object>> setResolverSettings = new Lazy<Action<DefaultODataPathHandler, object>>( GetResolverSettingsMutator );
 
         private static Action<DefaultODataPathHandler, object> GetResolverSettingsMutator()
@@ -192,46 +199,42 @@ public static IReadOnlyList<ODataRoute> MapVersionedODataRoutes(
             var routeConventions = EnsureConventions( routingConventions.ToList() );
             var routes = configuration.Routes;
 
-            if ( !string.IsNullOrEmpty( routePrefix ) )
+            if ( !IsNullOrEmpty( routePrefix ) )
             {
                 routePrefix = routePrefix.TrimEnd( '/' );
             }
 
             if ( batchHandler != null )
             {
-                var batchTemplate = string.IsNullOrEmpty( routePrefix ) ? ODataRouteConstants.Batch : routePrefix + '/' + ODataRouteConstants.Batch;
+                var batchTemplate = IsNullOrEmpty( routePrefix ) ? ODataRouteConstants.Batch : routePrefix + '/' + ODataRouteConstants.Batch;
                 routes.MapHttpBatchRoute( routeName + "Batch", batchTemplate, batchHandler );
             }
 
             configuration.SetResolverSettings( pathHandler );
             routeConventions.Insert( 0, null );
 
             var odataRoutes = new List<ODataRoute>();
+            var unversionedConstraints = new List<IHttpRouteConstraint>();
 
             foreach ( var model in models )
             {
                 var versionedRouteName = routeName;
-                var apiVersion = model.GetAnnotationValue<ApiVersionAnnotation>( model )?.ApiVersion;
                 var routeConstraint = default( ODataPathRouteConstraint );
 
                 routeConventions[0] = new VersionedAttributeRoutingConvention( model, configuration );
-
-                if ( apiVersion == null )
-                {
-                    routeConstraint = new ODataPathRouteConstraint( pathHandler, model, versionedRouteName, routeConventions.ToArray() );
-                }
-                else
-                {
-                    versionedRouteName += "-" + apiVersion.ToString();
-                    routeConstraint = new VersionedODataPathRouteConstraint( pathHandler, model, versionedRouteName, routeConventions.ToArray(), apiVersion );
-                }
+                routeConstraint = new ODataPathRouteConstraint( pathHandler, model, versionedRouteName, routeConventions.ToArray() );
+                unversionedConstraints.Add( routeConstraint );
+                routeConstraint = MakeVersionedODataRouteConstraint( routeConstraint, pathHandler, routeConventions, model, ref versionedRouteName );
 
                 var route = new ODataRoute( routePrefix, routeConstraint );
 
+                AddApiVersionConstraintIfNecessary( route );
                 routes.Add( versionedRouteName, route );
                 odataRoutes.Add( route );
             }
 
+            AddRouteToRespondWithBadRequestWhenAtLeastOneRouteCouldMatch( routeName, routePrefix, routes, odataRoutes, unversionedConstraints );
+
             return odataRoutes;
         }
 
@@ -329,14 +332,14 @@ public static ODataRoute MapVersionedODataRoute(
             var routeConventions = EnsureConventions( routingConventions.ToList() );
             var routes = configuration.Routes;
 
-            if ( !string.IsNullOrEmpty( routePrefix ) )
+            if ( !IsNullOrEmpty( routePrefix ) )
             {
                 routePrefix = routePrefix.TrimEnd( '/' );
             }
 
             if ( batchHandler != null )
             {
-                var batchTemplate = string.IsNullOrEmpty( routePrefix ) ? ODataRouteConstants.Batch : routePrefix + '/' + ODataRouteConstants.Batch;
+                var batchTemplate = IsNullOrEmpty( routePrefix ) ? ODataRouteConstants.Batch : routePrefix + '/' + ODataRouteConstants.Batch;
                 routes.MapHttpBatchRoute( routeName + "Batch", batchTemplate, batchHandler );
             }
 
@@ -347,9 +350,80 @@ public static ODataRoute MapVersionedODataRoute(
             var routeConstraint = new VersionedODataPathRouteConstraint( pathHandler, model, routeName, routeConventions.ToArray(), apiVersion );
             var route = new ODataRoute( routePrefix, routeConstraint );
 
+            AddApiVersionConstraintIfNecessary( route );
             routes.Add( routeName, route );
 
+            var unversionedRouteConstraint = new ODataPathRouteConstraint( pathHandler, model, routeName, routeConventions.ToArray() );
+            var unversionedRoute = new ODataRoute( routePrefix, new UnversionedODataPathRouteConstraint( unversionedRouteConstraint, apiVersion ) );
+
+            AddApiVersionConstraintIfNecessary( unversionedRoute );
+            routes.Add( routeName + UnversionedRouteSuffix, unversionedRoute );
+
             return route;
         }
+
+        private static ODataPathRouteConstraint MakeVersionedODataRouteConstraint(
+            ODataPathRouteConstraint routeConstraint,
+            IODataPathHandler pathHandler,
+            IList<IODataRoutingConvention> routeConventions,
+            IEdmModel model,
+            ref string versionedRouteName )
+        {
+            Contract.Requires( routeConstraint != null );
+            Contract.Requires( pathHandler != null );
+            Contract.Requires( routeConventions != null );
+            Contract.Requires( model != null );
+            Contract.Requires( !IsNullOrEmpty( versionedRouteName ) );
+            Contract.Ensures( Contract.Result<ODataPathRouteConstraint>() != null );
+
+            var apiVersion = model.GetAnnotationValue<ApiVersionAnnotation>( model )?.ApiVersion;
+
+            if ( apiVersion == null )
+            {
+                return routeConstraint;
+            }
+
+            versionedRouteName += "-" + apiVersion.ToString();
+            return new VersionedODataPathRouteConstraint( pathHandler, model, versionedRouteName, routeConventions.ToArray(), apiVersion );
+        }
+
+        private static void AddApiVersionConstraintIfNecessary( ODataRoute route )
+        {
+            Contract.Requires( route != null );
+
+            var routePrefix = route.RoutePrefix;
+
+            if ( routePrefix == null || routePrefix.IndexOf( ApiVersionConstraint, Ordinal ) < 0 || route.Constraints.ContainsKey( ApiVersionConstraintName ) )
+            {
+                return;
+            }
+
+            // note: even though the constraints are a dictionary, it's important to rebuild the entire collection
+            // to make sure the api version constraint is evaluated first; otherwise, the current api version will
+            // not be resolved when the odata versioning constraint is evaluated
+            var originalConstraints = new Dictionary<string, object>( route.Constraints );
+
+            route.Constraints.Clear();
+            route.Constraints.Add( ApiVersionConstraintName, new ApiVersionRouteConstraint() );
+
+            foreach ( var constraint in originalConstraints )
+            {
+                route.Constraints.Add( constraint.Key, constraint.Value );
+            }
+        }
+
+        private static void AddRouteToRespondWithBadRequestWhenAtLeastOneRouteCouldMatch( string routeName, string routePrefix, HttpRouteCollection routes, List<ODataRoute> odataRoutes, List<IHttpRouteConstraint> unversionedConstraints )
+        {
+            Contract.Requires( !IsNullOrEmpty( routeName ) );
+            Contract.Requires( routes != null );
+            Contract.Requires( odataRoutes != null );
+            Contract.Requires( unversionedConstraints != null );
+
+            var unversionedRoute = new ODataRoute( routePrefix, new UnversionedODataPathRouteConstraint( unversionedConstraints ) );
+
+            AddApiVersionConstraintIfNecessary( unversionedRoute );
+            routes.Add( routeName + UnversionedRouteSuffix, unversionedRoute );
+            odataRoutes.Add( unversionedRoute );
+        }
     }
 }

src/Microsoft.AspNet.OData.Versioning/System.Web.Http/HttpRequestMessageExtensions.cs

@@ -0,0 +1,27 @@
+namespace System.Web.Http
+{
+    using Diagnostics.Contracts;
+    using Microsoft.OData.Core;
+    using Microsoft.Web.Http;
+    using Microsoft.Web.Http.Versioning;
+    using System.Net.Http;
+    using static System.Net.HttpStatusCode;
+
+    internal static class HttpRequestMessageExtensions
+    {
+        internal static ApiVersion GetRequestedApiVersionOrReturnBadRequest( this HttpRequestMessage request )
+        {
+            Contract.Requires( request != null );
+
+            try
+            {
+                return request.GetRequestedApiVersion();
+            }
+            catch ( AmbiguousApiVersionException ex )
+            {
+                var error = new ODataError() { ErrorCode = "AmbiguousApiVersion", Message = ex.Message };
+                throw new HttpResponseException( request.CreateResponse( BadRequest, error ) );
+            }
+        }
+    }
+}

src/Microsoft.AspNet.OData.Versioning/Web.OData/Builder/VersionedODataModelBuilder.cs

@@ -1,6 +1,8 @@
 namespace Microsoft.Web.OData.Builder
 {
+    using Controllers;
     using Http;
+    using Http.Versioning.Conventions;
     using Microsoft.OData.Edm;
     using System;
     using System.Collections.Generic;
@@ -102,21 +104,31 @@ public virtual IEnumerable<IEdmModel> GetEdmModels()
             var typeResolver = services.GetHttpControllerTypeResolver();
             var controllerTypes = typeResolver.GetControllerTypes( assembliesResolver ).Where( c => c.IsODataController() );
             var options = configuration.GetApiVersioningOptions();
-            var apiVersions = new HashSet<ApiVersion>();
+            var supported = new HashSet<ApiVersion>();
+            var deprecated = new HashSet<ApiVersion>();
 
             foreach ( var controllerType in controllerTypes )
             {
                 var descriptor = new HttpControllerDescriptor( configuration, string.Empty, controllerType );
 
                 options.Conventions.ApplyTo( descriptor );
 
-                foreach ( var apiVersion in descriptor.GetImplementedApiVersions() )
+                var model = descriptor.GetApiVersionModel();
+
+                foreach ( var apiVersion in model.SupportedApiVersions )
                 {
-                    apiVersions.Add( apiVersion );
+                    supported.Add( apiVersion );
+                }
+
+                foreach ( var apiVersion in model.DeprecatedApiVersions )
+                {
+                    deprecated.Add( apiVersion );
                 }
             }
 
-            foreach ( var apiVersion in apiVersions )
+            deprecated.ExceptWith( supported );
+
+            foreach ( var apiVersion in supported.Union( deprecated ) )
             {
                 var builder = ModelBuilderFactory();
 
@@ -132,7 +144,35 @@ public virtual IEnumerable<IEdmModel> GetEdmModels()
                 models.Add( model );
             }
 
+            ConfigureMetadataController( configuration, supported, deprecated );
+
             return models;
         }
+
+        /// <summary>
+        /// Configures the metadata controller using the specified configuration and API versions.
+        /// </summary>
+        /// <param name="configuration">The current <see cref="HttpConfiguration">configuration</see>.</param>
+        /// <param name="supportedApiVersions">The discovered <see cref="IEnumerable{T}">sequence</see> of
+        /// supported OData controller <see cref="ApiVersion">API versions</see>.</param>
+        /// <param name="deprecatedApiVersions">The discovered <see cref="IEnumerable{T}">sequence</see> of
+        /// deprecated OData controller <see cref="ApiVersion">API versions</see>.</param>
+        protected virtual void ConfigureMetadataController( HttpConfiguration configuration, IEnumerable<ApiVersion> supportedApiVersions, IEnumerable<ApiVersion> deprecatedApiVersions )
+        {
+            var controllerMapping = configuration.Services.GetHttpControllerSelector().GetControllerMapping();
+            var controllerDescriptor = default( HttpControllerDescriptor );
+
+            if ( !controllerMapping.TryGetValue( "VersionedMetadata", out controllerDescriptor ))
+            {
+                return;
+            }
+
+            var options = configuration.GetApiVersioningOptions();
+            var controllerBuilder = options.Conventions.Controller<VersionedMetadataController>()
+                                                       .HasApiVersions( supportedApiVersions )
+                                                       .HasDeprecatedApiVersions( deprecatedApiVersions );
+
+            controllerBuilder.ApplyTo( controllerDescriptor );
+        }
     }
 }