Merge pull request #3816 from wallabag/validate-import-entry

Validate imported entry to avoid error on import

10 files changed, 108 insertions, 4 deletions

diff --git a/src/Wallabag/ImportBundle/Consumer/AbstractConsumer.php b/src/Wallabag/ImportBundle/Consumer/AbstractConsumer.php
index b035f5cc..e4bfbdf0 100644
--- a/src/Wallabag/ImportBundle/Consumer/AbstractConsumer.php
+++ b/src/Wallabag/ImportBundle/Consumer/AbstractConsumer.php
@@ -52,6 +52,13 @@ abstract class AbstractConsumer
 
         $this->import->setUser($user);
 
+        if (false === $this->import->validateEntry($storedEntry)) {
+            $this->logger->warning('Entry is invalid', ['entry' => $storedEntry]);
+
+            // return true to skip message
+            return true;
+        }
+
         $entry = $this->import->parseEntry($storedEntry);
 
         if (null === $entry) {
diff --git a/src/Wallabag/ImportBundle/Import/AbstractImport.php b/src/Wallabag/ImportBundle/Import/AbstractImport.php
index 58a234f4..d39d71b6 100644
--- a/src/Wallabag/ImportBundle/Import/AbstractImport.php
+++ b/src/Wallabag/ImportBundle/Import/AbstractImport.php
@@ -119,6 +119,15 @@ abstract class AbstractImport implements ImportInterface
     abstract public function parseEntry(array $importedEntry);
 
     /**
+     * Validate that an entry is valid (like has some required keys, etc.).
+     *
+     * @param array $importedEntry
+     *
+     * @return bool
+     */
+    abstract public function validateEntry(array $importedEntry);
+
+    /**
      * Fetch content from the ContentProxy (using graby).
      * If it fails return the given entry to be saved in all case (to avoid user to loose the content).
      *
@@ -141,9 +150,9 @@ abstract class AbstractImport implements ImportInterface
     /**
      * Parse and insert all given entries.
      *
-     * @param $entries
+     * @param array $entries
      */
-    protected function parseEntries($entries)
+    protected function parseEntries(array $entries)
     {
         $i = 1;
         $entryToBeFlushed = [];
@@ -153,6 +162,10 @@ abstract class AbstractImport implements ImportInterface
                 $importedEntry = $this->setEntryAsRead($importedEntry);
             }
 
+            if (false === $this->validateEntry($importedEntry)) {
+                continue;
+            }
+
             $entry = $this->parseEntry($importedEntry);
 
             if (null === $entry) {
diff --git a/src/Wallabag/ImportBundle/Import/BrowserImport.php b/src/Wallabag/ImportBundle/Import/BrowserImport.php
index 225f1791..4678ae0c 100644
--- a/src/Wallabag/ImportBundle/Import/BrowserImport.php
+++ b/src/Wallabag/ImportBundle/Import/BrowserImport.php
@@ -149,9 +149,9 @@ abstract class BrowserImport extends AbstractImport
     /**
      * Parse and insert all given entries.
      *
-     * @param $entries
+     * @param array $entries
      */
-    protected function parseEntries($entries)
+    protected function parseEntries(array $entries)
     {
         $i = 1;
         $entryToBeFlushed = [];
diff --git a/src/Wallabag/ImportBundle/Import/ChromeImport.php b/src/Wallabag/ImportBundle/Import/ChromeImport.php
index 09183abe..eccee698 100644
--- a/src/Wallabag/ImportBundle/Import/ChromeImport.php
+++ b/src/Wallabag/ImportBundle/Import/ChromeImport.php
@@ -33,6 +33,18 @@ class ChromeImport extends BrowserImport
     /**
      * {@inheritdoc}
      */
+    public function validateEntry(array $importedEntry)
+    {
+        if (empty($importedEntry['url'])) {
+            return false;
+        }
+
+        return true;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
     protected function prepareEntry(array $entry = [])
     {
         $data = [
diff --git a/src/Wallabag/ImportBundle/Import/FirefoxImport.php b/src/Wallabag/ImportBundle/Import/FirefoxImport.php
index 73269fe1..8999e3f3 100644
--- a/src/Wallabag/ImportBundle/Import/FirefoxImport.php
+++ b/src/Wallabag/ImportBundle/Import/FirefoxImport.php
@@ -33,6 +33,18 @@ class FirefoxImport extends BrowserImport
     /**
      * {@inheritdoc}
      */
+    public function validateEntry(array $importedEntry)
+    {
+        if (empty($importedEntry['uri'])) {
+            return false;
+        }
+
+        return true;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
     protected function prepareEntry(array $entry = [])
     {
         $data = [
diff --git a/src/Wallabag/ImportBundle/Import/InstapaperImport.php b/src/Wallabag/ImportBundle/Import/InstapaperImport.php
index e4f0970c..5a18c7c0 100644
--- a/src/Wallabag/ImportBundle/Import/InstapaperImport.php
+++ b/src/Wallabag/ImportBundle/Import/InstapaperImport.php
@@ -108,6 +108,18 @@ class InstapaperImport extends AbstractImport
     /**
      * {@inheritdoc}
      */
+    public function validateEntry(array $importedEntry)
+    {
+        if (empty($importedEntry['url'])) {
+            return false;
+        }
+
+        return true;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
     public function parseEntry(array $importedEntry)
     {
         $existingEntry = $this->em
diff --git a/src/Wallabag/ImportBundle/Import/PinboardImport.php b/src/Wallabag/ImportBundle/Import/PinboardImport.php
index 110b0464..995d1f2c 100644
--- a/src/Wallabag/ImportBundle/Import/PinboardImport.php
+++ b/src/Wallabag/ImportBundle/Import/PinboardImport.php
@@ -83,6 +83,18 @@ class PinboardImport extends AbstractImport
     /**
      * {@inheritdoc}
      */
+    public function validateEntry(array $importedEntry)
+    {
+        if (empty($importedEntry['href'])) {
+            return false;
+        }
+
+        return true;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
     public function parseEntry(array $importedEntry)
     {
         $existingEntry = $this->em
diff --git a/src/Wallabag/ImportBundle/Import/PocketImport.php b/src/Wallabag/ImportBundle/Import/PocketImport.php
index c1b35b7e..d3643389 100644
--- a/src/Wallabag/ImportBundle/Import/PocketImport.php
+++ b/src/Wallabag/ImportBundle/Import/PocketImport.php
@@ -170,6 +170,18 @@ class PocketImport extends AbstractImport
 
     /**
      * {@inheritdoc}
+     */
+    public function validateEntry(array $importedEntry)
+    {
+        if (empty($importedEntry['resolved_url']) && empty($importedEntry['given_url'])) {
+            return false;
+        }
+
+        return true;
+    }
+
+    /**
+     * {@inheritdoc}
      *
      * @see https://getpocket.com/developer/docs/v3/retrieve
      */
diff --git a/src/Wallabag/ImportBundle/Import/ReadabilityImport.php b/src/Wallabag/ImportBundle/Import/ReadabilityImport.php
index 002b27f4..a5f3798e 100644
--- a/src/Wallabag/ImportBundle/Import/ReadabilityImport.php
+++ b/src/Wallabag/ImportBundle/Import/ReadabilityImport.php
@@ -83,6 +83,18 @@ class ReadabilityImport extends AbstractImport
     /**
      * {@inheritdoc}
      */
+    public function validateEntry(array $importedEntry)
+    {
+        if (empty($importedEntry['article__url'])) {
+            return false;
+        }
+
+        return true;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
     public function parseEntry(array $importedEntry)
     {
         $existingEntry = $this->em
diff --git a/src/Wallabag/ImportBundle/Import/WallabagImport.php b/src/Wallabag/ImportBundle/Import/WallabagImport.php
index c64ccd64..350d0600 100644
--- a/src/Wallabag/ImportBundle/Import/WallabagImport.php
+++ b/src/Wallabag/ImportBundle/Import/WallabagImport.php
@@ -89,6 +89,18 @@ abstract class WallabagImport extends AbstractImport
     /**
      * {@inheritdoc}
      */
+    public function validateEntry(array $importedEntry)
+    {
+        if (empty($importedEntry['url'])) {
+            return false;
+        }
+
+        return true;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
     public function parseEntry(array $importedEntry)
     {
         $existingEntry = $this->em