From 9f8f188d928b47503d39348c5990379a572b570a Mon Sep 17 00:00:00 2001 From: Jeremy Benoist 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(-) (limited to 'src/Wallabag') 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