Add OData query option support for non-OData actions. Resolves #522

Chris Martinez · commonsensesoftware · commit d380666fd71e · 2019-10-30T08:56:36.000-07:00
diff --git a/src/Common.OData.ApiExplorer/AspNet.OData/Builder/DefaultODataQueryOptionDescriptionProvider.cs b/src/Common.OData.ApiExplorer/AspNet.OData/Builder/DefaultODataQueryOptionDescriptionProvider.cs
@@ -30,26 +30,18 @@ public virtual string Describe( AllowedQueryOptions queryOption, ODataQueryOptio
                 throw new ArgumentException( SR.MultipleQueryOptionsNotAllowed, nameof( queryOption ) );
             }
 
-            switch ( queryOption )
+            return queryOption switch
             {
-                case Filter:
-                    return DescribeFilter( context );
-                case Expand:
-                    return DescribeExpand( context );
-                case Select:
-                    return DescribeSelect( context );
-                case OrderBy:
-                    return DescribeOrderBy( context );
-                case Top:
-                    return DescribeTop( context );
-                case Skip:
-                    return DescribeSkip( context );
-                case Count:
-                    return DescribeCount( context );
-            }
-
+                Filter => DescribeFilter( context ),
+                Expand => DescribeExpand( context ),
+                Select => DescribeSelect( context ),
+                OrderBy => DescribeOrderBy( context ),
+                Top => DescribeTop( context ),
+                Skip => DescribeSkip( context ),
+                Count => DescribeCount( context ),
 #pragma warning disable CA1308 // Normalize strings to uppercase
-            throw new ArgumentException( SR.UnsupportedQueryOption.FormatDefault( queryOption.ToString().ToLowerInvariant() ), nameof( queryOption ) );
+                _ => throw new ArgumentException( SR.UnsupportedQueryOption.FormatDefault( queryOption.ToString().ToLowerInvariant() ), nameof( queryOption ) ),
+            };
 #pragma warning restore CA1308
         }
 
diff --git a/src/Common.OData.ApiExplorer/AspNet.OData/Builder/ODataActionQueryOptionsConventionBuilderExtensions.cs b/src/Common.OData.ApiExplorer/AspNet.OData/Builder/ODataActionQueryOptionsConventionBuilderExtensions.cs
@@ -137,7 +137,7 @@ public static ODataActionQueryOptionsConventionBuilder Action( this IODataAction
                     return builder.Action( methods[0] );
             }
 
-            argumentTypes = argumentTypes ?? Type.EmptyTypes;
+            argumentTypes ??= Type.EmptyTypes;
             methods = methods.Where( m => SignatureMatches( m, argumentTypes ) ).ToArray();
 
             if ( methods.Length == 1 )
diff --git a/src/Common.OData.ApiExplorer/AspNet.OData/Builder/ODataAttributeVisitor.cs b/src/Common.OData.ApiExplorer/AspNet.OData/Builder/ODataAttributeVisitor.cs
@@ -68,28 +68,6 @@ internal void Visit( ApiDescription apiDescription )
             }
         }
 
