OpenAPITools / openapi-generator

OpenAPI Generator allows generation of API client libraries (SDK generation), server stubs, documentation and configuration automatically given an OpenAPI Spec (v2, v3)
https://openapi-generator.tech
Apache License 2.0
21.23k stars 6.43k forks source link

PHP - pass array(key-value pair) as an argument instead of comma separated individual arguments #641

Closed rajanmodi closed 5 years ago

rajanmodi commented 6 years ago
Description

Functions generated automatically in SDK should support array(key-value pair) as an argument.

Suppose there is search function with 66 arguments. search($this->arg1 = null, $this->arg2 = null, ...., $this->arg66 = null)

When we need to make a call to this search function ,we need to provide all the arguments $apiInstance->search($this->arg1 = 5, $this->arg2 = 10, $this->arg3 = 20,.....,$this->arg66 = 1)

What if I only want to pass only two arguments $apiInstance->search($this->arg1 = 5,$this->arg31 = 95). now this will assign 5 to arg1 and 95 to arg2. I need to assign value of 95 to arg31

I think solution will be generating function argument as an array with key-value pair for such cases .

Instead of keeping comma separated arguments, array should be there..

openapi-generator version
OpenAPI declaration file content or url
Command line used for generation
Steps to reproduce
Related issues/PRs
Suggest a fix/enhancement
wing328 commented 6 years ago

@rajanmodi thanks for the suggestion.

cc @jebentier @dkarlovi @mandrean @jfastnacht @ackintosh @ybelenko

ybelenko commented 6 years ago

@rajanmodi Hmm... Give us an example? I can't find any function with more than one argument in current PHPClient samples. We're talking about models, right?

/**
 * Constructor
 *
 * @param mixed[] $data Associated array of property values
 *                      initializing the model
 */
public function __construct(array $data = null)
{
    $this->container['bill_id'] = isset($data['bill_id']) ? $data['bill_id'] : null;
    $this->container['bill_date'] = isset($data['bill_date']) ? $data['bill_date'] : null;
    $this->container['charges'] = isset($data['charges']) ? $data['charges'] : null;
    $this->container['charge_desc'] = isset($data['charge_desc']) ? $data['charge_desc'] : null;
}
wing328 commented 6 years ago

@ybelenko I think @rajanmodi is referring to method arguments, e.g. updatePetWithForm with 3 parameters only and let's imagine it has 20 additional optional parameters.

ybelenko commented 6 years ago

@wing328 If so, I think that more common practice is to create endpoints with just few parameters, or with one complex JSON object parameter. I like current "full list of parameters" appouch more and user always can rewrite mustache template a bit.

But key-values pairs very handy when you add both required and optional parameters and rebuild client SDK. You can use fresh SDK right away without need to rearrange arguments in functional call.

In total, 1 pro and 1 con from me :smile:

rajanmodi commented 6 years ago

@wing328 https://github.com/wing328 @ybelenko https://github.com/ybelenko - Yes I am referring to method arguments. There are 66 arguments in my search method. According to my requirement, to access search method, I need not pass all the arguments. I will pass only desired few arguments to search methods and accordingly my api returns JSON response.

But current SDK code is forcing me to pass all the arguments to make a search call.

So my suggestion is that instead of keeping comma separated arguments in search method, it should receive one array(key ,value pair).

So in this case if I need to make a call to search method, I will construct an array consisting of only needed arguments at my end and pass it to search method.

On receiving that array, search method will postmortem this array and accordingly construct those 66 parameters keeping unnecessary argument as null.

I hope I am able to represent the problem in a concise way. Still In case of of missing info please do let me know.

Thank you, Rajan

On Wed, Jul 25, 2018 at 8:06 PM, Yuriy Belenko notifications@github.com wrote:

@wing328 https://github.com/wing328 If so, I think that more common practice is to create endpoints with just few parameters, or with one complex JSON object parameter. I like current "full list of parameters" appouch more and user always can rewrite mustache template a bit.

But key-values pairs very handy when you add both required and optional parameters and rebuild client SDK. You can use fresh SDK right away without need to rearrange arguments in functional call.

In total, 1 pro and 1 con from me 😄

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OpenAPITools/openapi-generator/issues/641#issuecomment-407777027, or mute the thread https://github.com/notifications/unsubscribe-auth/AYv-BOgeqjn93xPEG4vjJNpV_h5iewIjks5uKIJ8gaJpZM4Vf28h .

ybelenko commented 6 years ago

@rajanmodi 66 arguments in search operation, huh? Even Facebook and Twitter doesn't have API methods with huge arguments list like that. Anyway, I understand why you don't wanna use object as input parameter, obviosly your HTTP method is GET for a search. Maybe you can use enum filters to make arguments list shorter, in way:

/jobs/?status_filter=closed|canceled|abandoned

@rajanmodi Can you give us endpoint example, when you need huge stack of input parameters? Another suggestion to solve this problem, we can extend PHPClient config with useArrayAsArgument which will be false by default.

rajanmodi commented 6 years ago

@ybelenko https://github.com/ybelenko Basically it is CarSearch Api. You can refer this documentation for more detail - https://developer.marketcheck.com/#/Listings/search https://apidocs.marketcheck.com/

