Make feed failing more verbose

5 files changed, 31 insertions, 54 deletions

diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php
index b2773a224..c70a2fb6f 100644
--- a/lib/AppInfo/Application.php
+++ b/lib/AppInfo/Application.php
@@ -72,7 +72,7 @@ class Application extends App
         $container->registerParameter('configFile', 'config.ini');
 
         // factories
-        $container->registerService(ItemMapper::class, function (IContainer $c) {
+        $container->registerService(ItemMapper::class, function (IContainer $c): ItemMapper {
             return $c->query(MapperFactory::class)->build();
         });
 
@@ -80,7 +80,7 @@ class Application extends App
         /**
          * App config parser.
          */
-        $container->registerService(AppConfig::class, function (IContainer $c) {
+        $container->registerService(AppConfig::class, function (IContainer $c): AppConfig {
             $config = new AppConfig(
                 $c->query(INavigationManager::class),
                 $c->query(IURLGenerator::class),
@@ -116,7 +116,7 @@ class Application extends App
         /**
          * Logger base
          */
-        $container->registerService(PsrLogger::class, function (IContainer $c) {
+        $container->registerService(PsrLogger::class, function (IContainer $c): PsrLogger {
             return new PsrLogger(
                 $c->query('ServerContainer')->getLogger(),
                 $c->query('AppName')
@@ -160,7 +160,7 @@ class Application extends App
         /**
          * Fetchers
          */
-        $container->registerService(FetcherConfig::class, function (IContainer $c) {
+        $container->registerService(FetcherConfig::class, function (IContainer $c): FetcherConfig {
             // FIXME: move this into a separate class for testing?
             $config = $c->query(Config::class);
             $proxy  = $c->query(ProxyConfigParser::class);
@@ -172,7 +172,7 @@ class Application extends App
             return $fConfig;
         });
 
-        $container->registerService(FeedIo::class, function (IContainer $c) {
+        $container->registerService(FeedIo::class, function (IContainer $c): FeedIo {
             $config = $c->query(FetcherConfig::class);
             return new FeedIo($config->getClient(), $c->query(PsrLogger::class));
         });
@@ -180,7 +180,7 @@ class Application extends App
         /**
          * @noinspection PhpParamsInspection
          */
-        $container->registerService(Fetcher::class, function (IContainer $c) {
+        $container->registerService(Fetcher::class, function (IContainer $c): Fetcher {
             $fetcher = new Fetcher();
 
             // register fetchers in order, the most generic fetcher should be
diff --git a/lib/Fetcher/FeedFetcher.php b/lib/Fetcher/FeedFetcher.php
index ae338ca09..abfd0095b 100644
--- a/lib/Fetcher/FeedFetcher.php
+++ b/lib/Fetcher/FeedFetcher.php
@@ -18,7 +18,6 @@ use Favicon\Favicon;
 use FeedIo\Feed\ItemInterface;
 use FeedIo\FeedInterface;
 use FeedIo\FeedIo;
-use OCP\Http\Client\IClientService;
 
 use OCP\IL10N;
 
@@ -33,20 +32,13 @@ class FeedFetcher implements IFeedFetcher
     private $reader;
     private $l10n;
     private $time;
-    private $clientService;
-
-    public function __construct(
-        FeedIo $fetcher,
-        Favicon $favicon,
-        IL10N $l10n,
-        Time $time,
-        IClientService $clientService
-    ) {
-        $this->faviconFactory = $favicon;
+
+    public function __construct(FeedIo $fetcher, Favicon $favicon, IL10N $l10n, Time $time)
+    {
         $this->reader         = $fetcher;
-        $this->time           = $time;
+        $this->faviconFactory = $favicon;
         $this->l10n           = $l10n;
-        $this->clientService  = $clientService;
+        $this->time           = $time;
     }
 
 
@@ -57,7 +49,7 @@ class FeedFetcher implements IFeedFetcher
      *
      * @return bool
      */
-    public function canHandle($url)
+    public function canHandle($url): bool
     {
         return true;
     }
@@ -79,7 +71,7 @@ class FeedFetcher implements IFeedFetcher
      * @return array an array containing the new feed and its items, first
      * element being the Feed and second element being an array of Items
      */
-    public function fetch($url, $getFavicon = true, $lastModified = null, $user = null, $password = null)
+    public function fetch(string $url, $getFavicon = true, $lastModified = null, $user = null, $password = null): array
     {
         if ($user !== null && trim($user) !== '') {
             $url = explode('://', $url);
@@ -88,7 +80,7 @@ class FeedFetcher implements IFeedFetcher
         $resource = $this->reader->readSince($url, new DateTime($lastModified));
 
         if (!$resource->getResponse()->isModified()) {
-            return [null, null];
+            throw new FetcherException('Feed was not modified since last fetch');
         }
 
         $location     = $resource->getUrl();
@@ -115,7 +107,7 @@ class FeedFetcher implements IFeedFetcher
      *
      * @return string
      */
-    private function decodeTwice($string)
+    private function decodeTwice($string): string
     {
         return html_entity_decode(
             html_entity_decode(
@@ -135,7 +127,7 @@ class FeedFetcher implements IFeedFetcher
      *
      * @return bool
      */
-    protected function determineRtl($parsedFeed)
+    protected function determineRtl(FeedInterface $parsedFeed): bool
     {
         $language = $parsedFeed->getLanguage();
 
@@ -166,7 +158,7 @@ class FeedFetcher implements IFeedFetcher
      *
      * @return Item
      */
-    protected function buildItem($parsedItem, $parsedFeed)
+    protected function buildItem(ItemInterface $parsedItem, FeedInterface $parsedFeed): Item
     {
         $item = new Item();
         $item->setUnread(true);
@@ -223,12 +215,12 @@ class FeedFetcher implements IFeedFetcher
      *
      * @param FeedInterface $feed       Feed to build from
      * @param string        $url        URL to use
-     * @param bool          $getFavicon To get the favicon
+     * @param boolean       $getFavicon To get the favicon
      * @param string        $location   String base URL
      *
      * @return Feed
      */
-    protected function buildFeed($feed, $url, $getFavicon, $location)
+    protected function buildFeed(FeedInterface $feed, string $url, boolean $getFavicon, string $location): Feed
     {
         $newFeed = new Feed();
 
diff --git a/lib/Fetcher/IFeedFetcher.php b/lib/Fetcher/IFeedFetcher.php
index d5994a076..70f153d2e 100644
--- a/lib/Fetcher/IFeedFetcher.php
+++ b/lib/Fetcher/IFeedFetcher.php
@@ -30,7 +30,7 @@ interface IFeedFetcher
      * @return array an array containing the new feed and its items, first
      * element being the Feed and second element being an array of Items
      */
-    public function fetch($url, $getFavicon = true, $lastModified = null, $user = null, $password = null);
+    public function fetch($url, $getFavicon = true, $lastModified = null, $user = null, $password = null): array;
 
     /**
      * Can a fetcher handle a feed.
@@ -40,5 +40,5 @@ interface IFeedFetcher
      * @return boolean if the fetcher can handle the url. This fetcher will be
      * used exclusively to fetch the feed and the items of the page
      */
-    public function canHandle($url);
+    public function canHandle($url): bool;
 }
diff --git a/lib/Fetcher/YoutubeFetcher.php b/lib/Fetcher/YoutubeFetcher.php
index 9ccce4463..fd4e7d2fb 100644
--- a/lib/Fetcher/YoutubeFetcher.php
+++ b/lib/Fetcher/YoutubeFetcher.php
@@ -39,7 +39,7 @@ class YoutubeFetcher implements IFeedFetcher
     /**
      * This fetcher handles all the remaining urls therefore always returns true
      */
-    public function canHandle($url)
+    public function canHandle($url): bool
     {
         return $this->buildUrl($url) !== $url;
     }
@@ -59,8 +59,7 @@ class YoutubeFetcher implements IFeedFetcher
      * @return array an array containing the new feed and its items, first
      * element being the Feed and second element being an array of Items
      */
-    public function fetch($url, $getFavicon = true, $lastModified = null, $user = null, $password = null
-    )
+    public function fetch($url, $getFavicon = true, $lastModified = null, $user = null, $password = null): array
     {
         $transformedUrl = $this->buildUrl($url);
 
diff --git a/lib/Service/FeedService.php b/lib/Service/FeedService.php
index fade77df8..f46171868 100644
--- a/lib/Service/FeedService.php
+++ b/lib/Service/FeedService.php
@@ -105,37 +105,24 @@ class FeedService extends Service
      * @throws ServiceNotFoundException if the url points to an invalid feed
      * @return Feed the newly created feed
      */
-    public function create(
-        $feedUrl,
-        $folderId,
-        $userId,
-        $title = null,
-        $user = null,
-        $password = null
-    ) {
+    public function create($feedUrl, $folderId, $userId, $title = null, $user = null, $password = null)
+    {
         // first try if the feed exists already
         try {
             /**
              * @var Feed   $feed
              * @var Item[] $items
              */
-            list($feed, $items) = $this->feedFetcher->fetch(
-                $feedUrl,
-                true,
-                null,
-                $user,
-                $password
-            );
-
+            list($feed, $items) = $this->feedFetcher->fetch($feedUrl, true, null, $user, $password);
             // try again if feed exists depending on the reported link
             try {
-                $this->feedMapper->findByUrlHash($feed->getUrlHash(), $userId);
+                $hash = $feed->getUrlHash();
+                $this->feedMapper->findByUrlHash($hash, $userId);
                 throw new ServiceConflictException(
                     $this->l10n->t('Can not add feed: Exists already')
                 );
-
-                // If no matching feed was found everything was ok
             } catch (DoesNotExistException $ex) {
+                // If no matching feed was found everything was ok
             }
 
             // insert feed
@@ -146,7 +133,7 @@ class FeedService extends Service
             $feed->setUserId($userId);
             $feed->setArticlesPerUpdate($itemCount);
 
-            if ($title !== null && $title !== '') {
+            if (!empty($title)) {
                 $feed->setTitle($title);
             }
 
@@ -155,8 +142,7 @@ class FeedService extends Service
             // insert items in reverse order because the first one is usually
             // the newest item
             $unreadCount = 0;
-            for ($i = $itemCount - 1; $i >= 0; $i--) {
-                $item = $items[$i];
+            foreach (array_reverse($items) as $item) {
                 $item->setFeedId($feed->getId());
 
                 // check if item exists (guidhash is the same)