Replacing Zend HTTP client with Guzzle
merged branch mtdowling/guzzle (PR #29)

Commits
-------

b218d17 Replacing Zend HTTP client with Guzzle

Discussion
----------

Replacing Zend HTTP client with Guzzle

Here's how Goutte would look using Guzzle.  What do you think?

---------------------------------------------------------------------------

by stof at 2011-11-12T20:00:55Z

There was a discussion on Twitter yesterday to replace Zend Http Client with [Buzz](https://github.com/kriswallsmith/Buzz) and @fabpot said he already started the implementation locally

---------------------------------------------------------------------------

by stof at 2011-11-12T20:26:06Z

Btw, a good point about both ZHC and Buzz is that they support different ways to do the request whereas Guzzle requires having the curl extension AFAICS (correct me if I missed something there).

And another point in favor of Buzz is that the library is more lightweight as it focuses on being an HTTP client whereas Guzzle has more features (which is great when dealing with a webservice client but not needed for Goutte).

However, nothing forbids to have 2 different BrowserKit implementations, one based on Buzz and the other on Guzzle and letting the user choose which one he wants to use (for instance someone using Guzzle in its project may prefer a version based on Guzzle).

@fabpot what do you think about this ?

---------------------------------------------------------------------------

by mtdowling at 2011-11-12T22:19:35Z

You're right -- Guzzle requires curl.  Every major linux distro includes curl and a version of PHP with the libcurl bindings enabled.  I'm not sure about Windows, but I imagine it's trivial to install if it is not installed already.  This is not an issue.

Guzzle is primarily an HTTP client and provides a very small namespace dedicated to building web service clients.  There are currently only 14 classes in the ``Service`` layer that would not normally be used by Goutte.  This again, in my opinion, is not an issue.

Guzzle has 100% code coverage and is a perfect match for Goutte.  From what fabpot said on the Buzz implementation was that Buzz was not ready.  One of the things that Guzzle can handle that I don't think is yet implemented in Buzz, is properly handling multiple Cookies of the same name (correct me if I'm wrong).

If it is decided to use multiple BrowserKit implementations, then let me know.

---------------------------------------------------------------------------

by stof at 2011-11-12T22:23:45Z

Buzz was not ready when he started to try it. There were many improvements in the previous weeks with the 0.3 release.

Buzz supports multiple cookies properly. The place where they were not supported properly (and may still not be as I'm not totally sure if a fix has been merged) is BrowserKit.

---------------------------------------------------------------------------

by mtdowling at 2011-11-12T23:11:44Z

You're right.  Looks like it was [fixed](https://github.com/kriswallsmith/Buzz/pull/11) yesterday.  It also looks like [Buzz currently supports only two kinds of clients](https://github.com/kriswallsmith/Buzz/tree/master/lib/Buzz/Client) -- file_get_contents and curl based clients.  file_get_contents requires ``allow_url_fopen = On`` and the curl implementation, of course, requires curl.

Another reason I think Guzzle would be a smart move for Goutte is because [Guzzle manages persistent connections](http://guzzlephp.org/docs/tour/http/#managed-persistent-http-connections).  Goutte, being tool for scraping websites, is going to send a large number of requests in succession to the same server.  Buzz creates a new curl handle for each request (per client), whereas Guzzle implicitly manages a pool of persistent connections that are reused when possible (file_get_contents does not reuse connections either).  Using persistent connections will make Goutte more efficient, faster, cause less strain on a network (fewer TCP connections), and reduced load on the remote server.

---------------------------------------------------------------------------

by mtdowling at 2011-11-14T18:23:08Z

I think there could be some value in being able to choose which http client you want to use with Goutte (buzz, Zend, guzzle, etc).  I think most people will use the generated phar file, so adding these other clients would only slow down cloning the repo and running the tests (which didn't exist before this PR).  The generated phar could use whichever client is deemed as the default so that it doesn't bloat the end product.

I'm not sure what the problem is with the Zend implementation.  If it's because we need to include the full ZF project, then couldn't we maintain a subtree split or use the one that's maintained by knplabs (iirc)?

I could probably submit a pull request with guzzle, buzz, and Zend browser kit adapters within a day or two if that's something of interest.

-Michael

On Nov 12, 2011, at 4:23 PM, Christophe Coevoet<reply@reply.github.com> wrote:

> Buzz was not ready when he started to try it. There were many improvements in the previous weeks with the 0.3 release.
>
> Buzz supports multiple cookies properly. The place where they were not supported properly (and may still not be as I'm not totally sure if a fix has been merged) is BrowserKit.
>
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/fabpot/Goutte/pull/29#issuecomment-2719707

---------------------------------------------------------------------------

by stof at 2011-11-14T18:32:06Z

The issue is that ZF components are heavily coupled: we need to use 6 or 7 subtrees.

---------------------------------------------------------------------------

by jc- at 2012-01-17T12:34:21Z

Another issue also is that the Zend HTTP client doesn't support multiple cookie headers.

---------------------------------------------------------------------------

by cystbear at 2012-02-22T10:37:45Z

:+1:
fixed phpunit configuration file name
fixed CS
switched to composer for dependency management
fixed CS
moved Goutte outside of a src/ directory
moved tests inside the Goutte directory
simplified compiler
tweaked README file
added a CHANGELOG
Updating to v2.6.0 of Guzzle.  Getting tests passing again.
Updating phar
merged branch mtdowling/master (PR #46)

Commits
-------

fe4152d Updating to v2.6.0 of Guzzle.  Getting tests passing again. Updating phar

Discussion
----------

Updating Goutte to work with Guzzle 2.6.0

Fixed the tests, removed dependency on Guzzle\Service namespace, and updated the phar.  I will possibly add subtree splits to Guzzle that will make it so that you no longer need to pull in the Service layer.
tweaked composer.json
merge BrowserKit request headers before into Guzzle request

this fixes HTTP_REFERRER and couple of other unset headers bugs
return proper response on 404 instead of exception

this commit makes client return proper response object on
"page not found" instead of throwing exception.
set raw cookies to Guzzle for proper evaluation

Seems that Guzzle doesn't evaluate properly clean cookie values
when there's some cookie with `some:value;`, setting clean value
to Guzzle makes him to lose `;`. Using `allRawValues()` instead
fixed issue.
merged branch everzet/patch-1 (PR #50)

Commits
-------

f476e56 merge BrowserKit request headers before into Guzzle request

Discussion
----------

Merge BrowserKit request headers before into Guzzle request

this fixes HTTP_REFERRER and couple of other unset headers bugs
merged branch everzet/patch-2 (PR #51)

Commits
-------

e4c067e return proper response on 404 instead of exception

Discussion
----------

Return proper response on 404 instead of exception

this commit makes client return proper response object on
"page not found" instead of throwing exception.
merged branch everzet/patch-3 (PR #52)

Commits
-------

a10349f set raw cookies to Guzzle for proper evaluation

Discussion
----------

set raw cookies to Guzzle for proper evaluation

Seems that Guzzle doesn't evaluate properly clean cookie values
when there's some cookie with `some:value;`, setting clean value
to Guzzle makes him to lose `;`. Using `allRawValues()` instead
fixed issue.