-
-
Notifications
You must be signed in to change notification settings - Fork 317
Guzzle Resolver added #72
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
Conversation
Since some Guzzle\Client configuration can't be provided by array values (such as subscribers), I'd like to add this static method to provide my own Guzzle client instance:
Embed\RequestResolvers\Guzzle::setClient($client);
any feedbacks?
What about set the client as a config value? Something like this:
$info = Embed::create($url, [ 'resolver' => [ 'class' => 'Embed\\RequestResolvers\\Guzzle', 'config' => [ 'client' => $client, ] ] ]);
done
Great, some considerations:
I've executed the tests usign guzzle and many of them returns this error warning:
49) SoundcloudTest::testTwo
Argument 3 passed to GuzzleHttp\Client::request() must be of the type array, string given, called in /Volumes/Projects/Embed/vendor/guzzlehttp/guzzle/src/Client.php on line 88 and defined
/Volumes/Projects/Embed/vendor/guzzlehttp/guzzle/src/Client.php:127
/Volumes/Projects/Embed/vendor/guzzlehttp/guzzle/src/Client.php:88
/Volumes/Projects/Embed/src/RequestResolvers/Guzzle.php:33
/Volumes/Projects/Embed/src/RequestResolvers/Guzzle.php:33
/Volumes/Projects/Embed/src/Request.php:65
/Volumes/Projects/Embed/src/Request.php:139
/Volumes/Projects/Embed/src/Request.php:251
/Volumes/Projects/Embed/src/Adapters/File.php:43
/Volumes/Projects/Embed/src/Embed.php:96
/Volumes/Projects/Embed/src/Embed.php:28
/Volumes/Projects/Embed/tests/TestCaseBase.php:25
/Volumes/Projects/Embed/tests/SoundcloudTest.php:19
Currently the curl resolver prevents to load binary files (for example images, videos, pdfs, etc) here: https://github.com/oscarotero/Embed/blob/master/src/RequestResolvers/Curl.php#L179 so it only loads html documents files. This allows use url to binary files and returns the info like any other webpage without download it. Is Guzzle any option to do something similar?
Thanks
Hi, again. If you merge the latest commit in Embed in your fork, you can test the library with the guzzle resolver. All test cases use this function to get the info, so you can change the function to use guzzle.
I'll look into the binary files problem.
Thanks for the function.
I'm not sure if it's the best way to include Guzzle in tests: I added this in tests/boostrap.php
require_once __DIR__ . '/../vendor/autoload.php';
Most projects do this
By the way, which Guzzle version do you use? Using 5.3 latest, I did not encounter the issue you mentionned when limiting tests to Soundcloud (I do have other tests failing, happening also with the CURL resolver)
With latest commit, phpunit reports:
Curl
PHPUnit 4.8.5 by Sebastian Bergmann and contributors. ...........................FF...............................F.... 65 / 72 ( 90%) E.F.... Time: 2.46 minutes, Memory: 23.25Mb There was 1 error: 1) ViddlerTest::testOne Embed\Exceptions\InvalidUrlException: The url 'http://www.viddler.com/v/bdce8c7' returns the http code '0' /www/yoda/vendor/embed/embed/src/Embed.php:48 /www/yoda/vendor/embed/embed/tests/TestCaseBase.php:33 /www/yoda/vendor/embed/embed/tests/ViddlerTest.php:6 -- There were 4 failures: 1) ImagesBlacklistTest::testPlainText Failed asserting that two strings are identical. --- Expected +++ Actual @@ @@ -https://assets-cdn.github.com/images/spinners/octocat-spinner-32.gif +https://avatars1.githubusercontent.com/u/377873?v=3&s=400 /www/yoda/vendor/embed/embed/tests/TestCaseBase.php:16 /www/yoda/vendor/embed/embed/tests/ImagesBlacklistTest.php:16 2) ImagesBlacklistTest::testPlainUrlMatch Failed asserting that two strings are identical. --- Expected +++ Actual @@ @@ -https://assets-cdn.github.com/images/spinners/octocat-spinner-32.gif +https://avatars1.githubusercontent.com/u/377873?v=3&s=400 /www/yoda/vendor/embed/embed/tests/TestCaseBase.php:16 /www/yoda/vendor/embed/embed/tests/ImagesBlacklistTest.php:31 3) TwitterTest::testOmitScript Failed asserting that two strings are identical. --- Expected +++ Actual @@ @@ -<blockquote class="twitter-tweet"><p lang="es" dir="ltr">RT <a href="https://twitter.com/PabloHerreros">@PabloHerreros</a> Pepephone rompe la baraja - <a href="http://t.co/mFn7mcB1vy">http://t.co/mFn7mcB1vy</a></p>— pepephone (@pepephone) <a href="https://twitter.com/pepephone/status/436461 658601713664">February 20, 2014</a></blockquote> <script async src="//platform.twitter.com/widgets.js" charset="utf-8"></script> +<blockquote class="twitter-tweet"><p lang="es" dir="ltr">RT <a href="https://twitter.com/PabloHerreros">@PabloHerreros</a> Pepephone rompe la baraja - <a href="http://t.co/mFn7mcB1vy">http://t.co/mFn7mcB1vy</a></p>— pepephone (@pepephone) <a href="https://twitter.com/pepephone/status/436461 658601713664">February 20, 2014</a></blockquote> /www/yoda/vendor/embed/embed/tests/TestCaseBase.php:16 /www/yoda/vendor/embed/embed/tests/TwitterTest.php:21 4) WordPressTest::testOne Failed asserting that two strings are identical. --- Expected +++ Actual @@ @@ -Dave Ross: Optimize Image Files Like a Pro +Dave Ross: Optimize Image Files Like a Pro /www/yoda/vendor/embed/embed/tests/TestCaseBase.php:16 /www/yoda/vendor/embed/embed/tests/WordPressTest.php:8 FAILURES! Tests: 72, Assertions: 343, Errors: 1, Failures: 4.
Guzzle
PHPUnit 4.8.5 by Sebastian Bergmann and contributors. ........E..................FF...............................F.... 65 / 72 ( 90%) ..F.... Time: 2.65 minutes, Memory: 34.25Mb There was 1 error: 1) CodepenTest::testAnonymous GuzzleHttp\Exception\ClientException: Client error response [url] http://codepen.io/anon/details/QwPVNW [status code] 404 [reason phrase] Not Found /www/yoda/vendor/embed/embed/vendor/guzzlehttp/guzzle/src/Exception/RequestException.php:89 /www/yoda/vendor/embed/embed/vendor/guzzlehttp/guzzle/src/Subscriber/HttpError.php:33 /www/yoda/vendor/embed/embed/vendor/guzzlehttp/guzzle/src/Event/Emitter.php:109 /www/yoda/vendor/embed/embed/vendor/guzzlehttp/guzzle/src/RequestFsm.php:91 /www/yoda/vendor/embed/embed/vendor/guzzlehttp/guzzle/src/RequestFsm.php:132 /www/yoda/vendor/embed/embed/vendor/react/promise/src/FulfilledPromise.php:25 /www/yoda/vendor/embed/embed/vendor/guzzlehttp/ringphp/src/Future/CompletedFutureValue.php:55 /www/yoda/vendor/embed/embed/vendor/guzzlehttp/guzzle/src/Message/FutureResponse.php:43 /www/yoda/vendor/embed/embed/vendor/guzzlehttp/guzzle/src/RequestFsm.php:135 /www/yoda/vendor/embed/embed/vendor/guzzlehttp/guzzle/src/Client.php:165 /www/yoda/vendor/embed/embed/src/RequestResolvers/Guzzle.php:97 /www/yoda/vendor/embed/embed/src/RequestResolvers/Guzzle.php:76 /www/yoda/vendor/embed/embed/src/Request.php:66 /www/yoda/vendor/embed/embed/src/Request.php:139 /www/yoda/vendor/embed/embed/src/Request.php:251 /www/yoda/vendor/embed/embed/src/Adapters/Adapter.php:91 /www/yoda/vendor/embed/embed/src/Adapters/Adapter.php:75 /www/yoda/vendor/embed/embed/src/Embed.php:100 /www/yoda/vendor/embed/embed/src/Embed.php:43 /www/yoda/vendor/embed/embed/tests/TestCaseBase.php:32 /www/yoda/vendor/embed/embed/tests/CodepenTest.php:6 -- There were 4 failures: 1) ImagesBlacklistTest::testPlainText Failed asserting that two strings are identical. --- Expected +++ Actual @@ @@ -https://assets-cdn.github.com/images/spinners/octocat-spinner-32.gif +https://avatars1.githubusercontent.com/u/377873?v=3&s=400 /www/yoda/vendor/embed/embed/tests/TestCaseBase.php:16 /www/yoda/vendor/embed/embed/tests/ImagesBlacklistTest.php:16 2) ImagesBlacklistTest::testPlainUrlMatch Failed asserting that two strings are identical. --- Expected +++ Actual @@ @@ -https://assets-cdn.github.com/images/spinners/octocat-spinner-32.gif +https://avatars1.githubusercontent.com/u/377873?v=3&s=400 /www/yoda/vendor/embed/embed/tests/TestCaseBase.php:16 /www/yoda/vendor/embed/embed/tests/ImagesBlacklistTest.php:31 3) TwitterTest::testOmitScript Failed asserting that two strings are identical. --- Expected +++ Actual @@ @@ -<blockquote class="twitter-tweet"><p lang="es" dir="ltr">RT <a href="https://twitter.com/PabloHerreros">@PabloHerreros</a> Pepephone rompe la baraja - <a href="http://t.co/mFn7mcB1vy">http://t.co/mFn7mcB1vy</a></p>— pepephone (@pepephone) <a href="https://twitter.com/pepephone/status/436461 658601713664">February 20, 2014</a></blockquote> <script async src="//platform.twitter.com/widgets.js" charset="utf-8"></script> +<blockquote class="twitter-tweet"><p lang="es" dir="ltr">RT <a href="https://twitter.com/PabloHerreros">@PabloHerreros</a> Pepephone rompe la baraja - <a href="http://t.co/mFn7mcB1vy">http://t.co/mFn7mcB1vy</a></p>— pepephone (@pepephone) <a href="https://twitter.com/pepephone/status/436461 658601713664">February 20, 2014</a></blockquote> /www/yoda/vendor/embed/embed/tests/TestCaseBase.php:16 /www/yoda/vendor/embed/embed/tests/TwitterTest.php:21 4) WordPressTest::testOne Failed asserting that two strings are identical. --- Expected +++ Actual @@ @@ -Dave Ross: Optimize Image Files Like a Pro +Dave Ross: Optimize Image Files Like a Pro /www/yoda/vendor/embed/embed/tests/TestCaseBase.php:16 /www/yoda/vendor/embed/embed/tests/WordPressTest.php:8
I'm using the latest version of Guzzle (6.0) (installed by default with composer require guzzlehttp/guzzle)
And about the failing tests, it's very hard to keep all these tests in green, because the same url returns different info deppending of the machine that execute the code. For example, all tests in my local environment are pased, but not in travis (the result of the same request is different). I don't know the reason, maybe it's about the php configuration.
Oh! I see the failures now, due to major BC in 6.x.
I'll fix this.
Looks like Guzzle 6 has removed getEffectiveUrl() : guzzle/guzzle#1166
Maybe we could wait for 6.x support ?
We can create a Embed\RequestResolvers\Guzzle5 and Embed\RequestResolvers\Guzzle6 (or just Guzzle). What you think?
Done!
Thanks
Please, can you rename the file Guzzle.php to Guzzle5.php? it's to keep psr-4 compatibility
my bad, done
Hi, can this be merged or is there anything else to do?
About the binary files problems, I think guzzle has events that can stop the download at any time, but I'm not sure.
I think you can merge even if there's this binary files download, since IMHO the main purpose of Guzzle is to allow Request mocking (which is important in my scraping project for instance)
Ok, thanks for the contribution 😄
you're welcome 👍
#71