Skip to content
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

no more json() method for responses? #1106

Closed
rap2hpoutre opened this issue Jun 2, 2015 · 18 comments
Closed

no more json() method for responses? #1106

rap2hpoutre opened this issue Jun 2, 2015 · 18 comments

Comments

@rap2hpoutre
Copy link

No more json() method for responses?

Guzzle 5:

$response = $client->get('http://httpbin.org/get');
$array = $response->json(); // Yoohoo
var_dump($array[0]['origin']);

Guzzle 6:

$response = $client->get('http://httpbin.org/get');
$array = json_decode($response->getBody()->getContents(), true); // :'(
var_dump($array[0]['origin']);

Will json() method be back ? (I miss it)

@GrahamCampbell
Copy link
Member

Will json() method be back ? (I miss it)

No. It's because of psr7.

@rap2hpoutre
Copy link
Author

Ok, thanks.
PSR 7 gives interfaces but does it disallow to add method in implementation?

@GrahamCampbell
Copy link
Member

disallow to add method in implementation?

Well, technically it does. Guzzle aims to be an implementation of psr7, allowing you to swap it out of anything else psr7, which is the entire point of psr7. If we stared adding methods, the value of implementing psr7 is lost.

@rap2hpoutre
Copy link
Author

Ok, thank you 😢

@rap2hpoutre
Copy link
Author

If we stared adding methods, the value of implementing psr7 is lost.

I don't understand (sorry if it's a time lost for you, I'm trying to understand).
Let's take the psr3 example with its well known implementation: Monolog

In Psr\Log\LoggerInterface, there is no public method named warn, but there is one in Monolog\Logger. And there are many other examples in this class. So if I want to use Monolog\Logger and its warn method, I cannot "swap it out of anything else psr". But it's a psr3 implementation. I think it could be additional methods in Guzzle without breaking the psr7 compatibility.

(sorry if I misunderstood something, or if my argumentation is unclear)

@mtdowling
Copy link
Member

Guzzle has switched over to PSR-7 only interfaces. In order to keep some of the old methods on responses like json, xml, effectiveUri, Guzzle would need to rely on a concrete implementation of a PSR-7 response. This would then require Guzzle users to rely on a concretion rather than an abstraction (an interface). This would then lead to needing to incessantly wrap normal PSR-7 responses with Guzzle specific responses or you would need to check the class of each response object you receive from Guzzle to ensure that it has the json method. Because Guzzle has a middleware system that is meant to essentially decorate clients, it makes much more sense to just get rid of the old Guzzle specific methods and use only PSR-7 methods.

Furthermore, the json method was a polarizing feature: some people wanted it to return associative arrays and others wanted it to return stdclass objects. Arguably, it shouldn't have been on the response object anyways as it's too specific IMO (i.e., we did not have yaml(), ini(), and other specific format parsers built-in). It's now just a matter of calling json_decode($response->getBody()).

@rap2hpoutre
Copy link
Author

Ok thanks for your answer!

@SvenRtbg
Copy link

The Monolog example demonstrates the problem. If you would use a PSR-3 logger implementation, you could NOT use the warn() method of Monolog (the correct method warning is available). Every IDE would yell at us for using an undefined method of an object, and it would break for any object that doesn't implement this method. You could also rely (i.e. typehint it) on Monolog\Logger being passed, which would ensure that the warn method is present, but this would defeat the purpose of interchangeable implementations. You could not pass anything else.

If you want to have an easily available json function, you can always write a wrapper object that on the one side provides this method, and on the other side expects a PSR-7 response message.

@landons
Copy link

landons commented Feb 16, 2016

Sorry to dig up an old post, but I just ran across this very issue. Google results showed old documentation of the json() method, which obviously doesn't work in PSR-7 compatible versions.

