search

guzzle / guzzle-services

Provides an implementation of the Guzzle Command library that uses Guzzle service descriptions to describe web services, serialize requests, and parse responses into easy to use model structures.
MIT License
253 stars 78 forks source link

Fix serialization of query params #129

Closed bakura10 closed 7 years ago

bakura10 commented 7 years ago

Addresses #128

This PR is likely a BC of 1, but as the behaviour of 1.0.0 is wrong and completely different from previous version, I think it's more a bug fix rather than a BC. Also there was no test to verify this behaviour.

Currently, the Guzzle Service serializes incorrectly any query params that are array or object.

For instance, considering this operation's param:

'foo' => [
  'type' => 'array',
  'location' => 'query'
]

Specifying this parameter: $client->myOperation(['foo' => [1, 2]]) would serialize this query parameter : foo=1&foo=2, which is completely different from the older behaviour and, from my understanding, incorrect.

Similarily, when specifying an object type, this: $client->myOperation(['foo' => ['bar' => 'baz']]) would serialize this query "foo=baz" (hence completely loosing the context of bar).

After this PR, first operation would be serialized 'foo[0]=1&foo[1]=2' and second operation would be serialized 'foo[bar]=baz'.

Thanks a lot!

Konafets commented 7 years ago

Thank you very much for your contribution. You fix looks good and you even add tests. Thumbs up!

bakura10 commented 7 years ago

Thanks a lot ! It seems there is a lot of missing coverage with serializing query attributes, as I'm maintaining some big descriptors and starting to upgrade them, it's possible that I encounter other regressions. I'll make sure to fix them as they arise :).

In the mean time would it be possible to tag this ? :)

bakura10 commented 7 years ago

Hey !

Apparently I've messed and I've made the PR against the "develop" branch. Should I re-do the PR to master so that this can be tagged?

Konafets commented 7 years ago

Nope. I changed it to develop intentionally because master only should contain tagged versions. I follow git flow here: http://danielkummer.github.io/git-flow-cheatsheet/

Konafets commented 7 years ago

@bakura10 Just tagged version 1.0.1. Hope it works for now. You are very welcome to open issues and/or PRs. I will try to get everything fixed as soon as possible.