-        static void ClearCollectionIfItContainsAllProperties( IList<string> list, ISet<string> set )
-        {
-            Contract.Requires( list != null );
-            Contract.Requires( set != null );
-
-            if ( list.Count != set.Count )
-            {
-                return;
-            }
-
-            for ( var i = 0; i < list.Count; i++ )
-            {
-                if ( !set.Contains( list[i] ) )
-                {
-                    return;
-                }
-            }
-
-            // avoid verbose messages by clearing the list when it contains all possible properties
-            list.Clear();
-        }
-
         void VisitModel( IEdmStructuredType modelType )
         {
             Contract.Requires( modelType != null );
diff --git a/src/Common.OData.ApiExplorer/AspNet.OData/Builder/ODataQueryOptionsConventionBuilder.cs b/src/Common.OData.ApiExplorer/AspNet.OData/Builder/ODataQueryOptionsConventionBuilder.cs
@@ -137,13 +137,13 @@ public virtual void ApplyTo( IEnumerable<ApiDescription> apiDescriptions, ODataQ
             {
                 var controller = GetController( description );
 
-                if ( !controller.IsODataController() )
-                {
-                    continue;
-                }
-
                 if ( !conventions.TryGetValue( controller, out var convention ) )
                 {
+                    if ( !controller.IsODataController() && !IsODataLike( description ) )
+                    {
+                        continue;
+                    }
+
                     if ( !ConventionBuilders.TryGetValue( controller, out var builder ) )
                     {
                         builder = new ODataControllerQueryOptionsConventionBuilder( controller );
diff --git a/src/Common.OData.ApiExplorer/AspNet.OData/Routing/ODataRouteBuilder.cs b/src/Common.OData.ApiExplorer/AspNet.OData/Routing/ODataRouteBuilder.cs
@@ -354,7 +354,7 @@ string FixUpArrayParameters( string template, IEdmOperation operation )
                 return template;
             }
 
-            int IndexOfToken( StringBuilder builder, string token )
+            static int IndexOfToken( StringBuilder builder, string token )
             {
                 var index = -1;
 
@@ -393,7 +393,7 @@ int IndexOfToken( StringBuilder builder, string token )
                 return index;
             }
 
-            void InsertBrackets( StringBuilder builder, string token )
+            static void InsertBrackets( StringBuilder builder, string token )
             {
                 var index = IndexOfToken( builder, token );
 
diff --git a/src/Common.OData.ApiExplorer/AspNet.OData/StructuredTypeResolver.cs b/src/Common.OData.ApiExplorer/AspNet.OData/StructuredTypeResolver.cs
@@ -16,6 +16,11 @@ internal IEdmStructuredType GetStructuredType( Type type )
         {
             Contract.Requires( type != null );
 
+            if ( model == null )
+            {
+                return default;
+            }
+
             var structuredTypes = model.SchemaElements.OfType<IEdmStructuredType>();
             var structuredType = structuredTypes.FirstOrDefault( t => type.Equals( t.GetClrType( model ) ) );
 
diff --git a/src/Microsoft.AspNet.OData.Versioning.ApiExplorer/AspNet.OData/Builder/ODataQueryOptionsConventionBuilder.cs b/src/Microsoft.AspNet.OData.Versioning.ApiExplorer/AspNet.OData/Builder/ODataQueryOptionsConventionBuilder.cs
@@ -10,6 +10,10 @@ public partial class ODataQueryOptionsConventionBuilder
     {
         static Type GetKey( Type type ) => type;
 
-        static Type GetController( ApiDescription apiDescription ) => apiDescription.ActionDescriptor.ControllerDescriptor.ControllerType;
+        static Type GetController( ApiDescription apiDescription ) =>
+            apiDescription.ActionDescriptor.ControllerDescriptor.ControllerType;
+
+        static bool IsODataLike( ApiDescription description ) =>
+            description.ActionDescriptor.GetCustomAttributes<EnableQueryAttribute>( inherit: true ).Count > 0;
     }
 }
diff --git a/src/Microsoft.AspNet.OData.Versioning.ApiExplorer/AspNet.OData/Builder/ODataValidationSettingsConvention.cs b/src/Microsoft.AspNet.OData.Versioning.ApiExplorer/AspNet.OData/Builder/ODataValidationSettingsConvention.cs
@@ -157,17 +157,12 @@ static bool IsSupported( ApiDescription apiDescription )
         {
             Contract.Requires( apiDescription != null );
 
-            switch ( apiDescription.HttpMethod.Method.ToUpperInvariant() )
+            return apiDescription.HttpMethod.Method.ToUpperInvariant() switch
             {
-                case "GET":
-                    // query or function
-                    return true;
-                case "POST":
-                    // action
-                    return apiDescription.Operation()?.IsAction() == true;
-            }
-
-            return false;
+                "GET" => true,
+                "POST" => apiDescription.Operation()?.IsAction() == true,
+                _ => false,
+            };
         }
     }
 }
diff --git a/src/Microsoft.AspNetCore.OData.Versioning.ApiExplorer/AspNet.OData/Builder/ODataQueryOptionsConventionBuilder.cs b/src/Microsoft.AspNetCore.OData.Versioning.ApiExplorer/AspNet.OData/Builder/ODataQueryOptionsConventionBuilder.cs
@@ -1,9 +1,7 @@
 ﻿namespace Microsoft.AspNet.OData.Builder
 {
-    using Microsoft.AspNet.OData.Query;
     using Microsoft.AspNetCore.Mvc.ApiExplorer;
     using Microsoft.AspNetCore.Mvc.Controllers;
-    using Microsoft.AspNetCore.Mvc.ModelBinding;
     using System;
     using System.Reflection;
 
@@ -24,5 +22,15 @@ static TypeInfo GetController( ApiDescription apiDescription )
 
             return typeof( object ).GetTypeInfo();
         }
+
+        static bool IsODataLike( ApiDescription description )
+        {
+            if ( description.ActionDescriptor is ControllerActionDescriptor action )
+            {
+                return Attribute.IsDefined( action.ControllerTypeInfo, typeof( EnableQueryAttribute ), inherit: true );
+            }
+
+            return false;
+        }
     }
 }
