diff options
author | Jeremy Benoist <jbenoist@20minutes.fr> | 2017-05-30 17:48:24 +0200 |
---|---|---|
committer | Jeremy Benoist <jbenoist@20minutes.fr> | 2017-06-01 09:49:15 +0200 |
commit | d5c2cc54b5490b0bec46f39d7706d5d46869e872 (patch) | |
tree | a40c4b73e13038d268b4f1f46c02bca5525bddee | |
parent | 432a24f5028446f1bc5c184905f8406bbef8bf05 (diff) | |
download | wallabag-d5c2cc54b5490b0bec46f39d7706d5d46869e872.tar.gz wallabag-d5c2cc54b5490b0bec46f39d7706d5d46869e872.tar.zst wallabag-d5c2cc54b5490b0bec46f39d7706d5d46869e872.zip |
Fix tests
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 | |||
231 | $this->validateAuthentication(); | 231 | $this->validateAuthentication(); |
232 | 232 | ||
233 | $urls = json_decode($request->query->get('urls', [])); | 233 | $urls = json_decode($request->query->get('urls', [])); |
234 | $results = []; | ||
235 | 234 | ||
236 | $limit = $this->container->getParameter('wallabag_core.api_limit_mass_actions'); | 235 | $limit = $this->container->getParameter('wallabag_core.api_limit_mass_actions'); |
237 | 236 | ||
@@ -239,32 +238,34 @@ class EntryRestController extends WallabagRestController | |||
239 | throw new HttpException(400, 'API limit reached'); | 238 | throw new HttpException(400, 'API limit reached'); |
240 | } | 239 | } |
241 | 240 | ||
241 | $results = []; | ||
242 | if (empty($urls)) { | ||
243 | return $this->sendResponse($results); | ||
244 | } | ||
245 | |||
242 | // handle multiple urls | 246 | // handle multiple urls |
243 | if (!empty($urls)) { | 247 | foreach ($urls as $key => $url) { |
244 | foreach ($urls as $key => $url) { | 248 | $entry = $this->get('wallabag_core.entry_repository')->findByUrlAndUserId( |
245 | $entry = $this->get('wallabag_core.entry_repository')->findByUrlAndUserId( | 249 | $url, |
246 | $url, | 250 | $this->getUser()->getId() |
247 | $this->getUser()->getId() | 251 | ); |
248 | ); | ||
249 | |||
250 | $results[$key]['url'] = $url; | ||
251 | |||
252 | if (false === $entry) { | ||
253 | $entry = $this->get('wallabag_core.content_proxy')->updateEntry( | ||
254 | new Entry($this->getUser()), | ||
255 | $url | ||
256 | ); | ||
257 | } | ||
258 | 252 | ||
259 | $em = $this->getDoctrine()->getManager(); | 253 | $results[$key]['url'] = $url; |
260 | $em->persist($entry); | ||
261 | $em->flush(); | ||
262 | 254 | ||
263 | $results[$key]['entry'] = $entry instanceof Entry ? $entry->getId() : false; | 255 | if (false === $entry) { |
256 | $entry = new Entry($this->getUser()); | ||
264 | 257 | ||
265 | // entry saved, dispatch event about it! | 258 | $this->get('wallabag_core.content_proxy')->updateEntry($entry, $url); |
266 | $this->get('event_dispatcher')->dispatch(EntrySavedEvent::NAME, new EntrySavedEvent($entry)); | ||
267 | } | 259 | } |
260 | |||
261 | $em = $this->getDoctrine()->getManager(); | ||
262 | $em->persist($entry); | ||
263 | $em->flush(); | ||
264 | |||
265 | $results[$key]['entry'] = $entry instanceof Entry ? $entry->getId() : false; | ||
266 | |||
267 | // entry saved, dispatch event about it! | ||
268 | $this->get('event_dispatcher')->dispatch(EntrySavedEvent::NAME, new EntrySavedEvent($entry)); | ||
268 | } | 269 | } |
269 | 270 | ||
270 | return $this->sendResponse($results); | 271 | 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 | |||
34 | /** | 34 | /** |
35 | * Update existing entry by fetching from URL using Graby. | 35 | * Update existing entry by fetching from URL using Graby. |
36 | * | 36 | * |
37 | * @param Entry $entry Entry to update | 37 | * @param Entry $entry Entry to update |
38 | * @param string $url Url to grab content for | 38 | * @param string $url Url to grab content for |
39 | */ | 39 | */ |
40 | public function updateEntry(Entry $entry, $url) | 40 | public function updateEntry(Entry $entry, $url) |
41 | { | 41 | { |
42 | $content = $this->graby->fetchContent($url); | 42 | $content = $this->graby->fetchContent($url); |
43 | 43 | ||
44 | // be sure to keep the url in case of error | ||
45 | // so we'll be able to refetch it in the future | ||
46 | $content['url'] = $content['url'] ?: $url; | ||
47 | |||
44 | $this->stockEntry($entry, $content); | 48 | $this->stockEntry($entry, $content); |
45 | } | 49 | } |
46 | 50 | ||
@@ -53,7 +57,14 @@ class ContentProxy | |||
53 | */ | 57 | */ |
54 | public function importEntry(Entry $entry, array $content, $disableContentUpdate = false) | 58 | public function importEntry(Entry $entry, array $content, $disableContentUpdate = false) |
55 | { | 59 | { |
56 | $this->validateContent($content); | 60 | try { |
61 | $this->validateContent($content); | ||
62 | } catch (\Exception $e) { | ||
63 | // validation failed but do we want to disable updating content? | ||
64 | if (true === $disableContentUpdate) { | ||
65 | throw $e; | ||
66 | } | ||
67 | } | ||
57 | 68 | ||
58 | if (false === $disableContentUpdate) { | 69 | if (false === $disableContentUpdate) { |
59 | try { | 70 | try { |
@@ -79,8 +90,8 @@ class ContentProxy | |||
79 | * Stock entry with fetched or imported content. | 90 | * Stock entry with fetched or imported content. |
80 | * Will fall back to OpenGraph data if available. | 91 | * Will fall back to OpenGraph data if available. |
81 | * | 92 | * |
82 | * @param Entry $entry Entry to stock | 93 | * @param Entry $entry Entry to stock |
83 | * @param array $content Array with at least title and URL | 94 | * @param array $content Array with at least title and URL |
84 | */ | 95 | */ |
85 | private function stockEntry(Entry $entry, array $content) | 96 | private function stockEntry(Entry $entry, array $content) |
86 | { | 97 | { |
@@ -162,15 +173,15 @@ class ContentProxy | |||
162 | */ | 173 | */ |
163 | private function validateContent(array $content) | 174 | private function validateContent(array $content) |
164 | { | 175 | { |
165 | if (!empty($content['title']))) { | 176 | if (empty($content['title'])) { |
166 | throw new Exception('Missing title from imported entry!'); | 177 | throw new Exception('Missing title from imported entry!'); |
167 | } | 178 | } |
168 | 179 | ||
169 | if (!empty($content['url']))) { | 180 | if (empty($content['url'])) { |
170 | throw new Exception('Missing URL from imported entry!'); | 181 | throw new Exception('Missing URL from imported entry!'); |
171 | } | 182 | } |
172 | 183 | ||
173 | if (!empty($content['html']))) { | 184 | if (empty($content['html'])) { |
174 | throw new Exception('Missing html from imported entry!'); | 185 | throw new Exception('Missing html from imported entry!'); |
175 | } | 186 | } |
176 | } | 187 | } |
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 | |||
20 | ->addArgument('filepath', InputArgument::REQUIRED, 'Path to the JSON file') | 20 | ->addArgument('filepath', InputArgument::REQUIRED, 'Path to the JSON file') |
21 | ->addOption('importer', null, InputArgument::OPTIONAL, 'The importer to use: v1, v2, instapaper, pinboard, readability, firefox or chrome', 'v1') | 21 | ->addOption('importer', null, InputArgument::OPTIONAL, 'The importer to use: v1, v2, instapaper, pinboard, readability, firefox or chrome', 'v1') |
22 | ->addOption('markAsRead', null, InputArgument::OPTIONAL, 'Mark all entries as read', false) | 22 | ->addOption('markAsRead', null, InputArgument::OPTIONAL, 'Mark all entries as read', false) |
23 | ->addOption('useUserId', null, InputArgument::OPTIONAL, 'Use user id instead of username to find account', false) | ||
23 | ->addOption('disableContentUpdate', null, InputOption::VALUE_NONE, 'Disable fetching updated content from URL') | 24 | ->addOption('disableContentUpdate', null, InputOption::VALUE_NONE, 'Disable fetching updated content from URL') |
24 | ; | 25 | ; |
25 | } | 26 | } |
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 | |||
24 | protected $producer; | 24 | protected $producer; |
25 | protected $user; | 25 | protected $user; |
26 | protected $markAsRead; | 26 | protected $markAsRead; |
27 | protected $disableContentUpdate; | 27 | protected $disableContentUpdate = false; |
28 | protected $skippedEntries = 0; | 28 | protected $skippedEntries = 0; |
29 | protected $importedEntries = 0; | 29 | protected $importedEntries = 0; |
30 | protected $queuedEntries = 0; | 30 | protected $queuedEntries = 0; |
@@ -115,6 +115,9 @@ abstract class AbstractImport implements ImportInterface | |||
115 | */ | 115 | */ |
116 | protected function fetchContent(Entry $entry, $url, array $content = []) | 116 | protected function fetchContent(Entry $entry, $url, array $content = []) |
117 | { | 117 | { |
118 | // be sure to set at least the given url | ||
119 | $content['url'] = isset($content['url']) ? $content['url'] : $url; | ||
120 | |||
118 | try { | 121 | try { |
119 | $this->contentProxy->importEntry($entry, $content, $this->disableContentUpdate); | 122 | $this->contentProxy->importEntry($entry, $content, $this->disableContentUpdate); |
120 | } catch (\Exception $e) { | 123 | } 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 | |||
7 | protected $fetchingErrorMessage; | 7 | protected $fetchingErrorMessage; |
8 | protected $fetchingErrorMessageTitle; | 8 | protected $fetchingErrorMessageTitle; |
9 | 9 | ||
10 | public function __construct($em, $contentProxy, $eventDispatcher, $fetchingErrorMessageTitle, $fetchingErrorMessage) | 10 | public function __construct($em, $contentProxy, $tagsAssigner, $eventDispatcher, $fetchingErrorMessageTitle, $fetchingErrorMessage) |
11 | { | 11 | { |
12 | $this->fetchingErrorMessageTitle = $fetchingErrorMessageTitle; | 12 | $this->fetchingErrorMessageTitle = $fetchingErrorMessageTitle; |
13 | $this->fetchingErrorMessage = $fetchingErrorMessage; | 13 | $this->fetchingErrorMessage = $fetchingErrorMessage; |
14 | 14 | ||
15 | parent::__construct($em, $contentProxy, $eventDispatcher); | 15 | parent::__construct($em, $contentProxy, $tagsAssigner, $eventDispatcher); |
16 | } | 16 | } |
17 | 17 | ||
18 | /** | 18 | /** |
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 @@ | |||
3 | namespace Tests\Wallabag\CoreBundle\Helper; | 3 | namespace Tests\Wallabag\CoreBundle\Helper; |
4 | 4 | ||
5 | use Psr\Log\NullLogger; | 5 | use Psr\Log\NullLogger; |
6 | use Monolog\Logger; | ||
7 | use Monolog\Handler\TestHandler; | ||
6 | use Wallabag\CoreBundle\Helper\ContentProxy; | 8 | use Wallabag\CoreBundle\Helper\ContentProxy; |
7 | use Wallabag\CoreBundle\Entity\Entry; | 9 | use Wallabag\CoreBundle\Entity\Entry; |
8 | use Wallabag\CoreBundle\Entity\Tag; | 10 | use Wallabag\CoreBundle\Entity\Tag; |
@@ -197,7 +199,8 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase | |||
197 | ]); | 199 | ]); |
198 | 200 | ||
199 | $proxy = new ContentProxy($graby, $tagger, $this->getLogger(), $this->fetchingErrorMessage); | 201 | $proxy = new ContentProxy($graby, $tagger, $this->getLogger(), $this->fetchingErrorMessage); |
200 | $entry = $proxy->updateEntry(new Entry(new User()), 'http://0.0.0.0'); | 202 | $entry = new Entry(new User()); |
203 | $proxy->updateEntry($entry, 'http://0.0.0.0'); | ||
201 | 204 | ||
202 | $this->assertEquals('http://1.1.1.1', $entry->getUrl()); | 205 | $this->assertEquals('http://1.1.1.1', $entry->getUrl()); |
203 | $this->assertEquals('this is my title', $entry->getTitle()); | 206 | $this->assertEquals('this is my title', $entry->getTitle()); |
@@ -255,7 +258,10 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase | |||
255 | $tagger->expects($this->once()) | 258 | $tagger->expects($this->once()) |
256 | ->method('tag'); | 259 | ->method('tag'); |
257 | 260 | ||
258 | $proxy = new ContentProxy((new Graby()), $tagger, $this->getLogger(), $this->fetchingErrorMessage); | 261 | $logHandler = new TestHandler(); |
262 | $logger = new Logger('test', array($logHandler)); | ||
263 | |||
264 | $proxy = new ContentProxy((new Graby()), $tagger, $logger, $this->fetchingErrorMessage); | ||
259 | $entry = new Entry(new User()); | 265 | $entry = new Entry(new User()); |
260 | $proxy->importEntry( | 266 | $proxy->importEntry( |
261 | $entry, | 267 | $entry, |