From 97b0134e56375e3ccb2b8c03ece0d65fb9a825d4 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Mon, 2 Oct 2017 15:42:15 -0400 Subject: [PATCH] Merge master --- arsse.php | 7 +- bootstrap.php | 10 --- build.xml | 1 - lib/Arsse.php | 2 + lib/CLI.php | 2 +- lib/Database.php | 16 +++-- lib/Feed.php | 75 ++++++++++++++-------- lib/REST/NextCloudNews/V1_2.php | 4 +- lib/REST/TinyTinyRSS/API.php | 2 +- tests/Feed/TestFeed.php | 5 +- tests/REST/NextCloudNews/TestNCNV1_2.php | 4 +- tests/REST/TinyTinyRSS/TestTinyTinyAPI.php | 2 +- tests/bootstrap.php | 8 +++ tests/lib/Database/SeriesSubscription.php | 26 ++++++-- tests/phpunit.xml | 2 +- tests/server.php | 2 +- 16 files changed, 109 insertions(+), 59 deletions(-) delete mode 100644 bootstrap.php create mode 100644 tests/bootstrap.php diff --git a/arsse.php b/arsse.php index 8634bf5..c1fd508 100644 --- a/arsse.php +++ b/arsse.php @@ -1,7 +1,12 @@ - diff --git a/lib/Arsse.php b/lib/Arsse.php index 2475ed6..a453bd7 100644 --- a/lib/Arsse.php +++ b/lib/Arsse.php @@ -3,6 +3,8 @@ declare(strict_types=1); namespace JKingWeb\Arsse; class Arsse { + const VERSION = "0.1.1"; + /** @var Lang */ public static $lang; /** @var Conf */ diff --git a/lib/CLI.php b/lib/CLI.php index 971197f..36aa8db 100644 --- a/lib/CLI.php +++ b/lib/CLI.php @@ -27,7 +27,7 @@ USAGE_TEXT; $this->args = \Docopt::handle($this->usage(), [ 'argv' => $argv, 'help' => true, - 'version' => VERSION, + 'version' => Arsse::VERSION, ]); } diff --git a/lib/Database.php b/lib/Database.php index 5a90b19..9ed18af 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -464,13 +464,19 @@ class Database { throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); } // check to see if the feed exists - $feedID = $this->db->prepare("SELECT id from arsse_feeds where url is ? and username is ? and password is ?", "str", "str", "str")->run($url, $fetchUser, $fetchPassword)->getValue(); + $check = $this->db->prepare("SELECT id from arsse_feeds where url is ? and username is ? and password is ?", "str", "str", "str"); + $feedID = $check->run($url, $fetchUser, $fetchPassword)->getValue(); + if ($discover && is_null($feedID)) { + // if the feed doesn't exist, first perform discovery if requested and check for the existence of that URL + $url = Feed::discover($url, $fetchUser, $fetchPassword); + $feedID = $check->run($url, $fetchUser, $fetchPassword)->getValue(); + } if (is_null($feedID)) { - // if the feed doesn't exist add it to the database; we do this unconditionally so as to lock SQLite databases for as little time as possible + // if the feed still doesn't exist in the database, add it to the database; we do this unconditionally so as to lock SQLite databases for as little time as possible $feedID = $this->db->prepare('INSERT INTO arsse_feeds(url,username,password) values(?,?,?)', 'str', 'str', 'str')->run($url, $fetchUser, $fetchPassword)->lastId(); try { // perform an initial update on the newly added feed - $this->feedUpdate($feedID, true, $discover); + $this->feedUpdate($feedID, true); } catch (\Throwable $e) { // if the update fails, delete the feed we just added $this->db->prepare('DELETE from arsse_feeds where id is ?', 'int')->run($feedID); @@ -601,7 +607,7 @@ class Database { return array_column($feeds, 'id'); } - public function feedUpdate($feedID, bool $throwError = false, bool $discover = false): bool { + public function feedUpdate($feedID, bool $throwError = false): bool { $tr = $this->db->begin(); // check to make sure the feed exists if (!ValueInfo::id($feedID)) { @@ -617,7 +623,7 @@ class Database { // here. When an exception is thrown it should update the database with the // error instead of failing; if other exceptions are thrown, we should simply roll back try { - $feed = new Feed((int) $feedID, $f['url'], (string) Date::transform($f['modified'], "http", "sql"), $f['etag'], $f['username'], $f['password'], $scrape, $discover); + $feed = new Feed((int) $feedID, $f['url'], (string) Date::transform($f['modified'], "http", "sql"), $f['etag'], $f['username'], $f['password'], $scrape); if (!$feed->modified) { // if the feed hasn't changed, just compute the next fetch time and record it $this->db->prepare("UPDATE arsse_feeds SET updated = CURRENT_TIMESTAMP, next_fetch = ? WHERE id is ?", 'datetime', 'int')->run($feed->nextFetch, $feedID); diff --git a/lib/Feed.php b/lib/Feed.php index ee6114c..a390659 100644 --- a/lib/Feed.php +++ b/lib/Feed.php @@ -5,6 +5,7 @@ namespace JKingWeb\Arsse; use JKingWeb\Arsse\Misc\Date; use PicoFeed\PicoFeedException; use PicoFeed\Config\Config; +use PicoFeed\Client\Client; use PicoFeed\Reader\Reader; use PicoFeed\Reader\Favicon; use PicoFeed\Scraper\Scraper; @@ -12,8 +13,6 @@ use PicoFeed\Scraper\Scraper; class Feed { public $data = null; public $favicon; - public $parser; - public $reader; public $resource; public $modified = false; public $lastModified; @@ -21,22 +20,30 @@ class Feed { public $newItems = []; public $changedItems = []; - public function __construct(int $feedID = null, string $url, string $lastModified = '', string $etag = '', string $username = '', string $password = '', bool $scrape = false, bool $discover = false) { - // set the configuration - $userAgent = Arsse::$conf->fetchUserAgentString ?? sprintf('Arsse/%s (%s %s; %s; https://code.jkingweb.ca/jking/arsse) PicoFeed (https://github.com/fguillot/picoFeed)', - VERSION, // Arsse version - php_uname('s'), // OS - php_uname('r'), // OS version - php_uname('m') // platform architecture - ); - $this->config = new Config; - $this->config->setMaxBodySize(Arsse::$conf->fetchSizeLimit); - $this->config->setClientTimeout(Arsse::$conf->fetchTimeout); - $this->config->setGrabberTimeout(Arsse::$conf->fetchTimeout); - $this->config->setClientUserAgent($userAgent); - $this->config->setGrabberUserAgent($userAgent); + public static function discover(string $url, string $username = '', string $password = ''): string { + // fetch the candidate feed + $f = self::download($url, "", "", $username, $password); + if ($f->reader->detectFormat($f->getContent())) { + // if the prospective URL is a feed, use it + $out = $url; + } else { + $links = $f->reader->find($f->getUrl(), $f->getContent()); + if (!$links) { + // work around a PicoFeed memory leak FIXME: remove this hack (or not) once PicoFeed stops leaking memory + libxml_use_internal_errors(false); + throw new Feed\Exception($url, new \PicoFeed\Reader\SubscriptionNotFoundException('Unable to find a subscription')); + } else { + $out = $links[0]; + } + } + // work around a PicoFeed memory leak FIXME: remove this hack (or not) once PicoFeed stops leaking memory + libxml_use_internal_errors(false); + return $out; + } + + public function __construct(int $feedID = null, string $url, string $lastModified = '', string $etag = '', string $username = '', string $password = '', bool $scrape = false) { // fetch the feed - $this->download($url, $lastModified, $etag, $username, $password, $discover); + $this->resource = self::download($url, $lastModified, $etag, $username, $password); // format the HTTP Last-Modified date returned $lastMod = $this->resource->getLastModified(); if (strlen($lastMod)) { @@ -65,26 +72,40 @@ class Feed { $this->nextFetch = $this->computeNextFetch(); } - protected function download(string $url, string $lastModified, string $etag, string $username, string $password, bool $discover): bool { - $action = $discover ? "discover" : "download"; + protected static function configure(): Config { + $userAgent = Arsse::$conf->fetchUserAgentString ?? sprintf('Arsse/%s (%s %s; %s; https://thearsse.com/) PicoFeed (https://github.com/miniflux/picoFeed)', + Arsse::VERSION, // Arsse version + php_uname('s'), // OS + php_uname('r'), // OS version + php_uname('m') // platform architecture + ); + $config = new Config; + $config->setMaxBodySize(Arsse::$conf->fetchSizeLimit); + $config->setClientTimeout(Arsse::$conf->fetchTimeout); + $config->setGrabberTimeout(Arsse::$conf->fetchTimeout); + $config->setClientUserAgent($userAgent); + $config->setGrabberUserAgent($userAgent); + return $config; + } + + protected static function download(string $url, string $lastModified, string $etag, string $username, string $password): Client { try { - $this->reader = new Reader($this->config); - $this->resource = $this->reader->$action($url, $lastModified, $etag, $username, $password); + $reader = new Reader(self::configure()); + $client = $reader->download($url, $lastModified, $etag, $username, $password); + $client->reader = $reader; + return $client; } catch (PicoFeedException $e) { throw new Feed\Exception($url, $e); } - return true; } protected function parse(): bool { try { - $this->parser = $this->reader->getParser( + $feed = $this->resource->reader->getParser( $this->resource->getUrl(), $this->resource->getContent(), $this->resource->getEncoding() - ); - $feed = $this->parser->execute(); - + )->execute(); // Grab the favicon for the feed; returns an empty string if it cannot find one. // Some feeds might use a different domain (eg: feedburner), so the site url is // used instead of the feed's url. @@ -388,7 +409,7 @@ class Feed { } protected function scrape(): bool { - $scraper = new Scraper($this->config); + $scraper = new Scraper(self::configure()); foreach (array_merge($this->newItems, $this->changedItems) as $item) { $scraper->setUrl($item->url); $scraper->execute(); diff --git a/lib/REST/NextCloudNews/V1_2.php b/lib/REST/NextCloudNews/V1_2.php index 8ece274..bab902e 100644 --- a/lib/REST/NextCloudNews/V1_2.php +++ b/lib/REST/NextCloudNews/V1_2.php @@ -691,14 +691,14 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { protected function serverVersion(array $url, array $data): Response { return new Response(200, [ 'version' => self::VERSION, - 'arsse_version' => \JKingWeb\Arsse\VERSION, + 'arsse_version' => Arsse::VERSION, ]); } protected function serverStatus(array $url, array $data): Response { return new Response(200, [ 'version' => self::VERSION, - 'arsse_version' => \JKingWeb\Arsse\VERSION, + 'arsse_version' => Arsse::VERSION, 'warnings' => [ 'improperlyConfiguredCron' => !Service::hasCheckedIn(), ] diff --git a/lib/REST/TinyTinyRSS/API.php b/lib/REST/TinyTinyRSS/API.php index 0db6601..d9ad532 100644 --- a/lib/REST/TinyTinyRSS/API.php +++ b/lib/REST/TinyTinyRSS/API.php @@ -118,7 +118,7 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { public function opGetVersion(array $data): array { return [ 'version' => self::VERSION, - 'arsse_version' => \JKingWeb\Arsse\VERSION, + 'arsse_version' => Arsse::VERSION, ]; } diff --git a/tests/Feed/TestFeed.php b/tests/Feed/TestFeed.php index 8957ba8..0ef1eb4 100644 --- a/tests/Feed/TestFeed.php +++ b/tests/Feed/TestFeed.php @@ -134,12 +134,13 @@ class TestFeed extends Test\AbstractTest { } public function testDiscoverAFeedSuccessfully() { - $this->assertInstanceOf(Feed::class, new Feed(null, $this->base."Discovery/Valid", "", "", "", "", false, true)); + $this->assertSame($this->base."Discovery/Feed", Feed::discover($this->base."Discovery/Valid")); + $this->assertSame($this->base."Discovery/Feed", Feed::discover($this->base."Discovery/Feed")); } public function testDiscoverAFeedUnsuccessfully() { $this->assertException("subscriptionNotFound", "Feed"); - new Feed(null, $this->base."Discovery/Invalid", "", "", "", "", false, true); + Feed::discover($this->base."Discovery/Invalid"); } public function testParseEntityExpansionAttack() { diff --git a/tests/REST/NextCloudNews/TestNCNV1_2.php b/tests/REST/NextCloudNews/TestNCNV1_2.php index 713267b..0d8e08e 100644 --- a/tests/REST/NextCloudNews/TestNCNV1_2.php +++ b/tests/REST/NextCloudNews/TestNCNV1_2.php @@ -458,7 +458,7 @@ class TestNCNV1_2 extends Test\AbstractTest { public function testRetrieveServerVersion() { $exp = new Response(200, [ - 'arsse_version' => \JKingWeb\Arsse\VERSION, + 'arsse_version' => Arsse::VERSION, 'version' => REST\NextCloudNews\V1_2::VERSION, ]); $this->assertEquals($exp, $this->h->dispatch(new Request("GET", "/version"))); @@ -842,7 +842,7 @@ class TestNCNV1_2 extends Test\AbstractTest { Phake::when(Arsse::$db)->metaGet("service_last_checkin")->thenReturn(Date::transform($valid, "sql"))->thenReturn(Date::transform($invalid, "sql")); $arr1 = $arr2 = [ 'version' => REST\NextCloudNews\V1_2::VERSION, - 'arsse_version' => VERSION, + 'arsse_version' => Arsse::VERSION, 'warnings' => [ 'improperlyConfiguredCron' => false, ] diff --git a/tests/REST/TinyTinyRSS/TestTinyTinyAPI.php b/tests/REST/TinyTinyRSS/TestTinyTinyAPI.php index 37aa594..7e35c9e 100644 --- a/tests/REST/TinyTinyRSS/TestTinyTinyAPI.php +++ b/tests/REST/TinyTinyRSS/TestTinyTinyAPI.php @@ -235,7 +235,7 @@ class TestTinyTinyAPI extends Test\AbstractTest { ]; $exp = $this->respGood([ 'version' => \JKingWeb\Arsse\REST\TinyTinyRSS\API::VERSION, - 'arsse_version' => \JKingWeb\Arsse\VERSION, + 'arsse_version' => Arsse::VERSION, ]); $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "", json_encode($data)))); } diff --git a/tests/bootstrap.php b/tests/bootstrap.php new file mode 100644 index 0000000..c74e551 --- /dev/null +++ b/tests/bootstrap.php @@ -0,0 +1,8 @@ +nextID("arsse_feeds"); $subID = $this->nextID("arsse_subscriptions"); Phake::when(Arsse::$db)->feedUpdate->thenReturn(true); - $this->assertSame($subID, Arsse::$db->subscriptionAdd($this->user, $url)); + $this->assertSame($subID, Arsse::$db->subscriptionAdd($this->user, $url, "", "", false)); Phake::verify(Arsse::$user)->authorize($this->user, "subscriptionAdd"); - Phake::verify(Arsse::$db)->feedUpdate($feedID, true, true); + Phake::verify(Arsse::$db)->feedUpdate($feedID, true); $state = $this->primeExpectations($this->data, [ 'arsse_feeds' => ['id','url','username','password'], 'arsse_subscriptions' => ['id','owner','feed'], @@ -145,15 +145,33 @@ trait SeriesSubscription { $this->compareExpectations($state); } + public function testAddASubscriptionToANewFeedViaDiscovery() { + $url = "http://localhost:8000/Feed/Discovery/Valid"; + $discovered = "http://localhost:8000/Feed/Discovery/Feed"; + $feedID = $this->nextID("arsse_feeds"); + $subID = $this->nextID("arsse_subscriptions"); + Phake::when(Arsse::$db)->feedUpdate->thenReturn(true); + $this->assertSame($subID, Arsse::$db->subscriptionAdd($this->user, $url, "", "", true)); + Phake::verify(Arsse::$user)->authorize($this->user, "subscriptionAdd"); + Phake::verify(Arsse::$db)->feedUpdate($feedID, true); + $state = $this->primeExpectations($this->data, [ + 'arsse_feeds' => ['id','url','username','password'], + 'arsse_subscriptions' => ['id','owner','feed'], + ]); + $state['arsse_feeds']['rows'][] = [$feedID,$discovered,"",""]; + $state['arsse_subscriptions']['rows'][] = [$subID,$this->user,$feedID]; + $this->compareExpectations($state); + } + public function testAddASubscriptionToAnInvalidFeed() { $url = "http://example.org/feed1"; $feedID = $this->nextID("arsse_feeds"); Phake::when(Arsse::$db)->feedUpdate->thenThrow(new FeedException($url, new \PicoFeed\Client\InvalidUrlException())); try { - Arsse::$db->subscriptionAdd($this->user, $url); + Arsse::$db->subscriptionAdd($this->user, $url, "", "", false); } catch (FeedException $e) { Phake::verify(Arsse::$user)->authorize($this->user, "subscriptionAdd"); - Phake::verify(Arsse::$db)->feedUpdate($feedID, true, true); + Phake::verify(Arsse::$db)->feedUpdate($feedID, true); $state = $this->primeExpectations($this->data, [ 'arsse_feeds' => ['id','url','username','password'], 'arsse_subscriptions' => ['id','owner','feed'], diff --git a/tests/phpunit.xml b/tests/phpunit.xml index d1e8ea0..6d486bc 100644 --- a/tests/phpunit.xml +++ b/tests/phpunit.xml @@ -1,7 +1,7 @@