From bf9ace0643f654e7ccd9c020b8b501ad56cd19de Mon Sep 17 00:00:00 2001 From: adev Date: Tue, 24 Oct 2017 22:55:40 +0200 Subject: [PATCH 1/8] Use httplug --- app/AppKernel.php | 1 + app/config/config.yml | 19 + composer.json | 12 +- .../CoreBundle/Helper/DownloadImages.php | 21 +- .../CoreBundle/Helper/HttpClientFactory.php | 59 +-- .../CoreBundle/Resources/config/services.yml | 11 +- .../ImportBundle/Import/PocketImport.php | 90 ++-- .../Resources/config/services.yml | 8 +- .../CoreBundle/Helper/DownloadImagesTest.php | 79 +--- .../ImportBundle/Import/PocketImportTest.php | 402 ++++++++---------- 10 files changed, 329 insertions(+), 373 deletions(-) diff --git a/app/AppKernel.php b/app/AppKernel.php index 7d19e9abc..4a54da298 100644 --- a/app/AppKernel.php +++ b/app/AppKernel.php @@ -34,6 +34,7 @@ class AppKernel extends Kernel new FOS\JsRoutingBundle\FOSJsRoutingBundle(), new BD\GuzzleSiteAuthenticatorBundle\BDGuzzleSiteAuthenticatorBundle(), new OldSound\RabbitMqBundle\OldSoundRabbitMqBundle(), + new Http\HttplugBundle\HttplugBundle(), // wallabag bundles new Wallabag\CoreBundle\WallabagCoreBundle(), diff --git a/app/config/config.yml b/app/config/config.yml index 078f277ac..309945c50 100644 --- a/app/config/config.yml +++ b/app/config/config.yml @@ -370,3 +370,22 @@ jms_serializer: sensio_framework_extra: router: annotations: false + +httplug: + clients: + wallabag_core: + factory: 'wallabag_core.http_client_factory' + plugins: ['httplug.plugin.logger'] + wallabag_core.entry.download_images: + factory: 'httplug.factory.auto' + plugins: ['httplug.plugin.logger'] + wallabag_import.pocket.client: + factory: 'httplug.factory.auto' + plugins: + - 'httplug.plugin.logger' + - header_defaults: + headers: + 'content-type': 'application/json' + 'X-Accept': 'application/json' + discovery: + client: false diff --git a/composer.json b/composer.json index b1c144c72..55e7f7651 100644 --- a/composer.json +++ b/composer.json @@ -66,8 +66,9 @@ "simplepie/simplepie": "~1.5", "willdurand/hateoas-bundle": "~1.3", "liip/theme-bundle": "^1.4.6", - "lexik/form-filter-bundle": "^5.0", - "j0k3r/graby": "^1.0", + "lexik/form-filter-bundle": "^5.0.4", + "j0k3r/graby": "^2.0", + "php-http/guzzle5-adapter": "^2.0", "friendsofsymfony/user-bundle": "2.0.*", "friendsofsymfony/oauth-server-bundle": "^1.5", "stof/doctrine-extensions-bundle": "^1.2", @@ -89,7 +90,8 @@ "bdunogier/guzzle-site-authenticator": "^1.0.0", "defuse/php-encryption": "^2.1", "html2text/html2text": "^4.1", - "pragmarx/recovery": "^0.1.0" + "pragmarx/recovery": "^0.1.0", + "php-http/httplug-bundle": "^1.14" }, "require-dev": { "doctrine/doctrine-fixtures-bundle": "~3.0", @@ -101,7 +103,9 @@ "phpstan/phpstan": "^0.11.0", "phpstan/phpstan-phpunit": "^0.11.0", "phpstan/phpstan-symfony": "^0.11.0", - "phpstan/phpstan-doctrine": "^0.11.0" + "phpstan/phpstan-doctrine": "^0.11.0", + "php-http/mock-client": "^1.0", + "guzzlehttp/psr7": "^1.0" }, "suggest": { "ext-imagick": "To keep GIF animation when downloading image is enabled" diff --git a/src/Wallabag/CoreBundle/Helper/DownloadImages.php b/src/Wallabag/CoreBundle/Helper/DownloadImages.php index c1645e45a..e57490608 100644 --- a/src/Wallabag/CoreBundle/Helper/DownloadImages.php +++ b/src/Wallabag/CoreBundle/Helper/DownloadImages.php @@ -2,8 +2,13 @@ namespace Wallabag\CoreBundle\Helper; -use GuzzleHttp\Client; -use GuzzleHttp\Message\Response; +use Http\Client\Common\HttpMethodsClient; +use Http\Client\Common\Plugin\ErrorPlugin; +use Http\Client\Common\PluginClient; +use Http\Client\HttpClient; +use Http\Discovery\MessageFactoryDiscovery; +use Http\Message\MessageFactory; +use Psr\Http\Message\ResponseInterface; use Psr\Log\LoggerInterface; use Symfony\Component\DomCrawler\Crawler; use Symfony\Component\Finder\Finder; @@ -19,9 +24,9 @@ class DownloadImages private $mimeGuesser; private $wallabagUrl; - public function __construct(Client $client, $baseFolder, $wallabagUrl, LoggerInterface $logger) + public function __construct(HttpClient $client, $baseFolder, $wallabagUrl, LoggerInterface $logger, MessageFactory $messageFactory = null) { - $this->client = $client; + $this->client = new HttpMethodsClient(new PluginClient($client, [new ErrorPlugin()]), $messageFactory ?: MessageFactoryDiscovery::find()); $this->baseFolder = $baseFolder; $this->wallabagUrl = rtrim($wallabagUrl, '/'); $this->logger = $logger; @@ -135,7 +140,7 @@ class DownloadImages $localPath = $folderPath . '/' . $hashImage . '.' . $ext; try { - $im = imagecreatefromstring($res->getBody()); + $im = imagecreatefromstring((string) $res->getBody()); } catch (\Exception $e) { $im = false; } @@ -306,14 +311,14 @@ class DownloadImages /** * Retrieve and validate the extension from the response of the url of the image. * - * @param Response $res Guzzle Response + * @param ResponseInterface $res Http Response * @param string $imagePath Path from the src image from the content (used for log only) * * @return string|false Extension name or false if validation failed */ - private function getExtensionFromResponse(Response $res, $imagePath) + private function getExtensionFromResponse(ResponseInterface $res, $imagePath) { - $ext = $this->mimeGuesser->guess($res->getHeader('content-type')); + $ext = $this->mimeGuesser->guess(current($res->getHeader('content-type'))); $this->logger->debug('DownloadImages: Checking extension', ['ext' => $ext, 'header' => $res->getHeader('content-type')]); // ok header doesn't have the extension, try a different way diff --git a/src/Wallabag/CoreBundle/Helper/HttpClientFactory.php b/src/Wallabag/CoreBundle/Helper/HttpClientFactory.php index 4602a6841..4899d3d41 100644 --- a/src/Wallabag/CoreBundle/Helper/HttpClientFactory.php +++ b/src/Wallabag/CoreBundle/Helper/HttpClientFactory.php @@ -2,16 +2,18 @@ namespace Wallabag\CoreBundle\Helper; -use Graby\Ring\Client\SafeCurlHandler; -use GuzzleHttp\Client; +use GuzzleHttp\Client as GuzzleClient; use GuzzleHttp\Cookie\CookieJar; use GuzzleHttp\Event\SubscriberInterface; +use Http\Adapter\Guzzle5\Client as GuzzleAdapter; use Psr\Log\LoggerInterface; +use Http\Client\HttpClient; +use Http\HttplugBundle\ClientFactory\ClientFactory; /** - * Builds and configures the Guzzle HTTP client. + * Builds and configures the HTTP client. */ -class HttpClientFactory +class HttpClientFactory implements ClientFactory { /** @var [\GuzzleHttp\Event\SubscriberInterface] */ private $subscribers = []; @@ -36,29 +38,6 @@ class HttpClientFactory $this->logger = $logger; } - /** - * @return \GuzzleHttp\Client|null - */ - public function buildHttpClient() - { - $this->logger->log('debug', 'Restricted access config enabled?', ['enabled' => (int) $this->restrictedAccess]); - - if (0 === (int) $this->restrictedAccess) { - return; - } - - // we clear the cookie to avoid websites who use cookies for analytics - $this->cookieJar->clear(); - // need to set the (shared) cookie jar - $client = new Client(['handler' => new SafeCurlHandler(), 'defaults' => ['cookies' => $this->cookieJar]]); - - foreach ($this->subscribers as $subscriber) { - $client->getEmitter()->attach($subscriber); - } - - return $client; - } - /** * Adds a subscriber to the HTTP client. * @@ -68,4 +47,30 @@ class HttpClientFactory { $this->subscribers[] = $subscriber; } + + /** + * Input an array of configuration to be able to create a HttpClient. + * + * @param array $config + * + * @return HttpClient + */ + public function createClient(array $config = []) + { + $this->logger->log('debug', 'Restricted access config enabled?', ['enabled' => (int) $this->restrictedAccess]); + + if (0 === (int) $this->restrictedAccess) { + return new GuzzleAdapter(new GuzzleClient()); + } + + // we clear the cookie to avoid websites who use cookies for analytics + $this->cookieJar->clear(); + // need to set the (shared) cookie jar + $guzzle = new GuzzleClient(['defaults' => ['cookies' => $this->cookieJar]]); + foreach ($this->subscribers as $subscriber) { + $guzzle->getEmitter()->attach($subscriber); + } + + return new GuzzleAdapter($guzzle); + } } diff --git a/src/Wallabag/CoreBundle/Resources/config/services.yml b/src/Wallabag/CoreBundle/Resources/config/services.yml index 280d779da..319869513 100644 --- a/src/Wallabag/CoreBundle/Resources/config/services.yml +++ b/src/Wallabag/CoreBundle/Resources/config/services.yml @@ -42,7 +42,7 @@ services: - error_message: '%wallabag_core.fetching_error_message%' error_message_title: '%wallabag_core.fetching_error_message_title%' - - "@wallabag_core.guzzle.http_client" + - "@wallabag_core.http_client" - "@wallabag_core.graby.config_builder" calls: - [ setLogger, [ "@logger" ] ] @@ -55,9 +55,8 @@ services: - {} - "@logger" - wallabag_core.guzzle.http_client: - class: GuzzleHttp\ClientInterface - factory: ["@wallabag_core.guzzle.http_client_factory", buildHttpClient] + wallabag_core.http_client: + alias: 'httplug.client.wallabag_core' wallabag_core.guzzle_authenticator.config_builder: class: Wallabag\CoreBundle\GuzzleSiteAuthenticator\GrabySiteConfigBuilder @@ -73,7 +72,7 @@ services: bd_guzzle_site_authenticator.site_config_builder: alias: wallabag_core.guzzle_authenticator.config_builder - wallabag_core.guzzle.http_client_factory: + wallabag_core.http_client_factory: class: Wallabag\CoreBundle\Helper\HttpClientFactory arguments: - "@wallabag_core.guzzle.cookie_jar" @@ -212,7 +211,7 @@ services: - "@logger" wallabag_core.entry.download_images.client: - class: GuzzleHttp\Client + alias: 'httplug.client.wallabag_core.entry.download_images' wallabag_core.helper.crypto_proxy: class: Wallabag\CoreBundle\Helper\CryptoProxy diff --git a/src/Wallabag/ImportBundle/Import/PocketImport.php b/src/Wallabag/ImportBundle/Import/PocketImport.php index a39d81568..9467fae24 100644 --- a/src/Wallabag/ImportBundle/Import/PocketImport.php +++ b/src/Wallabag/ImportBundle/Import/PocketImport.php @@ -2,13 +2,22 @@ namespace Wallabag\ImportBundle\Import; -use GuzzleHttp\Client; -use GuzzleHttp\Exception\RequestException; +use Http\Client\Common\HttpMethodsClient; +use Http\Client\Common\Plugin\ErrorPlugin; +use Http\Client\Common\PluginClient; +use Http\Client\HttpClient; +use Http\Discovery\MessageFactoryDiscovery; +use Http\Message\MessageFactory; +use Http\Client\Exception\RequestException; use Wallabag\CoreBundle\Entity\Entry; +use Psr\Http\Message\ResponseInterface; class PocketImport extends AbstractImport { const NB_ELEMENTS = 5000; + /** + * @var HttpMethodsClient + */ private $client; private $accessToken; @@ -55,24 +64,18 @@ class PocketImport extends AbstractImport */ public function getRequestToken($redirectUri) { - $request = $this->client->createRequest('POST', 'https://getpocket.com/v3/oauth/request', - [ - 'body' => json_encode([ - 'consumer_key' => $this->user->getConfig()->getPocketConsumerKey(), - 'redirect_uri' => $redirectUri, - ]), - ] - ); - try { - $response = $this->client->send($request); + $response = $this->client->post('https://getpocket.com/v3/oauth/request', [], json_encode([ + 'consumer_key' => $this->user->getConfig()->getPocketConsumerKey(), + 'redirect_uri' => $redirectUri, + ])); } catch (RequestException $e) { $this->logger->error(sprintf('PocketImport: Failed to request token: %s', $e->getMessage()), ['exception' => $e]); return false; } - return $response->json()['code']; + return $this->jsonDecode($response)['code']; } /** @@ -85,24 +88,19 @@ class PocketImport extends AbstractImport */ public function authorize($code) { - $request = $this->client->createRequest('POST', 'https://getpocket.com/v3/oauth/authorize', - [ - 'body' => json_encode([ - 'consumer_key' => $this->user->getConfig()->getPocketConsumerKey(), - 'code' => $code, - ]), - ] - ); try { - $response = $this->client->send($request); + $response = $this->client->post('https://getpocket.com/v3/oauth/authorize', [], json_encode([ + 'consumer_key' => $this->user->getConfig()->getPocketConsumerKey(), + 'code' => $code, + ])); } catch (RequestException $e) { $this->logger->error(sprintf('PocketImport: Failed to authorize client: %s', $e->getMessage()), ['exception' => $e]); return false; } - $this->accessToken = $response->json()['access_token']; + $this->accessToken = $this->jsonDecode($response)['access_token']; return true; } @@ -114,29 +112,23 @@ class PocketImport extends AbstractImport { static $run = 0; - $request = $this->client->createRequest('POST', 'https://getpocket.com/v3/get', - [ - 'body' => json_encode([ - 'consumer_key' => $this->user->getConfig()->getPocketConsumerKey(), - 'access_token' => $this->accessToken, - 'detailType' => 'complete', - 'state' => 'all', - 'sort' => 'newest', - 'count' => self::NB_ELEMENTS, - 'offset' => $offset, - ]), - ] - ); - try { - $response = $this->client->send($request); + $response = $this->client->post('https://getpocket.com/v3/get', [], json_encode([ + 'consumer_key' => $this->user->getConfig()->getPocketConsumerKey(), + 'access_token' => $this->accessToken, + 'detailType' => 'complete', + 'state' => 'all', + 'sort' => 'newest', + 'count' => self::NB_ELEMENTS, + 'offset' => $offset, + ])); } catch (RequestException $e) { $this->logger->error(sprintf('PocketImport: Failed to import: %s', $e->getMessage()), ['exception' => $e]); return false; } - $entries = $response->json(); + $entries = $this->jsonDecode($response); if ($this->producer) { $this->parseEntriesForProducer($entries['list']); @@ -159,13 +151,14 @@ class PocketImport extends AbstractImport } /** - * Set the Guzzle client. + * Set the Http client. * - * @param Client $client + * @param HttpClient $client + * @param MessageFactory|null $messageFactory */ - public function setClient(Client $client) + public function setClient(HttpClient $client, MessageFactory $messageFactory = null) { - $this->client = $client; + $this->client = new HttpMethodsClient(new PluginClient($client, [new ErrorPlugin()]), $messageFactory ?: MessageFactoryDiscovery::find()); } /** @@ -252,4 +245,15 @@ class PocketImport extends AbstractImport return $importedEntry; } + + protected function jsonDecode(ResponseInterface $response) + { + $data = \json_decode((string) $response->getBody(), true); + + if (JSON_ERROR_NONE !== json_last_error()) { + throw new \InvalidArgumentException('Unable to parse JSON data: ' . json_last_error_msg()); + } + + return $data; + } } diff --git a/src/Wallabag/ImportBundle/Resources/config/services.yml b/src/Wallabag/ImportBundle/Resources/config/services.yml index 2dd7dff8c..973c0d03e 100644 --- a/src/Wallabag/ImportBundle/Resources/config/services.yml +++ b/src/Wallabag/ImportBundle/Resources/config/services.yml @@ -7,13 +7,7 @@ services: class: Wallabag\ImportBundle\Import\ImportChain wallabag_import.pocket.client: - class: GuzzleHttp\Client - arguments: - - - defaults: - headers: - content-type: "application/json" - X-Accept: "application/json" + alias: 'httplug.client.wallabag_import.pocket.client' wallabag_import.pocket.import: class: Wallabag\ImportBundle\Import\PocketImport diff --git a/tests/Wallabag/CoreBundle/Helper/DownloadImagesTest.php b/tests/Wallabag/CoreBundle/Helper/DownloadImagesTest.php index cda5f8431..537583647 100644 --- a/tests/Wallabag/CoreBundle/Helper/DownloadImagesTest.php +++ b/tests/Wallabag/CoreBundle/Helper/DownloadImagesTest.php @@ -3,9 +3,10 @@ namespace Tests\Wallabag\CoreBundle\Helper; use GuzzleHttp\Client; -use GuzzleHttp\Message\Response; use GuzzleHttp\Stream\Stream; use GuzzleHttp\Subscriber\Mock; +use Http\Mock\Client as HttpMockClient; +use GuzzleHttp\Psr7\Response; use Monolog\Handler\TestHandler; use Monolog\Logger; use PHPUnit\Framework\TestCase; @@ -32,18 +33,14 @@ class DownloadImagesTest extends TestCase */ public function testProcessHtml($html, $url) { - $client = new Client(); + $httpMockClient = new HttpMockClient(); - $mock = new Mock([ - new Response(200, ['content-type' => 'image/png'], Stream::factory(file_get_contents(__DIR__ . '/../fixtures/unnamed.png'))), - ]); - - $client->getEmitter()->attach($mock); + $httpMockClient->addResponse(new Response(200, ['content-type' => 'image/png'], file_get_contents(__DIR__ . '/../fixtures/unnamed.png'))); $logHandler = new TestHandler(); $logger = new Logger('test', [$logHandler]); - $download = new DownloadImages($client, sys_get_temp_dir() . '/wallabag_test', 'http://wallabag.io/', $logger); + $download = new DownloadImages($httpMockClient, sys_get_temp_dir() . '/wallabag_test', 'http://wallabag.io/', $logger); $res = $download->processHtml(123, $html, $url); @@ -53,18 +50,13 @@ class DownloadImagesTest extends TestCase public function testProcessHtmlWithBadImage() { - $client = new Client(); - - $mock = new Mock([ - new Response(200, ['content-type' => 'application/json'], Stream::factory('')), - ]); - - $client->getEmitter()->attach($mock); + $httpMockClient = new HttpMockClient(); + $httpMockClient->addResponse(new Response(200, ['content-type' => 'application/json'], '')); $logHandler = new TestHandler(); $logger = new Logger('test', [$logHandler]); - $download = new DownloadImages($client, sys_get_temp_dir() . '/wallabag_test', 'http://wallabag.io/', $logger); + $download = new DownloadImages($httpMockClient, sys_get_temp_dir() . '/wallabag_test', 'http://wallabag.io/', $logger); $res = $download->processHtml(123, '
', 'http://imgur.com/gallery/WxtWY'); $this->assertContains('http://i.imgur.com/T9qgcHc.jpg', $res, 'Image were not replace because of content-type'); @@ -85,18 +77,13 @@ class DownloadImagesTest extends TestCase */ public function testProcessSingleImage($header, $extension) { - $client = new Client(); - - $mock = new Mock([ - new Response(200, ['content-type' => $header], Stream::factory(file_get_contents(__DIR__ . '/../fixtures/unnamed.png'))), - ]); - - $client->getEmitter()->attach($mock); + $httpMockClient = new HttpMockClient(); + $httpMockClient->addResponse(new Response(200, ['content-type' => $header], file_get_contents(__DIR__ . '/../fixtures/unnamed.png'))); $logHandler = new TestHandler(); $logger = new Logger('test', [$logHandler]); - $download = new DownloadImages($client, sys_get_temp_dir() . '/wallabag_test', 'http://wallabag.io/', $logger); + $download = new DownloadImages($httpMockClient, sys_get_temp_dir() . '/wallabag_test', 'http://wallabag.io/', $logger); $res = $download->processSingleImage(123, 'T9qgcHc.jpg', 'http://imgur.com/gallery/WxtWY'); $this->assertContains('/assets/images/9/b/9b0ead26/ebe60399.' . $extension, $res); @@ -104,18 +91,13 @@ class DownloadImagesTest extends TestCase public function testProcessSingleImageWithBadUrl() { - $client = new Client(); - - $mock = new Mock([ - new Response(404, []), - ]); - - $client->getEmitter()->attach($mock); + $httpMockClient = new HttpMockClient(); + $httpMockClient->addResponse(new Response(404, [])); $logHandler = new TestHandler(); $logger = new Logger('test', [$logHandler]); - $download = new DownloadImages($client, sys_get_temp_dir() . '/wallabag_test', 'http://wallabag.io/', $logger); + $download = new DownloadImages($httpMockClient, sys_get_temp_dir() . '/wallabag_test', 'http://wallabag.io/', $logger); $res = $download->processSingleImage(123, 'T9qgcHc.jpg', 'http://imgur.com/gallery/WxtWY'); $this->assertFalse($res, 'Image can not be found, so it will not be replaced'); @@ -123,18 +105,13 @@ class DownloadImagesTest extends TestCase public function testProcessSingleImageWithBadImage() { - $client = new Client(); - - $mock = new Mock([ - new Response(200, ['content-type' => 'image/png'], Stream::factory('')), - ]); - - $client->getEmitter()->attach($mock); + $httpMockClient = new HttpMockClient(); + $httpMockClient->addResponse(new Response(200, ['content-type' => 'image/png'], '')); $logHandler = new TestHandler(); $logger = new Logger('test', [$logHandler]); - $download = new DownloadImages($client, sys_get_temp_dir() . '/wallabag_test', 'http://wallabag.io/', $logger); + $download = new DownloadImages($httpMockClient, sys_get_temp_dir() . '/wallabag_test', 'http://wallabag.io/', $logger); $res = $download->processSingleImage(123, 'http://i.imgur.com/T9qgcHc.jpg', 'http://imgur.com/gallery/WxtWY'); $this->assertFalse($res, 'Image can not be loaded, so it will not be replaced'); @@ -142,18 +119,13 @@ class DownloadImagesTest extends TestCase public function testProcessSingleImageFailAbsolute() { - $client = new Client(); - - $mock = new Mock([ - new Response(200, ['content-type' => 'image/png'], Stream::factory(file_get_contents(__DIR__ . '/../fixtures/unnamed.png'))), - ]); - - $client->getEmitter()->attach($mock); + $httpMockClient = new HttpMockClient(); + $httpMockClient->addResponse(new Response(200, ['content-type' => 'image/png'], file_get_contents(__DIR__ . '/../fixtures/unnamed.png'))); $logHandler = new TestHandler(); $logger = new Logger('test', [$logHandler]); - $download = new DownloadImages($client, sys_get_temp_dir() . '/wallabag_test', 'http://wallabag.io/', $logger); + $download = new DownloadImages($httpMockClient, sys_get_temp_dir() . '/wallabag_test', 'http://wallabag.io/', $logger); $res = $download->processSingleImage(123, '/i.imgur.com/T9qgcHc.jpg', 'imgur.com/gallery/WxtWY'); $this->assertFalse($res, 'Absolute image can not be determined, so it will not be replaced'); @@ -161,18 +133,13 @@ class DownloadImagesTest extends TestCase public function testProcessRealImage() { - $client = new Client(); - - $mock = new Mock([ - new Response(200, ['content-type' => null], Stream::factory(file_get_contents(__DIR__ . '/../fixtures/image-no-content-type.jpg'))), - ]); - - $client->getEmitter()->attach($mock); + $httpMockClient = new HttpMockClient(); + $httpMockClient->addResponse(new Response(200, ['content-type' => null], file_get_contents(__DIR__ . '/../fixtures/image-no-content-type.jpg'))); $logHandler = new TestHandler(); $logger = new Logger('test', [$logHandler]); - $download = new DownloadImages($client, sys_get_temp_dir() . '/wallabag_test', 'http://wallabag.io/', $logger); + $download = new DownloadImages($httpMockClient, sys_get_temp_dir() . '/wallabag_test', 'http://wallabag.io/', $logger); $res = $download->processSingleImage( 123, diff --git a/tests/Wallabag/ImportBundle/Import/PocketImportTest.php b/tests/Wallabag/ImportBundle/Import/PocketImportTest.php index 8083f1a88..ec76f1681 100644 --- a/tests/Wallabag/ImportBundle/Import/PocketImportTest.php +++ b/tests/Wallabag/ImportBundle/Import/PocketImportTest.php @@ -2,10 +2,8 @@ namespace Tests\Wallabag\ImportBundle\Import; -use GuzzleHttp\Client; -use GuzzleHttp\Message\Response; -use GuzzleHttp\Stream\Stream; -use GuzzleHttp\Subscriber\Mock; +use Http\Mock\Client as HttpMockClient; +use GuzzleHttp\Psr7\Response; use M6Web\Component\RedisMock\RedisMockFactory; use Monolog\Handler\TestHandler; use Monolog\Logger; @@ -38,16 +36,11 @@ class PocketImportTest extends TestCase public function testOAuthRequest() { - $client = new Client(); - - $mock = new Mock([ - new Response(200, ['Content-Type' => 'application/json'], Stream::factory(json_encode(['code' => 'wunderbar_code']))), - ]); - - $client->getEmitter()->attach($mock); + $httpMockClient = new HttpMockClient(); + $httpMockClient->addResponse(new Response(200, ['Content-Type' => 'application/json'], json_encode(['code' => 'wunderbar_code']))); $pocketImport = $this->getPocketImport(); - $pocketImport->setClient($client); + $pocketImport->setClient($httpMockClient); $code = $pocketImport->getRequestToken('http://0.0.0.0/redirect'); @@ -56,16 +49,11 @@ class PocketImportTest extends TestCase public function testOAuthRequestBadResponse() { - $client = new Client(); - - $mock = new Mock([ - new Response(403), - ]); - - $client->getEmitter()->attach($mock); + $httpMockClient = new HttpMockClient(); + $httpMockClient->addResponse(new Response(403)); $pocketImport = $this->getPocketImport(); - $pocketImport->setClient($client); + $pocketImport->setClient($httpMockClient); $code = $pocketImport->getRequestToken('http://0.0.0.0/redirect'); @@ -78,16 +66,11 @@ class PocketImportTest extends TestCase public function testOAuthAuthorize() { - $client = new Client(); - - $mock = new Mock([ - new Response(200, ['Content-Type' => 'application/json'], Stream::factory(json_encode(['access_token' => 'wunderbar_token']))), - ]); - - $client->getEmitter()->attach($mock); + $httpMockClient = new HttpMockClient(); + $httpMockClient->addResponse(new Response(200, ['Content-Type' => 'application/json'], json_encode(['access_token' => 'wunderbar_token']))); $pocketImport = $this->getPocketImport(); - $pocketImport->setClient($client); + $pocketImport->setClient($httpMockClient); $res = $pocketImport->authorize('wunderbar_code'); @@ -97,16 +80,11 @@ class PocketImportTest extends TestCase public function testOAuthAuthorizeBadResponse() { - $client = new Client(); - - $mock = new Mock([ - new Response(403), - ]); - - $client->getEmitter()->attach($mock); + $httpMockClient = new HttpMockClient(); + $httpMockClient->addResponse(new Response(403)); $pocketImport = $this->getPocketImport(); - $pocketImport->setClient($client); + $pocketImport->setClient($httpMockClient); $res = $pocketImport->authorize('wunderbar_code'); @@ -122,94 +100,90 @@ class PocketImportTest extends TestCase */ public function testImport() { - $client = new Client(); - - $mock = new Mock([ - new Response(200, ['Content-Type' => 'application/json'], Stream::factory(json_encode(['access_token' => 'wunderbar_token']))), - new Response(200, ['Content-Type' => 'application/json'], Stream::factory(' - { - "status": 1, - "list": { - "229279689": { - "item_id": "229279689", - "resolved_id": "229279689", - "given_url": "http://www.grantland.com/blog/the-triangle/post/_/id/38347/ryder-cup-preview", - "given_title": "The Massive Ryder Cup Preview - The Triangle Blog - Grantland", - "favorite": "1", - "status": "1", - "time_added": "1473020899", - "time_updated": "1473020899", - "time_read": "0", - "time_favorited": "0", - "sort_id": 0, - "resolved_title": "The Massive Ryder Cup Preview", - "resolved_url": "http://www.grantland.com/blog/the-triangle/post/_/id/38347/ryder-cup-preview", - "excerpt": "The list of things I love about the Ryder Cup is so long that it could fill a (tedious) novel, and golf fans can probably guess most of them.", - "is_article": "1", - "is_index": "0", - "has_video": "1", - "has_image": "1", - "word_count": "3197", - "images": { - "1": { - "item_id": "229279689", - "image_id": "1", - "src": "http://a.espncdn.com/combiner/i?img=/photo/2012/0927/grant_g_ryder_cr_640.jpg&w=640&h=360", - "width": "0", - "height": "0", - "credit": "Jamie Squire/Getty Images", - "caption": "" - } - }, - "videos": { - "1": { - "item_id": "229279689", - "video_id": "1", - "src": "http://www.youtube.com/v/Er34PbFkVGk?version=3&hl=en_US&rel=0", - "width": "420", - "height": "315", - "type": "1", - "vid": "Er34PbFkVGk" - } - }, - "tags": { - "grantland": { - "item_id": "1147652870", - "tag": "grantland" - }, - "Ryder Cup": { - "item_id": "1147652870", - "tag": "Ryder Cup" - } + $httpMockClient = new HttpMockClient(); + $httpMockClient->addResponse(new Response(200, ['Content-Type' => 'application/json'], json_encode(['access_token' => 'wunderbar_token']))); + $httpMockClient->addResponse(new Response(200, ['Content-Type' => 'application/json'], <<<'JSON' + { + "status": 1, + "list": { + "229279689": { + "item_id": "229279689", + "resolved_id": "229279689", + "given_url": "http://www.grantland.com/blog/the-triangle/post/_/id/38347/ryder-cup-preview", + "given_title": "The Massive Ryder Cup Preview - The Triangle Blog - Grantland", + "favorite": "1", + "status": "1", + "time_added": "1473020899", + "time_updated": "1473020899", + "time_read": "0", + "time_favorited": "0", + "sort_id": 0, + "resolved_title": "The Massive Ryder Cup Preview", + "resolved_url": "http://www.grantland.com/blog/the-triangle/post/_/id/38347/ryder-cup-preview", + "excerpt": "The list of things I love about the Ryder Cup is so long that it could fill a (tedious) novel, and golf fans can probably guess most of them.", + "is_article": "1", + "is_index": "0", + "has_video": "1", + "has_image": "1", + "word_count": "3197", + "images": { + "1": { + "item_id": "229279689", + "image_id": "1", + "src": "http://a.espncdn.com/combiner/i?img=/photo/2012/0927/grant_g_ryder_cr_640.jpg&w=640&h=360", + "width": "0", + "height": "0", + "credit": "Jamie Squire/Getty Images", + "caption": "" } }, - "229279690": { - "item_id": "229279689", - "resolved_id": "229279689", - "given_url": "http://www.grantland.com/blog/the-triangle/post/_/id/38347/ryder-cup-preview", - "given_title": "The Massive Ryder Cup Preview - The Triangle Blog - Grantland", - "favorite": "1", - "status": "1", - "time_added": "1473020899", - "time_updated": "1473020899", - "time_read": "0", - "time_favorited": "0", - "sort_id": 1, - "resolved_title": "The Massive Ryder Cup Preview", - "resolved_url": "http://www.grantland.com/blog/the-triangle/post/_/id/38347/ryder-cup-preview", - "excerpt": "The list of things I love about the Ryder Cup is so long that it could fill a (tedious) novel, and golf fans can probably guess most of them.", - "is_article": "1", - "is_index": "0", - "has_video": "0", - "has_image": "0", - "word_count": "3197" + "videos": { + "1": { + "item_id": "229279689", + "video_id": "1", + "src": "http://www.youtube.com/v/Er34PbFkVGk?version=3&hl=en_US&rel=0", + "width": "420", + "height": "315", + "type": "1", + "vid": "Er34PbFkVGk" + } + }, + "tags": { + "grantland": { + "item_id": "1147652870", + "tag": "grantland" + }, + "Ryder Cup": { + "item_id": "1147652870", + "tag": "Ryder Cup" + } } + }, + "229279690": { + "item_id": "229279689", + "resolved_id": "229279689", + "given_url": "http://www.grantland.com/blog/the-triangle/post/_/id/38347/ryder-cup-preview", + "given_title": "The Massive Ryder Cup Preview - The Triangle Blog - Grantland", + "favorite": "1", + "status": "1", + "time_added": "1473020899", + "time_updated": "1473020899", + "time_read": "0", + "time_favorited": "0", + "sort_id": 1, + "resolved_title": "The Massive Ryder Cup Preview", + "resolved_url": "http://www.grantland.com/blog/the-triangle/post/_/id/38347/ryder-cup-preview", + "excerpt": "The list of things I love about the Ryder Cup is so long that it could fill a (tedious) novel, and golf fans can probably guess most of them.", + "is_article": "1", + "is_index": "0", + "has_video": "0", + "has_image": "0", + "word_count": "3197" } } - ')), - ]); - - $client->getEmitter()->attach($mock); + } +JSON +)); $pocketImport = $this->getPocketImport('ConsumerKey', 1); @@ -240,7 +214,7 @@ class PocketImportTest extends TestCase ->method('updateEntry') ->willReturn($entry); - $pocketImport->setClient($client); + $pocketImport->setClient($httpMockClient); $pocketImport->authorize('wunderbar_code'); $res = $pocketImport->import(); @@ -254,56 +228,52 @@ class PocketImportTest extends TestCase */ public function testImportAndMarkAllAsRead() { - $client = new Client(); - - $mock = new Mock([ - new Response(200, ['Content-Type' => 'application/json'], Stream::factory(json_encode(['access_token' => 'wunderbar_token']))), - new Response(200, ['Content-Type' => 'application/json'], Stream::factory(' - { - "status": 1, - "list": { - "229279689": { - "item_id": "229279689", - "resolved_id": "229279689", - "given_url": "http://www.grantland.com/blog/the-triangle/post/_/id/38347/ryder-cup-preview", - "given_title": "The Massive Ryder Cup Preview - The Triangle Blog - Grantland", - "favorite": "1", - "status": "1", - "time_added": "1473020899", - "time_updated": "1473020899", - "time_read": "0", - "time_favorited": "0", - "sort_id": 0, - "excerpt": "The list of things I love about the Ryder Cup is so long that it could fill a (tedious) novel, and golf fans can probably guess most of them.", - "is_article": "1", - "has_video": "1", - "has_image": "1", - "word_count": "3197" - }, - "229279690": { - "item_id": "229279689", - "resolved_id": "229279689", - "given_url": "http://www.grantland.com/blog/the-triangle/post/_/id/38347/ryder-cup-preview/2", - "given_title": "The Massive Ryder Cup Preview - The Triangle Blog - Grantland", - "favorite": "1", - "status": "0", - "time_added": "1473020899", - "time_updated": "1473020899", - "time_read": "0", - "time_favorited": "0", - "sort_id": 1, - "excerpt": "The list of things I love about the Ryder Cup is so long that it could fill a (tedious) novel, and golf fans can probably guess most of them.", - "is_article": "1", - "has_video": "0", - "has_image": "0", - "word_count": "3197" - } + $httpMockClient = new HttpMockClient(); + $httpMockClient->addResponse(new Response(200, ['Content-Type' => 'application/json'], json_encode(['access_token' => 'wunderbar_token']))); + $httpMockClient->addResponse(new Response(200, ['Content-Type' => 'application/json'], <<<'JSON' + { + "status": 1, + "list": { + "229279689": { + "item_id": "229279689", + "resolved_id": "229279689", + "given_url": "http://www.grantland.com/blog/the-triangle/post/_/id/38347/ryder-cup-preview", + "given_title": "The Massive Ryder Cup Preview - The Triangle Blog - Grantland", + "favorite": "1", + "status": "1", + "time_added": "1473020899", + "time_updated": "1473020899", + "time_read": "0", + "time_favorited": "0", + "sort_id": 0, + "excerpt": "The list of things I love about the Ryder Cup is so long that it could fill a (tedious) novel, and golf fans can probably guess most of them.", + "is_article": "1", + "has_video": "1", + "has_image": "1", + "word_count": "3197" + }, + "229279690": { + "item_id": "229279689", + "resolved_id": "229279689", + "given_url": "http://www.grantland.com/blog/the-triangle/post/_/id/38347/ryder-cup-preview/2", + "given_title": "The Massive Ryder Cup Preview - The Triangle Blog - Grantland", + "favorite": "1", + "status": "0", + "time_added": "1473020899", + "time_updated": "1473020899", + "time_read": "0", + "time_favorited": "0", + "sort_id": 1, + "excerpt": "The list of things I love about the Ryder Cup is so long that it could fill a (tedious) novel, and golf fans can probably guess most of them.", + "is_article": "1", + "has_video": "0", + "has_image": "0", + "word_count": "3197" } } - ')), - ]); - - $client->getEmitter()->attach($mock); + } +JSON +)); $pocketImport = $this->getPocketImport('ConsumerKey', 2); @@ -335,7 +305,7 @@ class PocketImportTest extends TestCase ->method('updateEntry') ->willReturn($entry); - $pocketImport->setClient($client); + $pocketImport->setClient($httpMockClient); $pocketImport->authorize('wunderbar_code'); $res = $pocketImport->setMarkAsRead(true)->import(); @@ -349,7 +319,7 @@ class PocketImportTest extends TestCase */ public function testImportWithRabbit() { - $client = new Client(); + $httpMockClient = new HttpMockClient(); $body = <<<'JSON' { @@ -374,19 +344,16 @@ class PocketImportTest extends TestCase } JSON; - $mock = new Mock([ - new Response(200, ['Content-Type' => 'application/json'], Stream::factory(json_encode(['access_token' => 'wunderbar_token']))), - new Response(200, ['Content-Type' => 'application/json'], Stream::factory(' - { - "status": 1, - "list": { - "229279690": ' . $body . ' - } + $httpMockClient->addResponse(new Response(200, ['Content-Type' => 'application/json'], json_encode(['access_token' => 'wunderbar_token']))); + $httpMockClient->addResponse(new Response(200, ['Content-Type' => 'application/json'], <<getEmitter()->attach($mock); + } +JSON + )); $pocketImport = $this->getPocketImport(); @@ -420,7 +387,7 @@ JSON; ->method('publish') ->with(json_encode($bodyAsArray)); - $pocketImport->setClient($client); + $pocketImport->setClient($httpMockClient); $pocketImport->setProducer($producer); $pocketImport->authorize('wunderbar_code'); @@ -435,7 +402,7 @@ JSON; */ public function testImportWithRedis() { - $client = new Client(); + $httpMockClient = new HttpMockClient(); $body = <<<'JSON' { @@ -460,19 +427,16 @@ JSON; } JSON; - $mock = new Mock([ - new Response(200, ['Content-Type' => 'application/json'], Stream::factory(json_encode(['access_token' => 'wunderbar_token']))), - new Response(200, ['Content-Type' => 'application/json'], Stream::factory(' - { - "status": 1, - "list": { - "229279690": ' . $body . ' - } + $httpMockClient->addResponse(new Response(200, ['Content-Type' => 'application/json'], json_encode(['access_token' => 'wunderbar_token']))); + $httpMockClient->addResponse(new Response(200, ['Content-Type' => 'application/json'], <<getEmitter()->attach($mock); + } +JSON + )); $pocketImport = $this->getPocketImport(); @@ -499,7 +463,7 @@ JSON; $queue = new RedisQueue($redisMock, 'pocket'); $producer = new Producer($queue); - $pocketImport->setClient($client); + $pocketImport->setClient($httpMockClient); $pocketImport->setProducer($producer); $pocketImport->authorize('wunderbar_code'); @@ -513,17 +477,13 @@ JSON; public function testImportBadResponse() { - $client = new Client(); + $httpMockClient = new HttpMockClient(); - $mock = new Mock([ - new Response(200, ['Content-Type' => 'application/json'], Stream::factory(json_encode(['access_token' => 'wunderbar_token']))), - new Response(403), - ]); - - $client->getEmitter()->attach($mock); + $httpMockClient->addResponse(new Response(200, ['Content-Type' => 'application/json'], json_encode(['access_token' => 'wunderbar_token']))); + $httpMockClient->addResponse(new Response(403)); $pocketImport = $this->getPocketImport(); - $pocketImport->setClient($client); + $pocketImport->setClient($httpMockClient); $pocketImport->authorize('wunderbar_code'); $res = $pocketImport->import(); @@ -537,25 +497,23 @@ JSON; public function testImportWithExceptionFromGraby() { - $client = new Client(); + $httpMockClient = new HttpMockClient(); - $mock = new Mock([ - new Response(200, ['Content-Type' => 'application/json'], Stream::factory(json_encode(['access_token' => 'wunderbar_token']))), - new Response(200, ['Content-Type' => 'application/json'], Stream::factory(' - { - "status": 1, - "list": { - "229279689": { - "status": "1", - "favorite": "1", - "resolved_url": "http://www.grantland.com/blog/the-triangle/post/_/id/38347/ryder-cup-preview" - } + $httpMockClient->addResponse(new Response(200, ['Content-Type' => 'application/json'], json_encode(['access_token' => 'wunderbar_token']))); + $httpMockClient->addResponse(new Response(200, ['Content-Type' => 'application/json'], <<<'JSON' + { + "status": 1, + "list": { + "229279689": { + "status": "1", + "favorite": "1", + "resolved_url": "http://www.grantland.com/blog/the-triangle/post/_/id/38347/ryder-cup-preview" } } - ')), - ]); - - $client->getEmitter()->attach($mock); + } + +JSON + )); $pocketImport = $this->getPocketImport('ConsumerKey', 1); @@ -579,7 +537,7 @@ JSON; ->method('updateEntry') ->will($this->throwException(new \Exception())); - $pocketImport->setClient($client); + $pocketImport->setClient($httpMockClient); $pocketImport->authorize('wunderbar_code'); $res = $pocketImport->import(); From 5f08426201c336f96d593954fb45b284d7e60f4a Mon Sep 17 00:00:00 2001 From: adev Date: Sat, 11 Nov 2017 20:04:15 +0100 Subject: [PATCH 2/8] Fix because of some breaking changes of Graby 2.0 --- .../Controller/EntryRestController.php | 4 +- .../CoreBundle/Helper/ContentProxy.php | 20 +++--- .../ImportBundle/Import/WallabagV2Import.php | 4 +- .../CoreBundle/Helper/ContentProxyTest.php | 70 ++++++++++--------- 4 files changed, 50 insertions(+), 48 deletions(-) diff --git a/src/Wallabag/ApiBundle/Controller/EntryRestController.php b/src/Wallabag/ApiBundle/Controller/EntryRestController.php index aff0534a0..d9d99c85b 100644 --- a/src/Wallabag/ApiBundle/Controller/EntryRestController.php +++ b/src/Wallabag/ApiBundle/Controller/EntryRestController.php @@ -369,9 +369,7 @@ class EntryRestController extends WallabagRestController 'language' => !empty($data['language']) ? $data['language'] : $entry->getLanguage(), 'date' => !empty($data['publishedAt']) ? $data['publishedAt'] : $entry->getPublishedAt(), // faking the open graph preview picture - 'open_graph' => [ - 'og_image' => !empty($data['picture']) ? $data['picture'] : $entry->getPreviewPicture(), - ], + 'image' => !empty($data['picture']) ? $data['picture'] : $entry->getPreviewPicture(), 'authors' => \is_string($data['authors']) ? explode(',', $data['authors']) : $entry->getPublishedBy(), ] ); diff --git a/src/Wallabag/CoreBundle/Helper/ContentProxy.php b/src/Wallabag/CoreBundle/Helper/ContentProxy.php index ca01dec8d..ac27e50a9 100644 --- a/src/Wallabag/CoreBundle/Helper/ContentProxy.php +++ b/src/Wallabag/CoreBundle/Helper/ContentProxy.php @@ -253,16 +253,14 @@ class ContentProxy if (!empty($content['title'])) { $entry->setTitle($content['title']); - } elseif (!empty($content['open_graph']['og_title'])) { - $entry->setTitle($content['open_graph']['og_title']); } if (empty($content['html'])) { $content['html'] = $this->fetchingErrorMessage; - if (!empty($content['open_graph']['og_description'])) { + if (!empty($content['description'])) { $content['html'] .= '

