]> git.immae.eu Git - github/wallabag/wallabag.git/commitdiff
Fix tests
authorJeremy Benoist <jbenoist@20minutes.fr>
Tue, 30 May 2017 15:48:24 +0000 (17:48 +0200)
committerJeremy Benoist <jbenoist@20minutes.fr>
Thu, 1 Jun 2017 07:49:15 +0000 (09:49 +0200)
src/Wallabag/ApiBundle/Controller/EntryRestController.php
src/Wallabag/CoreBundle/Helper/ContentProxy.php
src/Wallabag/ImportBundle/Command/ImportCommand.php
src/Wallabag/ImportBundle/Import/AbstractImport.php
src/Wallabag/ImportBundle/Import/WallabagV1Import.php
tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php

index d154c18b37d71a623fe28926f8a3cef0d466e452..93c8157e758a934905663846516fccc27b46e50b 100644 (file)
@@ -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);
index 88873bd5305bab6dba0f553b158c0a862468bc22..cd18c668f7ea3b1b342260bca27591a6022c4103 100644 (file)
@@ -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!');
         }
     }
index bca800e6cf23bb5d614c7396f959f0b6d7d7b67c..0829a1da34ace2cc135fd75b2d742faf0bacafc6 100644 (file)
@@ -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')
         ;
     }
index 1f9042923478365a51d8436f33656c91aa596d05..bf568a1af4ec5a4d4a44963f7df7d0fff612cd09 100644 (file)
@@ -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) {
index 872fd642f5b6465773773cd5d8584547de56dbca..1f0df646bc85908e1e194ef403d3a31144c1ef8b 100644 (file)
@@ -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);
     }
 
     /**
index 1ad21d147103d326f22c240fe4f252a848afe507..be287d847c61b9bda087dafb6373003d020495c9 100644 (file)
@@ -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,