From d61a57bd2dbbc6ecbddfaa22c347248210703f02 Mon Sep 17 00:00:00 2001 From: Sean Molenaar Date: Thu, 31 Jan 2019 10:23:56 +0100 Subject: Make feed failing more verbose --- lib/AppInfo/Application.php | 12 ++++++------ lib/Fetcher/FeedFetcher.php | 34 +++++++++++++--------------------- lib/Fetcher/IFeedFetcher.php | 4 ++-- lib/Fetcher/YoutubeFetcher.php | 5 ++--- lib/Service/FeedService.php | 30 ++++++++---------------------- 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) -- cgit v1.2.3