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 · 16 comments

Comments

Projects
None yet
8 participants
@rap2hpoutre
Copy link

rap2hpoutre commented Jun 2, 2015

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

This comment has been minimized.

Copy link
Contributor

GrahamCampbell commented Jun 2, 2015

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

No. It's because of psr7.

@rap2hpoutre

This comment has been minimized.

Copy link
Author

rap2hpoutre commented Jun 2, 2015

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

@GrahamCampbell

This comment has been minimized.

Copy link
Contributor

GrahamCampbell commented Jun 2, 2015

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

This comment has been minimized.

Copy link
Author

rap2hpoutre commented Jun 2, 2015

Ok, thank you 😢

@rap2hpoutre

This comment has been minimized.

Copy link
Author

rap2hpoutre commented Jun 2, 2015

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

This comment has been minimized.

Copy link
Member

mtdowling commented Jun 2, 2015

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()).

@mtdowling mtdowling closed this Jun 2, 2015

@rap2hpoutre

This comment has been minimized.

Copy link
Author

rap2hpoutre commented Jun 2, 2015

Ok thanks for your answer!

@SvenRtbg

This comment has been minimized.

Copy link

SvenRtbg commented Jul 23, 2015

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

This comment has been minimized.

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

This comment has been minimized.

Copy link

SvenRtbg commented Feb 17, 2016

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

This comment has been minimized.

Copy link

mailopl commented Feb 24, 2016

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

@artickc

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor

alexeyshockov commented Sep 18, 2017

@artickc, call json_last_error_msg() and see.

@artickc

This comment has been minimized.

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

This comment has been minimized.

Copy link
Author

rap2hpoutre commented Sep 18, 2017

@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

This comment has been minimized.

Copy link

SvenRtbg commented Sep 18, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment