[php] use http_build_query for deepObject support

[nadar]

This fixes #11222

The Query::build() function does not support deepObjects in query params, but the OpenAPI defintion does.

/users?id[role]=admin&id[firstName]=Alex an example from the docs.

I have replaced the Query::build() function with PHP's internal http_build_query function. I have adjust PHP default encoding to match the default encoding from Query::build() which is PHP_QUERY_RFC3986.

This change will only have an effect for the query string of the URL, encoding POST bodys is still done by Query::build().

[ybelenko]

cc @jebentier, @dkarlovi, @mandrean, @jfastnacht, @ackintosh, @renepardon

[nadar]

Is there something i can improve to fix bitrise?

[wing328]

For the bitrise CI failure, please ignore it.

[wing328]

@nadar thanks for the PR. Can we add some tests in https://github.com/OpenAPITools/openapi-generator/blob/master/samples/client/petstore/php/OpenAPIClient-php/tests/ParametersTest.php ?

[nadar]

Sure!

[nadar]

@wing328 I have to admit i am not sure what kind of test you expect. As i have changed the default behavior in tests (i have modified all tests apis with the new code), if they are not failing there should be no BC break. I can add a very simple nested object tested for URLs, i will do that, but not sure if it helps, or makes anything more stable :-) but i'll do it.

[ybelenko]

I'm taking care of it. Please don't merge or push anything. Gonna push commits soon.

[nadar]

@ybelenko thanks for your help on this 👍 i have at least added a tests which shows that the default behavior is the same for query param generating.

[ybelenko]

By the way, why don't we ask for deepObject support at Guzzle repo?

[nadar]

Sure you can 👍 The be honest i have no idea what that function is made for, seems to be a sort of "ensure only scalar values are provided", but i assume there is a reason to have that function, otherwise they would just use http_build_query instead. So this might be a huge "BC break" for the guzzle eco system. This also might take very long to introduce deep object params here. So i would suggest, to ask them by issue, but in the meantime merge this PR, as we are really looking forward to have this option.

[nadar]

@ybelenko @wing328 Any news regarding this PR? Is there something i can do? It would be really nice to have that feature soon. Thanks

[wing328]

@nadar thanks for adding the tests. It looks good from my side and happy to see this PR gets merged.

[ybelenko]

Doesn't look good to me…

[nadar]

@ybelenko i see, those data is generated diff checked, now i can more understand how the test works.

[ybelenko]

I'm working on a fix which provide not only deepObject serialisation style, but
pipeDelimited and spaceDelimilted. Please wait for a day or two. I think we still can use Guzzle/Query::build.

[ybelenko]

@nadar Please, check it out. I've added anonymous function to flatten query array so it solves Guzzle error. Should work with nested arrays now since all tests are passed.

Also wrote small static function to wrap query build, we can migrate to http_build_query with one line of code in future:

openapi-generator/samples/client/petstore/php/OpenAPIClient-php/lib/ObjectSerializer.php

Lines 452 to 471 in 920cfe5

	/**
	* Native `http_build_query` wrapper.
	* @see https://www.php.net/manual/en/function.http-build-query
	*
	* @param array|object $data May be an array or object containing properties.
	* @param string $numeric_prefix If numeric indices are used in the base array and this parameter is provided, it will be prepended to the numeric index for elements in the base array only.
	* @param string|null $arg_separator arg_separator.output is used to separate arguments but may be overridden by specifying this parameter.
	* @param int $encoding_type Encoding type. By default, PHP_QUERY_RFC1738.
	*
	* @return string
	*/
	public static function buildQuery(
	$data,
	string $numeric_prefix = '',
	?string $arg_separator = null,
	int $encoding_type = \PHP_QUERY_RFC3986
	): string {
	return \GuzzleHttp\Psr7\Query::build($data, $encoding_type);
	}
	}

vendor/bin/phpunit tests/ObjectSerializerTest.php
PHPUnit 9.5.18

..................................................                50 / 50 (100%)