One endpoint is search in which we can pass possible combinations of search parameters. for e.g http://api.marketcheck.com/v1/search?api_key={your_api_key}&latitude=39.73&longitude=-104.99&radius=200&car_type=used&start=0&rows=10&sort_by=dist&sort_order=desc http://api.marketcheck.com/v1/search?api_key={your_api_key}&latitude=39.73&longitude=-104.99&radius=200&car_type=used&start=0&rows=10&sort_by=dist&sort_order=desc&plot=true http://api.marketcheck.com/v1/search?api_key={your_api_key}&lease_term=36&lease_down_payment=2000-5000&lease_emp=200-400&sort_by=lease_emp&sort_order=asc

So here I am passing only few parameters and accordingly I will get response in JSON.

While making a call to search method, I have to pass all 66 parameters to search, even though I am interested in passing this parameters only - search($ latitude=39.73,$longitude=-104.99,$radius=200,$car_type=used,$start=0,$rows=10,$sort_by=dist,$sort_order=desc) rest all arguments I have to pass as null.

On Thu, Jul 26, 2018 at 10:43 AM, Yuriy Belenko notifications@github.com wrote:

@rajanmodi https://github.com/rajanmodi 66 arguments in search operation, huh? Even Facebook and Twitter doesn't have API methods with huge arguments list like that. Anyway, I understand why you don't wanna use object as input parameter, obviosly your HTTP method is GET for a search. Maybe you can use enum filters to make arguments list shorter, in way:

/jobs/?status_filter=closed|canceled|abandoned

@rajanmodi https://github.com/rajanmodi Can you give us endpoint example, when you need huge stack of input parameters? Another suggestion to solve this problem, we can extend PHPClient config with useArrayAsArgument which will be false by default.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OpenAPITools/openapi-generator/issues/641#issuecomment-407978494, or mute the thread https://github.com/notifications/unsubscribe-auth/AYv-BJWeYbN_sbQnVmFAI8naH3aqL5Ehks5uKVAEgaJpZM4Vf28h .

ackintosh commented 6 years ago

Ideally, the issue should be resolved with named parameters but PHP doesn't have that mechanism. 🤔

How about implementing the wrapper class for the search method like below?

<?php
// You only have to pass the necessary arguments
WrapperForExampleApi::search(['arg2' => 'x', 'arg4' => 'x']);

/**
 * hand-written wrapper class for ExampleApi
 */
class WrapperForExampleApi
{
    public static function search()
    {
        $args = self::defaultArguments();
        $passed = func_get_args()[0];
        $merged = array_merge($args, $passed);

        return call_user_func_array([new ExampleApi, 'search'], $merged);
    }

    private static function defaultArguments()
    {
        $exampleApi = new ExampleApi;
        $method = new \ReflectionMethod($exampleApi, 'search');

        $arguments = [];
        foreach ($method->getParameters() as $parameter) {
            $arguments[$parameter->getName()] = $parameter->getDefaultValue();
        }

        return $arguments;
    }
}

/**
 * auto-generated api class
 */
class ExampleApi
{
    public function search($arg1 = null, $arg2 = null, $arg3 = null, $arg4 = null)
    {
        var_dump('$arg1 = ' . $arg1);
        var_dump('$arg2 = ' . $arg2);
        var_dump('$arg3 = ' . $arg3);
        var_dump('$arg4 = ' . $arg4);
    }
}
rajanmodi commented 6 years ago

This solution will work, but I am wandering if there is anything like java adapter classes in php. Automatic sdk generation, if supports this java adapter class concept, then this will be great help and this problem will be solved and great rescue for devs.

ackintosh commented 6 years ago

(This is just my personal opinion 😉)

I think that adopting the adapter class workaround to SDK generation should avoid as much as possible.

because...

So I suggest SDK users who has the issue like you reported implement a adapter class like https://github.com/OpenAPITools/openapi-generator/issues/641#issuecomment-408292155 .

ackintosh commented 5 years ago

@rajanmodi The group parameter support in PHP client has been implemented by @wing328 and released as v3.3.2. Please give it a try. 😉


Samples

https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/resources/3_0/petstore-with-fake-endpoints-models-for-testing.yaml#L774

https://github.com/OpenAPITools/openapi-generator/blob/master/samples/openapi3/client/petstore/php/OpenAPIClient-php/lib/Api/FakeApi.php#L2538-L2541

jeandonaldroselin commented 5 months ago

@rajanmodi The group parameter support in PHP client has been implemented by @wing328 and released as v3.3.2. Please give it a try. 😉

Samples

  • Spec

https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/resources/3_0/petstore-with-fake-endpoints-models-for-testing.yaml#L774

  • Generated code

https://github.com/OpenAPITools/openapi-generator/blob/master/samples/openapi3/client/petstore/php/OpenAPIClient-php/lib/Api/FakeApi.php#L2538-L2541

Hello @ackintosh,

First thank you for your great contributions.

I have adopted your tools for a while now but I face the same issue as @rajanmodi.

The consequences of that are multiple :

I could adopt the given workaround https://github.com/OpenAPITools/openapi-generator/issues/641#issuecomment-408292155, but I want to keep my projects the most simple as possible because I want to be able to integrate juniors in the project as well. So the more I use standard they can fin in documentation, the best it is.

I don't find any documentation on "how to generate methods supporting array instead of separate arguments as parameters".

Please can you help me ?