From bd6f23692c368c90abe56599f5ae8ecda17011fc Mon Sep 17 00:00:00 2001 From: "J. King" Date: Tue, 3 Oct 2017 10:43:09 -0400 Subject: [PATCH] Implement TTRSS feed subscription; fixes #92 --- lib/Database.php | 2 +- lib/REST/TinyTinyRSS/API.php | 85 ++++++++++++++++++++- tests/REST/TinyTinyRSS/TestTinyTinyAPI.php | 86 +++++++++++++++++++++- 3 files changed, 169 insertions(+), 4 deletions(-) diff --git a/lib/Database.php b/lib/Database.php index 9ed18af..a7fea0e 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -496,7 +496,7 @@ class Database { // create a complex query $q = new Query( "SELECT - arsse_subscriptions.id, + arsse_subscriptions.id as id, url,favicon,source,folder,pinned,err_count,err_msg,order_type,added, topmost.top as top_folder, coalesce(arsse_subscriptions.title, arsse_feeds.title) as title, diff --git a/lib/REST/TinyTinyRSS/API.php b/lib/REST/TinyTinyRSS/API.php index d9ad532..43c8f2c 100644 --- a/lib/REST/TinyTinyRSS/API.php +++ b/lib/REST/TinyTinyRSS/API.php @@ -2,6 +2,7 @@ declare(strict_types=1); namespace JKingWeb\Arsse\REST\TinyTinyRSS; +use JKingWeb\Arsse\Feed; use JKingWeb\Arsse\Arsse; use JKingWeb\Arsse\User; use JKingWeb\Arsse\Service; @@ -164,7 +165,7 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { return (int) $folder['id']; } } - return false; + return false; // @codeCoverageIgnore case 10235: // parent folder does not exist; this returns false as an ID return false; default: // other errors related to input @@ -226,6 +227,88 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { return null; } + protected function feedError(FeedException $e): array { + // N.B.: we don't return code 4 (multiple feeds discovered); we simply pick the first feed discovered + switch ($e->getCode()) { + case 10502: // invalid URL + return ['code' => 2, 'message' => $e->getMessage()]; + case 10521: // no feeds discovered + return ['code' => 3, 'message' => $e->getMessage()]; + case 10511: + case 10512: + case 10522: // malformed data + return ['code' => 6, 'message' => $e->getMessage()]; + default: // unable to download + return ['code' => 5, 'message' => $e->getMessage()]; + } + } + + public function opSubscribeToFeed(array $data): array { + if (!isset($data['feed_url']) || !(ValueInfo::str($data['feed_url']) & ValueInfo::VALID)) { + // if the feed URL is invalid, throw an error + throw new Exception("INCORRECT_USAGE"); + } + // normalize input data + if ( + (isset($data['category_id']) && !ValueInfo::id($data['category_id'], true)) || + (isset($data['login']) && !(ValueInfo::str($data['login']) & ValueInfo::VALID)) || + (isset($data['password']) && !(ValueInfo::str($data['password']) & ValueInfo::VALID)) + ) { + // if the category is not a valid ID or the feed username or password are not convertible to strings, also throw an error + throw new Exception("INCORRECT_USAGE"); + } + $url = (string) $data['feed_url']; + $folder = isset($data['category_id']) ? (int) $data['category_id'] : null; + $fetchUser = isset($data['login']) ? (string) $data['login'] : ""; + $fetchPassword = isset($data['password']) ? (string) $data['password'] : ""; + // check to make sure the requested folder exists before doing anything else, if one is specified + if ($folder) { + try { + Arsse::$db->folderPropertiesGet(Arsse::$user->id, $folder); + } catch (ExceptionInput $e) { + // folder does not exist: TT-RSS is a bit weird in this case and returns a feed ID of 0. It checks the feed first, but we do not + return ['code' => 1, 'feed_id' => 0]; + } + } + try { + $id = Arsse::$db->subscriptionAdd(Arsse::$user->id, $url, $fetchUser, $fetchPassword); + } catch (ExceptionInput $e) { + // subscription already exists; retrieve the existing ID and return that with the correct code + for ($triedDiscovery = 0; $triedDiscovery <= 1; $triedDiscovery++) { + $subs = Arsse::$db->subscriptionList(Arsse::$user->id); + $id = false; + foreach ($subs as $sub) { + if ($sub['url']===$url) { + $id = (int) $sub['id']; + break; + } + } + if ($id) { + break; + } elseif (!$triedDiscovery) { + // if we didn't find the ID we perform feed discovery for the next iteration; this is pretty messy: discovery ends up being done twice because it was already done in $db->subscriptionAdd() + try { + $url = Feed::discover($url, $fetchUser, $fetchPassword); + } catch(FeedException $e) { + // feed errors (handled above) + return $this->feedError($e); + } + } + } + return ['code' => 0, 'feed_id' => $id]; + } catch (FeedException $e) { + // feed errors (handled above) + return $this->feedError($e); + } + // if all went well, move the new subscription to the requested folder (if one was requested) + try { + Arsse::$db->subscriptionPropertiesSet(Arsse::$user->id, $id, ['folder' => $folder]); + } catch (ExceptionInput $e) { + // ignore errors + } + return ['code' => 1, 'feed_id' => $id]; + } + public function opUnsubscribeFeed(array $data): array { if (!isset($data['feed_id']) || !ValueInfo::id($data['feed_id'])) { // if the feed is invalid, throw an error diff --git a/tests/REST/TinyTinyRSS/TestTinyTinyAPI.php b/tests/REST/TinyTinyRSS/TestTinyTinyAPI.php index 7e35c9e..3b4b152 100644 --- a/tests/REST/TinyTinyRSS/TestTinyTinyAPI.php +++ b/tests/REST/TinyTinyRSS/TestTinyTinyAPI.php @@ -11,7 +11,8 @@ use JKingWeb\Arsse\Db\ExceptionInput; use JKingWeb\Arsse\Db\Transaction; use Phake; -/** @covers \JKingWeb\Arsse\REST\TinyTinyRSS\API */ +/** @covers \JKingWeb\Arsse\REST\TinyTinyRSS\API + * @covers \JKingWeb\Arsse\REST\TinyTinyRSS\Exception */ class TestTinyTinyAPI extends Test\AbstractTest { protected $h; protected $feeds = [ // expected sample output of a feed list from the database, and the resultant expected transformation by the REST handler @@ -253,6 +254,7 @@ class TestTinyTinyAPI extends Test\AbstractTest { $in = [ ['op' => "addCategory", 'sid' => "PriestsOfSyrinx", 'caption' => "Software"], ['op' => "addCategory", 'sid' => "PriestsOfSyrinx", 'caption' => "Hardware", 'parent_id' => 1], + ['op' => "addCategory", 'sid' => "PriestsOfSyrinx", 'caption' => "Hardware", 'parent_id' => 2112], ['op' => "addCategory", 'sid' => "PriestsOfSyrinx"], ['op' => "addCategory", 'sid' => "PriestsOfSyrinx", 'caption' => ""], ['op' => "addCategory", 'sid' => "PriestsOfSyrinx", 'caption' => " "], @@ -260,6 +262,7 @@ class TestTinyTinyAPI extends Test\AbstractTest { $db = [ ['name' => "Software", 'parent' => null], ['name' => "Hardware", 'parent' => 1], + ['name' => "Hardware", 'parent' => 2112], ]; $out = [ ['id' => 2, 'name' => "Software", 'parent' => null], @@ -272,6 +275,7 @@ class TestTinyTinyAPI extends Test\AbstractTest { Phake::when(Arsse::$db)->folderList(Arsse::$user->id, null, false)->thenReturn(new Result([$out[0], $out[2]])); Phake::when(Arsse::$db)->folderList(Arsse::$user->id, 1, false)->thenReturn(new Result([$out[1]])); // set up mocks that produce errors + Phake::when(Arsse::$db)->folderAdd(Arsse::$user->id, $db[2])->thenThrow(new ExceptionInput("idMissing")); // parent folder does not exist Phake::when(Arsse::$db)->folderAdd(Arsse::$user->id, [])->thenThrow(new ExceptionInput("missing")); Phake::when(Arsse::$db)->folderAdd(Arsse::$user->id, ['name' => "", 'parent' => null])->thenThrow(new ExceptionInput("missing")); Phake::when(Arsse::$db)->folderAdd(Arsse::$user->id, ['name' => " ", 'parent' => null])->thenThrow(new ExceptionInput("whitespace")); @@ -287,11 +291,14 @@ class TestTinyTinyAPI extends Test\AbstractTest { $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "", json_encode($in[1])))); Phake::verify(Arsse::$db)->folderList(Arsse::$user->id, null, false); Phake::verify(Arsse::$db)->folderList(Arsse::$user->id, 1, false); + // add a folder to a missing parent (silently fails) + $exp = $this->respGood(false); + $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "", json_encode($in[2])))); // add some invalid folders $exp = $this->respErr("INCORRECT_USAGE"); - $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "", json_encode($in[2])))); $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "", json_encode($in[3])))); $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "", json_encode($in[4])))); + $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "", json_encode($in[5])))); } public function testRemoveACategory() { @@ -399,6 +406,81 @@ class TestTinyTinyAPI extends Test\AbstractTest { Phake::verify(Arsse::$db, Phake::times(3))->folderPropertiesSet(Arsse::$user->id, $this->anything(), $this->anything()); } + public function testAddASubscription() { + $in = [ + ['op' => "subscribeToFeed", 'sid' => "PriestsOfSyrinx", 'feed_url' => "http://example.com/0"], + ['op' => "subscribeToFeed", 'sid' => "PriestsOfSyrinx", 'feed_url' => "http://example.com/1", 'category_id' => 42], + ['op' => "subscribeToFeed", 'sid' => "PriestsOfSyrinx", 'feed_url' => "http://example.com/2", 'category_id' => 2112], + ['op' => "subscribeToFeed", 'sid' => "PriestsOfSyrinx", 'feed_url' => "http://example.com/3"], + ['op' => "subscribeToFeed", 'sid' => "PriestsOfSyrinx", 'feed_url' => "http://localhost:8000/Feed/Discovery/Valid"], + ['op' => "subscribeToFeed", 'sid' => "PriestsOfSyrinx", 'feed_url' => "http://localhost:8000/Feed/Discovery/Invalid"], + ['op' => "subscribeToFeed", 'sid' => "PriestsOfSyrinx", 'feed_url' => "http://example.com/6"], + ['op' => "subscribeToFeed", 'sid' => "PriestsOfSyrinx", 'feed_url' => "http://example.com/7"], + ['op' => "subscribeToFeed", 'sid' => "PriestsOfSyrinx", 'feed_url' => "http://example.com/8", 'category_id' => 47], + ['op' => "subscribeToFeed", 'sid' => "PriestsOfSyrinx", 'feed_url' => "http://example.com/9", 'category_id' => 1], + // these don't even query the database as the input is syntactically invalid + ['op' => "subscribeToFeed", 'sid' => "PriestsOfSyrinx"], + ['op' => "subscribeToFeed", 'sid' => "PriestsOfSyrinx", 'feed_url' => "http://example.com/", 'login' => []], + ['op' => "subscribeToFeed", 'sid' => "PriestsOfSyrinx", 'feed_url' => "http://example.com/", 'login' => "", 'password' => []], + ['op' => "subscribeToFeed", 'sid' => "PriestsOfSyrinx", 'feed_url' => "http://example.com/", 'category_id' => -1], + ]; + $db = [ + [Arsse::$user->id, "http://example.com/0", "", ""], + [Arsse::$user->id, "http://example.com/1", "", ""], + [Arsse::$user->id, "http://example.com/2", "", ""], + [Arsse::$user->id, "http://example.com/3", "", ""], + [Arsse::$user->id, "http://localhost:8000/Feed/Discovery/Valid", "", ""], + [Arsse::$user->id, "http://localhost:8000/Feed/Discovery/Invalid", "", ""], + [Arsse::$user->id, "http://example.com/6", "", ""], + [Arsse::$user->id, "http://example.com/7", "", ""], + [Arsse::$user->id, "http://example.com/8", "", ""], + [Arsse::$user->id, "http://example.com/9", "", ""], + ]; + $out = [ + ['code' => 1, 'feed_id' => 2], + ['code' => 5, 'message' => (new \JKingWeb\Arsse\Feed\Exception("http://example.com/1", new \PicoFeed\Client\UnauthorizedException()))->getMessage()], + ['code' => 1, 'feed_id' => 0], + ['code' => 0, 'feed_id' => 3], + ['code' => 0, 'feed_id' => 1], + ['code' => 3, 'message' => (new \JKingWeb\Arsse\Feed\Exception("http://localhost:8000/Feed/Discovery/Invalid", new \PicoFeed\Reader\SubscriptionNotFoundException()))->getMessage()], + ['code' => 2, 'message' => (new \JKingWeb\Arsse\Feed\Exception("http://example.com/6", new \PicoFeed\Client\InvalidUrlException()))->getMessage()], + ['code' => 6, 'message' => (new \JKingWeb\Arsse\Feed\Exception("http://example.com/7", new \PicoFeed\Parser\MalformedXmlException()))->getMessage()], + ['code' => 1, 'feed_id' => 4], + ['code' => 0, 'feed_id' => 4], + ]; + $list = [ + ['id' => 1, 'url' => "http://localhost:8000/Feed/Discovery/Feed"], + ['id' => 2, 'url' => "http://example.com/0"], + ['id' => 3, 'url' => "http://example.com/3"], + ['id' => 4, 'url' => "http://example.com/9"], + ]; + Phake::when(Arsse::$db)->subscriptionAdd(...$db[0])->thenReturn(2); + Phake::when(Arsse::$db)->subscriptionAdd(...$db[1])->thenThrow(new \JKingWeb\Arsse\Feed\Exception("http://example.com/1", new \PicoFeed\Client\UnauthorizedException())); + Phake::when(Arsse::$db)->subscriptionAdd(...$db[2])->thenReturn(2); + Phake::when(Arsse::$db)->subscriptionAdd(...$db[3])->thenThrow(new ExceptionInput("constraintViolation")); + Phake::when(Arsse::$db)->subscriptionAdd(...$db[4])->thenThrow(new ExceptionInput("constraintViolation")); + Phake::when(Arsse::$db)->subscriptionAdd(...$db[5])->thenThrow(new ExceptionInput("constraintViolation")); + Phake::when(Arsse::$db)->subscriptionAdd(...$db[6])->thenThrow(new \JKingWeb\Arsse\Feed\Exception("http://example.com/6", new \PicoFeed\Client\InvalidUrlException())); + Phake::when(Arsse::$db)->subscriptionAdd(...$db[7])->thenThrow(new \JKingWeb\Arsse\Feed\Exception("http://example.com/7", new \PicoFeed\Parser\MalformedXmlException())); + Phake::when(Arsse::$db)->subscriptionAdd(...$db[8])->thenReturn(4); + Phake::when(Arsse::$db)->subscriptionAdd(...$db[9])->thenThrow(new ExceptionInput("constraintViolation")); + Phake::when(Arsse::$db)->folderPropertiesGet(Arsse::$user->id, 42)->thenReturn(['id' => 42]); + Phake::when(Arsse::$db)->folderPropertiesGet(Arsse::$user->id, 47)->thenReturn(['id' => 47]); + Phake::when(Arsse::$db)->folderPropertiesGet(Arsse::$user->id, 2112)->thenThrow(new ExceptionInput("subjectMissing")); + Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, $this->anything(), $this->anything())->thenReturn(true); + Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 4, $this->anything())->thenThrow(new ExceptionInput("idMissing")); + Phake::when(Arsse::$db)->subscriptionList(Arsse::$user->id)->thenReturn(new Result($list)); + for ($a = 0; $a < (sizeof($in) - 4); $a++) { + $exp = $this->respGood($out[$a]); + $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "", json_encode($in[$a]))), "Failed test $a"); + } + $exp = $this->respErr("INCORRECT_USAGE"); + for ($a = (sizeof($in) - 4); $a < sizeof($in); $a++) { + $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "", json_encode($in[$a]))), "Failed test $a"); + } + Phake::verify(Arsse::$db, Phake::times(0))->subscriptionPropertiesSet(Arsse::$user->id, 4, ['folder' => 1]); + } + public function testRemoveASubscription() { $in = [ ['op' => "unsubscribeFeed", 'sid' => "PriestsOfSyrinx", 'feed_id' => 42],