From 9f8f188d928b47503d39348c5990379a572b570a Mon Sep 17 00:00:00 2001
From: Jeremy Benoist <jeremy.benoist@gmail.com>
Date: Tue, 18 Dec 2018 13:14:42 +0100
Subject: Validate imported entry to avoid error on import

We got some imports with a missing `url` field generating some errors while trying to retrieve an existing entry with that url.
Introducing the `validateEntry` allow us to dismiss a message when it doesn't have an url (or other missing stuff in the future)
---
 src/Wallabag/ImportBundle/Consumer/AbstractConsumer.php |  7 +++++++
 src/Wallabag/ImportBundle/Import/AbstractImport.php     | 17 +++++++++++++++--
 src/Wallabag/ImportBundle/Import/BrowserImport.php      |  4 ++--
 src/Wallabag/ImportBundle/Import/ChromeImport.php       | 12 ++++++++++++
 src/Wallabag/ImportBundle/Import/FirefoxImport.php      | 12 ++++++++++++
 src/Wallabag/ImportBundle/Import/InstapaperImport.php   | 12 ++++++++++++
 src/Wallabag/ImportBundle/Import/PinboardImport.php     | 12 ++++++++++++
 src/Wallabag/ImportBundle/Import/PocketImport.php       | 12 ++++++++++++
 src/Wallabag/ImportBundle/Import/ReadabilityImport.php  | 12 ++++++++++++
 src/Wallabag/ImportBundle/Import/WallabagImport.php     | 12 ++++++++++++
 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
@@ -118,6 +118,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
@@ -30,6 +30,18 @@ class ChromeImport extends BrowserImport
         return 'import.chrome.description';
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function validateEntry(array $importedEntry)
+    {
+        if (empty($importedEntry['url'])) {
+            return false;
+        }
+
+        return true;
+    }
+
     /**
      * {@inheritdoc}
      */
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
@@ -30,6 +30,18 @@ class FirefoxImport extends BrowserImport
         return 'import.firefox.description';
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function validateEntry(array $importedEntry)
+    {
+        if (empty($importedEntry['uri'])) {
+            return false;
+        }
+
+        return true;
+    }
+
     /**
      * {@inheritdoc}
      */
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
@@ -105,6 +105,18 @@ class InstapaperImport extends AbstractImport
         return true;
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function validateEntry(array $importedEntry)
+    {
+        if (empty($importedEntry['url'])) {
+            return false;
+        }
+
+        return true;
+    }
+
     /**
      * {@inheritdoc}
      */
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
@@ -80,6 +80,18 @@ class PinboardImport extends AbstractImport
         return true;
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function validateEntry(array $importedEntry)
+    {
+        if (empty($importedEntry['href'])) {
+            return false;
+        }
+
+        return true;
+    }
+
     /**
      * {@inheritdoc}
      */
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
@@ -168,6 +168,18 @@ class PocketImport extends AbstractImport
         $this->client = $client;
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function validateEntry(array $importedEntry)
+    {
+        if (empty($importedEntry['resolved_url']) && empty($importedEntry['given_url'])) {
+            return false;
+        }
+
+        return true;
+    }
+
     /**
      * {@inheritdoc}
      *
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
@@ -80,6 +80,18 @@ class ReadabilityImport extends AbstractImport
         return true;
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function validateEntry(array $importedEntry)
+    {
+        if (empty($importedEntry['article__url'])) {
+            return false;
+        }
+
+        return true;
+    }
+
     /**
      * {@inheritdoc}
      */
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
@@ -86,6 +86,18 @@ abstract class WallabagImport extends AbstractImport
         return $this;
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function validateEntry(array $importedEntry)
+    {
+        if (empty($importedEntry['url'])) {
+            return false;
+        }
+
+        return true;
+    }
+
     /**
      * {@inheritdoc}
      */
-- 
cgit v1.2.3