search

misd-service-development / guzzle-bundle

[NOT MAINTAINED] Integrates Guzzle into your Symfony2 application
99 stars 54 forks source link

Guzzle 3.6.0 support #25

Closed mtdowling closed 11 years ago

mtdowling commented 11 years ago

Added a custom ResponseParserInterface instead of extending from DefaultResponseParser. Fixed some tests.

petermanser commented 11 years ago

:+1:

thewilkybarkid commented 11 years ago

Thanks for this. With the tests, I think we lose the mocked responses/execute() calls by using $command->prepare()->getRequest() can't we?

With the the JMSSerializerResponseParser, I think it might be better (/more Symfonic, if that's a word! :smiley:) to typehint ResponseParserInterface and inject an instance of OperationResponseParser (it musn't be nullable too).

By improving it to use the OperationResponseParser instead of just the DefaultResponseParser would resolve #21. Are there any cases in Guzzle where you would expect someone to use just the default one?

When this is complete I'll cherry-pick it back to the 1.0 branch (=Symfony 2.1).

mtdowling commented 11 years ago

I updated the PR to not require the mocking that was going on in the JMSSerializerRequestTest. I updated the JMSSerializerResponseParser to now use a ResponseParserInterface in the constructor.

Lots of tests broke when I removed the creation of a default DefaultResponseParser in the JMSSerializerResponseParser constructor. I think this is because the DI configs are not wired up to inject a ResponseParserInterface into the JMSSerializerResponseParser, but I wasn't sure how to update them. Is that something that you can handle?

thewilkybarkid commented 11 years ago

That's fine thanks, I'll merge and finish it off later in the week.

thewilkybarkid commented 11 years ago

Now finished and merged, thanks!