Time: 00:00.059, Memory: 6.00 MB

OK (50 tests, 60 assertions)

[nadar]

So you could fix the original issue without the --additional-properties option? That's great news! So my provided test case works now out of the box?

[ybelenko]

So you could fix the original issue without the --additional-properties option? That's great news! So my provided test case works now out of the box?

I think so, at least minimal examples list works out of the box, including yours. I'm scared to test it with crazy fixtures(nested arrays of unexpected values) 😅 😅 😅

[ybelenko]

@nadar I'm pretty satisfied with the result here. I wait for your feedback, hope you can check it locally then I can merge.

[nadar]

i will do a test tomorrow, but if my provided example works then i am totally happy!

['filter' => ['tags' => ['alias' => 'food']]]

should be evaluated as

?filter[tags][alias]=food

[ybelenko]

{ "filter": { "or": [ {"name":"John"}, {"email":"[email protected]"} ] } } 

should be encoded to:

filter[or][0][name]=John&filter[or][1][email][email protected]

It encodes deepObject that way above.

[nadar]

Then, there is nothing to test for me. Amazing @ybelenko, thank you!

[ybelenko]

Check locally anyway. We spent huge amount of time with this, might be huge disappointment if it doesn't work for you.

[nadar]

@ybelenko very hard to test i have to admit. Or is there any good command how i could make the openapi generator fetch the data from that branch?

openapi-generator-cli generate -i https://mysuperjson.json -g php --remote-generator=.......

or can you give me a hint?

[nadar]

@ybelenko i just copy the code quickly to my vendor files and adjust accordingly

$queryParams = array_merge($queryParams, ObjectSerializer::toQueryValue(
            $filter,
            'filter', // param base name
            'object', // openApiType
            'deepObject', // style
            true // explode
        ) ?? []);

And it works. 👍 Thanks

[nadar]

great journey, thanks for the time @ybelenko

[ksvirkou-hubspot]

Hey @ybelenko
My app doesn’t work because it passes empty value to query string
example bellow
https://api.*.com/crm/v3/objects/contacts?limit=10&after=&properties=website&properties
Except string (without after)
https://api.*.com/crm/v3/objects/contacts?limit=1&properties=website&properties

[ksvirkou-hubspot]

can I fix it here ?
CC @ybelenko

[wing328]

Looks like this broke some of the PHP petstore integration tests:

PHPUnit 9.5.19 #StandWithUkraine

..............* Closing connection 0
.* Closing connection 0
................................................  63 / 124 ( 50%)
.........................................FF..........F.......   124 / 124 (100%)

Time: 00:01.443, Memory: 12.00 MB

There were 3 failures:

1) OpenAPI\Client\PetApiTest::testUpdatePetWithFormWithHttpInfo
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'updatePet'
+'update pet with form with http info'

/Users/williamcheng/Code/openapi-generator3/samples/client/petstore/php/OpenAPIClient-php/tests/PetApiTest.php:165
phpvfscomposer:///Users/williamcheng/Code/openapi-generator3/samples/client/petstore/php/OpenAPIClient-php/vendor/phpunit/phpunit/phpunit:97

2) OpenAPI\Client\PetApiTest::testUpdatePetWithForm
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'updatePet'
+'update pet with form'

/Users/williamcheng/Code/openapi-generator3/samples/client/petstore/php/OpenAPIClient-php/tests/PetApiTest.php:178
phpvfscomposer:///Users/williamcheng/Code/openapi-generator3/samples/client/petstore/php/OpenAPIClient-php/vendor/phpunit/phpunit/phpunit:97

3) OpenAPI\Client\RequestTest::testFormDataEncodingToJson
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'param=value&param2=value2'
+''

/Users/williamcheng/Code/openapi-generator3/samples/client/petstore/php/OpenAPIClient-php/tests/RequestTest.php:34
phpvfscomposer:///Users/williamcheng/Code/openapi-generator3/samples/client/petstore/php/OpenAPIClient-php/vendor/phpunit/phpunit/phpunit:97