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

Add support for optional parameters in function #782

Closed
jatin085 opened this Issue Mar 27, 2017 · 9 comments

Comments

Projects
None yet
6 participants
@jatin085

jatin085 commented Mar 27, 2017

The library already supports OData actions with optional parameters i.e. a client can execute an OData action with one or more optional parameters missing. For functions, we need a similar support. Presently, it seems the code flows through method FilterFunctionsByParameterNames that picks a function only if its parameter count matches with count of input parameters.

Assemblies affected

OData .Net lib 7.0.0

Reproduce steps

Define a function with optional parameter. i.e. one that does not have Nullable="true" field value set. Make call to the OData function without passing the optional parameter.

Expected result

OData function call is successful

Actual result

"Resource not found" error.

@xuzhg

This comment has been minimized.

Member

xuzhg commented Mar 27, 2017

Similar requirement at: #403

@mikepizzo

This comment has been minimized.

Member

mikepizzo commented Apr 20, 2017

The OData protocol doesn't support optional parameters for function invocation because it wants the service to be able to correctly route function overloads based on the URL (actions don't have overloads based on non-binding parameters for this same reason -- non-binding parameters are passed in the body).

The OData-compliant way to do this would be to define two versions of the function, one with the optional parameter and one without.

Does that not work for your scenario?

@jatin085

This comment has been minimized.

jatin085 commented Apr 21, 2017

@jatin085

This comment has been minimized.

jatin085 commented Apr 22, 2017

Hi Mike, if I get it correctly, a single routing pattern won't work even if the caller changes the order of input arguments to the function but the OData specification supports reordering of the function arguments. Let me know if I am missing something.

@mikepizzo

This comment has been minimized.

Member

mikepizzo commented Apr 25, 2017

OData requires that both of the following are true:
-the unordered set of parameters names must uniquely identify an overload
-the ordered set of parameter types must uniquely identify an overload

The first rule is to support routing (since OData doesn't require the parameters to be passed in order) and the second rule was added largely for compatibility with programming languages which may want to generate corresponding strongly typed functions.

As long as we can live with these rules still being supported for all required parameters, I believe we could add optional parameters without breaking routing or language overload rules.

Specifically:
-Optional parameters must be defined after any required parameters (for language compatibility)
-Unordered list of required parameter names must uniquely identify the overload
-Ordered list of required parameter types must uniquely identify the overload

Would that address your requirements?

@jatin085

This comment has been minimized.

jatin085 commented Apr 26, 2017

Thanks Mike.. this should address our requirements. Dynamics 365 internally does not have a mechanism to specify APIs with same name but different set of request parameters. I know of only two functions for which we added custom overloads within the OData layer. The API owners add optional parameters whenever there's a change in API to ensure that the backward compatibility is not lost (if API is called using SOAP protocol).

That should answer the last two constraints. Regarding the first one, it should be totally okay for clients to pass optional parameters in the end. Presently, whenever a change is made to the API, we have to reach all places where the API is called and add default values for the new optional parameters. This will be avoided with support for optional parameters, even under the constraints that you have mentioned.

@markdstafford

This comment has been minimized.

Member

markdstafford commented Jun 12, 2017

@ysanghi for tracking purposes

@bdebaere

This comment has been minimized.

bdebaere commented Mar 12, 2018

Is this already implemented in the latest release? I'm currently having issues calling the following function signature:

public IHttpActionResult Price([FromODataUri] string key, string requiredParameter1, string requiredParameter2, string optionalParameter1 = null, string optionalParameter2 = null)`

I'm using the following builder:

                FunctionConfiguration functionCustomerPrice = builder
                    .EntitySet<Item>("Items")
                    .EntityType
                    .Function("Price")
                    .Returns<Result>();
                functionCustomerPrice.Parameter<string>("requiredParameter1");
                functionCustomerPrice.Parameter<string>("requiredParameter2");
                functionCustomerPrice.Parameter<string>("optionalParameter1").OptionalParameter = true;
                functionCustomerPrice.Parameter<string>("optionalParameter2").OptionalParameter = true;
{
    "error": {
        "code": "",
        "message": "No HTTP resource was found that matches the request URI 'http://localhost/Items('')/Price(requiredParameter1='',requiredParameter2='',optionalParameter1='')'.",
        "innererror": {
            "message": "No routing convention was found to select an action for the OData path with template '~/entityset/key/unresolved'.",
            "type": "",
            "stacktrace": ""
        }
    }
}
@mikepizzo

This comment has been minimized.

Member

mikepizzo commented Jun 18, 2018

This is supported in OData Library, but not yet supported in the WebAPI OData Stack.

Unfortunately, in the current WebAPI OData stack, the "OptionalParameter" doesn't actually mean optional, it means nullable (i.e., you can provide a null value, rather than you can omit the value).

Opened OData/WebApi#1488 to track this in WebAPI OData Stack, and closing this issue.

@mikepizzo mikepizzo closed this Jun 18, 2018

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