But we found a short description:

'; - $content['html'] .= $content['open_graph']['og_description']; + $content['html'] .= $content['description']; } } @@ -277,8 +275,8 @@ class ContentProxy $entry->setPublishedBy($content['authors']); } - if (!empty($content['all_headers']) && $this->storeArticleHeaders) { - $entry->setHeaders($content['all_headers']); + if (!empty($content['headers'])) { + $entry->setHeaders($content['headers']); } if (!empty($content['date'])) { @@ -290,12 +288,12 @@ class ContentProxy } $previewPictureUrl = ''; - if (!empty($content['open_graph']['og_image'])) { - $previewPictureUrl = $content['open_graph']['og_image']; + if (!empty($content['image'])) { + $previewPictureUrl = $content['image']; } // if content is an image, define it as a preview too - if (!empty($content['content_type']) && \in_array($this->mimeGuesser->guess($content['content_type']), ['jpeg', 'jpg', 'gif', 'png'], true)) { + if (!empty($content['headers']['content_type']) && \in_array($this->mimeGuesser->guess($content['headers']['content_type']), ['jpeg', 'jpg', 'gif', 'png'], true)) { $previewPictureUrl = $content['url']; } elseif (empty($previewPictureUrl)) { $this->logger->debug('Extracting images from content to provide a default preview picture'); @@ -310,8 +308,8 @@ class ContentProxy $this->updatePreviewPicture($entry, $previewPictureUrl); } - if (!empty($content['content_type'])) { - $entry->setMimetype($content['content_type']); + if (!empty($content['headers']['content-type'])) { + $entry->setMimetype($content['headers']['content-type']); } try { diff --git a/src/Wallabag/ImportBundle/Import/WallabagV2Import.php b/src/Wallabag/ImportBundle/Import/WallabagV2Import.php index 3e085ecff..2ba26003f 100644 --- a/src/Wallabag/ImportBundle/Import/WallabagV2Import.php +++ b/src/Wallabag/ImportBundle/Import/WallabagV2Import.php @@ -35,7 +35,9 @@ class WallabagV2Import extends WallabagImport { return [ 'html' => $entry['content'], - 'content_type' => $entry['mimetype'], + 'headers' => [ + 'content-type' => $entry['mimetype'], + ], 'is_archived' => (bool) ($entry['is_archived'] || $this->markAsRead), 'is_starred' => (bool) $entry['is_starred'], ] + $entry; diff --git a/tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php b/tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php index c7caac1d3..40a6cc802 100644 --- a/tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php +++ b/tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php @@ -104,15 +104,12 @@ class ContentProxyTest extends TestCase ->method('fetchContent') ->willReturn([ 'html' => false, - 'title' => '', + 'title' => 'my title', 'url' => '', 'content_type' => '', 'language' => '', 'status' => '', - 'open_graph' => [ - 'og_title' => 'my title', - 'og_description' => 'desc', - ], + 'description' => 'desc', ]); $proxy = new ContentProxy($graby, $tagger, $this->getValidator(), $this->getLogger(), $this->fetchingErrorMessage); @@ -147,13 +144,12 @@ class ContentProxyTest extends TestCase 'html' => str_repeat('this is my content', 325), 'title' => 'this is my title', 'url' => 'http://1.1.1.1', - 'content_type' => 'text/html', 'language' => 'fr', 'status' => '200', - 'open_graph' => [ - 'og_title' => 'my OG title', - 'og_description' => 'OG desc', - 'og_image' => 'http://3.3.3.3/cover.jpg', + 'description' => 'OG desc', + 'image' => 'http://3.3.3.3/cover.jpg', + 'headers' => [ + 'content-type' => 'text/html', ], ]); @@ -189,13 +185,12 @@ class ContentProxyTest extends TestCase 'html' => str_repeat('this is my content', 325), 'title' => 'this is my title', 'url' => 'http://1.1.1.1', - 'content_type' => 'text/html', 'language' => 'fr', 'status' => '200', - 'open_graph' => [ - 'og_title' => 'my OG title', - 'og_description' => 'OG desc', - 'og_image' => null, + 'description' => 'OG desc', + 'image' => null, + 'headers' => [ + 'content-type' => 'text/html', ], ]); @@ -320,9 +315,11 @@ class ContentProxyTest extends TestCase 'html' => str_repeat('this is my content', 325), 'title' => 'this is my title', 'url' => 'http://1.1.1.1', - 'content_type' => 'text/html', 'language' => 'dontexist', 'status' => '200', + 'headers' => [ + 'content-type' => 'text/html', + ], ]); $proxy = new ContentProxy($graby, $tagger, $validator, $this->getLogger(), $this->fetchingErrorMessage); @@ -367,10 +364,10 @@ class ContentProxyTest extends TestCase 'content_type' => 'text/html', 'language' => 'fr', 'status' => '200', - 'open_graph' => [ - 'og_title' => 'my OG title', - 'og_description' => 'OG desc', - 'og_image' => 'https://', + 'description' => 'OG desc', + 'image' => 'https://', + 'headers' => [ + 'content-type' => 'text/html', ], ]); @@ -404,12 +401,12 @@ class ContentProxyTest extends TestCase 'html' => str_repeat('this is my content', 325), 'title' => 'this is my title', 'url' => 'http://1.1.1.1', - 'content_type' => 'text/html', 'language' => 'fr', 'date' => '1395635872', 'authors' => ['Jeremy', 'Nico', 'Thomas'], - 'all_headers' => [ - 'Cache-Control' => 'no-cache', + 'headers' => [ + 'cache-control' => 'no-cache', + 'content-type' => 'text/html', ], ] ); @@ -447,9 +444,11 @@ class ContentProxyTest extends TestCase 'html' => str_repeat('this is my content', 325), 'title' => 'this is my title', 'url' => 'http://1.1.1.1', - 'content_type' => 'text/html', 'language' => 'fr', 'date' => '2016-09-08T11:55:58+0200', + 'headers' => [ + 'content-type' => 'text/html', + ], ] ); @@ -482,9 +481,11 @@ class ContentProxyTest extends TestCase 'html' => str_repeat('this is my content', 325), 'title' => 'this is my title', 'url' => 'http://1.1.1.1', - 'content_type' => 'text/html', 'language' => 'fr', 'date' => '01 02 2012', + 'headers' => [ + 'content-type' => 'text/html', + ], ] ); @@ -519,8 +520,10 @@ class ContentProxyTest extends TestCase 'html' => str_repeat('this is my content', 325), 'title' => 'this is my title', 'url' => 'http://1.1.1.1', - 'content_type' => 'text/html', 'language' => 'fr', + 'headers' => [ + 'content-type' => 'text/html', + ], ] ); @@ -559,13 +562,13 @@ class ContentProxyTest extends TestCase 'html' => $html, 'title' => 'this is my title', 'url' => 'http://1.1.1.1', - 'content_type' => 'text/html', 'language' => 'fr', 'status' => '200', - 'open_graph' => [ - 'og_title' => 'my OG title', - 'og_description' => 'OG desc', - 'og_image' => 'http://3.3.3.3/cover.jpg', + //'og_title' => 'my OG title', + 'description' => 'OG desc', + 'image' => 'http://3.3.3.3/cover.jpg', + 'headers' => [ + 'content-type' => 'text/html', ], ] ); @@ -597,9 +600,10 @@ class ContentProxyTest extends TestCase 'html' => '