diff --git a/src/Microsoft.AspNetCore.OData.Versioning.ApiExplorer/AspNet.OData/Builder/ODataValidationSettingsConvention.cs b/src/Microsoft.AspNetCore.OData.Versioning.ApiExplorer/AspNet.OData/Builder/ODataValidationSettingsConvention.cs
@@ -165,17 +165,12 @@ static bool IsSupported( ApiDescription apiDescription )
         {
             Contract.Requires( apiDescription != null );
 
-            switch ( apiDescription.HttpMethod.ToUpperInvariant() )
+            return apiDescription.HttpMethod.ToUpperInvariant() switch
             {
-                case "GET":
-                    // query or function
-                    return true;
-                case "POST":
-                    // action
-                    return apiDescription.Operation()?.IsAction() == true;
-            }
-
-            return false;
+                "GET" => true,
+                "POST" => apiDescription.Operation()?.IsAction() == true,
+                _ => false,
+            };
         }
     }
 }
diff --git a/src/Microsoft.AspNetCore.OData.Versioning.ApiExplorer/AspNetCore.Mvc.ApiExplorer/ODataApiDescriptionProvider.cs b/src/Microsoft.AspNetCore.OData.Versioning.ApiExplorer/AspNetCore.Mvc.ApiExplorer/ODataApiDescriptionProvider.cs
@@ -330,18 +330,6 @@ static Type GetDeclaredReturnType( ControllerActionDescriptor action )
 
         static Type GetRuntimeReturnType( Type declaredReturnType ) => declaredReturnType == typeof( object ) ? default : declaredReturnType;
 