Out of curiosity, this seems to be more of a design philosophy question. I would expect PSR-7 compatible classes to implement all required methods of the interface (it's hardly an interface otherwise, no?). However, implementing additional/custom interfaces shouldn't inherently violate PSR-7. It does add a concrete dependency of the consuming application on the extended interface, yes, but AFAIK that doesn't undermine the usefulness of PSR-7. Frameworks and other libraries can continue calling for PSR-7 compatibility, but consuming applications can still get the benefit of helper methods outside the scope of the interface spec.

Am I missing something here?

@SvenRtbg
Copy link

Using interfaces is entirely design philosophy. The key point is: Guzzle decided to provide a PSR-7 implementation. If the team would add json() as a method to their response object, people would use it. By doing so they depend on Guzzle, not PSR-7, and this violates the idea behind PSR-7, or interface implementations in general. The different implementations have to be interchangeable.

If you decide that you want to have convenience methods to quickly access the json decoded content (or anything else), you can easily write a wrapper object that accepts a PSR-7 compatible object. And the added benefit of this solution would be that you are not depending on Guzzle - you are depending on ANY PSR-7 implementation. You could even start a new library with just a fancy wrapper object allowing direct access to anything - and this would be beneficial for every user of a PSR-7 compatible library, not only one specific implementation.

@mailopl
Copy link

mailopl commented Feb 24, 2016

@SvenRtbg Great explanation. Thanks. Although it was very usable.

@artickc
Copy link

artickc commented Sep 18, 2017

i cant't understand why?
$array = json_decode($response1->getBody()->getContents(), true); // {"status":"false","message":"Site not found"}
var_dump($array); // NULL

@alexeyshockov
Copy link
Collaborator

@artickc, call json_last_error_msg() and see.

@artickc
Copy link

artickc commented Sep 18, 2017

Case 1:
$array = json_decode($response1->getBody()->getContents(), true);
echo $response1->getBody()->getContents();//
var_dump($array); // NULL
echo json_last_error_msg();//Syntax error

Case 2
$array = json_decode($response1->getBody(), true);
echo $response1->getBody();//{"status":"false","message":"Site not found"}
var_dump($array); // NULL
echo json_last_error_msg();//Syntax error

@rap2hpoutre
Copy link
Author

@artickc I'm not totally sure it's the right place for asking your specific question. You will have more visibility and more quality answers if you ask it on Stackoverflow.

@SvenRtbg
Copy link

SvenRtbg commented Sep 18, 2017 via email

@geofanytobing
Copy link

Call to undefined method GuzzleHttp\Psr7\Response::json()

@rattacresh
Copy link

IMHO this is actually simply an omission in the Psr-7 Standard that needs to be fixed. They completely forgot to provide an interface for client-side responses. Crazy but true. If you read the standard, you see "Our recommendation is that implementations use read-only streams for server-side requests and client-side responses" (my emphasis). So the concept of a client-side response is not uncommon to them. Yet, they only have interfaces for:

  • RequestInterface -- "Representation of an outgoing, client-side request."
  • ServerRequestInterface -- "Representation of an incoming, server-side HTTP request."
  • ResponseInterface -- "Representation of an outgoing, server-side response."

What is obviously lacking is ClientResponseInterface -- Representation of an incoming, client-side response.

That interface would obivously have the getParsedBody() method the same way as ServerRequestInterface has.

Please file a bug against the Psr-7 standard.

duncan-brown added a commit to duncan-brown/comanage-registry that referenced this issue Nov 9, 2019
duncan-brown added a commit to duncan-brown/comanage-registry that referenced this issue Nov 10, 2019
duncan-brown added a commit to duncan-brown/comanage-registry that referenced this issue Nov 12, 2019
boshrin pushed a commit to Internet2/comanage-registry that referenced this issue Mar 27, 2020
* fix path to plugin

* fix undefined variable error in commented out code

* Vendor with capital V to match existing

* update guzzle version to 6.x
php composer.phar require guzzlehttp/guzzle:~6.0

* update php github api to current version
php composer.phar require knplabs/github-api php-http/guzzle6-adapter ^1.1

* update composer.lock with new dependencies
php composer.phar update --with-dependencies

* install new vendor modules
php composer.phar install

* request is PUT/POST
https://stackoverflow.com/questions/14713258/cakephp-this-request-ispost-return-false-for-just-one-form-so-strange

* switch to guzzle 6 syntax
https://stackoverflow.com/questions/27825667/php-guzzlehttp-how-to-make-a-post-request-with-params

* use uri per guzzle 6 syntax
https://stackoverflow.com/questions/29722822/guzzle-returns-curl-error-3-url-malformed/53831345#53831345

* json() is deprecated in guzzle 6
guzzle/guzzle#1106

* install filesystem adapter for caching
php composer.phar require cache/filesystem-adapter

* update to current github api using file cache
https://github.com/KnpLabs/php-github-api using https://github.com/php-cache/filesystem-adapter as cache

* v3 api only returns orgs that user can operate on
https://developer.github.com/v3/orgs/#list-your-organizations

* remove CoGroupMember as it is no longer given
need to use the method that @skoranda uses at
https://github.com/Internet2/comanage-registry/blob/db3712032ad743039c5d9a322b031da758ba6373/app/AvailablePlugin/MailmanProvisioner/Model/CoMailmanProvisionerTarget.php#L738

* fixed adding member to team to use v3 api
fixes https://todos.internet2.edu/browse/CO-942

* report the id of user missing a github id in error

* we cannot remove pending invitations
this is due to KnpLabs/php-github-api#713
see also https://todos.internet2.edu/browse/CO-1818

* fix CoGroup to GitHub team provisioning

* just skip CoPerson if no GitHub identifier
a non-trivial number of CoPersons in a CO might not have a GitHub ID, so
just skip them when doing the provisioning otherwise users will get
confused by the error

* set the github redirect uri
this allows the user to set a single base callback url on GitHub for multiple provisioners
see https://developer.github.com/apps/building-oauth-apps/authorizing-oauth-apps/#redirect-urls
however see https://todos.internet2.edu/browse/CO-1819

* Revert "just skip CoPerson if no GitHub identifier"

This reverts commit da42e2b.

* only provision if the coperson is active

* strip whitespace from github username

* update for syracuse contributions
@guzzle guzzle deleted a comment from typoworx-de Aug 19, 2021
@guzzle guzzle deleted a comment from eexit Aug 19, 2021
@guzzle guzzle locked as off-topic and limited conversation to collaborators Aug 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants