From d5c2cc54b5490b0bec46f39d7706d5d46869e872 Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Tue, 30 May 2017 17:48:24 +0200 Subject: [PATCH] Fix tests --- .../Controller/EntryRestController.php | 45 ++++++++++--------- .../CoreBundle/Helper/ContentProxy.php | 27 +++++++---- .../ImportBundle/Command/ImportCommand.php | 1 + .../ImportBundle/Import/AbstractImport.php | 5 ++- .../ImportBundle/Import/WallabagV1Import.php | 4 +- .../CoreBundle/Helper/ContentProxyTest.php | 10 ++++- 6 files changed, 57 insertions(+), 35 deletions(-) diff --git a/src/Wallabag/ApiBundle/Controller/EntryRestController.php b/src/Wallabag/ApiBundle/Controller/EntryRestController.php index d154c18b..93c8157e 100644 --- a/src/Wallabag/ApiBundle/Controller/EntryRestController.php +++ b/src/Wallabag/ApiBundle/Controller/EntryRestController.php @@ -231,7 +231,6 @@ class EntryRestController extends WallabagRestController $this->validateAuthentication(); $urls = json_decode($request->query->get('urls', [])); - $results = []; $limit = $this->container->getParameter('wallabag_core.api_limit_mass_actions'); @@ -239,32 +238,34 @@ class EntryRestController extends WallabagRestController throw new HttpException(400, 'API limit reached'); } + $results = []; + if (empty($urls)) { + return $this->sendResponse($results); + } + // handle multiple urls - if (!empty($urls)) { - foreach ($urls as $key => $url) { - $entry = $this->get('wallabag_core.entry_repository')->findByUrlAndUserId( - $url, - $this->getUser()->getId() - ); - - $results[$key]['url'] = $url; - - if (false === $entry) { - $entry = $this->get('wallabag_core.content_proxy')->updateEntry( - new Entry($this->getUser()), - $url - ); - } + foreach ($urls as $key => $url) { + $entry = $this->get('wallabag_core.entry_repository')->findByUrlAndUserId( + $url, + $this->getUser()->getId() + ); - $em = $this->getDoctrine()->getManager(); - $em->persist($entry); - $em->flush(); + $results[$key]['url'] = $url; - $results[$key]['entry'] = $entry instanceof Entry ? $entry->getId() : false; + if (false === $entry) { + $entry = new Entry($this->getUser()); - // entry saved, dispatch event about it! - $this->get('event_dispatcher')->dispatch(EntrySavedEvent::NAME, new EntrySavedEvent($entry)); + $this->get('wallabag_core.content_proxy')->updateEntry($entry, $url); } + + $em = $this->getDoctrine()->getManager(); + $em->persist($entry); + $em->flush(); + + $results[$key]['entry'] = $entry instanceof Entry ? $entry->getId() : false; + + // entry saved, dispatch event about it! + $this->get('event_dispatcher')->dispatch(EntrySavedEvent::NAME, new EntrySavedEvent($entry)); } return $this->sendResponse($results); diff --git a/src/Wallabag/CoreBundle/Helper/ContentProxy.php b/src/Wallabag/CoreBundle/Helper/ContentProxy.php index 88873bd5..cd18c668 100644 --- a/src/Wallabag/CoreBundle/Helper/ContentProxy.php +++ b/src/Wallabag/CoreBundle/Helper/ContentProxy.php @@ -34,13 +34,17 @@ class ContentProxy /** * Update existing entry by fetching from URL using Graby. * - * @param Entry $entry Entry to update - * @param string $url Url to grab content for + * @param Entry $entry Entry to update + * @param string $url Url to grab content for */ public function updateEntry(Entry $entry, $url) { $content = $this->graby->fetchContent($url); + // be sure to keep the url in case of error + // so we'll be able to refetch it in the future + $content['url'] = $content['url'] ?: $url; + $this->stockEntry($entry, $content); } @@ -53,7 +57,14 @@ class ContentProxy */ public function importEntry(Entry $entry, array $content, $disableContentUpdate = false) { - $this->validateContent($content); + try { + $this->validateContent($content); + } catch (\Exception $e) { + // validation failed but do we want to disable updating content? + if (true === $disableContentUpdate) { + throw $e; + } + } if (false === $disableContentUpdate) { try { @@ -79,8 +90,8 @@ class ContentProxy * Stock entry with fetched or imported content. * Will fall back to OpenGraph data if available. * - * @param Entry $entry Entry to stock - * @param array $content Array with at least title and URL + * @param Entry $entry Entry to stock + * @param array $content Array with at least title and URL */ private function stockEntry(Entry $entry, array $content) { @@ -162,15 +173,15 @@ class ContentProxy */ private function validateContent(array $content) { - if (!empty($content['title']))) { + if (empty($content['title'])) { throw new Exception('Missing title from imported entry!'); } - if (!empty($content['url']))) { + if (empty($content['url'])) { throw new Exception('Missing URL from imported entry!'); } - if (!empty($content['html']))) { + if (empty($content['html'])) { throw new Exception('Missing html from imported entry!'); } } diff --git a/src/Wallabag/ImportBundle/Command/ImportCommand.php b/src/Wallabag/ImportBundle/Command/ImportCommand.php index bca800e6..0829a1da 100644 --- a/src/Wallabag/ImportBundle/Command/ImportCommand.php +++ b/src/Wallabag/ImportBundle/Command/ImportCommand.php @@ -20,6 +20,7 @@ class ImportCommand extends ContainerAwareCommand ->addArgument('filepath', InputArgument::REQUIRED, 'Path to the JSON file') ->addOption('importer', null, InputArgument::OPTIONAL, 'The importer to use: v1, v2, instapaper, pinboard, readability, firefox or chrome', 'v1') ->addOption('markAsRead', null, InputArgument::OPTIONAL, 'Mark all entries as read', false) + ->addOption('useUserId', null, InputArgument::OPTIONAL, 'Use user id instead of username to find account', false) ->addOption('disableContentUpdate', null, InputOption::VALUE_NONE, 'Disable fetching updated content from URL') ; } diff --git a/src/Wallabag/ImportBundle/Import/AbstractImport.php b/src/Wallabag/ImportBundle/Import/AbstractImport.php index 1f904292..bf568a1a 100644 --- a/src/Wallabag/ImportBundle/Import/AbstractImport.php +++ b/src/Wallabag/ImportBundle/Import/AbstractImport.php @@ -24,7 +24,7 @@ abstract class AbstractImport implements ImportInterface protected $producer; protected $user; protected $markAsRead; - protected $disableContentUpdate; + protected $disableContentUpdate = false; protected $skippedEntries = 0; protected $importedEntries = 0; protected $queuedEntries = 0; @@ -115,6 +115,9 @@ abstract class AbstractImport implements ImportInterface */ protected function fetchContent(Entry $entry, $url, array $content = []) { + // be sure to set at least the given url + $content['url'] = isset($content['url']) ? $content['url'] : $url; + try { $this->contentProxy->importEntry($entry, $content, $this->disableContentUpdate); } catch (\Exception $e) { diff --git a/src/Wallabag/ImportBundle/Import/WallabagV1Import.php b/src/Wallabag/ImportBundle/Import/WallabagV1Import.php index 872fd642..1f0df646 100644 --- a/src/Wallabag/ImportBundle/Import/WallabagV1Import.php +++ b/src/Wallabag/ImportBundle/Import/WallabagV1Import.php @@ -7,12 +7,12 @@ class WallabagV1Import extends WallabagImport protected $fetchingErrorMessage; protected $fetchingErrorMessageTitle; - public function __construct($em, $contentProxy, $eventDispatcher, $fetchingErrorMessageTitle, $fetchingErrorMessage) + public function __construct($em, $contentProxy, $tagsAssigner, $eventDispatcher, $fetchingErrorMessageTitle, $fetchingErrorMessage) { $this->fetchingErrorMessageTitle = $fetchingErrorMessageTitle; $this->fetchingErrorMessage = $fetchingErrorMessage; - parent::__construct($em, $contentProxy, $eventDispatcher); + parent::__construct($em, $contentProxy, $tagsAssigner, $eventDispatcher); } /** diff --git a/tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php b/tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php index 1ad21d14..be287d84 100644 --- a/tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php +++ b/tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php @@ -3,6 +3,8 @@ namespace Tests\Wallabag\CoreBundle\Helper; use Psr\Log\NullLogger; +use Monolog\Logger; +use Monolog\Handler\TestHandler; use Wallabag\CoreBundle\Helper\ContentProxy; use Wallabag\CoreBundle\Entity\Entry; use Wallabag\CoreBundle\Entity\Tag; @@ -197,7 +199,8 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase ]); $proxy = new ContentProxy($graby, $tagger, $this->getLogger(), $this->fetchingErrorMessage); - $entry = $proxy->updateEntry(new Entry(new User()), 'http://0.0.0.0'); + $entry = new Entry(new User()); + $proxy->updateEntry($entry, 'http://0.0.0.0'); $this->assertEquals('http://1.1.1.1', $entry->getUrl()); $this->assertEquals('this is my title', $entry->getTitle()); @@ -255,7 +258,10 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase $tagger->expects($this->once()) ->method('tag'); - $proxy = new ContentProxy((new Graby()), $tagger, $this->getLogger(), $this->fetchingErrorMessage); + $logHandler = new TestHandler(); + $logger = new Logger('test', array($logHandler)); + + $proxy = new ContentProxy((new Graby()), $tagger, $logger, $this->fetchingErrorMessage); $entry = new Entry(new User()); $proxy->importEntry( $entry, -- 2.41.0