[PHP] Use String instead of Byte Array

[jqr]

When { "type": "string", format": "binary" } is requested, it's internally represented as a ByteArray, which was then translated into PHP as a literal byte array. A literal byte array in PHP is [72, 101, 108, 108, 111] and is not a format supported by any file, echoing, or any standard library operations other than general array handling.

I propose that a "binary string" should represent just that in PHP, an unencoded string. This allows access to the full standard library of tools for dealing with strings as bytes, including file writing and echoing.

This should have some notable performance improvements for end-users as the previous fix was essentially to do the following, which ends up making 4 copies of the data as it passes through the various layers of unpacking, and repacking.

# customer code                                                 swagger code
#            copy                      copy                     copy         original
$string = call_user_func_array('pack', array_merge(array('C*'), unpack("C*", (string)response)) )

I tried fixing the response type in documentation to string instead of ByteArray by manipulating the mapping, but this runs into issues with bypassing the raw decoding in ApiClient.php and then gets decoded as a JSON object. I'm open to advice on how to fix this.

[wing328]

@jqr thanks for the PR. For the issue, I think you're referring to this line: https://github.com/swagger-api/swagger-codegen/blob/master/samples/client/petstore/php/SwaggerClient-php/lib/ApiClient.php#L237

What about adding another check (else) for response type at https://github.com/swagger-api/swagger-codegen/blob/master/samples/client/petstore/php/SwaggerClient-php/lib/ApiClient.php#L235 and return the raw body if the response type is string?

[jqr]

Won't ByteArray effectively disappear with this change? I was thinking of changing the mapping in PhpClientCodegen.java#L104 so the generated comments would say string instead of ByteArray.

I think I have combined the approaches to get what we're after, let me know what you think.

[wing328]

Thinking about this more. I suggest we rename "ByteArray" to "binary(string)" as PHP developers may not have a clue what ByteArray means or string actually refers to "binary" in the method help text, e.g. https://github.com/swagger-api/swagger-codegen/blob/master/samples/client/petstore/php/SwaggerClient-php/lib/Api/PetApi.php#L983

[wing328]

@jqr what's your view on my suggestion?

[jqr]

I prefer just plain string, as implemented now. I think that's the actual type involved and what should be expressed.

I get that there are consistency issues across a bunch of languages though so I'm happy to defer to your opinion. string(binary) makes slightly more sense to me than binary(string) but you let me know what you want and I'll make it happen. Or feel free to just take this branch forward as you see fit.

[wing328]

@jqr I respect your opinion as well. Would you please rebase on the latest master to resolve the merge conflicts? Then I'll review and approve accordingly.

[jqr]

Generating the clients locally adds a good amount of new code to the petstore php client which I have opted not to check in.

[wing328]

OK. Those are new fake endpoints we added to unit test some bugs we've addressed.

I'll add those in my next PHP PR.