Optional method parameters not worked on simple action methods GET/POST/...

[GitHub-TC]

If i defined a single method for GET in the ApiVersioning/SwaggerODataSample public SingleResult<Product> Get( [FromQuery] int? dummy = 1 ) ... i will get the result { "error": { "code": "UnsupportedApiVersion", "message": "The HTTP resource that matches the request URI 'http://localhost:59918/api/Products' does not support the API version '3.0'.", "innerError": null } } when i called http://localhost:59918/api/Products?api-version=3.0

for a function public SingleResult<Product> Read( [FromQuery] int? dummy = null )... with the configuration var fctRead = product.Collection.Function( "Read" ); fctRead.Parameter<int?>( "dummy" ).Optional(); fctRead.ReturnsFromEntitySet<Product>( "Products" );

it will work.

Reason: For the action there is no ODataRouteConstants.OptionalParameters set in the RouteData.Values when i fake it with a custom IODataRoutingConvention handler the action GET works fine.

Here is my simple routing convention handler

using Microsoft.AspNet.OData.Routing; 
using Microsoft.AspNet.OData.Routing.Conventions;
using Microsoft.AspNetCore.Mvc.Controllers;
using Microsoft.AspNetCore.Routing;
using Microsoft.OData.Edm;
using System;
using System.Collections.Generic;
using System.Linq;
 
namespace Microsoft.Examples
{
    public class OptionalParameterCustomConvention : EntitySetRoutingConvention
    {
        class CustomOptionalParameter : IEdmOptionalParameter
        {
            public string DefaultValueString { get; set; }
            public IEdmTypeReference Type { get; set; }
            public IEdmOperation DeclaringOperation { get; set; }
            public string Name { get; set; }
        }
        public override string SelectAction( RouteContext routeContext, SelectControllerResult controllerResult, IEnumerable<ControllerActionDescriptor> actionDescriptors )
        {
            if ( !routeContext.HttpContext.Request.Path.Value.EndsWith( controllerResult.ControllerName, StringComparison.InvariantCultureIgnoreCase ) ) return null;

            var foundAction = actionDescriptors.FirstOrDefault( action => string.Equals( action.ActionName, routeContext.HttpContext.Request.Method, StringComparison.InvariantCultureIgnoreCase ) );
            if ( foundAction == null ) return null;

            var oDataOptionalParameterType  = Type.GetType( "Microsoft.AspNet.OData.Routing.ODataOptionalParameter, Microsoft.AspNetCore.OData", throwOnError: true, ignoreCase: false );
            var constructor                 = oDataOptionalParameterType.GetConstructors()[0];
            var oDataOptionalParameter      = constructor.Invoke( null );
            var addMethod                   = oDataOptionalParameterType.GetMethod( "Add" );

            foundAction.Parameters
                .OfType<ControllerParameterDescriptor>()
                .Where(param => param.ParameterInfo.IsOptional)
                .ToList()
                .ForEach(param => addMethod.Invoke( oDataOptionalParameter, new[]{ new CustomOptionalParameter(){ 
                    Name                = param.ParameterInfo.Name, 
                    DefaultValueString  = param.ParameterInfo.DefaultValue?.ToString() } 
                }));

            routeContext.RouteData.Values.Add( ODataRouteConstants.OptionalParameters, oDataOptionalParameter );

            return foundAction.ActionName;
        }
    }
}

[commonsensesoftware]

Interesting trick. Sadly, this is a bug. Unfortunately, the OData routing quite difficult to work with under the hood. I've had to fork a big chunk of it. Although 7.x does support Endpoint Routing, it's a 🐶 and 🐴 show. The implementation still calls back through IActionSelector 🤦🏽. I wouldn't be surprised if this was a bug in the original OData implementation that was fixed, but I never knew about. That's been a huge pain to stay in front of.

[commonsensesoftware]

6.0 with support for OData 8.x is now available. Is this issue still relevant? This definitely is a bug, but if there is no interest in continuing to work on OData 7.x (which I wouldn't recommend), I'd rather close it out unsolved and move on. If this is still needed, then I will reprioritize it in the backlog.

[commonsensesoftware]

@GitHub-TC

There's a very real chance this was a bug and not functioning as expected. For 5.x with OData 7.x we may never know. Until MS unblocks my ability to publish 5.x once again, I'm not going to invest more time in troubleshooting it as it may never happen.

I can confirm that your desired setup will work as expected without any custom conventions, but your repro setup is wrong. What wasn't clear is whether you want dummy to be a query parameter or an OData function parameter. As defined, the wires are crossed.

Solution 1 - An OData Function

You defined an OData function and parameter in the EDM. The correct setup should be:

var product = builder.EntitySet<Product>( "Products" ).EntityType.HasKey( p => p.Id );
var function = product.Collection.Function( "Read" );

function.ReturnsFromEntitySet<Product>( "Products" );
function.Parameter<int?>( "dummy" ).Optional().Nullable = true;

Your action can then be defined as:

[HttpGet]
[Produces( "application/json" )]
[ProducesResponseType( typeof( Product ), Status200OK )]
[ProducesResponseType( Status404NotFound )]
public SingleResult<Product> Read( int? dummy = null ) => Get( 1 );

It should be noted that your expected URL is incorrect as well. An OData function name is part of the path. If it weren't, then the request would be ambiguous with a vanilla GET request. The OData convention generated will therefore be:

api/Products/Read(dummy={dummy})

This poses a challenge for OpenAPI (formerly Swagger). It is assumed anything in the path is required. This means that any of the following are valid from OData's perspective:

• api/Products/Read?api-version=3.0
• api/Products/Read(dummy=42)?api-version=3.0
• api/Products/Read(dummy=null)?api-version=3.0

I'm not entirely sure how, but there is probably a way to make the OpenAPI document use a literal null for the value - maybe.

Solution 2 - A Query String Parameter

Looking at the action definition, it appears you intend for dummy to be part of the query string. If that's the case, then it should not be part of the OData function definition. The function definition is instead simply:

var product = builder.EntitySet<Product>( "Products" ).EntityType.HasKey( p => p.Id );
product.Collection.Function( "Read" ).ReturnsFromEntitySet<Product>( "Products" );

The action implementation need not change because dummy will be treated as a query string parameter by default; however, if you want to add the [FromQuery] annotation to be explicit, that will work too. This will produce the OData template:

api/Products/Read

Where the following are valid:

• api/Products/Read?api-version=3.0
• api/Products/Read?api-version=3.0&dummy=42

---

cc: @icnocop, @pil0t

[commonsensesoftware]

This issue has a working solution and several alternatives. 5.1 has been published standalone and the 6.1. packages have been published, which will definitely resolve this issue the correct way. If something else comes up that I've missed, I'm happy to discuss further or reopen the issue, but for now, this issue has reached its conclusion. Thanks.