-        static IReadOnlyList<IApiRequestMetadataProvider> GetRequestMetadataAttributes( ControllerActionDescriptor action )
-        {
-            Contract.Requires( action != null );
-
-            if ( action.FilterDescriptors == null )
-            {
-                return default;
-            }
-
-            return action.FilterDescriptors.Select( fd => fd.Filter ).OfType<IApiRequestMetadataProvider>().ToArray();
-        }
-
         static IReadOnlyList<IApiResponseMetadataProvider> GetResponseMetadataAttributes( ControllerActionDescriptor action )
         {
             Contract.Requires( action != null );
@@ -378,7 +366,6 @@ IEnumerable<ApiDescription> NewODataApiDescriptions( ControllerActionDescriptor
             Contract.Requires( mapping != null );
             Contract.Ensures( Contract.Result<ApiDescription>() != null );
 
-            var requestMetadataAttributes = GetRequestMetadataAttributes( action );
             var responseMetadataAttributes = GetResponseMetadataAttributes( action );
             var declaredReturnType = GetDeclaredReturnType( action );
             var runtimeReturnType = GetRuntimeReturnType( declaredReturnType );

Original file line number	Diff line number	Diff line change
`@@ -137,7 +137,7 @@ public static ODataActionQueryOptionsConventionBuilder Action( this IODataAction`
`137`	`137`	`return builder.Action( methods[0] );`
`138`	`138`	`}`
`139`	`139`
`140`		`- argumentTypes = argumentTypes ?? Type.EmptyTypes;`
	`140`	`+ argumentTypes ??= Type.EmptyTypes;`
`141`	`141`	`methods = methods.Where( m => SignatureMatches( m, argumentTypes ) ).ToArray();`
`142`	`142`
`143`	`143`	`if ( methods.Length == 1 )`
Original file line number	Diff line number	Diff line change
`@@ -137,13 +137,13 @@ public virtual void ApplyTo( IEnumerable<ApiDescription> apiDescriptions, ODataQ`
`137`	`137`	`{`
`138`	`138`	`var controller = GetController( description );`
`139`	`139`
`140`		`- if ( !controller.IsODataController() )`
`141`		`- {`
`142`		`- continue;`
`143`		`- }`
`144`		`-`
`145`	`140`	`if ( !conventions.TryGetValue( controller, out var convention ) )`
`146`	`141`	`{`
	`142`	`+ if ( !controller.IsODataController() && !IsODataLike( description ) )`
	`143`	`+ {`
	`144`	`+ continue;`
	`145`	`+ }`
	`146`	`+`
`147`	`147`	`if ( !ConventionBuilders.TryGetValue( controller, out var builder ) )`
`148`	`148`	`{`
`149`	`149`	`builder = new ODataControllerQueryOptionsConventionBuilder( controller );`
Original file line number	Diff line number	Diff line change
`@@ -354,7 +354,7 @@ string FixUpArrayParameters( string template, IEdmOperation operation )`
`354`	`354`	`return template;`
`355`	`355`	`}`
`356`	`356`
`357`		`- int IndexOfToken( StringBuilder builder, string token )`
	`357`	`+ static int IndexOfToken( StringBuilder builder, string token )`
`358`	`358`	`{`
`359`	`359`	`var index = -1;`
`360`	`360`
`@@ -393,7 +393,7 @@ int IndexOfToken( StringBuilder builder, string token )`
`393`	`393`	`return index;`
`394`	`394`	`}`
`395`	`395`
`396`		`- void InsertBrackets( StringBuilder builder, string token )`
	`396`	`+ static void InsertBrackets( StringBuilder builder, string token )`
`397`	`397`	`{`
`398`	`398`	`var index = IndexOfToken( builder, token );`
`399`	`399`
Original file line number	Diff line number	Diff line change
`@@ -10,6 +10,10 @@ public partial class ODataQueryOptionsConventionBuilder`
`10`	`10`	`{`
`11`	`11`	`static Type GetKey( Type type ) => type;`
`12`	`12`
`13`		`- static Type GetController( ApiDescription apiDescription ) => apiDescription.ActionDescriptor.ControllerDescriptor.ControllerType;`
	`13`	`+ static Type GetController( ApiDescription apiDescription ) =>`
	`14`	`+ apiDescription.ActionDescriptor.ControllerDescriptor.ControllerType;`
	`15`	`+`
	`16`	`+ static bool IsODataLike( ApiDescription description ) =>`
	`17`	`+ description.ActionDescriptor.GetCustomAttributes<EnableQueryAttribute>( inherit: true ).Count > 0;`
`14`	`18`	`}`
`15`	`19`	`}`
Original file line number	Diff line number	Diff line change
`@@ -157,17 +157,12 @@ static bool IsSupported( ApiDescription apiDescription )`
`157`	`157`	`{`
`158`	`158`	`Contract.Requires( apiDescription != null );`
`159`	`159`
`160`		`- switch ( apiDescription.HttpMethod.Method.ToUpperInvariant() )`
	`160`	`+ return apiDescription.HttpMethod.Method.ToUpperInvariant() switch`
`161`	`161`	`{`
`162`		`- case "GET":`
`163`		`- // query or function`
`164`		`- return true;`
`165`		`- case "POST":`
`166`		`- // action`
`167`		`- return apiDescription.Operation()?.IsAction() == true;`
`168`		`- }`
`169`		`-`
`170`		`- return false;`
	`162`	`+ "GET" => true,`
	`163`	`+ "POST" => apiDescription.Operation()?.IsAction() == true,`
	`164`	`+ _ => false,`
	`165`	`+ };`
`171`	`166`	`}`
`172`	`167`	`}`
`173`	`168`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,9 +1,7 @@`
`1`	`1`	`namespace Microsoft.AspNet.OData.Builder`
`2`	`2`	`{`
`3`		`- using Microsoft.AspNet.OData.Query;`
`4`	`3`	`using Microsoft.AspNetCore.Mvc.ApiExplorer;`
`5`	`4`	`using Microsoft.AspNetCore.Mvc.Controllers;`
`6`		`- using Microsoft.AspNetCore.Mvc.ModelBinding;`
`7`	`5`	`using System;`
`8`	`6`	`using System.Reflection;`
`9`	`7`
`@@ -24,5 +22,15 @@ static TypeInfo GetController( ApiDescription apiDescription )`
`24`	`22`
`25`	`23`	`return typeof( object ).GetTypeInfo();`
`26`	`24`	`}`
	`25`	`+`
	`26`	`+ static bool IsODataLike( ApiDescription description )`
	`27`	`+ {`
	`28`	`+ if ( description.ActionDescriptor is ControllerActionDescriptor action )`
	`29`	`+ {`
	`30`	`+ return Attribute.IsDefined( action.ControllerTypeInfo, typeof( EnableQueryAttribute ), inherit: true );`
	`31`	`+ }`
	`32`	`+`
	`33`	`+ return false;`
	`34`	`+ }`
`27`	`35`	`}`
`28`	`36`	`}`
Original file line number	Diff line number	Diff line change
`@@ -165,17 +165,12 @@ static bool IsSupported( ApiDescription apiDescription )`
`165`	`165`	`{`
`166`	`166`	`Contract.Requires( apiDescription != null );`
`167`	`167`
`168`		`- switch ( apiDescription.HttpMethod.ToUpperInvariant() )`
	`168`	`+ return apiDescription.HttpMethod.ToUpperInvariant() switch`
`169`	`169`	`{`
`170`		`- case "GET":`
`171`		`- // query or function`
`172`		`- return true;`
`173`		`- case "POST":`
`174`		`- // action`
`175`		`- return apiDescription.Operation()?.IsAction() == true;`
`176`		`- }`
`177`		`-`
`178`		`- return false;`
	`170`	`+ "GET" => true,`
	`171`	`+ "POST" => apiDescription.Operation()?.IsAction() == true,`
	`172`	`+ _ => false,`
	`173`	`+ };`
`179`	`174`	`}`
`180`	`175`	`}`
`181`	`176`	`}`