From 6c8598d8973879879482eecfc7047790aad41a3b Mon Sep 17 00:00:00 2001 From: "J. King" Date: Thu, 16 Nov 2017 15:56:14 -0500 Subject: [PATCH] Implement contexts for non-recursive folders, and any/no label Adjusted TTRSS handler accordingly --- lib/Database.php | 11 +++++++- lib/Misc/Context.php | 10 +++++++ lib/REST/TinyTinyRSS/API.php | 33 ++++------------------ tests/REST/TinyTinyRSS/TestTinyTinyAPI.php | 26 ++++------------- tests/lib/Database/SeriesArticle.php | 14 ++++++--- 5 files changed, 40 insertions(+), 54 deletions(-) diff --git a/lib/Database.php b/lib/Database.php index 82a09e2..963c53b 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -845,6 +845,11 @@ class Database { $q->setCTE("folders(folder)", "SELECT ? union select id from arsse_folders join folders on parent is folder", "int", $context->folder); // add another CTE for the subscriptions within the folder $q->setCTE("subscribed_feeds(id,sub)", "SELECT feed,id from arsse_subscriptions join user on user is owner join folders on arsse_subscriptions.folder is folders.folder", [], [], "join subscribed_feeds on feed is subscribed_feeds.id"); + } elseif ($context->folderShallow()) { + // if a shallow folder is specified, make sure it exists + $this->folderValidateId($user, $context->folderShallow); + // if it does exist, add a CTE with only its subscriptions (and not those of its descendents) + $q->setCTE("subscribed_feeds(id,sub)", "SELECT feed,id from arsse_subscriptions join user on user is owner and coalesce(folder,0) is ?", "strict int", $context->folderShallow, "join subscribed_feeds on feed is subscribed_feeds.id"); } else { // otherwise add a CTE for all the user's subscriptions $q->setCTE("subscribed_feeds(id,sub)", "SELECT feed,id from arsse_subscriptions join user on user is owner", [], [], "join subscribed_feeds on feed is subscribed_feeds.id"); @@ -889,7 +894,11 @@ class Database { $q->setCTE("requested_articles(id,edition)", "SELECT 'empty','table' where 1 is 0"); } // filter based on label by ID or name - if ($context->label() || $context->labelName()) { + if ($context->labelled()) { + // any label (true) or no label (false) + $q->setWhere((!$context->labelled ? "not " : "")."exists(select article from arsse_label_members where assigned is 1 and article is arsse_articles.id and subscription in (select sub from subscribed_feeds))"); + } elseif ($context->label() || $context->labelName()) { + // specific label ID or name if ($context->label()) { $id = $this->labelValidateId($user, $context->label, false)['id']; } else { diff --git a/lib/Misc/Context.php b/lib/Misc/Context.php index a4fdd8c..82195df 100644 --- a/lib/Misc/Context.php +++ b/lib/Misc/Context.php @@ -10,6 +10,7 @@ class Context { public $limit = 0; public $offset = 0; public $folder; + public $folderShallow; public $subscription; public $oldestEdition; public $latestEdition; @@ -23,6 +24,7 @@ class Context { public $articles; public $label; public $labelName; + public $labelled = null; protected $props = []; @@ -64,6 +66,10 @@ class Context { return $this->act(__FUNCTION__, func_num_args(), $spec); } + public function folderShallow(int $spec = null) { + return $this->act(__FUNCTION__, func_num_args(), $spec); + } + public function subscription(int $spec = null) { return $this->act(__FUNCTION__, func_num_args(), $spec); } @@ -123,4 +129,8 @@ class Context { public function labelName(string $spec = null) { return $this->act(__FUNCTION__, func_num_args(), $spec); } + + public function labelled(bool $spec = null) { + return $this->act(__FUNCTION__, func_num_args(), $spec); + } } diff --git a/lib/REST/TinyTinyRSS/API.php b/lib/REST/TinyTinyRSS/API.php index 78eddcc..2260bd8 100644 --- a/lib/REST/TinyTinyRSS/API.php +++ b/lib/REST/TinyTinyRSS/API.php @@ -1039,35 +1039,12 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { // not valid return $out; case self::CAT_UNCATEGORIZED: - // this is a special case - try { - $tr = Arsse::$db->begin(); - // filter the subscription list to return only uncategorized, and get their IDs - $list = array_column(Arsse::$db->subscriptionList(Arsse::$user->id, null, false)->getAll(), "id"); - // perform marking for each applicable subscription - foreach ($list as $id) { - Arsse::$db->articleMark(Arsse::$user->id, ['read' => true], (new Context)->subscription($id)); - } - $tr->commit(); - } catch (ExceptionInput $e) { // @codeCoverageIgnore - // ignore errors; none should occur - } - return $out; + // this requires a shallow context since in TTRSS folder zero/null is apart from the tree rather than at the root + $c->folderShallow(0); + break; case self::CAT_LABELS: - // this is also a special case - try { - $tr = Arsse::$db->begin(); - // list all non-empty labels - $list = array_column(Arsse::$db->labelList(Arsse::$user->id, false)->getAll(), "id"); - // perform marking for each label - foreach ($list as $id) { - Arsse::$db->articleMark(Arsse::$user->id, ['read' => true], (new Context)->label($id)); - } - $tr->commit(); - } catch (ExceptionInput $e) { // @codeCoverageIgnore - // ignore errors; none should occur - } - return $out; + $c->labelled(true); + break; default: // any actual category $c->folder($id); diff --git a/tests/REST/TinyTinyRSS/TestTinyTinyAPI.php b/tests/REST/TinyTinyRSS/TestTinyTinyAPI.php index 1900bc7..c7e2b98 100644 --- a/tests/REST/TinyTinyRSS/TestTinyTinyAPI.php +++ b/tests/REST/TinyTinyRSS/TestTinyTinyAPI.php @@ -941,13 +941,10 @@ class TestTinyTinyAPI extends Test\AbstractTest { ['op' => "catchupFeed", 'sid' => "PriestsOfSyrinx", 'feed_id' => -2112], ['op' => "catchupFeed", 'sid' => "PriestsOfSyrinx", 'feed_id' => 2112], ['op' => "catchupFeed", 'sid' => "PriestsOfSyrinx", 'feed_id' => 42, 'is_cat' => true], - ]; - $in3 = [ - // complex context ['op' => "catchupFeed", 'sid' => "PriestsOfSyrinx", 'feed_id' => 0, 'is_cat' => true], ['op' => "catchupFeed", 'sid' => "PriestsOfSyrinx", 'feed_id' => -2, 'is_cat' => true], ]; - $in4 = [ + $in3 = [ // this one has a tricky time-based context ['op' => "catchupFeed", 'sid' => "PriestsOfSyrinx", 'feed_id' => -3], ]; @@ -967,25 +964,12 @@ class TestTinyTinyAPI extends Test\AbstractTest { Phake::verify(Arsse::$db)->articleMark($this->anything(), ['read' => true], (new Context)->label(1088)); Phake::verify(Arsse::$db)->articleMark($this->anything(), ['read' => true], (new Context)->subscription(2112)); Phake::verify(Arsse::$db)->articleMark($this->anything(), ['read' => true], (new Context)->folder(42)); - // reset the database mock - $this->setUp(); - Phake::when(Arsse::$db)->articleMark->thenReturn(42); - Phake::when(Arsse::$db)->subscriptionList->thenReturn(new Result($this->subscriptions)); - Phake::when(Arsse::$db)->subscriptionList($this->anything(), null, false)->thenReturn(new Result($this->filterSubs(null))); - Phake::when(Arsse::$db)->labelList->thenReturn(new Result($this->labels)); - Phake::when(Arsse::$db)->labelList($this->anything(), false)->thenReturn(new Result($this->usedLabels)); - // verify the complex contexts - for ($a = 0; $a < sizeof($in3); $a++) { - $this->assertResponse($exp, $this->h->dispatch(new Request("POST", "", json_encode($in3[$a]))), "Test $a failed"); - } - Phake::verify(Arsse::$db)->articleMark($this->anything(), ['read' => true], (new Context)->subscription(6)); - Phake::verify(Arsse::$db)->articleMark($this->anything(), ['read' => true], (new Context)->label(3)); - Phake::verify(Arsse::$db)->articleMark($this->anything(), ['read' => true], (new Context)->label(1)); - Phake::verify(Arsse::$db, Phake::times(3))->articleMark; + Phake::verify(Arsse::$db)->articleMark($this->anything(), ['read' => true], (new Context)->folderShallow(0)); + Phake::verify(Arsse::$db)->articleMark($this->anything(), ['read' => true], (new Context)->labelled(true)); // verify the time-based mock $t = Date::sub("PT24H"); - for ($a = 0; $a < sizeof($in4); $a++) { - $this->assertResponse($exp, $this->h->dispatch(new Request("POST", "", json_encode($in4[$a]))), "Test $a failed"); + for ($a = 0; $a < sizeof($in3); $a++) { + $this->assertResponse($exp, $this->h->dispatch(new Request("POST", "", json_encode($in3[$a]))), "Test $a failed"); } Phake::verify(Arsse::$db)->articleMark($this->anything(), ['read' => true], (new Context)->modifiedSince($t)); } diff --git a/tests/lib/Database/SeriesArticle.php b/tests/lib/Database/SeriesArticle.php index 90c36ea..65c1569 100644 --- a/tests/lib/Database/SeriesArticle.php +++ b/tests/lib/Database/SeriesArticle.php @@ -237,6 +237,8 @@ trait SeriesArticle { [2,20,5,1], [1, 5,3,0], [2, 5,3,1], + [4, 7,4,0], + [4, 8,4,1], ], ], ]; @@ -362,11 +364,12 @@ trait SeriesArticle { $this->compareIds($exp, new Context); $this->compareIds($exp, (new Context)->articles(range(1, Database::LIMIT_ARTICLES * 3))); // get items from a folder tree - $exp = [5,6,7,8]; - $this->compareIds($exp, (new Context)->folder(1)); + $this->compareIds([5,6,7,8], (new Context)->folder(1)); // get items from a leaf folder - $exp = [7,8]; - $this->compareIds($exp, (new Context)->folder(6)); + $this->compareIds([7,8], (new Context)->folder(6)); + // get items from a non-leaf folder without descending + $this->compareIds([1,2,3,4], (new Context)->folderShallow(0)); + $this->compareIds([5,6], (new Context)->folderShallow(1)); // get items from a single subscription $exp = [19,20]; $this->compareIds($exp, (new Context)->subscription(5)); @@ -405,6 +408,9 @@ trait SeriesArticle { // label by name $this->compareIds([1,19], (new Context)->labelName("Interesting")); $this->compareIds([1,5,20], (new Context)->labelName("Fascinating")); + // any or no label + $this->compareIds([1,5,8,19,20], (new Context)->labelled(true)); + $this->compareIds([2,3,4,6,7], (new Context)->labelled(false)); } public function testListArticlesOfAMissingFolder() {