', 'title' => 'this is my title', 'url' => 'http://1.1.1.1/image.jpg', - 'content_type' => 'image/jpeg', 'status' => '200', - 'open_graph' => [], + 'headers' => [ + 'content-type' => 'image/jpeg', + ], ]); $proxy = new ContentProxy($graby, $tagger, $this->getValidator(), $this->getLogger(), $this->fetchingErrorMessage); From 1048c9c4a811821b00cc04bfec905bebcc22bac4 Mon Sep 17 00:00:00 2001 From: adev Date: Sun, 12 Nov 2017 12:15:02 +0100 Subject: [PATCH 3/8] Configure timeout --- app/config/config.yml | 3 +++ src/Wallabag/CoreBundle/Helper/HttpClientFactory.php | 10 +++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/app/config/config.yml b/app/config/config.yml index 309945c50..bbcc682f9 100644 --- a/app/config/config.yml +++ b/app/config/config.yml @@ -375,6 +375,9 @@ httplug: clients: wallabag_core: factory: 'wallabag_core.http_client_factory' + config: + defaults: + timeout: 10 plugins: ['httplug.plugin.logger'] wallabag_core.entry.download_images: factory: 'httplug.factory.auto' diff --git a/src/Wallabag/CoreBundle/Helper/HttpClientFactory.php b/src/Wallabag/CoreBundle/Helper/HttpClientFactory.php index 4899d3d41..3e19a7be1 100644 --- a/src/Wallabag/CoreBundle/Helper/HttpClientFactory.php +++ b/src/Wallabag/CoreBundle/Helper/HttpClientFactory.php @@ -60,13 +60,17 @@ class HttpClientFactory implements ClientFactory $this->logger->log('debug', 'Restricted access config enabled?', ['enabled' => (int) $this->restrictedAccess]); if (0 === (int) $this->restrictedAccess) { - return new GuzzleAdapter(new GuzzleClient()); + return new GuzzleAdapter(new GuzzleClient($config)); } // we clear the cookie to avoid websites who use cookies for analytics $this->cookieJar->clear(); - // need to set the (shared) cookie jar - $guzzle = new GuzzleClient(['defaults' => ['cookies' => $this->cookieJar]]); + if (!isset($config['defaults']['cookies'])) { + // need to set the (shared) cookie jar + $config['defaults']['cookies'] = $this->cookieJar; + } + + $guzzle = new GuzzleClient($config); foreach ($this->subscribers as $subscriber) { $guzzle->getEmitter()->attach($subscriber); } From 448d99f84e93697ce49ec31224addb1da1a37a9f Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Mon, 28 Jan 2019 06:10:26 +0100 Subject: [PATCH 4/8] CS --- src/Wallabag/CoreBundle/Helper/DownloadImages.php | 2 +- src/Wallabag/CoreBundle/Helper/HttpClientFactory.php | 2 +- src/Wallabag/ImportBundle/Import/PocketImport.php | 7 +++---- tests/Wallabag/CoreBundle/Helper/DownloadImagesTest.php | 2 +- tests/Wallabag/ImportBundle/Import/PocketImportTest.php | 2 +- 5 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/Wallabag/CoreBundle/Helper/DownloadImages.php b/src/Wallabag/CoreBundle/Helper/DownloadImages.php index e57490608..7a39a2e4a 100644 --- a/src/Wallabag/CoreBundle/Helper/DownloadImages.php +++ b/src/Wallabag/CoreBundle/Helper/DownloadImages.php @@ -312,7 +312,7 @@ class DownloadImages * Retrieve and validate the extension from the response of the url of the image. * * @param ResponseInterface $res Http Response - * @param string $imagePath Path from the src image from the content (used for log only) + * @param string $imagePath Path from the src image from the content (used for log only) * * @return string|false Extension name or false if validation failed */ diff --git a/src/Wallabag/CoreBundle/Helper/HttpClientFactory.php b/src/Wallabag/CoreBundle/Helper/HttpClientFactory.php index 3e19a7be1..b8e95381d 100644 --- a/src/Wallabag/CoreBundle/Helper/HttpClientFactory.php +++ b/src/Wallabag/CoreBundle/Helper/HttpClientFactory.php @@ -6,9 +6,9 @@ use GuzzleHttp\Client as GuzzleClient; use GuzzleHttp\Cookie\CookieJar; use GuzzleHttp\Event\SubscriberInterface; use Http\Adapter\Guzzle5\Client as GuzzleAdapter; -use Psr\Log\LoggerInterface; use Http\Client\HttpClient; use Http\HttplugBundle\ClientFactory\ClientFactory; +use Psr\Log\LoggerInterface; /** * Builds and configures the HTTP client. diff --git a/src/Wallabag/ImportBundle/Import/PocketImport.php b/src/Wallabag/ImportBundle/Import/PocketImport.php index 9467fae24..b35a561bb 100644 --- a/src/Wallabag/ImportBundle/Import/PocketImport.php +++ b/src/Wallabag/ImportBundle/Import/PocketImport.php @@ -5,12 +5,12 @@ namespace Wallabag\ImportBundle\Import; use Http\Client\Common\HttpMethodsClient; use Http\Client\Common\Plugin\ErrorPlugin; use Http\Client\Common\PluginClient; +use Http\Client\Exception\RequestException; use Http\Client\HttpClient; use Http\Discovery\MessageFactoryDiscovery; use Http\Message\MessageFactory; -use Http\Client\Exception\RequestException; -use Wallabag\CoreBundle\Entity\Entry; use Psr\Http\Message\ResponseInterface; +use Wallabag\CoreBundle\Entity\Entry; class PocketImport extends AbstractImport { @@ -88,7 +88,6 @@ class PocketImport extends AbstractImport */ public function authorize($code) { - try { $response = $this->client->post('https://getpocket.com/v3/oauth/authorize', [], json_encode([ 'consumer_key' => $this->user->getConfig()->getPocketConsumerKey(), @@ -153,7 +152,7 @@ class PocketImport extends AbstractImport /** * Set the Http client. * - * @param HttpClient $client + * @param HttpClient $client * @param MessageFactory|null $messageFactory */ public function setClient(HttpClient $client, MessageFactory $messageFactory = null) diff --git a/tests/Wallabag/CoreBundle/Helper/DownloadImagesTest.php b/tests/Wallabag/CoreBundle/Helper/DownloadImagesTest.php index 537583647..0199b3e44 100644 --- a/tests/Wallabag/CoreBundle/Helper/DownloadImagesTest.php +++ b/tests/Wallabag/CoreBundle/Helper/DownloadImagesTest.php @@ -3,10 +3,10 @@ namespace Tests\Wallabag\CoreBundle\Helper; use GuzzleHttp\Client; +use GuzzleHttp\Psr7\Response; use GuzzleHttp\Stream\Stream; use GuzzleHttp\Subscriber\Mock; use Http\Mock\Client as HttpMockClient; -use GuzzleHttp\Psr7\Response; use Monolog\Handler\TestHandler; use Monolog\Logger; use PHPUnit\Framework\TestCase; diff --git a/tests/Wallabag/ImportBundle/Import/PocketImportTest.php b/tests/Wallabag/ImportBundle/Import/PocketImportTest.php index ec76f1681..40e1626ba 100644 --- a/tests/Wallabag/ImportBundle/Import/PocketImportTest.php +++ b/tests/Wallabag/ImportBundle/Import/PocketImportTest.php @@ -2,8 +2,8 @@ namespace Tests\Wallabag\ImportBundle\Import; -use Http\Mock\Client as HttpMockClient; use GuzzleHttp\Psr7\Response; +use Http\Mock\Client as HttpMockClient; use M6Web\Component\RedisMock\RedisMockFactory; use Monolog\Handler\TestHandler; use Monolog\Logger; From a91a3150fbc4446e379cc23618db8f74e4044515 Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Thu, 7 Feb 2019 17:30:38 +0100 Subject: [PATCH 5/8] CS --- src/Wallabag/ImportBundle/Import/PocketImport.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Wallabag/ImportBundle/Import/PocketImport.php b/src/Wallabag/ImportBundle/Import/PocketImport.php index b35a561bb..746120af1 100644 --- a/src/Wallabag/ImportBundle/Import/PocketImport.php +++ b/src/Wallabag/ImportBundle/Import/PocketImport.php @@ -247,7 +247,7 @@ class PocketImport extends AbstractImport protected function jsonDecode(ResponseInterface $response) { - $data = \json_decode((string) $response->getBody(), true); + $data = json_decode((string) $response->getBody(), true); if (JSON_ERROR_NONE !== json_last_error()) { throw new \InvalidArgumentException('Unable to parse JSON data: ' . json_last_error_msg()); From b6c1e1bacc59ba761d1b47ac6611d1db800f7252 Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Thu, 7 Feb 2019 17:56:05 +0100 Subject: [PATCH 6/8] Fix some tests --- .../CoreBundle/Helper/ContentProxy.php | 19 ++++---- .../CoreBundle/Helper/DownloadImagesTest.php | 44 ++++++------------- 2 files changed, 24 insertions(+), 39 deletions(-) diff --git a/src/Wallabag/CoreBundle/Helper/ContentProxy.php b/src/Wallabag/CoreBundle/Helper/ContentProxy.php index ac27e50a9..59465ad11 100644 --- a/src/Wallabag/CoreBundle/Helper/ContentProxy.php +++ b/src/Wallabag/CoreBundle/Helper/ContentProxy.php @@ -54,7 +54,11 @@ class ContentProxy if ((empty($content) || false === $this->validateContent($content)) && false === $disableContentUpdate) { $fetchedContent = $this->graby->fetchContent($url); - $fetchedContent['title'] = $this->sanitizeContentTitle($fetchedContent['title'], $fetchedContent['content_type']); + + $fetchedContent['title'] = $this->sanitizeContentTitle( + $fetchedContent['title'], + isset($fetchedContent['headers']['content-type']) ? $fetchedContent['headers']['content-type'] : '' + ); // when content is imported, we have information in $content // in case fetching content goes bad, we'll keep the imported information instead of overriding them @@ -188,8 +192,8 @@ class ContentProxy /** * Try to sanitize the title of the fetched content from wrong character encodings and invalid UTF-8 character. * - * @param $title - * @param $contentType + * @param string $title + * @param string $contentType * * @return string */ @@ -293,12 +297,15 @@ class ContentProxy } // if content is an image, define it as a preview too - if (!empty($content['headers']['content_type']) && \in_array($this->mimeGuesser->guess($content['headers']['content_type']), ['jpeg', 'jpg', 'gif', 'png'], true)) { + if (!empty($content['headers']['content-type']) && \in_array($this->mimeGuesser->guess($content['headers']['content-type']), ['jpeg', 'jpg', 'gif', 'png'], true)) { $previewPictureUrl = $content['url']; + + $entry->setMimetype($content['headers']['content-type']); } elseif (empty($previewPictureUrl)) { $this->logger->debug('Extracting images from content to provide a default preview picture'); $imagesUrls = DownloadImages::extractImagesUrlsFromHtml($content['html']); $this->logger->debug(\count($imagesUrls) . ' pictures found'); + if (!empty($imagesUrls)) { $previewPictureUrl = $imagesUrls[0]; } @@ -308,10 +315,6 @@ class ContentProxy $this->updatePreviewPicture($entry, $previewPictureUrl); } - if (!empty($content['headers']['content-type'])) { - $entry->setMimetype($content['headers']['content-type']); - } - try { $this->tagger->tag($entry); } catch (\Exception $e) { diff --git a/tests/Wallabag/CoreBundle/Helper/DownloadImagesTest.php b/tests/Wallabag/CoreBundle/Helper/DownloadImagesTest.php index 0199b3e44..3c720425a 100644 --- a/tests/Wallabag/CoreBundle/Helper/DownloadImagesTest.php +++ b/tests/Wallabag/CoreBundle/Helper/DownloadImagesTest.php @@ -2,10 +2,7 @@ namespace Tests\Wallabag\CoreBundle\Helper; -use GuzzleHttp\Client; use GuzzleHttp\Psr7\Response; -use GuzzleHttp\Stream\Stream; -use GuzzleHttp\Subscriber\Mock; use Http\Mock\Client as HttpMockClient; use Monolog\Handler\TestHandler; use Monolog\Logger; @@ -153,20 +150,15 @@ class DownloadImagesTest extends TestCase public function testProcessImageWithSrcset() { - $client = new Client(); - - $mock = new Mock([ - new Response(200, ['content-type' => 'image/jpeg'], Stream::factory(file_get_contents(__DIR__ . '/../fixtures/image-no-content-type.jpg'))), - new Response(200, ['content-type' => 'image/jpeg'], Stream::factory(file_get_contents(__DIR__ . '/../fixtures/image-no-content-type.jpg'))), - new Response(200, ['content-type' => 'image/jpeg'], Stream::factory(file_get_contents(__DIR__ . '/../fixtures/image-no-content-type.jpg'))), - ]); - - $client->getEmitter()->attach($mock); + $httpMockClient = new HttpMockClient(); + $httpMockClient->addResponse(new Response(200, ['content-type' => null], file_get_contents(__DIR__ . '/../fixtures/image-no-content-type.jpg'))); + $httpMockClient->addResponse(new Response(200, ['content-type' => null], file_get_contents(__DIR__ . '/../fixtures/image-no-content-type.jpg'))); + $httpMockClient->addResponse(new Response(200, ['content-type' => null], file_get_contents(__DIR__ . '/../fixtures/image-no-content-type.jpg'))); $logHandler = new TestHandler(); $logger = new Logger('test', [$logHandler]); - $download = new DownloadImages($client, sys_get_temp_dir() . '/wallabag_test', 'http://wallabag.io/', $logger); + $download = new DownloadImages($httpMockClient, sys_get_temp_dir() . '/wallabag_test', 'http://wallabag.io/', $logger); $res = $download->processHtml(123, '

', 'http://piketty.blog.lemonde.fr/2017/10/12/budget-2018-la-jeunesse-sacrifiee/'); $this->assertNotContains('http://piketty.blog.lemonde.fr/', $res, 'Image srcset attribute were not replaced'); @@ -174,20 +166,15 @@ class DownloadImagesTest extends TestCase public function testProcessImageWithTrickySrcset() { - $client = new Client(); - - $mock = new Mock([ - new Response(200, ['content-type' => 'image/jpeg'], Stream::factory(file_get_contents(__DIR__ . '/../fixtures/image-no-content-type.jpg'))), - new Response(200, ['content-type' => 'image/jpeg'], Stream::factory(file_get_contents(__DIR__ . '/../fixtures/image-no-content-type.jpg'))), - new Response(200, ['content-type' => 'image/jpeg'], Stream::factory(file_get_contents(__DIR__ . '/../fixtures/image-no-content-type.jpg'))), - ]); - - $client->getEmitter()->attach($mock); + $httpMockClient = new HttpMockClient(); + $httpMockClient->addResponse(new Response(200, ['content-type' => null], file_get_contents(__DIR__ . '/../fixtures/image-no-content-type.jpg'))); + $httpMockClient->addResponse(new Response(200, ['content-type' => null], file_get_contents(__DIR__ . '/../fixtures/image-no-content-type.jpg'))); + $httpMockClient->addResponse(new Response(200, ['content-type' => null], file_get_contents(__DIR__ . '/../fixtures/image-no-content-type.jpg'))); $logHandler = new TestHandler(); $logger = new Logger('test', [$logHandler]); - $download = new DownloadImages($client, sys_get_temp_dir() . '/wallabag_test', 'http://wallabag.io/', $logger); + $download = new DownloadImages($httpMockClient, sys_get_temp_dir() . '/wallabag_test', 'http://wallabag.io/', $logger); $res = $download->processHtml(123, '
Test

", 'title' => 'this is my title', 'url' => 'http://1.1.1.1', - 'content_type' => 'text/html', + 'headers' => [ + 'content-type' => 'text/html', + ], 'language' => 'fr', 'status' => '200', - 'open_graph' => [ - 'og_title' => 'my OG title', - 'og_description' => 'OG desc', - 'og_image' => null, - ], + 'image' => null, ]); $proxy = new ContentProxy($graby, $tagger, $this->getValidator(), $this->getLogger(), $this->fetchingErrorMessage); @@ -274,14 +272,12 @@ class ContentProxyTest extends TestCase 'html' => "

Test

", 'title' => 'this is my title', 'url' => 'http://1.1.1.1', - 'content_type' => 'text/html', + 'headers' => [ + 'content-type' => 'text/html', + ], 'language' => 'fr', 'status' => '200', - 'open_graph' => [ - 'og_title' => 'my OG title', - 'og_description' => 'OG desc', - 'og_image' => 'http://3.3.3.3/cover.jpg', - ], + 'image' => 'http://3.3.3.3/cover.jpg', ]); $proxy = new ContentProxy($graby, $tagger, $this->getValidator(), $this->getLogger(), $this->fetchingErrorMessage);