From 5d994f3dadad6d26134678afd919de25a3837454 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Thu, 28 Mar 2019 14:54:31 -0400 Subject: [PATCH 01/32] Normalize Fever input consistently Two parameters are undocumented, but other implementations consistently accept them from clients --- lib/REST/Fever/API.php | 80 +++++++++++++++++++++++++++++++++++------- 1 file changed, 67 insertions(+), 13 deletions(-) diff --git a/lib/REST/Fever/API.php b/lib/REST/Fever/API.php index 47b4038..5dcb9b0 100644 --- a/lib/REST/Fever/API.php +++ b/lib/REST/Fever/API.php @@ -11,7 +11,7 @@ use JKingWeb\Arsse\Database; use JKingWeb\Arsse\User; use JKingWeb\Arsse\Service; use JKingWeb\Arsse\Context\Context; -use JKingWeb\Arsse\Misc\ValueInfo; +use JKingWeb\Arsse\Misc\ValueInfo as V; use JKingWeb\Arsse\Misc\Date; use JKingWeb\Arsse\AbstractException; use JKingWeb\Arsse\Db\ExceptionInput; @@ -26,17 +26,40 @@ use Zend\Diactoros\Response\EmptyResponse; class API extends \JKingWeb\Arsse\REST\AbstractHandler { const LEVEL = 3; + // GET parameters for which we only check presence: these will be converted to booleans + const PARAM_BOOL = ["groups", "feeds", "items", "favicons", "links", "unread_item_ids", "saved_item_ids"]; + // GET parameters which contain meaningful values + const PARAM_GET = [ + 'api' => V::T_STRING, // this parameter requires special handling + 'page' => V::T_INT, // parameter for hot links + 'range' => V::T_INT, // parameter for hot links + 'offset' => V::T_INT, // parameter for hot links + 'since_id' => V::T_INT, + 'max_id' => V::T_INT, + 'with_ids' => V::T_STRING, + 'group_ids' => V::T_STRING, // undocumented parameter for 'items' lookup + 'feed_ids' => V::T_STRING, // undocumented parameter for 'items' lookup + ]; + // POST parameters, all of which contain meaningful values + const PARAM_POST = [ + 'api_key' => V::T_STRING, + 'mark' => V::T_STRING, + 'as' => V::T_STRING, + 'id' => V::T_INT, + 'before' => V::T_DATE, + 'unread_recently_read' => V::T_BOOL, + ]; + public function __construct() { } public function dispatch(ServerRequestInterface $req): ResponseInterface { - $inR = $req->getQueryParams() ?? []; - $inW = $req->getParsedBody() ?? []; - if (!array_key_exists("api", $inR)) { + $G = $this->normalizeInputGet($req->getQueryParams() ?? []); + $P = $this->normalizeInputPost($req->getParsedBody() ?? []); + if (!isset($G['api'])) { // the original would have shown the Fever UI in the absence of the "api" parameter, but we'll return 404 return new EmptyResponse(404); } - $xml = $inR['api'] === "xml"; switch ($req->getMethod()) { case "OPTIONS": // do stuff @@ -58,32 +81,63 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { return new EmptyResponse(401); } // produce a full response if authenticated or a basic response otherwise - if ($this->logIn(strtolower($inW['api_key'] ?? ""))) { - $out = $this->processRequest($this->baseResponse(true), $inR, $inW); + if ($this->logIn(strtolower($P['api_key'] ?? ""))) { + $out = $this->processRequest($this->baseResponse(true), $G, $P); } else { $out = $this->baseResponse(false); } // return the result, possibly formatted as XML - return $this->formatResponse($out, $xml); - break; + return $this->formatResponse($out, ($G['api'] === "xml")); default: return new EmptyResponse(405, ['Allow' => "OPTIONS,POST"]); } } + protected function normalizeInputGet(array $data): array { + $out = []; + if (array_key_exists("api", $data)) { + // the "api" parameter must be handled specially as it a string, but null has special meaning + $data['api'] = $data['api'] ?? "json"; + } + foreach (self::PARAM_BOOL as $p) { + // first handle all the boolean parameters + $out[$p] = array_key_exists($p, $data); + } + foreach (self::PARAM_GET as $p => $t) { + $out[$p] = V::normalize($data[$p] ?? null, $t | V::M_DROP, "unix"); + } + return $out; + } + + protected function normalizeInputPost(array $data): array { + $out = []; + foreach (self::PARAM_POST as $p => $t) { + $out[$p] = V::normalize($data[$p] ?? null, $t | V::M_DROP, "unix"); + } + return $out; + } + protected function processRequest(array $out, array $G, array $P): array { - if (array_key_exists("feeds", $G) || array_key_exists("groups", $G)) { - if (array_key_exists("groups", $G)) { + if ($G['feeds'] || $G['groups']) { + if ($G['groups']) { $out['groups'] = $this->getGroups(); } - if (array_key_exists("feeds", $G)) { + if ($G['feeds']) { $out['feeds'] = $this->getFeeds(); } $out['feeds_groups'] = $this->getRelationships(); } - if (array_key_exists("favicons", $G)) { + if ($G['favicons']) { # deal with favicons } + if ($G['items']) { + $out['items'] = $this->getItems($G); + $out['total_items'] = Arsse::$db->articleCount(Arsse::$user->id); + } + if ($G['links']) { + // TODO: implement hot links + $out['inks'] = []; + } return $out; } From ba32ad2f1724cfd766df9579ce668063e57fa67d Mon Sep 17 00:00:00 2001 From: "J. King" Date: Tue, 2 Apr 2019 09:32:31 -0400 Subject: [PATCH 02/32] Add context options for multiple tags, labels, etc --- lib/Context/ExclusionContext.php | 68 +++++++++++++++++++++++++++++--- tests/cases/Misc/TestContext.php | 25 ++++++++++-- 2 files changed, 84 insertions(+), 9 deletions(-) diff --git a/lib/Context/ExclusionContext.php b/lib/Context/ExclusionContext.php index 1f91994..63cc97d 100644 --- a/lib/Context/ExclusionContext.php +++ b/lib/Context/ExclusionContext.php @@ -11,16 +11,23 @@ use JKingWeb\Arsse\Misc\Date; class ExclusionContext { public $folder; + public $folders; public $folderShallow; + public $foldersShallow; public $tag; + public $tags; public $tagName; + public $tagNames; public $subscription; + public $subscriptions; public $edition; - public $article; public $editions; + public $article; public $articles; public $label; + public $labels; public $labelName; + public $labelNames; public $annotationTerms; public $searchTerms; public $titleTerms; @@ -70,16 +77,18 @@ class ExclusionContext { } } - protected function cleanIdArray(array $spec): array { + protected function cleanIdArray(array $spec, bool $allowZero = false): array { $spec = array_values($spec); for ($a = 0; $a < sizeof($spec); $a++) { - if (ValueInfo::id($spec[$a])) { + if (ValueInfo::id($spec[$a], $allowZero)) { $spec[$a] = (int) $spec[$a]; } else { - $spec[$a] = 0; + $spec[$a] = null; } } - return array_values(array_unique(array_filter($spec))); + return array_values(array_unique(array_filter($spec, function ($v) { + return !is_null($v); + }))); } protected function cleanStringArray(array $spec): array { @@ -99,22 +108,57 @@ class ExclusionContext { return $this->act(__FUNCTION__, func_num_args(), $spec); } + public function folders(array $spec = null) { + if (isset($spec)) { + $spec = $this->cleanIdArray($spec, true); + } + return $this->act(__FUNCTION__, func_num_args(), $spec); + } + public function folderShallow(int $spec = null) { return $this->act(__FUNCTION__, func_num_args(), $spec); } + public function foldersShallow(array $spec = null) { + if (isset($spec)) { + $spec = $this->cleanIdArray($spec, true); + } + return $this->act(__FUNCTION__, func_num_args(), $spec); + } + public function tag(int $spec = null) { return $this->act(__FUNCTION__, func_num_args(), $spec); } + public function tags(array $spec = null) { + if (isset($spec)) { + $spec = $this->cleanIdArray($spec); + } + return $this->act(__FUNCTION__, func_num_args(), $spec); + } + public function tagName(string $spec = null) { return $this->act(__FUNCTION__, func_num_args(), $spec); } + public function tagNames(array $spec = null) { + if (isset($spec)) { + $spec = $this->cleanStringArray($spec); + } + return $this->act(__FUNCTION__, func_num_args(), $spec); + } + public function subscription(int $spec = null) { return $this->act(__FUNCTION__, func_num_args(), $spec); } + public function subscriptions(array $spec = null) { + if (isset($spec)) { + $spec = $this->cleanIdArray($spec); + } + return $this->act(__FUNCTION__, func_num_args(), $spec); + } + public function edition(int $spec = null) { return $this->act(__FUNCTION__, func_num_args(), $spec); } @@ -141,10 +185,24 @@ class ExclusionContext { return $this->act(__FUNCTION__, func_num_args(), $spec); } + public function labels(array $spec = null) { + if (isset($spec)) { + $spec = $this->cleanIdArray($spec); + } + return $this->act(__FUNCTION__, func_num_args(), $spec); + } + public function labelName(string $spec = null) { return $this->act(__FUNCTION__, func_num_args(), $spec); } + public function labelNames(array $spec = null) { + if (isset($spec)) { + $spec = $this->cleanStringArray($spec); + } + return $this->act(__FUNCTION__, func_num_args(), $spec); + } + public function annotationTerms(array $spec = null) { if (isset($spec)) { $spec = $this->cleanStringArray($spec); diff --git a/tests/cases/Misc/TestContext.php b/tests/cases/Misc/TestContext.php index e85d58e..f32f11e 100644 --- a/tests/cases/Misc/TestContext.php +++ b/tests/cases/Misc/TestContext.php @@ -29,10 +29,15 @@ class TestContext extends \JKingWeb\Arsse\Test\AbstractTest { 'limit' => 10, 'offset' => 5, 'folder' => 42, + 'folders' => [12,22], 'folderShallow' => 42, + 'foldersShallow' => [0,1], 'tag' => 44, + 'tags' => [44, 2112], 'tagName' => "XLIV", + 'tagNames' => ["XLIV", "MMCXII"], 'subscription' => 2112, + 'subscriptions' => [44, 2112], 'article' => 255, 'edition' => 65535, 'latestArticle' => 47, @@ -48,7 +53,9 @@ class TestContext extends \JKingWeb\Arsse\Test\AbstractTest { 'editions' => [1,2], 'articles' => [1,2], 'label' => 2112, + 'labels' => [2112, 1984], 'labelName' => "Rush", + 'labelNames' => ["Rush", "Orwell"], 'labelled' => true, 'annotated' => true, 'searchTerms' => ["foo", "bar"], @@ -79,9 +86,19 @@ class TestContext extends \JKingWeb\Arsse\Test\AbstractTest { } public function testCleanIdArrayValues() { - $methods = ["articles", "editions"]; - $in = [1, "2", 3.5, 3.0, "ook", 0, -20, true, false, null, new \DateTime(), -1.0]; - $out = [1,2, 3]; + $methods = ["articles", "editions", "tags", "labels", "subscriptions"]; + $in = [1, "2", 3.5, 4.0, 4, "ook", 0, -20, true, false, null, new \DateTime(), -1.0]; + $out = [1, 2, 4]; + $c = new Context; + foreach ($methods as $method) { + $this->assertSame($out, $c->$method($in)->$method, "Context method $method did not return the expected results"); + } + } + + public function testCleanFolderIdArrayValues() { + $methods = ["folders", "foldersShallow"]; + $in = [1, "2", 3.5, 4.0, 4, "ook", 0, -20, true, false, null, new \DateTime(), -1.0]; + $out = [1, 2, 4, 0]; $c = new Context; foreach ($methods as $method) { $this->assertSame($out, $c->$method($in)->$method, "Context method $method did not return the expected results"); @@ -89,7 +106,7 @@ class TestContext extends \JKingWeb\Arsse\Test\AbstractTest { } public function testCleanStringArrayValues() { - $methods = ["searchTerms", "annotationTerms", "titleTerms", "authorTerms"]; + $methods = ["searchTerms", "annotationTerms", "titleTerms", "authorTerms", "tagNames", "labelNames"]; $now = new \DateTime; $in = [1, 3.0, "ook", 0, true, false, null, $now, ""]; $out = ["1", "3", "ook", "0", valueInfo::normalize($now, ValueInfo::T_STRING)]; From ef1b761f9583373a1e1797c66902300206fdee42 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Tue, 2 Apr 2019 18:24:20 -0400 Subject: [PATCH 03/32] Implement most multiple-item context options Selecting multiple folder trees will require further effort --- lib/Database.php | 102 +++++++++++++++++-------- tests/cases/Database/SeriesArticle.php | 6 ++ 2 files changed, 78 insertions(+), 30 deletions(-) diff --git a/lib/Database.php b/lib/Database.php index 2486907..49f32e8 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -1323,7 +1323,9 @@ class Database { "markedSince" => ["marked_date", ">=", "datetime", "notMarkedSince"], "notMarkedSince" => ["marked_date", "<=", "datetime", "markedSince"], "folderShallow" => ["folder", "=", "int", ""], + "foldersShallow" => ["folder", "in", "int", ""], "subscription" => ["subscription", "=", "int", ""], + "subscriptions" => ["subscription", "in", "int", ""], "unread" => ["unread", "=", "bool", ""], "starred" => ["starred", "=", "bool", ""], ]; @@ -1374,6 +1376,76 @@ class Database { $q->setWhereNot("{$colDefs[$col]} $op ?", $type, $context->not->$m); } } + // handle labels and tags + $options = [ + 'label' => [ + 'match_col' => "arsse_articles.id", + 'cte_name' => "labelled", + 'cte_cols' => ["article", "label_id", "label_name"], + 'cte_body' => "SELECT m.article, l.id, l.name from arsse_label_members as m join arsse_labels as l on l.id = m.label where l.owner = ? and m.assigned = 1", + 'cte_types' => ["str"], + 'cte_values' => [$user], + 'options' => [ + 'label' => ['use_name' => false, 'multi' => false], + 'labels' => ['use_name' => false, 'multi' => true], + 'labelName' => ['use_name' => true, 'multi' => false], + 'labelNames' => ['use_name' => true, 'multi' => true], + ], + ], + 'tag' => [ + 'match_col' => "arsse_subscriptions.id", + 'cte_name' => "tagged", + 'cte_cols' => ["subscription", "tag_id", "tag_name"], + 'cte_body' => "SELECT m.subscription, t.id, t.name from arsse_tag_members as m join arsse_tags as t on t.id = m.tag where t.owner = ? and m.assigned = 1", + 'cte_types' => ["str"], + 'cte_values' => [$user], + 'options' => [ + 'tag' => ['use_name' => false, 'multi' => false], + 'tags' => ['use_name' => false, 'multi' => true], + 'tagName' => ['use_name' => true, 'multi' => false], + 'tagNames' => ['use_name' => true, 'multi' => true], + ], + ], + ]; + foreach ($options as $opt) { + $seen = false; + $match = $opt['match_col']; + $table = $opt['cte_name']; + foreach ($opt['options'] as $m => $props) { + $named = $props['use_name']; + $multi = $props['multi']; + $selection = $opt['cte_cols'][0]; + $col = $opt['cte_cols'][$named ? 2 : 1]; + if ($context->$m()) { + $seen = true; + if ($multi) { + list($test, $types, $values) = $this->generateIn($context->$m, $named ? "str" : "int"); + $test = "in ($test)"; + } else { + $test = "= ?"; + $types = $named ? "str" : "int"; + $values = $context->$m; + } + $q->setWhere("$match in (select $selection from $table where $col $test)", $types, $values); + } + if ($context->not->$m()) { + $seen = true; + if ($multi) { + list($test, $types, $values) = $this->generateIn($context->not->$m, $named ? "str" : "int"); + $test = "in ($test)"; + } else { + $test = "= ?"; + $types = $named ? "str" : "int"; + $values = $context->not->$m; + } + $q->setWhereNot("$match in (select $selection from $table where $col $test)", $types, $values); + } + } + if ($seen) { + $spec = $opt['cte_name']."(".implode(",",$opt['cte_cols']).")"; + $q->setCTE($spec, $opt['cte_body'], $opt['cte_types'], $opt['cte_values']); + } + } // handle complex context options if ($context->annotated()) { $comp = ($context->annotated) ? "<>" : "="; @@ -1384,36 +1456,6 @@ class Database { $op = $context->labelled ? ">" : "="; $q->setWhere("coalesce(label_stats.assigned,0) $op 0"); } - if ($context->label() || $context->not->label() || $context->labelName() || $context->not->labelName()) { - $q->setCTE("labelled(article,label_id,label_name)","SELECT m.article, l.id, l.name from arsse_label_members as m join arsse_labels as l on l.id = m.label where l.owner = ? and m.assigned = 1", "str", $user); - if ($context->label()) { - $q->setWhere("arsse_articles.id in (select article from labelled where label_id = ?)", "int", $context->label); - } - if ($context->not->label()) { - $q->setWhereNot("arsse_articles.id in (select article from labelled where label_id = ?)", "int", $context->not->label); - } - if ($context->labelName()) { - $q->setWhere("arsse_articles.id in (select article from labelled where label_name = ?)", "str", $context->labelName); - } - if ($context->not->labelName()) { - $q->setWhereNot("arsse_articles.id in (select article from labelled where label_name = ?)", "str", $context->not->labelName); - } - } - if ($context->tag() || $context->not->tag() || $context->tagName() || $context->not->tagName()) { - $q->setCTE("tagged(id,name,subscription)","SELECT arsse_tags.id, arsse_tags.name, arsse_tag_members.subscription FROM arsse_tag_members join arsse_tags on arsse_tags.id = arsse_tag_members.tag WHERE arsse_tags.owner = ? and assigned = 1", "str", $user); - if ($context->tag()) { - $q->setWhere("arsse_subscriptions.id in (select subscription from tagged where id = ?)", "int", $context->tag); - } - if ($context->not->tag()) { - $q->setWhereNot("arsse_subscriptions.id in (select subscription from tagged where id = ?)", "int", $context->not->tag); - } - if ($context->tagName()) { - $q->setWhere("arsse_subscriptions.id in (select subscription from tagged where name = ?)", "str", $context->tagName); - } - if ($context->not->tagName()) { - $q->setWhereNot("arsse_subscriptions.id in (select subscription from tagged where name = ?)", "str", $context->not->tagName); - } - } if ($context->folder()) { // add a common table expression to list the folder and its children so that we select from the entire subtree $q->setCTE("folders(folder)", "SELECT ? union select id from arsse_folders join folders on parent = folder", "int", $context->folder); diff --git a/tests/cases/Database/SeriesArticle.php b/tests/cases/Database/SeriesArticle.php index 5340fcc..51d9da5 100644 --- a/tests/cases/Database/SeriesArticle.php +++ b/tests/cases/Database/SeriesArticle.php @@ -427,7 +427,9 @@ trait SeriesArticle { 'Leaf folder' => [(new Context)->folder(6), [7,8]], 'Root folder only' => [(new Context)->folderShallow(0), [1,2,3,4]], 'Shallow folder' => [(new Context)->folderShallow(1), [5,6]], + 'Multiple shallow folders' => [(new Context)->foldersShallow([1,6]), [5,6,7,8]], 'Subscription' => [(new Context)->subscription(5), [19,20]], + 'Multiple subscriptions' => [(new Context)->subscriptions([4,5]), [7,8,19,20]], 'Unread' => [(new Context)->subscription(5)->unread(true), [20]], 'Read' => [(new Context)->subscription(5)->unread(false), [19]], 'Starred' => [(new Context)->starred(true), [1,20]], @@ -458,8 +460,10 @@ trait SeriesArticle { 'Reversed paged results' => [(new Context)->limit(2)->latestEdition(7)->reverse(true), [7,6]], 'With label ID 1' => [(new Context)->label(1), [1,19]], 'With label ID 2' => [(new Context)->label(2), [1,5,20]], + 'With label ID 1 or 2' => [(new Context)->labels([1,2]), [1,5,19,20]], 'With label "Interesting"' => [(new Context)->labelName("Interesting"), [1,19]], 'With label "Fascinating"' => [(new Context)->labelName("Fascinating"), [1,5,20]], + 'With label "Interesting" or "Fascinating"' => [(new Context)->labelNames(["Interesting","Fascinating"]), [1,5,19,20]], 'Article ID 20' => [(new Context)->article(20), [20]], 'Edition ID 1001' => [(new Context)->edition(1001), [20]], 'Multiple articles' => [(new Context)->articles([1,20,50]), [1,20]], @@ -494,8 +498,10 @@ trait SeriesArticle { 'Search 501 terms' => [(new Context)->searchTerms(array_merge(range(1,500),[str_repeat("a", 1000)])), []], 'With tag ID 1' => [(new Context)->tag(1), [5,6,7,8]], 'With tag ID 5' => [(new Context)->tag(5), [7,8,19,20]], + 'With tag ID 1 or 5' => [(new Context)->tags([1,5]), [5,6,7,8,19,20]], 'With tag "Technology"' => [(new Context)->tagName("Technology"), [5,6,7,8]], 'With tag "Politics"' => [(new Context)->tagName("Politics"), [7,8,19,20]], + 'With tag "Technology" or "Politics"' => [(new Context)->tagNames(["Technology","Politics"]), [5,6,7,8,19,20]], 'Excluding tag ID 1' => [(new Context)->not->tag(1), [1,2,3,4,19,20]], 'Excluding tag ID 5' => [(new Context)->not->tag(5), [1,2,3,4,5,6]], 'Excluding tag "Technology"' => [(new Context)->not->tagName("Technology"), [1,2,3,4,19,20]], From 98f6fca7e3d2ef88b8246e93514297b2d7c41a60 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Tue, 2 Apr 2019 18:37:46 -0400 Subject: [PATCH 04/32] Enforce minimum array size (for now) --- lib/Database.php | 3 +++ tests/cases/Database/SeriesArticle.php | 27 ++++++++++++-------------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/lib/Database.php b/lib/Database.php index 49f32e8..404f451 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -1418,6 +1418,9 @@ class Database { $col = $opt['cte_cols'][$named ? 2 : 1]; if ($context->$m()) { $seen = true; + if (!$context->$m) { + throw new Db\ExceptionInput("tooShort", ['field' => $m, 'action' => $this->caller(), 'min' => 1]); // must have at least one array element + } if ($multi) { list($test, $types, $values) = $this->generateIn($context->$m, $named ? "str" : "int"); $test = "in ($test)"; diff --git a/tests/cases/Database/SeriesArticle.php b/tests/cases/Database/SeriesArticle.php index 51d9da5..694aec5 100644 --- a/tests/cases/Database/SeriesArticle.php +++ b/tests/cases/Database/SeriesArticle.php @@ -789,11 +789,6 @@ trait SeriesArticle { $this->compareExpectations($state); } - public function testMarkTooFewMultipleArticles() { - $this->assertException("tooShort", "Db", "ExceptionInput"); - Arsse::$db->articleMark($this->user, ['read'=>false,'starred'=>true], (new Context)->articles([])); - } - public function testMarkTooManyMultipleArticles() { $this->assertSame(7, Arsse::$db->articleMark($this->user, ['read'=>false,'starred'=>true], (new Context)->articles(range(1, Database::LIMIT_SET_SIZE * 3)))); } @@ -860,11 +855,6 @@ trait SeriesArticle { $this->compareExpectations($state); } - public function testMarkTooFewMultipleEditions() { - $this->assertException("tooShort", "Db", "ExceptionInput"); - Arsse::$db->articleMark($this->user, ['read'=>false,'starred'=>true], (new Context)->editions([])); - } - public function testMarkTooManyMultipleEditions() { $this->assertSame(7, Arsse::$db->articleMark($this->user, ['read'=>false,'starred'=>true], (new Context)->editions(range(1, 51)))); } @@ -1036,13 +1026,20 @@ trait SeriesArticle { Arsse::$db->articleCategoriesGet($this->user, 19); } - public function testSearchTooFewTerms() { + /** @dataProvider provideArrayContextOptions */ + public function testUseTooFewValuesInArrayContext(string $option) { $this->assertException("tooShort", "Db", "ExceptionInput"); - Arsse::$db->articleList($this->user, (new Context)->searchTerms([])); + Arsse::$db->articleList($this->user, (new Context)->annotationTerms([])); } - public function testSearchTooFewTermsInNote() { - $this->assertException("tooShort", "Db", "ExceptionInput"); - Arsse::$db->articleList($this->user, (new Context)->annotationTerms([])); + public function provideArrayContextOptions() { + foreach([ + "articles", "editions", + "subscriptions", "foldersShallow", //"folders", + "tags", "tagNames", "labels", "labelNames", + "searchTerms", "authorTerms", "annotationTerms", + ] as $method) { + yield [$method]; + } } } From cce1089e10740bbf6a7ee38fc9227cfcd09f2aaf Mon Sep 17 00:00:00 2001 From: "J. King" Date: Tue, 2 Apr 2019 19:58:35 -0400 Subject: [PATCH 05/32] Handle edge case with folder 0 Folder 0 (the root folder) is a valid, though nonsensical selection: using it as a positive option is the same as not using the option at all, and using it as a negative option necessarily yields an empty set. However, it can in some contexts be validly specified, and so it should be handled consistently. It had not been previously, but is now. --- lib/Context/ExclusionContext.php | 1 + lib/Database.php | 4 ++-- tests/cases/Database/SeriesArticle.php | 2 ++ 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/Context/ExclusionContext.php b/lib/Context/ExclusionContext.php index 63cc97d..7cf45cb 100644 --- a/lib/Context/ExclusionContext.php +++ b/lib/Context/ExclusionContext.php @@ -49,6 +49,7 @@ class ExclusionContext { } public function __clone() { + // if the context was cloned because its parent was cloned, change the parent to the clone if ($this->parent) { $t = debug_backtrace(\DEBUG_BACKTRACE_IGNORE_ARGS | \DEBUG_BACKTRACE_PROVIDE_OBJECT, 2)[1]; if (($t['object'] ?? null) instanceof self && $t['function'] === "__clone") { diff --git a/lib/Database.php b/lib/Database.php index 404f451..1173ae6 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -1461,13 +1461,13 @@ class Database { } if ($context->folder()) { // add a common table expression to list the folder and its children so that we select from the entire subtree - $q->setCTE("folders(folder)", "SELECT ? union select id from arsse_folders join folders on parent = folder", "int", $context->folder); + $q->setCTE("folders(folder)", "SELECT ? union select id from arsse_folders join folders on coalesce(parent,0) = folder", "int", $context->folder); // limit subscriptions to the listed folders $q->setWhere("coalesce(arsse_subscriptions.folder,0) in (select folder from folders)"); } if ($context->not->folder()) { // add a common table expression to list the folder and its children so that we exclude from the entire subtree - $q->setCTE("folders_excluded(folder)", "SELECT ? union select id from arsse_folders join folders_excluded on parent = folder", "int", $context->not->folder); + $q->setCTE("folders_excluded(folder)", "SELECT ? union select id from arsse_folders join folders_excluded on coalesce(parent,0) = folder", "int", $context->not->folder); // excluded any subscriptions in the listed folders $q->setWhereNot("coalesce(arsse_subscriptions.folder,0) in (select folder from folders_excluded)"); } diff --git a/tests/cases/Database/SeriesArticle.php b/tests/cases/Database/SeriesArticle.php index 694aec5..31dcf96 100644 --- a/tests/cases/Database/SeriesArticle.php +++ b/tests/cases/Database/SeriesArticle.php @@ -424,6 +424,7 @@ trait SeriesArticle { return [ 'Blank context' => [new Context, [1,2,3,4,5,6,7,8,19,20]], 'Folder tree' => [(new Context)->folder(1), [5,6,7,8]], + 'Entire folder tree' => [(new Context)->folder(0), [1,2,3,4,5,6,7,8,19,20]], 'Leaf folder' => [(new Context)->folder(6), [7,8]], 'Root folder only' => [(new Context)->folderShallow(0), [1,2,3,4]], 'Shallow folder' => [(new Context)->folderShallow(1), [5,6]], @@ -506,6 +507,7 @@ trait SeriesArticle { 'Excluding tag ID 5' => [(new Context)->not->tag(5), [1,2,3,4,5,6]], 'Excluding tag "Technology"' => [(new Context)->not->tagName("Technology"), [1,2,3,4,19,20]], 'Excluding tag "Politics"' => [(new Context)->not->tagName("Politics"), [1,2,3,4,5,6]], + 'Excluding entire folder tree' => [(new Context)->not->folder(0), []], ]; } From 74fc39fca034a152d301baea9ea9f0b64a53d8ec Mon Sep 17 00:00:00 2001 From: "J. King" Date: Tue, 2 Apr 2019 22:44:09 -0400 Subject: [PATCH 06/32] Implement multi-folder context option --- lib/Database.php | 14 ++++++++++++++ tests/cases/Database/SeriesArticle.php | 6 +++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/lib/Database.php b/lib/Database.php index 1173ae6..459f467 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -1465,12 +1465,26 @@ class Database { // limit subscriptions to the listed folders $q->setWhere("coalesce(arsse_subscriptions.folder,0) in (select folder from folders)"); } + if ($context->folders()) { + list($inClause, $inTypes, $inValues) = $this->generateIn($context->folders, "int"); + // add a common table expression to list the folders and their children so that we select from the entire subtree + $q->setCTE("folders_multi(folder)", "SELECT id as folder from (select id from (select 0 as id union select id from arsse_folders where owner = ?) as f where id in ($inClause)) as folders_multi union select id from arsse_folders join folders_multi on coalesce(parent,0) = folder", ["str", $inTypes], [$user, $inValues]); + // limit subscriptions to the listed folders + $q->setWhere("coalesce(arsse_subscriptions.folder,0) in (select folder from folders_multi)"); + } if ($context->not->folder()) { // add a common table expression to list the folder and its children so that we exclude from the entire subtree $q->setCTE("folders_excluded(folder)", "SELECT ? union select id from arsse_folders join folders_excluded on coalesce(parent,0) = folder", "int", $context->not->folder); // excluded any subscriptions in the listed folders $q->setWhereNot("coalesce(arsse_subscriptions.folder,0) in (select folder from folders_excluded)"); } + if ($context->not->folders()) { + list($inClause, $inTypes, $inValues) = $this->generateIn($context->not->folders, "int"); + // add a common table expression to list the folders and their children so that we select from the entire subtree + $q->setCTE("folders_multi_excluded(folder)", "SELECT id as folder from (select id from (select 0 as id union select id from arsse_folders where owner = ?) as f where id in ($inClause)) as folders_multi_excluded union select id from arsse_folders join folders_multi_excluded on coalesce(parent,0) = folder", ["str", $inTypes], [$user, $inValues]); + // limit subscriptions to the listed folders + $q->setWhereNot("coalesce(arsse_subscriptions.folder,0) in (select folder from folders_multi_excluded)"); + } // handle text-matching context options $options = [ "titleTerms" => ["arsse_articles.title"], diff --git a/tests/cases/Database/SeriesArticle.php b/tests/cases/Database/SeriesArticle.php index 31dcf96..17b0ece 100644 --- a/tests/cases/Database/SeriesArticle.php +++ b/tests/cases/Database/SeriesArticle.php @@ -426,8 +426,10 @@ trait SeriesArticle { 'Folder tree' => [(new Context)->folder(1), [5,6,7,8]], 'Entire folder tree' => [(new Context)->folder(0), [1,2,3,4,5,6,7,8,19,20]], 'Leaf folder' => [(new Context)->folder(6), [7,8]], - 'Root folder only' => [(new Context)->folderShallow(0), [1,2,3,4]], + 'Multiple folder trees' => [(new Context)->folders([1,5]), [5,6,7,8,19,20]], + 'Multiple folder trees including root' => [(new Context)->folders([0,1,5]), [1,2,3,4,5,6,7,8,19,20]], 'Shallow folder' => [(new Context)->folderShallow(1), [5,6]], + 'Root folder only' => [(new Context)->folderShallow(0), [1,2,3,4]], 'Multiple shallow folders' => [(new Context)->foldersShallow([1,6]), [5,6,7,8]], 'Subscription' => [(new Context)->subscription(5), [19,20]], 'Multiple subscriptions' => [(new Context)->subscriptions([4,5]), [7,8,19,20]], @@ -508,6 +510,8 @@ trait SeriesArticle { 'Excluding tag "Technology"' => [(new Context)->not->tagName("Technology"), [1,2,3,4,19,20]], 'Excluding tag "Politics"' => [(new Context)->not->tagName("Politics"), [1,2,3,4,5,6]], 'Excluding entire folder tree' => [(new Context)->not->folder(0), []], + 'Excluding multiple folder trees' => [(new Context)->not->folders([1,5]), [1,2,3,4]], + 'Excluding multiple folder trees including root' => [(new Context)->not->folders([0,1,5]), []], ]; } From 4b133bddd640dcafd80c2fff5c1243119ab38842 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Wed, 3 Apr 2019 15:02:59 -0400 Subject: [PATCH 07/32] Prototype arbitrary result ordering --- lib/Database.php | 60 ++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 48 insertions(+), 12 deletions(-) diff --git a/lib/Database.php b/lib/Database.php index 459f467..341b267 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -1226,7 +1226,7 @@ class Database { * @param Context $context The search context * @param array $cols The columns to request in the result set */ - protected function articleQuery(string $user, Context $context, array $cols = ["id"]): Query { + protected function articleQuery(string $user, Context $context, array $cols = ["id"], array $sort = []): Query { // validate input if ($context->subscription()) { $this->subscriptionValidateId($user, $context->subscription); @@ -1275,23 +1275,55 @@ class Database { 'media_type' => "arsse_enclosures.type", ]; if (!$cols) { - // if no columns are specified return a count - $columns = "count(distinct arsse_articles.id) as count"; + // if no columns are specified return a count; don't borther with sorting + $outColumns = "count(distinct arsse_articles.id) as count"; + $sortColumns = []; } else { - $columns = []; + // normalize requested output and sorting columns + $norm = function($v) { + return trim(strtolower(ValueInfo::normalize($v, ValueInfo::T_STRING))); + }; + $cols = array_map($norm, $cols); + $sort = array_map($norm, $sort); + // make an output column list + $outColumns = []; foreach ($cols as $col) { - $col = trim(strtolower($col)); if (!isset($colDefs[$col])) { continue; } - $columns[] = $colDefs[$col]." as ".$col; + $outColumns[] = $colDefs[$col]." as ".$col; + } + $outColumns = implode(",", $outColumns); + // make an ORDER BY column list + $sortColumns = []; + foreach ($sort as $spec) { + $col = explode(" ", $spec, 1); + $order = $col[1] ?? ""; + $col = $col[0]; + if ($order === "desc") { + $order = " desc"; + } elseif ($order === "asc" || $order === "") { + $order = ""; + } else { + // column direction spec is bogus + continue; + } + if (!isset($colDefs[$col])) { + // column name spec is bogus + continue; + } elseif (in_array($col, $cols)) { + // if the sort column is also an output column, use it as-is + $sortColumns[] = $col.$order; + } else { + // otherwise if the column name is valid, use its expression + $sortColumns[] = $colDefs[$col].$order; + } } - $columns = implode(",", $columns); } // define the basic query, to which we add lots of stuff where necessary $q = new Query( "SELECT - $columns + $outColumns from arsse_articles join arsse_subscriptions on arsse_subscriptions.feed = arsse_articles.feed and arsse_subscriptions.owner = ? join arsse_feeds on arsse_subscriptions.feed = arsse_feeds.id @@ -1307,6 +1339,10 @@ class Database { [$user, $user] ); $q->setLimit($context->limit, $context->offset); + // apply the ORDER BY definition computed above + array_walk($sortColumns, function($v, $k, Query $q) { + $q->setOrder($v); + }, $q); // handle the simple context options $options = [ // each context array consists of a column identifier (see $colDefs above), a comparison operator, a data type, and an option to pair with for BETWEEN evaluation @@ -1492,20 +1528,20 @@ class Database { "authorTerms" => ["arsse_articles.author"], "annotationTerms" => ["arsse_marks.note"], ]; - foreach ($options as $m => $cols) { + foreach ($options as $m => $columns) { if (!$context->$m()) { continue; } elseif (!$context->$m) { throw new Db\ExceptionInput("tooShort", ['field' => $m, 'action' => $this->caller(), 'min' => 1]); // must have at least one array element } - $q->setWhere(...$this->generateSearch($context->$m, $cols)); + $q->setWhere(...$this->generateSearch($context->$m, $columns)); } // further handle exclusionary text-matching context options - foreach ($options as $m => $cols) { + foreach ($options as $m => $columns) { if (!$context->not->$m() || !$context->not->$m) { continue; } - $q->setWhereNot(...$this->generateSearch($context->not->$m, $cols, true)); + $q->setWhereNot(...$this->generateSearch($context->not->$m, $columns, true)); } // return the query return $q; From 156ce2d09970860dde25d200ce577d3d4e06576e Mon Sep 17 00:00:00 2001 From: "J. King" Date: Thu, 4 Apr 2019 11:20:40 -0400 Subject: [PATCH 08/32] Fix Unix Robo script --- robo | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/robo b/robo index 0b3be08..f525941 100755 --- a/robo +++ b/robo @@ -5,7 +5,7 @@ shift ulimit -n 2048 if [ "$1" = "clean" ]; then - "$base/vendor/bin/robo" "$roboCommand" $* + "$base/vendor/bin/robo" "$roboCommand" "$@" else - "$base/vendor/bin/robo" "$roboCommand" -- $* + "$base/vendor/bin/robo" "$roboCommand" -- "$@" fi From f72c85c9f64f0015e3b5e37e4c2303ac050b4c58 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Thu, 4 Apr 2019 11:22:50 -0400 Subject: [PATCH 09/32] Hopefully working but maybe broken custom sorting --- lib/Context/Context.php | 5 - lib/Database.php | 130 +++++++++++++------------ lib/REST/NextCloudNews/V1_2.php | 10 +- lib/REST/TinyTinyRSS/API.php | 13 +-- tests/cases/Database/SeriesArticle.php | 1 - 5 files changed, 79 insertions(+), 80 deletions(-) diff --git a/lib/Context/Context.php b/lib/Context/Context.php index 858409f..fb1236a 100644 --- a/lib/Context/Context.php +++ b/lib/Context/Context.php @@ -9,7 +9,6 @@ namespace JKingWeb\Arsse\Context; class Context extends ExclusionContext { /** @var ExclusionContext */ public $not; - public $reverse = false; public $limit = 0; public $offset = 0; public $unread; @@ -31,10 +30,6 @@ class Context extends ExclusionContext { unset($this->not); } - public function reverse(bool $spec = null) { - return $this->act(__FUNCTION__, func_num_args(), $spec); - } - public function limit(int $spec = null) { return $this->act(__FUNCTION__, func_num_args(), $spec); } diff --git a/lib/Database.php b/lib/Database.php index 341b267..ff74f65 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -1218,6 +1218,37 @@ class Database { )->run($feedID, $vId, $vHashUT, $vHashUC, $vHashTC); } + /** Returns an associative array of result column names and their SQL computations for article queries + * + * This is used for whitelisting and defining both output column and order-by columns, as well as for resolution of some context options + */ + protected function articleColumns(): array { + $greatest = $this->db->sqlToken("greatest"); + return [ + 'id' => "arsse_articles.id", + 'edition' => "latest_editions.edition", + 'url' => "arsse_articles.url", + 'title' => "arsse_articles.title", + 'author' => "arsse_articles.author", + 'content' => "arsse_articles.content", + 'guid' => "arsse_articles.guid", + 'fingerprint' => "arsse_articles.url_title_hash || ':' || arsse_articles.url_content_hash || ':' || arsse_articles.title_content_hash", + 'folder' => "coalesce(arsse_subscriptions.folder,0)", + 'subscription' => "arsse_subscriptions.id", + 'feed' => "arsse_subscriptions.feed", + 'starred' => "coalesce(arsse_marks.starred,0)", + 'unread' => "abs(coalesce(arsse_marks.read,0) - 1)", + 'note' => "coalesce(arsse_marks.note,'')", + 'published_date' => "arsse_articles.published", + 'edited_date' => "arsse_articles.edited", + 'modified_date' => "arsse_articles.modified", + 'marked_date' => "$greatest(arsse_articles.modified, coalesce(arsse_marks.modified, '0001-01-01 00:00:00'), coalesce(label_stats.modified, '0001-01-01 00:00:00'))", + 'subscription_title' => "coalesce(arsse_subscriptions.title, arsse_feeds.title)", + 'media_url' => "arsse_enclosures.url", + 'media_type' => "arsse_enclosures.type", + ]; + } + /** Computes an SQL query to find and retrieve data about articles in the database * * If an empty column list is supplied, a count of articles matching the context is queried instead @@ -1226,14 +1257,14 @@ class Database { * @param Context $context The search context * @param array $cols The columns to request in the result set */ - protected function articleQuery(string $user, Context $context, array $cols = ["id"], array $sort = []): Query { + protected function articleQuery(string $user, Context $context, array $cols = ["id"]): Query { // validate input if ($context->subscription()) { $this->subscriptionValidateId($user, $context->subscription); } if ($context->folder()) { $this->folderValidateId($user, $context->folder); - } + } if ($context->folderShallow()) { $this->folderValidateId($user, $context->folderShallow); } @@ -1250,41 +1281,16 @@ class Database { $this->labelValidateId($user, $context->labelName, true); } // prepare the output column list; the column definitions are also used later - $greatest = $this->db->sqlToken("greatest"); - $colDefs = [ - 'id' => "arsse_articles.id", - 'edition' => "latest_editions.edition", - 'url' => "arsse_articles.url", - 'title' => "arsse_articles.title", - 'author' => "arsse_articles.author", - 'content' => "arsse_articles.content", - 'guid' => "arsse_articles.guid", - 'fingerprint' => "arsse_articles.url_title_hash || ':' || arsse_articles.url_content_hash || ':' || arsse_articles.title_content_hash", - 'folder' => "coalesce(arsse_subscriptions.folder,0)", - 'subscription' => "arsse_subscriptions.id", - 'feed' => "arsse_subscriptions.feed", - 'starred' => "coalesce(arsse_marks.starred,0)", - 'unread' => "abs(coalesce(arsse_marks.read,0) - 1)", - 'note' => "coalesce(arsse_marks.note,'')", - 'published_date' => "arsse_articles.published", - 'edited_date' => "arsse_articles.edited", - 'modified_date' => "arsse_articles.modified", - 'marked_date' => "$greatest(arsse_articles.modified, coalesce(arsse_marks.modified, '0001-01-01 00:00:00'), coalesce(label_stats.modified, '0001-01-01 00:00:00'))", - 'subscription_title' => "coalesce(arsse_subscriptions.title, arsse_feeds.title)", - 'media_url' => "arsse_enclosures.url", - 'media_type' => "arsse_enclosures.type", - ]; + $colDefs = $this->articleColumns(); if (!$cols) { // if no columns are specified return a count; don't borther with sorting $outColumns = "count(distinct arsse_articles.id) as count"; - $sortColumns = []; } else { // normalize requested output and sorting columns $norm = function($v) { return trim(strtolower(ValueInfo::normalize($v, ValueInfo::T_STRING))); }; $cols = array_map($norm, $cols); - $sort = array_map($norm, $sort); // make an output column list $outColumns = []; foreach ($cols as $col) { @@ -1294,31 +1300,6 @@ class Database { $outColumns[] = $colDefs[$col]." as ".$col; } $outColumns = implode(",", $outColumns); - // make an ORDER BY column list - $sortColumns = []; - foreach ($sort as $spec) { - $col = explode(" ", $spec, 1); - $order = $col[1] ?? ""; - $col = $col[0]; - if ($order === "desc") { - $order = " desc"; - } elseif ($order === "asc" || $order === "") { - $order = ""; - } else { - // column direction spec is bogus - continue; - } - if (!isset($colDefs[$col])) { - // column name spec is bogus - continue; - } elseif (in_array($col, $cols)) { - // if the sort column is also an output column, use it as-is - $sortColumns[] = $col.$order; - } else { - // otherwise if the column name is valid, use its expression - $sortColumns[] = $colDefs[$col].$order; - } - } } // define the basic query, to which we add lots of stuff where necessary $q = new Query( @@ -1339,10 +1320,6 @@ class Database { [$user, $user] ); $q->setLimit($context->limit, $context->offset); - // apply the ORDER BY definition computed above - array_walk($sortColumns, function($v, $k, Query $q) { - $q->setOrder($v); - }, $q); // handle the simple context options $options = [ // each context array consists of a column identifier (see $colDefs above), a comparison operator, a data type, and an option to pair with for BETWEEN evaluation @@ -1553,16 +1530,47 @@ class Database { * * @param string $user The user whose articles are to be listed * @param Context $context The search context - * @param array $cols The columns to return in the result set, any of: id, edition, url, title, author, content, guid, fingerprint, folder, subscription, feed, starred, unread, note, published_date, edited_date, modified_date, marked_date, subscription_title, media_url, media_type + * @param array $fieldss The columns to return in the result set, any of: id, edition, url, title, author, content, guid, fingerprint, folder, subscription, feed, starred, unread, note, published_date, edited_date, modified_date, marked_date, subscription_title, media_url, media_type + * @param array $sort The columns to sort the result by eg. "edition desc" in decreasing order of importance */ - public function articleList(string $user, Context $context = null, array $fields = ["id"]): Db\Result { + public function articleList(string $user, Context $context = null, array $fields = ["id"], array $sort = []): Db\Result { if (!Arsse::$user->authorize($user, __FUNCTION__)) { throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); } + // make a base query based on context and output columns $context = $context ?? new Context; $q = $this->articleQuery($user, $context, $fields); - $q->setOrder("arsse_articles.edited".($context->reverse ? " desc" : "")); - $q->setOrder("latest_editions.edition".($context->reverse ? " desc" : "")); + // make an ORDER BY column list + $colDefs = $this->articleColumns(); + // normalize requested output and sorting columns + $norm = function($v) { + return trim(strtolower((string) $v)); + }; + $fields = array_map($norm, $fields); + $sort = array_map($norm, $sort); + foreach ($sort as $spec) { + $col = explode(" ", $spec, 1); + $order = $col[1] ?? ""; + $col = $col[0]; + if ($order === "desc") { + $order = " desc"; + } elseif ($order === "asc" || $order === "") { + $order = ""; + } else { + // column direction spec is bogus + continue; + } + if (!isset($colDefs[$col])) { + // column name spec is bogus + continue; + } elseif (in_array($col, $fields)) { + // if the sort column is also an output column, use it as-is + $q->setOrder($col.$order); + } else { + // otherwise if the column name is valid, use its expression + $q->setOrder($colDefs[$col].$order); + } + } // perform the query and return results return $this->db->prepare($q->getQuery(), $q->getTypes())->run($q->getValues()); } diff --git a/lib/REST/NextCloudNews/V1_2.php b/lib/REST/NextCloudNews/V1_2.php index 7f4301c..0df5032 100644 --- a/lib/REST/NextCloudNews/V1_2.php +++ b/lib/REST/NextCloudNews/V1_2.php @@ -521,14 +521,10 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { $c->limit($data['batchSize']); } // set the order of returned items - if ($data['oldestFirst']) { - $c->reverse(false); - } else { - $c->reverse(true); - } + $reverse = !$data['oldestFirst']; // set the edition mark-off; the database uses an or-equal comparison for internal consistency, but the protocol does not, so we must adjust by one if ($data['offset'] > 0) { - if ($c->reverse) { + if ($reverse) { $c->latestEdition($data['offset'] - 1); } else { $c->oldestEdition($data['offset'] + 1); @@ -579,7 +575,7 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { "starred", "modified_date", "fingerprint", - ]); + ], [$reverse ? "edition desc" : "edition"]); } catch (ExceptionInput $e) { // ID of subscription or folder is not valid return new EmptyResponse(422); diff --git a/lib/REST/TinyTinyRSS/API.php b/lib/REST/TinyTinyRSS/API.php index 8bf85bc..b274f20 100644 --- a/lib/REST/TinyTinyRSS/API.php +++ b/lib/REST/TinyTinyRSS/API.php @@ -23,6 +23,7 @@ use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\ResponseInterface; use Zend\Diactoros\Response\JsonResponse as Response; use Zend\Diactoros\Response\EmptyResponse; +use Robo\Task\Archive\Pack; class API extends \JKingWeb\Arsse\REST\AbstractHandler { const LEVEL = 14; // emulated API level @@ -1438,7 +1439,7 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { // no context needed here break; case self::FEED_READ: - $c->markedSince(Date::sub("PT24H"))->unread(false); // FIXME: this selects any recently touched article which is read, not necessarily a recently read one + $c->markedSince(Date::sub("PT24H"))->unread(false); // FIXME: this selects any recently touched (read, starred, annotated) article which is read, not necessarily a recently read one break; default: // any actual feed @@ -1491,15 +1492,15 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { switch ($data['order_by']) { case "date_reverse": // sort oldest first - $c->reverse(false); + $order = ["edited_date"]; break; case "feed_dates": // sort newest first - $c->reverse(true); + $order = ["edited_date desc"]; break; default: - // in TT-RSS the default sort order is unusual for some of the special feeds; we do not implement this - $c->reverse(true); + // sort most recently marked for special feeds, newest first otherwise + $order = (!$cat && ($id == self::FEED_READ || $id == self::FEED_STARRED)) ? ["marked_date desc"] : ["edited_date desc"]; break; } // set the limit and offset @@ -1514,6 +1515,6 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { $c->oldestArticle($data['since_id'] + 1); } // return results - return Arsse::$db->articleList(Arsse::$user->id, $c, $fields); + return Arsse::$db->articleList(Arsse::$user->id, $c, $fields, $order); } } diff --git a/tests/cases/Database/SeriesArticle.php b/tests/cases/Database/SeriesArticle.php index 17b0ece..fb547c1 100644 --- a/tests/cases/Database/SeriesArticle.php +++ b/tests/cases/Database/SeriesArticle.php @@ -460,7 +460,6 @@ trait SeriesArticle { 'Marked or labelled between 2000 and 2015' => [(new Context)->markedSince("2000-01-01T00:00:00Z")->notMarkedSince("2015-12-31T23:59:59Z"), [1,2,3,4,5,6,7,8,20]], 'Marked or labelled in 2010' => [(new Context)->markedSince("2010-01-01T00:00:00Z")->notMarkedSince("2010-12-31T23:59:59Z"), [2,4,6,20]], 'Paged results' => [(new Context)->limit(2)->oldestEdition(4), [4,5]], - 'Reversed paged results' => [(new Context)->limit(2)->latestEdition(7)->reverse(true), [7,6]], 'With label ID 1' => [(new Context)->label(1), [1,19]], 'With label ID 2' => [(new Context)->label(2), [1,5,20]], 'With label ID 1 or 2' => [(new Context)->labels([1,2]), [1,5,19,20]], From 12f23ddc164b332c3eb491c5b1c4bc80831ba11d Mon Sep 17 00:00:00 2001 From: "J. King" Date: Thu, 4 Apr 2019 17:21:23 -0400 Subject: [PATCH 10/32] Updated tests for arbitrary sorting --- lib/Database.php | 2 +- tests/cases/Database/SeriesArticle.php | 24 ++++++- tests/cases/REST/NextCloudNews/TestV1_2.php | 34 ++++----- tests/cases/REST/TinyTinyRSS/TestAPI.php | 80 ++++++++++----------- 4 files changed, 81 insertions(+), 59 deletions(-) diff --git a/lib/Database.php b/lib/Database.php index ff74f65..fa754fd 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -1549,7 +1549,7 @@ class Database { $fields = array_map($norm, $fields); $sort = array_map($norm, $sort); foreach ($sort as $spec) { - $col = explode(" ", $spec, 1); + $col = explode(" ", $spec, 2); $order = $col[1] ?? ""; $col = $col[0]; if ($order === "desc") { diff --git a/tests/cases/Database/SeriesArticle.php b/tests/cases/Database/SeriesArticle.php index fb547c1..d47f918 100644 --- a/tests/cases/Database/SeriesArticle.php +++ b/tests/cases/Database/SeriesArticle.php @@ -10,6 +10,7 @@ use JKingWeb\Arsse\Database; use JKingWeb\Arsse\Arsse; use JKingWeb\Arsse\Context\Context; use JKingWeb\Arsse\Misc\Date; +use JKingWeb\Arsse\Misc\ValueInfo; use Phake; trait SeriesArticle { @@ -508,6 +509,8 @@ trait SeriesArticle { 'Excluding tag ID 5' => [(new Context)->not->tag(5), [1,2,3,4,5,6]], 'Excluding tag "Technology"' => [(new Context)->not->tagName("Technology"), [1,2,3,4,19,20]], 'Excluding tag "Politics"' => [(new Context)->not->tagName("Politics"), [1,2,3,4,5,6]], + 'Excluding tags ID 1 and 5' => [(new Context)->not->tags([1,5]), [1,2,3,4]], + 'Excluding tags "Technology" and "Politics"' => [(new Context)->not->tagNames(["Technology","Politics"]), [1,2,3,4]], 'Excluding entire folder tree' => [(new Context)->not->folder(0), []], 'Excluding multiple folder trees' => [(new Context)->not->folders([1,5]), [1,2,3,4]], 'Excluding multiple folder trees including root' => [(new Context)->not->folders([0,1,5]), []], @@ -574,6 +577,25 @@ trait SeriesArticle { $this->assertEquals($this->fields, $test); } + /** @dataProvider provideOrderedLists */ + public function testListArticlesCheckingOrder(array $sortCols, array $exp) { + $act = ValueInfo::normalize(array_column(iterator_to_array(Arsse::$db->articleList("john.doe@example.com", null, ["id"], $sortCols)), "id"), ValueInfo::T_INT | ValueInfo::M_ARRAY); + $this->assertSame($exp, $act); + } + + public function provideOrderedLists() { + return [ + [["id"], [1,2,3,4,5,6,7,8,19,20]], + [["id asc"], [1,2,3,4,5,6,7,8,19,20]], + [["id desc"], [20,19,8,7,6,5,4,3,2,1]], + [["edition"], [1,2,3,4,5,6,7,8,19,20]], + [["edition asc"], [1,2,3,4,5,6,7,8,19,20]], + [["edition desc"], [20,19,8,7,6,5,4,3,2,1]], + [["id", "edition desk"], [1,2,3,4,5,6,7,8,19,20]], + [["id", "editio"], [1,2,3,4,5,6,7,8,19,20]], + ]; + } + public function testListArticlesWithoutAuthority() { Phake::when(Arsse::$user)->authorize->thenReturn(false); $this->assertException("notAuthorized", "User", "ExceptionAuthz"); @@ -1034,7 +1056,7 @@ trait SeriesArticle { /** @dataProvider provideArrayContextOptions */ public function testUseTooFewValuesInArrayContext(string $option) { $this->assertException("tooShort", "Db", "ExceptionInput"); - Arsse::$db->articleList($this->user, (new Context)->annotationTerms([])); + Arsse::$db->articleList($this->user, (new Context)->$option([])); } public function provideArrayContextOptions() { diff --git a/tests/cases/REST/NextCloudNews/TestV1_2.php b/tests/cases/REST/NextCloudNews/TestV1_2.php index 664db4e..52291cb 100644 --- a/tests/cases/REST/NextCloudNews/TestV1_2.php +++ b/tests/cases/REST/NextCloudNews/TestV1_2.php @@ -734,11 +734,11 @@ class TestV1_2 extends \JKingWeb\Arsse\Test\AbstractTest { ['lastModified' => $t->getTimestamp()], ['oldestFirst' => false, 'batchSize' => 5, 'offset' => 0], // offset=0 should not set the latestEdition context ]; - Phake::when(Arsse::$db)->articleList(Arsse::$user->id, $this->anything(), $this->anything())->thenReturn(new Result($this->v($this->articles['db']))); - Phake::when(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->subscription(42), $this->anything())->thenThrow(new ExceptionInput("idMissing")); - Phake::when(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->folder(2112), $this->anything())->thenThrow(new ExceptionInput("idMissing")); - Phake::when(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->subscription(-1), $this->anything())->thenThrow(new ExceptionInput("typeViolation")); - Phake::when(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->folder(-1), $this->anything())->thenThrow(new ExceptionInput("typeViolation")); + Phake::when(Arsse::$db)->articleList->thenReturn(new Result($this->v($this->articles['db']))); + Phake::when(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->subscription(42), $this->anything(), ["edition desc"])->thenThrow(new ExceptionInput("idMissing")); + Phake::when(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->folder(2112), $this->anything(), ["edition desc"])->thenThrow(new ExceptionInput("idMissing")); + Phake::when(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->subscription(-1), $this->anything(), ["edition desc"])->thenThrow(new ExceptionInput("typeViolation")); + Phake::when(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->folder(-1), $this->anything(), ["edition desc"])->thenThrow(new ExceptionInput("typeViolation")); $exp = new Response(['items' => $this->articles['rest']]); // check the contents of the response $this->assertMessage($exp, $this->req("GET", "/items")); // first instance of base context @@ -759,17 +759,17 @@ class TestV1_2 extends \JKingWeb\Arsse\Test\AbstractTest { $this->req("GET", "/items", json_encode($in[10])); $this->req("GET", "/items", json_encode($in[11])); // perform method verifications - Phake::verify(Arsse::$db, Phake::times(4))->articleList(Arsse::$user->id, (new Context)->reverse(true), $this->anything()); - Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->subscription(42), $this->anything()); - Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->folder(2112), $this->anything()); - Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->subscription(-1), $this->anything()); - Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->folder(-1), $this->anything()); - Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->starred(true), $this->anything()); - Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(false)->limit(10)->oldestEdition(6), $this->anything()); // offset is one more than specified - Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->limit(5)->latestEdition(4), $this->anything()); // offset is one less than specified - Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->unread(true), $this->anything()); - Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, $this->equalTo((new Context)->reverse(true)->markedSince($t), 2), $this->anything()); - Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->limit(5), $this->anything()); + Phake::verify(Arsse::$db, Phake::times(4))->articleList(Arsse::$user->id, new Context, $this->anything(), ["edition desc"]); + Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->subscription(42), $this->anything(), ["edition desc"]); + Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->folder(2112), $this->anything(), ["edition desc"]); + Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->subscription(-1), $this->anything(), ["edition desc"]); + Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->folder(-1), $this->anything(), ["edition desc"]); + Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->starred(true), $this->anything(), ["edition desc"]); + Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->limit(10)->oldestEdition(6), $this->anything(), ["edition"]); // offset is one more than specified + Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->limit(5)->latestEdition(4), $this->anything(), ["edition desc"]); // offset is one less than specified + Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->unread(true), $this->anything(), ["edition desc"]); + Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, $this->equalTo((new Context)->markedSince($t), 2), $this->anything(), ["edition desc"]); + Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->limit(5), $this->anything(), ["edition desc"]); } public function testMarkAFolderRead() { @@ -958,6 +958,6 @@ class TestV1_2 extends \JKingWeb\Arsse\Test\AbstractTest { $url = "/items?type=2"; Phake::when(Arsse::$db)->articleList->thenReturn(new Result([])); $this->req("GET", $url, json_encode($in)); - Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(false)->starred(true), $this->anything()); + Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->starred(true), $this->anything(), ["edition"]); } } diff --git a/tests/cases/REST/TinyTinyRSS/TestAPI.php b/tests/cases/REST/TinyTinyRSS/TestAPI.php index 91b370c..dfe4077 100644 --- a/tests/cases/REST/TinyTinyRSS/TestAPI.php +++ b/tests/cases/REST/TinyTinyRSS/TestAPI.php @@ -1749,19 +1749,19 @@ LONG_STRING; Phake::when(Arsse::$db)->articleList->thenReturn(new Result($this->v([['id' => 0]]))); Phake::when(Arsse::$db)->articleCount->thenReturn(0); Phake::when(Arsse::$db)->articleCount($this->anything(), (new Context)->unread(true))->thenReturn(1); - $c = (new Context)->reverse(true); - Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->subscription(2112), ["id"])->thenThrow(new ExceptionInput("subjectMissing")); - Phake::when(Arsse::$db)->articleList($this->anything(), $c, ["id"])->thenReturn(new Result($this->v($this->articles))); - Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->starred(true), ["id"])->thenReturn(new Result($this->v([['id' => 1]]))); - Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->label(1088), ["id"])->thenReturn(new Result($this->v([['id' => 2]]))); - Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->unread(true), ["id"])->thenReturn(new Result($this->v([['id' => 3]]))); - Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->label(1088)->unread(true), ["id"])->thenReturn(new Result($this->v([['id' => 4]]))); - Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->subscription(42)->starred(true), ["id"])->thenReturn(new Result($this->v([['id' => 5]]))); - Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->subscription(42)->annotated(true), ["id"])->thenReturn(new Result($this->v([['id' => 6]]))); - Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->limit(5), ["id"])->thenReturn(new Result($this->v([['id' => 7]]))); - Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->offset(2), ["id"])->thenReturn(new Result($this->v([['id' => 8]]))); - Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->limit(5)->offset(2), ["id"])->thenReturn(new Result($this->v([['id' => 9]]))); - Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->oldestArticle(48), ["id"])->thenReturn(new Result($this->v([['id' => 10]]))); + $c = (new Context); + Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->subscription(2112), ["id"], ["edited_date desc"])->thenThrow(new ExceptionInput("subjectMissing")); + Phake::when(Arsse::$db)->articleList($this->anything(), $c, ["id"], ["edited_date desc"])->thenReturn(new Result($this->v($this->articles))); + Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->starred(true), ["id"], ["marked_date desc"])->thenReturn(new Result($this->v([['id' => 1]]))); + Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->label(1088), ["id"], ["edited_date desc"])->thenReturn(new Result($this->v([['id' => 2]]))); + Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->unread(true), ["id"], ["edited_date desc"])->thenReturn(new Result($this->v([['id' => 3]]))); + Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->label(1088)->unread(true), ["id"], ["edited_date desc"])->thenReturn(new Result($this->v([['id' => 4]]))); + Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->subscription(42)->starred(true), ["id"], ["edited_date desc"])->thenReturn(new Result($this->v([['id' => 5]]))); + Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->subscription(42)->annotated(true), ["id"], ["edited_date desc"])->thenReturn(new Result($this->v([['id' => 6]]))); + Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->limit(5), ["id"], ["edited_date desc"])->thenReturn(new Result($this->v([['id' => 7]]))); + Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->offset(2), ["id"], ["edited_date desc"])->thenReturn(new Result($this->v([['id' => 8]]))); + Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->limit(5)->offset(2), ["id"], ["edited_date desc"])->thenReturn(new Result($this->v([['id' => 9]]))); + Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->oldestArticle(48), ["id"], ["edited_date desc"])->thenReturn(new Result($this->v([['id' => 10]]))); $out1 = [ $this->respErr("INCORRECT_USAGE"), $this->respGood([]), @@ -1793,9 +1793,9 @@ LONG_STRING; $this->assertMessage($out1[$a], $this->req($in1[$a]), "Test $a failed"); } for ($a = 0; $a < sizeof($in2); $a++) { - Phake::when(Arsse::$db)->articleList($this->anything(), $this->equalTo((clone $c)->unread(false)->markedSince(Date::sub("PT24H")), 2), ["id"])->thenReturn(new Result($this->v([['id' => 1001]]))); - Phake::when(Arsse::$db)->articleList($this->anything(), $this->equalTo((clone $c)->unread(true)->modifiedSince(Date::sub("PT24H")), 2), ["id"])->thenReturn(new Result($this->v([['id' => 1002]]))); - Phake::when(Arsse::$db)->articleList($this->anything(), $this->equalTo((clone $c)->unread(true)->modifiedSince(Date::sub("PT24H"))->starred(true), 2), ["id"])->thenReturn(new Result($this->v([['id' => 1003]]))); + Phake::when(Arsse::$db)->articleList($this->anything(), $this->equalTo((clone $c)->unread(false)->markedSince(Date::sub("PT24H")), 2), ["id"], ["marked_date desc"])->thenReturn(new Result($this->v([['id' => 1001]]))); + Phake::when(Arsse::$db)->articleList($this->anything(), $this->equalTo((clone $c)->unread(true)->modifiedSince(Date::sub("PT24H")), 2), ["id"], ["edited_date desc"])->thenReturn(new Result($this->v([['id' => 1002]]))); + Phake::when(Arsse::$db)->articleList($this->anything(), $this->equalTo((clone $c)->unread(true)->modifiedSince(Date::sub("PT24H"))->starred(true), 2), ["id"], ["edited_date desc"])->thenReturn(new Result($this->v([['id' => 1003]]))); $this->assertMessage($out2[$a], $this->req($in2[$a]), "Test $a failed"); } } @@ -1853,25 +1853,25 @@ LONG_STRING; Phake::when(Arsse::$db)->articleList->thenReturn($this->generateHeadlines(0)); Phake::when(Arsse::$db)->articleCount->thenReturn(0); Phake::when(Arsse::$db)->articleCount($this->anything(), (new Context)->unread(true))->thenReturn(1); - $c = (new Context)->limit(200)->reverse(true); - Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->subscription(2112), $this->anything())->thenThrow(new ExceptionInput("subjectMissing")); - Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->starred(true), $this->anything())->thenReturn($this->generateHeadlines(1)); - Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->label(1088), $this->anything())->thenReturn($this->generateHeadlines(2)); - Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->unread(true), $this->anything())->thenReturn($this->generateHeadlines(3)); - Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->label(1088)->unread(true), $this->anything())->thenReturn($this->generateHeadlines(4)); - Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->subscription(42)->starred(true), $this->anything())->thenReturn($this->generateHeadlines(5)); - Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->subscription(42)->annotated(true), $this->anything())->thenReturn($this->generateHeadlines(6)); - Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->limit(5), $this->anything())->thenReturn($this->generateHeadlines(7)); - Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->offset(2), $this->anything())->thenReturn($this->generateHeadlines(8)); - Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->limit(5)->offset(2), $this->anything())->thenReturn($this->generateHeadlines(9)); - Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->oldestArticle(48), $this->anything())->thenReturn($this->generateHeadlines(10)); - Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c), $this->anything())->thenReturn($this->generateHeadlines(11)); - Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->labelled(true), $this->anything())->thenReturn($this->generateHeadlines(12)); - Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->folderShallow(0), $this->anything())->thenReturn($this->generateHeadlines(13)); - Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->folderShallow(42), $this->anything())->thenReturn($this->generateHeadlines(14)); - Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->folder(42), $this->anything())->thenReturn($this->generateHeadlines(15)); - Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->reverse(false), $this->anything())->thenReturn($this->generateHeadlines(16)); - Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->subscription(42)->searchTerms(["interesting"]), $this->anything())->thenReturn($this->generateHeadlines(17)); + $c = (new Context)->limit(200); + Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->subscription(2112), $this->anything(), ["edited_date desc"])->thenThrow(new ExceptionInput("subjectMissing")); + Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->starred(true), $this->anything(), ["marked_date desc"])->thenReturn($this->generateHeadlines(1)); + Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->label(1088), $this->anything(), ["edited_date desc"])->thenReturn($this->generateHeadlines(2)); + Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->unread(true), $this->anything(), ["edited_date desc"])->thenReturn($this->generateHeadlines(3)); + Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->label(1088)->unread(true), $this->anything(), ["edited_date desc"])->thenReturn($this->generateHeadlines(4)); + Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->subscription(42)->starred(true), $this->anything(), ["edited_date desc"])->thenReturn($this->generateHeadlines(5)); + Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->subscription(42)->annotated(true), $this->anything(), ["edited_date desc"])->thenReturn($this->generateHeadlines(6)); + Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->limit(5), $this->anything(), ["edited_date desc"])->thenReturn($this->generateHeadlines(7)); + Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->offset(2), $this->anything(), ["edited_date desc"])->thenReturn($this->generateHeadlines(8)); + Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->limit(5)->offset(2), $this->anything(), ["edited_date desc"])->thenReturn($this->generateHeadlines(9)); + Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->oldestArticle(48), $this->anything(), ["edited_date desc"])->thenReturn($this->generateHeadlines(10)); + Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c), $this->anything(), ["edited_date desc"])->thenReturn($this->generateHeadlines(11)); + Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->labelled(true), $this->anything(), ["edited_date desc"])->thenReturn($this->generateHeadlines(12)); + Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->folderShallow(0), $this->anything(), ["edited_date desc"])->thenReturn($this->generateHeadlines(13)); + Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->folderShallow(42), $this->anything(), ["edited_date desc"])->thenReturn($this->generateHeadlines(14)); + Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->folder(42), $this->anything(), ["edited_date desc"])->thenReturn($this->generateHeadlines(15)); + Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c), $this->anything(), ["edited_date"])->thenReturn($this->generateHeadlines(16)); + Phake::when(Arsse::$db)->articleList($this->anything(), (clone $c)->subscription(42)->searchTerms(["interesting"]), $this->anything(), ["edited_date desc"])->thenReturn($this->generateHeadlines(17)); $out2 = [ $this->respErr("INCORRECT_USAGE"), $this->outputHeadlines(11), @@ -1909,9 +1909,9 @@ LONG_STRING; $this->assertMessage($out2[$a], $this->req($in2[$a]), "Test $a failed"); } for ($a = 0; $a < sizeof($in3); $a++) { - Phake::when(Arsse::$db)->articleList($this->anything(), $this->equalTo((clone $c)->unread(false)->markedSince(Date::sub("PT24H")), 2), $this->anything())->thenReturn($this->generateHeadlines(1001)); - Phake::when(Arsse::$db)->articleList($this->anything(), $this->equalTo((clone $c)->unread(true)->modifiedSince(Date::sub("PT24H")), 2), $this->anything())->thenReturn($this->generateHeadlines(1002)); - Phake::when(Arsse::$db)->articleList($this->anything(), $this->equalTo((clone $c)->unread(true)->modifiedSince(Date::sub("PT24H"))->starred(true), 2), $this->anything())->thenReturn($this->generateHeadlines(1003)); + Phake::when(Arsse::$db)->articleList($this->anything(), $this->equalTo((clone $c)->unread(false)->markedSince(Date::sub("PT24H")), 2), $this->anything(), ["marked_date desc"])->thenReturn($this->generateHeadlines(1001)); + Phake::when(Arsse::$db)->articleList($this->anything(), $this->equalTo((clone $c)->unread(true)->modifiedSince(Date::sub("PT24H")), 2), $this->anything(), ["edited_date desc"])->thenReturn($this->generateHeadlines(1002)); + Phake::when(Arsse::$db)->articleList($this->anything(), $this->equalTo((clone $c)->unread(true)->modifiedSince(Date::sub("PT24H"))->starred(true), 2), $this->anything(), ["edited_date desc"])->thenReturn($this->generateHeadlines(1003)); $this->assertMessage($out3[$a], $this->req($in3[$a]), "Test $a failed"); } } @@ -1990,7 +1990,7 @@ LONG_STRING; ]); $this->assertMessage($exp, $test); // test 'include_header' with an erroneous result - Phake::when(Arsse::$db)->articleList($this->anything(), (new Context)->limit(200)->reverse(true)->subscription(2112), $this->anything())->thenThrow(new ExceptionInput("subjectMissing")); + Phake::when(Arsse::$db)->articleList($this->anything(), (new Context)->limit(200)->subscription(2112), $this->anything(), ["edited_date desc"])->thenThrow(new ExceptionInput("subjectMissing")); $test = $this->req($in[6]); $exp = $this->respGood([ ['id' => 2112, 'is_cat' => false, 'first_id' => 0], @@ -2005,7 +2005,7 @@ LONG_STRING; ]); $this->assertMessage($exp, $test); // test 'include_header' with skip - Phake::when(Arsse::$db)->articleList($this->anything(), (new Context)->reverse(true)->limit(1)->subscription(42), $this->anything())->thenReturn($this->generateHeadlines(1867)); + Phake::when(Arsse::$db)->articleList($this->anything(), (new Context)->limit(1)->subscription(42), $this->anything(), ["edited_date desc"])->thenReturn($this->generateHeadlines(1867)); $test = $this->req($in[8]); $exp = $this->respGood([ ['id' => 42, 'is_cat' => false, 'first_id' => 1867], From c6d241e653345bd160ceac0f6be98982d4789e1b Mon Sep 17 00:00:00 2001 From: "J. King" Date: Thu, 4 Apr 2019 17:57:12 -0400 Subject: [PATCH 11/32] Implement Fever item list --- lib/REST/Fever/API.php | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/lib/REST/Fever/API.php b/lib/REST/Fever/API.php index 5dcb9b0..0b79c48 100644 --- a/lib/REST/Fever/API.php +++ b/lib/REST/Fever/API.php @@ -225,4 +225,41 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { } return $out; } + + protected function getItems(array $G): array { + $c = (new Context)->limit(50); + $reverse = false; + // handle the standard options + if ($G['with_ids']) { + $c->articles(explode(",", $G['with_ids'])); + } elseif ($G['since_id']) { + $c->oldestArticle($G['since_id'] + 1); + } elseif ($G['max_id']) { + $c->newestArticle($G['max_id'] - 1); + $reverse = true; + } + // handle the undocumented options + if ($G['group_ids']) { + $c->tags(explode(",", $G['group_ids'])); + } + if ($G['feed_ids']) { + $c->subscriptions(explode(",", $G['feed_ids'])); + } + // get results + $out = []; + $order = $reverse ? "id desc" : "id"; + foreach (Arsse::$db->articleList(Arsse::$user->id, $c, ["id", "subscription", "title", "author", "content", "url", "starred", "unread", "published_date"], [$order]) as $r) { + $out[] = [ + 'id' => (int) $r['id'], + 'feed_id' => (int) $r['subscription'], + 'title' => (string) $r['title'], + 'author' => (string) $r['author'], + 'html' => (string) $r['content'], + 'is_saved' => (int) $r['starred'], + 'is_read' => (int) !$r['unread'], + 'created_on_time' => Date::transform($r['published_date'], "unix", "sql"), + ]; + } + return $out; + } } From 7c85e837df2b6f92c64387bca82b39e965824219 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Thu, 4 Apr 2019 18:01:57 -0400 Subject: [PATCH 12/32] Documentation update --- CHANGELOG | 4 ++++ README.md | 1 - 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index edc4b0a..79a1be5 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -5,6 +5,10 @@ New features: - Support for the Fever protocol (see README.md for details) - Command line functionality for clearing a password, disabling the account - Command line options for dealing with Fever passwords +- Command line functionality for exporting subscriptions to OPML + +Bug fixes: +- Sort Tiny Tiny RSS special feeds according to special ordering Version 0.7.1 (2019-03-25) ========================== diff --git a/README.md b/README.md index ab7dc2e..6760072 100644 --- a/README.md +++ b/README.md @@ -148,7 +148,6 @@ We are not aware of any other extensions to the TTRSS protocol. If you know of a - Full-text search is not yet employed with any database, including PostgreSQL - Article hashes are normally SHA1; The Arsse uses SHA256 hashes - Article attachments normally have unique IDs; The Arsse always gives attachments an ID of `"0"` -- The default sort order of the `getHeadlines` operation normally uses custom sorting for "special" feeds; The Arsse's default sort order is equivalent to `feed_dates` for all feeds - The `getCounters` operation normally omits members with zero unread; The Arsse includes everything to appease some clients #### Other notes From 982f09c9aa78674e13bfc467fab1756c57baba3e Mon Sep 17 00:00:00 2001 From: "J. King" Date: Thu, 4 Apr 2019 18:05:26 -0400 Subject: [PATCH 13/32] Upgrade notes --- UPGRADING | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/UPGRADING b/UPGRADING index a837396..ea3c84a 100644 --- a/UPGRADING +++ b/UPGRADING @@ -10,6 +10,12 @@ usually prudent: - If installing from source, update dependencies with: `composer install -o --no-dev` +Upgrading from 0.7.1 to 0.8.0 +============================= + +- The database schema has changed from rev4 to rev5; if upgrading the database + manually, apply the 4.sql file + Upgrading from 0.5.1 to 0.6.0 ============================= From 0752e9cf3dc0352e9a7af8de3ddcdba68d622b30 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Thu, 4 Apr 2019 19:37:48 -0400 Subject: [PATCH 14/32] Implement Fever sync --- lib/REST/Fever/API.php | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/lib/REST/Fever/API.php b/lib/REST/Fever/API.php index 0b79c48..62212f8 100644 --- a/lib/REST/Fever/API.php +++ b/lib/REST/Fever/API.php @@ -138,6 +138,12 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { // TODO: implement hot links $out['inks'] = []; } + if ($G['unread_item_ids']) { + $out['unread_item_ids'] = $this->getItemIds((new Context)->unread(true)); + } + if ($G['saved_item_ids']) { + $out['saved_item_ids'] = $this->getItemIds((new Context)->starred(true)); + } return $out; } @@ -262,4 +268,12 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { } return $out; } + + protected function getItemIds(Context $c = null): array { + $out = []; + foreach (Arsse::$db->articleList(Arsse::$user->id, $c) as $r) { + $out[] = (int) $r['id']; + } + return $out; + } } From 0ef606aa03caed72da3e1abe0b78e932f324956b Mon Sep 17 00:00:00 2001 From: "J. King" Date: Fri, 5 Apr 2019 08:20:05 -0400 Subject: [PATCH 15/32] Return string list of item IDs --- lib/REST/Fever/API.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/REST/Fever/API.php b/lib/REST/Fever/API.php index 62212f8..2504463 100644 --- a/lib/REST/Fever/API.php +++ b/lib/REST/Fever/API.php @@ -269,11 +269,11 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { return $out; } - protected function getItemIds(Context $c = null): array { + protected function getItemIds(Context $c = null): string { $out = []; foreach (Arsse::$db->articleList(Arsse::$user->id, $c) as $r) { $out[] = (int) $r['id']; } - return $out; + return implode(",", $out); } } From e3d2215920538045001eacfcd770dcc1ef09b41b Mon Sep 17 00:00:00 2001 From: "J. King" Date: Fri, 5 Apr 2019 11:03:15 -0400 Subject: [PATCH 16/32] Style fixes --- lib/CLI.php | 1 + lib/Context/ExclusionContext.php | 2 +- lib/Database.php | 242 ++++++++++++------------- lib/Db/Driver.php | 14 +- lib/Db/SQLite3/PDODriver.php | 4 +- lib/Db/SQLite3/PDOStatement.php | 2 +- lib/Db/Statement.php | 2 +- lib/REST/TinyTinyRSS/API.php | 2 +- lib/REST/TinyTinyRSS/Search.php | 8 +- tests/cases/Database/SeriesArticle.php | 6 +- tests/cases/REST/Fever/TestAPI.php | 1 - 11 files changed, 145 insertions(+), 139 deletions(-) diff --git a/lib/CLI.php b/lib/CLI.php index 9693698..617a68b 100644 --- a/lib/CLI.php +++ b/lib/CLI.php @@ -128,6 +128,7 @@ USAGE_TEXT; } else { return $this->userAddOrSetPassword("passwordSet", $args[""], $args[""], $args["--oldpass"]); } + // no break case "unset-pass": if ($args['--fever']) { $this->getFever()->unregister($args[""]); diff --git a/lib/Context/ExclusionContext.php b/lib/Context/ExclusionContext.php index 7cf45cb..e7323ea 100644 --- a/lib/Context/ExclusionContext.php +++ b/lib/Context/ExclusionContext.php @@ -87,7 +87,7 @@ class ExclusionContext { $spec[$a] = null; } } - return array_values(array_unique(array_filter($spec, function ($v) { + return array_values(array_unique(array_filter($spec, function($v) { return !is_null($v); }))); } diff --git a/lib/Database.php b/lib/Database.php index fa754fd..69f1ae1 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -14,9 +14,9 @@ use JKingWeb\Arsse\Misc\Date; use JKingWeb\Arsse\Misc\ValueInfo; /** The high-level interface with the database - * + * * The database stores information on the following things: - * + * * - Users * - Subscriptions to feeds, which belong to users * - Folders, which belong to users and contain subscriptions @@ -28,9 +28,9 @@ use JKingWeb\Arsse\Misc\ValueInfo; * - Sessions, used by some protocols to identify users across periods of time * - Tokens, similar to sessions, but with more control over their properties * - Metadata, used internally by the server - * + * * The various methods of this class perform operations on these things, with - * each public method prefixed with the thing it concerns e.g. userRemove() + * each public method prefixed with the thing it concerns e.g. userRemove() * deletes a user from the database, and labelArticlesSet() changes a label's * associations with articles. There has been an effort to keep public method * names consistent throughout, but protected methods, having different @@ -54,7 +54,7 @@ class Database { public $db; /** Constructs the database interface - * + * * @param boolean $initialize Whether to attempt to upgrade the databse schema when constructing */ public function __construct($initialize = true) { @@ -71,7 +71,7 @@ class Database { return debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 3)[2]['function']; } - /** Lists the available database drivers, as an associative array with + /** Lists the available database drivers, as an associative array with * fully-qualified class names as keys, and human-readable descriptions as values */ public static function driverList(): array { @@ -105,9 +105,9 @@ class Database { } /** Computes the column and value text of an SQL "SET" clause, validating arbitrary input against a whitelist - * + * * Returns an indexed array containing the clause text, an array of types, and another array of values - * + * * @param array $props An associative array containing untrusted data; keys are column names * @param array $valid An associative array containing a whitelist: keys are column names, and values are strings representing data types */ @@ -130,9 +130,9 @@ class Database { } /** Computes the contents of an SQL "IN()" clause, for each input value either embedding the value or producing a parameter placeholder - * + * * Returns an indexed array containing the clause text, an array of types, and an array of values. Note that the array of output values may not match the array of input values - * + * * @param array $values Arbitrary values * @param string $type A single data type applied to each value */ @@ -147,7 +147,7 @@ class Database { $params = []; $count = 0; $convType = Db\AbstractStatement::TYPE_NORM_MAP[Statement::TYPES[$type]]; - foreach($values as $v) { + foreach ($values as $v) { $v = ValueInfo::normalize($v, $convType, null, "sql"); if (is_null($v)) { // nulls are pointless to have @@ -176,11 +176,11 @@ class Database { } /** Computes basic LIKE-based text search constraints for use in a WHERE clause - * + * * Returns an indexed array containing the clause text, an array of types, and another array of values - * + * * The clause is structured such that all terms must be present across any of the columns - * + * * @param string[] $terms The terms to search for * @param string[] $cols The columns to match against; these are -not- sanitized, so much -not- come directly from user input * @param boolean $matchAny Whether the search is successful when it matches any (true) or all (false) terms @@ -194,7 +194,7 @@ class Database { $values = []; $like = $this->db->sqlToken("like"); $embedSet = sizeof($terms) > ((int) (self::LIMIT_SET_SIZE / sizeof($cols))); - foreach($terms as $term) { + foreach ($terms as $term) { $embedTerm = ($embedSet && strlen($term) <= self::LIMIT_SET_STRING_LENGTH); $term = str_replace(["%", "_", "^"], ["^%", "^_", "^^"], $term); $term = "%$term%"; @@ -249,7 +249,7 @@ class Database { } /** Adds a user to the database - * + * * @param string $user The user to add * @param string $passwordThe user's password in cleartext. It will be stored hashed */ @@ -298,7 +298,7 @@ class Database { } /** Sets the password of an existing user - * + * * @param string $user The user for whom to set the password * @param string $password The new password, in cleartext. The password will be stored hashed. If null is passed, the password is unset and authentication not possible */ @@ -329,10 +329,10 @@ class Database { } /** Explicitly removes a session from the database - * - * Sessions may also be invalidated as they expire, and then be automatically pruned. + * + * Sessions may also be invalidated as they expire, and then be automatically pruned. * This function can be used to explicitly invalidate a session after a user logs out - * + * * @param string $user The user who owns the session to be destroyed * @param string $id The identifier of the session to destroy */ @@ -346,7 +346,7 @@ class Database { } /** Resumes a session, returning available session data - * + * * This also has the side effect of refreshing the session if it is near its timeout */ public function sessionResume(string $id): array { @@ -380,8 +380,8 @@ class Database { return (($now + $diff) >= $expiry->getTimestamp()); } - /** Creates a new token for the given user in the given class - * + /** Creates a new token for the given user in the given class + * * @param string $user The user for whom to create the token * @param string $class The class of the token e.g. the protocol name * @param string|null $id The value of the token; if none is provided a UUID will be generated @@ -403,7 +403,7 @@ class Database { } /** Revokes one or all tokens for a user in a class - * + * * @param string $user The user who owns the token to be revoked * @param string $class The class of the token e.g. the protocol name * @param string|null $id The ID of a specific token, or null for all tokens in the class @@ -436,14 +436,14 @@ class Database { } /** Adds a folder for containing newsfeed subscriptions, returning an integer identifying the created folder - * + * * The $data array may contain the following keys: - * + * * - "name": A folder name, which must be a non-empty string not composed solely of whitespace; this key is required * - "parent": An integer (or null) identifying a parent folder; this key is optional - * + * * If a folder with the same name and parent already exists, this is an error - * + * * @param string $user The user who will own the folder * @param array $data An associative array defining the folder */ @@ -462,15 +462,15 @@ class Database { } /** Returns a result set listing a user's folders - * + * * Each record in the result set contains: - * + * * - "id": The folder identifier, an integer * - "name": The folder's name, a string * - "parent": The integer identifier of the folder's parent, or null * - "children": The number of child folders contained in the given folder - * - "feeds": The number of newsfeed subscriptions contained in the given folder, not including subscriptions in descendent folders - * + * - "feeds": The number of newsfeed subscriptions contained in the given folder, not including subscriptions in descendent folders + * * @param string $uer The user whose folders are to be listed * @param integer|null $parent Restricts the list to the descendents of the specified folder identifier * @param boolean $recursive Whether to list all descendents (true) or only direct children (false) @@ -505,9 +505,9 @@ class Database { } /** Deletes a folder from the database - * + * * Any descendent folders are also deleted, as are all newsfeed subscriptions contained in the deleted folder tree - * + * * @param string $user The user to whom the folder to be deleted belongs * @param integer $id The identifier of the folder to delete */ @@ -541,14 +541,14 @@ class Database { } /** Modifies the properties of a folder - * + * * The $data array must contain one or more of the following keys: - * + * * - "name": A new folder name, which must be a non-empty string not composed solely of whitespace * - "parent": An integer (or null) identifying a parent folder - * + * * If a folder with the new name and parent combination already exists, this is an error; it is also an error to move a folder to itself or one of its descendents - * + * * @param string $user The user who owns the folder to be modified * @param integer $id The identifier of the folder to be modified * @param array $data An associative array of properties to modify. Anything not specified will remain unchanged @@ -590,9 +590,9 @@ class Database { } /** Ensures the specified folder exists and raises an exception otherwise - * - * Returns an associative array containing the id, name, and parent of the folder if it exists - * + * + * Returns an associative array containing the id, name, and parent of the folder if it exists + * * @param string $user The user who owns the folder to be validated * @param integer|null $id The identifier of the folder to validate; null or zero represent the implied root folder * @param boolean $subject Whether the folder is the subject (true) rather than the object (false) of the operation being performed; this only affects the semantics of the error message if validation fails @@ -668,7 +668,7 @@ class Database { } /** Ensures a prospective folder name is valid, and optionally ensure it is not a duplicate if renamed - * + * * @param string $name The name to check * @param boolean $checkDuplicates Whether to also check if the new name would cause a collision * @param integer|null $parent The parent folder context in which to check for duplication @@ -695,7 +695,7 @@ class Database { } /** Adds a subscription to a newsfeed, and returns the numeric identifier of the added subscription - * + * * @param string $user The user which will own the subscription * @param string $url The URL of the newsfeed or discovery source * @param string $fetchUser The user name required to access the newsfeed, if applicable @@ -731,7 +731,7 @@ class Database { } /** Lists a user's subscriptions, returning various data - * + * * @param string $user The user whose subscriptions are to be listed * @param integer|null $folder The identifier of the folder under which to list subscriptions; by default the root folder is used * @param boolean $recursive Whether to list subscriptions of descendent folders as well as the selected folder @@ -802,8 +802,8 @@ class Database { } /** Deletes a subscription from the database - * - * This has the side effect of deleting all marks the user has set on articles + * + * This has the side effect of deleting all marks the user has set on articles * belonging to the newsfeed, but may not delete the articles themselves, as * other users may also be subscribed to the same newsfeed. There is also a * configurable retention period for newsfeeds @@ -823,7 +823,7 @@ class Database { } /** Retrieves data about a particular subscription, as an associative array with the following keys: - * + * * - "id": The numeric identifier of the subscription * - "feed": The numeric identifier of the underlying newsfeed * - "url": The URL of the newsfeed, after discovery and HTTP redirects @@ -855,14 +855,14 @@ class Database { } /** Modifies the properties of a subscription - * + * * The $data array must contain one or more of the following keys: - * + * * - "title": The title of the newsfeed * - "folder": The numeric identifier (or null) of the subscription's folder * - "pinned": Whether the subscription is pinned * - "order_type": Whether articles should be sorted in reverse cronological order (2), chronological order (1), or the default (0) - * + * * @param string $user The user whose subscription is to be modified * @param integer $id the numeric identifier of the subscription to modfify * @param array $data An associative array of properties to modify; any keys not specified will be left unchanged @@ -908,7 +908,7 @@ class Database { } /** Returns an indexed array listing the tags assigned to a subscription - * + * * @param string $user The user whose tags are to be listed * @param integer $id The numeric identifier of the subscription whose tags are to be listed * @param boolean $byName Whether to return the tag names (true) instead of the numeric tag identifiers (false) @@ -924,14 +924,14 @@ class Database { } /** Retrieves the URL of the icon for a subscription. - * + * * Note that while the $user parameter is optional, it - * is NOT recommended to omit it, as this can lead to - * leaks of private information. The parameter is only + * is NOT recommended to omit it, as this can lead to + * leaks of private information. The parameter is only * optional because this is required for Tiny Tiny RSS, * the original implementation of which leaks private * information due to a design flaw. - * + * * @param integer $id The numeric identifier of the subscription * @param string|null $user The user who owns the subscription being queried */ @@ -965,9 +965,9 @@ class Database { } /** Ensures the specified subscription exists and raises an exception otherwise - * + * * Returns an associative array containing the id of the subscription and the id of the underlying newsfeed - * + * * @param string $user The user who owns the subscription to be validated * @param integer $id The identifier of the subscription to validate * @param boolean $subject Whether the subscription is the subject (true) rather than the object (false) of the operation being performed; this only affects the semantics of the error message if validation fails @@ -990,7 +990,7 @@ class Database { } /** Attempts to refresh a newsfeed, returning an indication of success - * + * * @param integer $feedID The numerical identifier of the newsfeed to refresh * @param boolean $throwError Whether to throw an exception on failure in addition to storing error information in the database */ @@ -1146,7 +1146,7 @@ class Database { } /** Deletes orphaned newsfeeds from the database - * + * * Newsfeeds are orphaned if no users are subscribed to them. Deleting a newsfeed also deletes its articles */ public function feedCleanup(): bool { @@ -1167,14 +1167,14 @@ class Database { } /** Retrieves various identifiers for the latest $count articles in the given newsfeed. The identifiers are: - * + * * - "id": The database record key for the article * - "guid": The (theoretically) unique identifier for the article * - "edited": The time at which the article was last edited, per the newsfeed * - "url_title_hash": A cryptographic hash of the article URL and its title * - "url_content_hash": A cryptographic hash of the article URL and its content * - "title_content_hash": A cryptographic hash of the article title and its content - * + * * @param integer $feedID The numeric identifier of the feed * @param integer $count The number of records to return */ @@ -1187,14 +1187,14 @@ class Database { } /** Retrieves various identifiers for articles in the given newsfeed which match the input identifiers. The output identifiers are: - * + * * - "id": The database record key for the article * - "guid": The (theoretically) unique identifier for the article * - "edited": The time at which the article was last edited, per the newsfeed * - "url_title_hash": A cryptographic hash of the article URL and its title * - "url_content_hash": A cryptographic hash of the article URL and its content * - "title_content_hash": A cryptographic hash of the article title and its content - * + * * @param integer $feedID The numeric identifier of the feed * @param array $ids An array of GUIDs of articles * @param array $hashesUT An array of hashes of articles' URL and title @@ -1219,7 +1219,7 @@ class Database { } /** Returns an associative array of result column names and their SQL computations for article queries - * + * * This is used for whitelisting and defining both output column and order-by columns, as well as for resolution of some context options */ protected function articleColumns(): array { @@ -1250,9 +1250,9 @@ class Database { } /** Computes an SQL query to find and retrieve data about articles in the database - * + * * If an empty column list is supplied, a count of articles matching the context is queried instead - * + * * @param string $user The user whose articles are to be queried * @param Context $context The search context * @param array $cols The columns to request in the result set @@ -1270,7 +1270,7 @@ class Database { } if ($context->edition()) { $this->articleValidateEdition($user, $context->edition); - } + } if ($context->article()) { $this->articleValidateId($user, $context->article); } @@ -1356,7 +1356,7 @@ class Database { } elseif ($pair && $context->$pair()) { // option is paired with another which is also being used if ($op === ">=") { - $q->setWhere("{$colDefs[$col]} BETWEEN ? AND ?", [$type, $type], [$context->$m, $context->$pair]); + $q->setWhere("{$colDefs[$col]} BETWEEN ? AND ?", [$type, $type], [$context->$m, $context->$pair]); } else { // option has already been paired continue; @@ -1380,7 +1380,7 @@ class Database { } elseif ($pair && $context->not->$pair()) { // option is paired with another which is also being used if ($op === ">=") { - $q->setWhereNot("{$colDefs[$col]} BETWEEN ? AND ?", [$type, $type], [$context->not->$m, $context->not->$pair]); + $q->setWhereNot("{$colDefs[$col]} BETWEEN ? AND ?", [$type, $type], [$context->not->$m, $context->not->$pair]); } else { // option has already been paired continue; @@ -1458,7 +1458,7 @@ class Database { } } if ($seen) { - $spec = $opt['cte_name']."(".implode(",",$opt['cte_cols']).")"; + $spec = $opt['cte_name']."(".implode(",", $opt['cte_cols']).")"; $q->setCTE($spec, $opt['cte_body'], $opt['cte_types'], $opt['cte_values']); } } @@ -1525,9 +1525,9 @@ class Database { } /** Lists articles in the database which match a given query context - * + * * If an empty column list is supplied, a count of articles is returned instead - * + * * @param string $user The user whose articles are to be listed * @param Context $context The search context * @param array $fieldss The columns to return in the result set, any of: id, edition, url, title, author, content, guid, fingerprint, folder, subscription, feed, starred, unread, note, published_date, edited_date, modified_date, marked_date, subscription_title, media_url, media_type @@ -1576,7 +1576,7 @@ class Database { } /** Returns a count of articles which match the given query context - * + * * @param string $user The user whose articles are to be counted * @param Context $context The search context */ @@ -1590,13 +1590,13 @@ class Database { } /** Applies one or multiple modifications to all articles matching the given query context - * + * * The $data array enumerates the modifications to perform and must contain one or more of the following keys: - * + * * - "read": Whether the article should be marked as read (true) or unread (false) * - "starred": Whether the article should (true) or should not (false) be marked as starred/favourite * - "note": A string containing a freeform plain-text note for the article - * + * * @param string $user The user who owns the articles to be modified * @param array $data An associative array of properties to modify. Anything not specified will remain unchanged * @param Context $context The query context to match articles against @@ -1680,9 +1680,9 @@ class Database { } /** Returns statistics about the articles starred by the given user - * + * * The associative array returned has the following keys: - * + * * - "total": The count of all starred articles * - "unread": The count of starred articles which are unread * - "read": The count of starred articles which are read @@ -1704,7 +1704,7 @@ class Database { } /** Returns an indexed array listing the labels assigned to an article - * + * * @param string $user The user whose labels are to be listed * @param integer $id The numeric identifier of the article whose labels are to be listed * @param boolean $byName Whether to return the label names (true) instead of the numeric label identifiers (false) @@ -1768,9 +1768,9 @@ class Database { } /** Ensures the specified article exists and raises an exception otherwise - * - * Returns an associative array containing the id and latest edition of the article if it exists - * + * + * Returns an associative array containing the id and latest edition of the article if it exists + * * @param string $user The user who owns the article to be validated * @param integer $id The identifier of the article to validate */ @@ -1795,9 +1795,9 @@ class Database { } /** Ensures the specified article edition exists and raises an exception otherwise - * - * Returns an associative array containing the edition id, article id, and latest edition of the edition if it exists - * + * + * Returns an associative array containing the edition id, article id, and latest edition of the edition if it exists + * * @param string $user The user who owns the edition to be validated * @param integer $id The identifier of the edition to validate */ @@ -1848,9 +1848,9 @@ class Database { } /** Creates a label, and returns its numeric identifier - * + * * Labels are discrete objects in the database and can be associated with multiple articles; an article may in turn be associated with multiple labels - * + * * @param string $user The user who will own the created label * @param array $data An associative array defining the label's properties; currently only "name" is understood */ @@ -1867,14 +1867,14 @@ class Database { } /** Lists a user's article labels - * + * * The following keys are included in each record: - * + * * - "id": The label's numeric identifier * - "name" The label's textual name * - "articles": The count of articles which have the label assigned to them * - "read": How many of the total articles assigned to the label are read - * + * * @param string $user The user whose labels are to be listed * @param boolean $includeEmpty Whether to include (true) or supress (false) labels which have no articles assigned to them */ @@ -1911,9 +1911,9 @@ class Database { } /** Deletes a label from the database - * + * * Any articles associated with the label remains untouched - * + * * @param string $user The owner of the label to remove * @param integer|string $id The numeric identifier or name of the label * @param boolean $byName Whether to interpret the $id parameter as the label's name (true) or identifier (false) @@ -1933,14 +1933,14 @@ class Database { } /** Retrieves the properties of a label - * + * * The following keys are included in the output array: - * + * * - "id": The label's numeric identifier * - "name" The label's textual name * - "articles": The count of articles which have the label assigned to them * - "read": How many of the total articles assigned to the label are read - * + * * @param string $user The owner of the label to remove * @param integer|string $id The numeric identifier or name of the label * @param boolean $byName Whether to interpret the $id parameter as the label's name (true) or identifier (false) @@ -1981,7 +1981,7 @@ class Database { } /** Sets the properties of a label - * + * * @param string $user The owner of the label to query * @param integer|string $id The numeric identifier or name of the label * @param array $data An associative array defining the label's properties; currently only "name" is understood @@ -2013,7 +2013,7 @@ class Database { } /** Returns an indexed array of article identifiers assigned to a label - * + * * @param string $user The owner of the label to query * @param integer|string $id The numeric identifier or name of the label * @param boolean $byName Whether to interpret the $id parameter as the label's name (true) or identifier (false) @@ -2039,7 +2039,7 @@ class Database { } /** Makes or breaks associations between a given label and articles matching the given query context - * + * * @param string $user The owner of the label * @param integer|string $id The numeric identifier or name of the label * @param Context $context The query context matching the desired articles @@ -2080,9 +2080,9 @@ class Database { } /** Ensures the specified label identifier or name is valid (and optionally whether it exists) and raises an exception otherwise - * - * Returns an associative array containing the id, name of the label if it exists - * + * + * Returns an associative array containing the id, name of the label if it exists + * * @param string $user The user who owns the label to be validated * @param integer|string $id The numeric identifier or name of the label to validate * @param boolean $byName Whether to interpret the $id parameter as the label's name (true) or identifier (false) @@ -2127,9 +2127,9 @@ class Database { } /** Creates a tag, and returns its numeric identifier - * + * * Tags are discrete objects in the database and can be associated with multiple subscriptions; a subscription may in turn be associated with multiple tags - * + * * @param string $user The user who will own the created tag * @param array $data An associative array defining the tag's properties; currently only "name" is understood */ @@ -2146,13 +2146,13 @@ class Database { } /** Lists a user's subscription tags - * + * * The following keys are included in each record: - * + * * - "id": The tag's numeric identifier * - "name" The tag's textual name * - "subscriptions": The count of subscriptions which have the tag assigned to them - * + * * @param string $user The user whose tags are to be listed * @param boolean $includeEmpty Whether to include (true) or supress (false) tags which have no subscriptions assigned to them */ @@ -2177,14 +2177,14 @@ class Database { } /** Lists the associations between all tags and subscription - * + * * The following keys are included in each record: - * + * * - "tag_id": The tag's numeric identifier * - "tag_name" The tag's textual name * - "subscription_id": The numeric identifier of the associated subscription * - "subscription_name" The subscription's textual name - * + * * @param string $user The user whose tags are to be listed */ public function tagSummarize(string $user): Db\Result { @@ -2205,9 +2205,9 @@ class Database { } /** Deletes a tag from the database - * + * * Any subscriptions associated with the tag remains untouched - * + * * @param string $user The owner of the tag to remove * @param integer|string $id The numeric identifier or name of the tag * @param boolean $byName Whether to interpret the $id parameter as the tag's name (true) or identifier (false) @@ -2227,13 +2227,13 @@ class Database { } /** Retrieves the properties of a tag - * + * * The following keys are included in the output array: - * + * * - "id": The tag's numeric identifier * - "name" The tag's textual name * - "subscriptions": The count of subscriptions which have the tag assigned to them - * + * * @param string $user The owner of the tag to remove * @param integer|string $id The numeric identifier or name of the tag * @param boolean $byName Whether to interpret the $id parameter as the tag's name (true) or identifier (false) @@ -2262,7 +2262,7 @@ class Database { } /** Sets the properties of a tag - * + * * @param string $user The owner of the tag to query * @param integer|string $id The numeric identifier or name of the tag * @param array $data An associative array defining the tag's properties; currently only "name" is understood @@ -2294,7 +2294,7 @@ class Database { } /** Returns an indexed array of subscription identifiers assigned to a tag - * + * * @param string $user The owner of the tag to query * @param integer|string $id The numeric identifier or name of the tag * @param boolean $byName Whether to interpret the $id parameter as the tag's name (true) or identifier (false) @@ -2320,7 +2320,7 @@ class Database { } /** Makes or breaks associations between a given tag and specified subscriptions - * + * * @param string $user The owner of the tag * @param integer|string $id The numeric identifier or name of the tag * @param integer[] $context The query context matching the desired subscriptions @@ -2339,7 +2339,7 @@ class Database { $q1 = $this->db->prepare( "UPDATE arsse_tag_members set assigned = ?, modified = CURRENT_TIMESTAMP - where tag = ? and assigned <> ? and subscription in (select id from arsse_subscriptions where owner = ? and id in ($inClause))", + where tag = ? and assigned <> ? and subscription in (select id from arsse_subscriptions where owner = ? and id in ($inClause))", "bool", "int", "bool", @@ -2370,9 +2370,9 @@ class Database { } /** Ensures the specified tag identifier or name is valid (and optionally whether it exists) and raises an exception otherwise - * - * Returns an associative array containing the id, name of the tag if it exists - * + * + * Returns an associative array containing the id, name of the tag if it exists + * * @param string $user The user who owns the tag to be validated * @param integer|string $id The numeric identifier or name of the tag to validate * @param boolean $byName Whether to interpret the $id parameter as the tag's name (true) or identifier (false) diff --git a/lib/Db/Driver.php b/lib/Db/Driver.php index b0f572c..7f04dc6 100644 --- a/lib/Db/Driver.php +++ b/lib/Db/Driver.php @@ -20,7 +20,7 @@ interface Driver { public static function driverName(): string; /** Returns the version of the schema of the opened database; if uninitialized should return 0 - * + * * Normally the version is stored under the 'schema_version' key in the arsse_meta table, but another method may be used if appropriate */ public function schemaVersion(): int; @@ -32,7 +32,7 @@ interface Driver { public function begin(bool $lock = false): Transaction; /** Manually begins a real or synthetic transactions, with real or synthetic nesting, and returns its numeric ID - * + * * If the database backend does not implement savepoints, IDs must still be tracked as if it does */ public function savepointCreate(): int; @@ -44,7 +44,7 @@ interface Driver { public function savepointUndo(int $index = null): bool; /** Performs an in-place upgrade of the database schema - * + * * The driver may choose not to implement in-place upgrading, in which case an exception should be thrown */ public function schemaUpdate(int $to): bool; @@ -62,15 +62,15 @@ interface Driver { public function prepareArray(string $query, array $paramTypes): Statement; /** Reports whether the database character set is correct/acceptable - * + * * The backend must be able to accept and provide UTF-8 text; information may be stored in any encoding capable of representing the entire range of Unicode */ public function charsetAcceptable(): bool; /** Returns an implementation-dependent form of a reference SQL function or operator - * + * * The tokens the implementation must understand are: - * + * * - "greatest": the GREATEST function implemented by PostgreSQL and MySQL * - "nocase": the name of a general-purpose case-insensitive collation sequence * - "like": the case-insensitive LIKE operator @@ -78,7 +78,7 @@ interface Driver { public function sqlToken(string $token): string; /** Returns a string literal which is properly escaped to guard against SQL injections. Delimiters are included in the output string - * + * * This functionality should be avoided in favour of using statement parameters whenever possible */ public function literalString(string $str): string; diff --git a/lib/Db/SQLite3/PDODriver.php b/lib/Db/SQLite3/PDODriver.php index b1cff19..c6d7ad4 100644 --- a/lib/Db/SQLite3/PDODriver.php +++ b/lib/Db/SQLite3/PDODriver.php @@ -50,7 +50,7 @@ class PDODriver extends AbstractPDODriver { /** @codeCoverageIgnore */ public function exec(string $query): bool { - // because PDO uses sqlite3_prepare() internally instead of sqlite3_prepare_v2(), + // because PDO uses sqlite3_prepare() internally instead of sqlite3_prepare_v2(), // we have to retry ourselves in cases of schema changes // the SQLite3 class is not similarly affected $attempts = 0; @@ -68,7 +68,7 @@ class PDODriver extends AbstractPDODriver { /** @codeCoverageIgnore */ public function query(string $query): \JKingWeb\Arsse\Db\Result { - // because PDO uses sqlite3_prepare() internally instead of sqlite3_prepare_v2(), + // because PDO uses sqlite3_prepare() internally instead of sqlite3_prepare_v2(), // we have to retry ourselves in cases of schema changes // the SQLite3 class is not similarly affected $attempts = 0; diff --git a/lib/Db/SQLite3/PDOStatement.php b/lib/Db/SQLite3/PDOStatement.php index 166fe31..eb4fdfe 100644 --- a/lib/Db/SQLite3/PDOStatement.php +++ b/lib/Db/SQLite3/PDOStatement.php @@ -12,7 +12,7 @@ class PDOStatement extends \JKingWeb\Arsse\Db\PDOStatement { /** @codeCoverageIgnore */ public function runArray(array $values = []): \JKingWeb\Arsse\Db\Result { - // because PDO uses sqlite3_prepare() internally instead of sqlite3_prepare_v2(), + // because PDO uses sqlite3_prepare() internally instead of sqlite3_prepare_v2(), // we have to retry ourselves in cases of schema changes // the SQLite3 class is not similarly affected $attempts = 0; diff --git a/lib/Db/Statement.php b/lib/Db/Statement.php index b85ceca..0ed8685 100644 --- a/lib/Db/Statement.php +++ b/lib/Db/Statement.php @@ -24,7 +24,7 @@ interface Statement { 'str' => self::T_STRING, 'bool' => self::T_BOOLEAN, 'boolean' => self::T_BOOLEAN, - 'bit' => self::T_BOOLEAN, + 'bit' => self::T_BOOLEAN, 'strict int' => self::T_NOT_NULL + self::T_INTEGER, 'strict integer' => self::T_NOT_NULL + self::T_INTEGER, 'strict float' => self::T_NOT_NULL + self::T_FLOAT, diff --git a/lib/REST/TinyTinyRSS/API.php b/lib/REST/TinyTinyRSS/API.php index b274f20..26cf441 100644 --- a/lib/REST/TinyTinyRSS/API.php +++ b/lib/REST/TinyTinyRSS/API.php @@ -1499,7 +1499,7 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { $order = ["edited_date desc"]; break; default: - // sort most recently marked for special feeds, newest first otherwise + // sort most recently marked for special feeds, newest first otherwise $order = (!$cat && ($id == self::FEED_READ || $id == self::FEED_STARRED)) ? ["marked_date desc"] : ["edited_date desc"]; break; } diff --git a/lib/REST/TinyTinyRSS/Search.php b/lib/REST/TinyTinyRSS/Search.php index 4ff634b..f791361 100644 --- a/lib/REST/TinyTinyRSS/Search.php +++ b/lib/REST/TinyTinyRSS/Search.php @@ -82,6 +82,7 @@ class Search { $state = self::STATE_IN_TOKEN_OR_TAG; continue 3; } + // no break case self::STATE_BEFORE_TOKEN_QUOTED: switch ($char) { case "": @@ -130,6 +131,7 @@ class Search { $state = self::STATE_IN_TOKEN_OR_TAG_QUOTED; continue 3; } + // no break case self::STATE_IN_DATE: while ($pos < $stop && $search[$pos] !== " ") { $buffer .= $search[$pos++]; @@ -169,6 +171,7 @@ class Search { $buffer .= $char; continue 3; } + // no break case self::STATE_IN_TOKEN: while ($pos < $stop && $search[$pos] !== " ") { $buffer .= $search[$pos++]; @@ -214,6 +217,7 @@ class Search { $buffer .= $char; continue 3; } + // no break case self::STATE_IN_TOKEN_OR_TAG: switch ($char) { case "": @@ -223,7 +227,7 @@ class Search { $flag_negative = false; $buffer = $tag = ""; continue 3; - case ":"; + case ":": $tag = $buffer; $buffer = ""; $state = self::STATE_IN_TOKEN; @@ -232,6 +236,7 @@ class Search { $buffer .= $char; continue 3; } + // no break case self::STATE_IN_TOKEN_OR_TAG_QUOTED: switch ($char) { case "": @@ -267,6 +272,7 @@ class Search { $buffer .= $char; continue 3; } + // no break default: throw new \Exception; // @codeCoverageIgnore } diff --git a/tests/cases/Database/SeriesArticle.php b/tests/cases/Database/SeriesArticle.php index d47f918..3871474 100644 --- a/tests/cases/Database/SeriesArticle.php +++ b/tests/cases/Database/SeriesArticle.php @@ -498,7 +498,7 @@ trait SeriesArticle { 'Excluded folder tree' => [(new Context)->not->folder(1), [1,2,3,4,19,20]], 'Excluding label ID 2' => [(new Context)->not->label(2), [2,3,4,6,7,8,19]], 'Excluding label "Fascinating"' => [(new Context)->not->labelName("Fascinating"), [2,3,4,6,7,8,19]], - 'Search 501 terms' => [(new Context)->searchTerms(array_merge(range(1,500),[str_repeat("a", 1000)])), []], + 'Search 501 terms' => [(new Context)->searchTerms(array_merge(range(1, 500), [str_repeat("a", 1000)])), []], 'With tag ID 1' => [(new Context)->tag(1), [5,6,7,8]], 'With tag ID 5' => [(new Context)->tag(5), [7,8,19,20]], 'With tag ID 1 or 5' => [(new Context)->tags([1,5]), [5,6,7,8,19,20]], @@ -1060,8 +1060,8 @@ trait SeriesArticle { } public function provideArrayContextOptions() { - foreach([ - "articles", "editions", + foreach ([ + "articles", "editions", "subscriptions", "foldersShallow", //"folders", "tags", "tagNames", "labels", "labelNames", "searchTerms", "authorTerms", "annotationTerms", diff --git a/tests/cases/REST/Fever/TestAPI.php b/tests/cases/REST/Fever/TestAPI.php index 272a25f..1986db0 100644 --- a/tests/cases/REST/Fever/TestAPI.php +++ b/tests/cases/REST/Fever/TestAPI.php @@ -26,7 +26,6 @@ use Zend\Diactoros\Response\EmptyResponse; /** @covers \JKingWeb\Arsse\REST\Fever\API */ class TestAPI extends \JKingWeb\Arsse\Test\AbstractTest { - protected function v($value) { return $value; } From 4ce371ece69e0e0763870297fc25af263f1cce64 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Mon, 8 Apr 2019 18:41:56 -0400 Subject: [PATCH 17/32] Tests and fixes for Fever item listing --- lib/REST/Fever/API.php | 7 +- tests/cases/REST/Fever/TestAPI.php | 147 +++++++++++++++++++++++++++++ 2 files changed, 151 insertions(+), 3 deletions(-) diff --git a/lib/REST/Fever/API.php b/lib/REST/Fever/API.php index 2504463..0849f61 100644 --- a/lib/REST/Fever/API.php +++ b/lib/REST/Fever/API.php @@ -238,11 +238,11 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { // handle the standard options if ($G['with_ids']) { $c->articles(explode(",", $G['with_ids'])); - } elseif ($G['since_id']) { - $c->oldestArticle($G['since_id'] + 1); } elseif ($G['max_id']) { - $c->newestArticle($G['max_id'] - 1); + $c->latestArticle($G['max_id'] - 1); $reverse = true; + } elseif ($G['since_id']) { + $c->oldestArticle($G['since_id'] + 1); } // handle the undocumented options if ($G['group_ids']) { @@ -261,6 +261,7 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { 'title' => (string) $r['title'], 'author' => (string) $r['author'], 'html' => (string) $r['content'], + 'url' => (string) $r['url'], 'is_saved' => (int) $r['starred'], 'is_read' => (int) !$r['unread'], 'created_on_time' => Date::transform($r['published_date'], "unix", "sql"), diff --git a/tests/cases/REST/Fever/TestAPI.php b/tests/cases/REST/Fever/TestAPI.php index 1986db0..490149f 100644 --- a/tests/cases/REST/Fever/TestAPI.php +++ b/tests/cases/REST/Fever/TestAPI.php @@ -26,6 +26,122 @@ use Zend\Diactoros\Response\EmptyResponse; /** @covers \JKingWeb\Arsse\REST\Fever\API */ class TestAPI extends \JKingWeb\Arsse\Test\AbstractTest { + protected $articles = [ + 'db' => [ + [ + 'id' => 101, + 'url' => 'http://example.com/1', + 'title' => 'Article title 1', + 'author' => '', + 'content' => '

Article content 1

', + 'published_date' => '2000-01-01 00:00:00', + 'unread' => 1, + 'starred' => 0, + 'subscription' => 8, + ], + [ + 'id' => 102, + 'url' => 'http://example.com/2', + 'title' => 'Article title 2', + 'author' => '', + 'content' => '

Article content 2

', + 'published_date' => '2000-01-02 00:00:00', + 'unread' => 0, + 'starred' => 0, + 'subscription' => 8, + ], + [ + 'id' => 103, + 'url' => 'http://example.com/3', + 'title' => 'Article title 3', + 'author' => '', + 'content' => '

Article content 3

', + 'published_date' => '2000-01-03 00:00:00', + 'unread' => 1, + 'starred' => 1, + 'subscription' => 9, + ], + [ + 'id' => 104, + 'url' => 'http://example.com/4', + 'title' => 'Article title 4', + 'author' => '', + 'content' => '

Article content 4

', + 'published_date' => '2000-01-04 00:00:00', + 'unread' => 0, + 'starred' => 1, + 'subscription' => 9, + ], + [ + 'id' => 105, + 'url' => 'http://example.com/5', + 'title' => 'Article title 5', + 'author' => '', + 'content' => '

Article content 5

', + 'published_date' => '2000-01-05 00:00:00', + 'unread' => 1, + 'starred' => 0, + 'subscription' => 10, + ], + ], + 'rest' => [ + [ + 'id' => 101, + 'feed_id' => 8, + 'title' => 'Article title 1', + 'author' => '', + 'html' => '

Article content 1

', + 'url' => 'http://example.com/1', + 'is_saved' => 0, + 'is_read' => 0, + 'created_on_time' => 946684800, + ], + [ + 'id' => 102, + 'feed_id' => 8, + 'title' => 'Article title 2', + 'author' => '', + 'html' => '

Article content 2

', + 'url' => 'http://example.com/2', + 'is_saved' => 0, + 'is_read' => 1, + 'created_on_time' => 946771200, + ], + [ + 'id' => 103, + 'feed_id' => 9, + 'title' => 'Article title 3', + 'author' => '', + 'html' => '

Article content 3

', + 'url' => 'http://example.com/3', + 'is_saved' => 1, + 'is_read' => 0, + 'created_on_time' => 946857600, + ], + [ + 'id' => 104, + 'feed_id' => 9, + 'title' => 'Article title 4', + 'author' => '', + 'html' => '

Article content 4

', + 'url' => 'http://example.com/4', + 'is_saved' => 1, + 'is_read' => 1, + 'created_on_time' => 946944000, + ], + [ + 'id' => 105, + 'feed_id' => 10, + 'title' => 'Article title 5', + 'author' => '', + 'html' => '

Article content 5

', + 'url' => 'http://example.com/5', + 'is_saved' => 0, + 'is_read' => 0, + 'created_on_time' => 947030400, + ], + ], + ]; protected function v($value) { return $value; } @@ -204,4 +320,35 @@ class TestAPI extends \JKingWeb\Arsse\Test\AbstractTest { $act = $this->req("api&feeds"); $this->assertMessage($exp, $act); } + + /** @dataProvider provideItemListContexts */ + public function testListItems(string $url, Context $c, bool $desc) { + $fields = ["id", "subscription", "title", "author", "content", "url", "starred", "unread", "published_date"]; + $order = [$desc ? "id desc" : "id"]; + \Phake::when(Arsse::$db)->articleList->thenReturn(new Result($this->articles['db'])); + \Phake::when(Arsse::$db)->articleCount($this->anything())->thenReturn(1024); + $exp = new JsonResponse([ + 'items' => $this->articles['rest'], + 'total_items' => 1024, + ]); + $act = $this->req("api&$url"); + $this->assertMessage($exp, $act); + \Phake::verify(Arsse::$db)->articleList($this->anything(), $c, $fields, $order); + } + + public function provideItemListContexts() { + $c = (new Context)->limit(50); + return [ + ["items", (clone $c), false], + ["items&group_ids=1,2,3,4", (clone $c)->tags([1,2,3,4]), false], + ["items&feed_ids=1,2,3,4", (clone $c)->subscriptions([1,2,3,4]), false], + ["items&with_ids=1,2,3,4", (clone $c)->articles([1,2,3,4]), false], + ["items&since_id=1", (clone $c)->oldestArticle(2), false], + ["items&max_id=2", (clone $c)->latestArticle(1), true], + ["items&with_ids=1,2,3,4&max_id=6", (clone $c)->articles([1,2,3,4]), false], + ["items&with_ids=1,2,3,4&since_id=6", (clone $c)->articles([1,2,3,4]), false], + ["items&max_id=3&since_id=6", (clone $c)->latestArticle(2), true], + ["items&feed_ids=1,2,3,4&since_id=6", (clone $c)->subscriptions([1,2,3,4])->oldestArticle(7), false], + ]; + } } From e8f4732b1f32c49c68ded4313c68346749e98265 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Mon, 8 Apr 2019 19:15:12 -0400 Subject: [PATCH 18/32] Tests for saved and unread item ID lists --- tests/cases/REST/Fever/TestAPI.php | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/cases/REST/Fever/TestAPI.php b/tests/cases/REST/Fever/TestAPI.php index 490149f..ffd7a63 100644 --- a/tests/cases/REST/Fever/TestAPI.php +++ b/tests/cases/REST/Fever/TestAPI.php @@ -351,4 +351,19 @@ class TestAPI extends \JKingWeb\Arsse\Test\AbstractTest { ["items&feed_ids=1,2,3,4&since_id=6", (clone $c)->subscriptions([1,2,3,4])->oldestArticle(7), false], ]; } + + public function testListItemIds() { + $saved = [['id' => 1],['id' => 2],['id' => 3]]; + $unread = [['id' => 4],['id' => 5],['id' => 6]]; + \Phake::when(Arsse::$db)->articleList($this->anything(), (new Context)->starred(true))->thenReturn(new Result($saved)); + \Phake::when(Arsse::$db)->articleList($this->anything(), (new Context)->unread(true))->thenReturn(new Result($unread)); + $exp = new JsonResponse([ + 'saved_item_ids' => "1,2,3" + ]); + $this->assertMessage($exp, $this->req("api&saved_item_ids")); + $exp = new JsonResponse([ + 'unread_item_ids' => "4,5,6" + ]); + $this->assertMessage($exp, $this->req("api&unread_item_ids")); + } } From 98fc3f4940867593fe533fcd7446d5faea825742 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Mon, 8 Apr 2019 19:21:21 -0400 Subject: [PATCH 19/32] Test for hot links --- lib/REST/Fever/API.php | 2 +- tests/cases/REST/Fever/TestAPI.php | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/REST/Fever/API.php b/lib/REST/Fever/API.php index 0849f61..8c14d69 100644 --- a/lib/REST/Fever/API.php +++ b/lib/REST/Fever/API.php @@ -136,7 +136,7 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { } if ($G['links']) { // TODO: implement hot links - $out['inks'] = []; + $out['links'] = []; } if ($G['unread_item_ids']) { $out['unread_item_ids'] = $this->getItemIds((new Context)->unread(true)); diff --git a/tests/cases/REST/Fever/TestAPI.php b/tests/cases/REST/Fever/TestAPI.php index ffd7a63..686ef22 100644 --- a/tests/cases/REST/Fever/TestAPI.php +++ b/tests/cases/REST/Fever/TestAPI.php @@ -366,4 +366,12 @@ class TestAPI extends \JKingWeb\Arsse\Test\AbstractTest { ]); $this->assertMessage($exp, $this->req("api&unread_item_ids")); } + + public function testListHotLinks() { + // hot links are not actually implemented, so an empty array should be all we get + $exp = new JsonResponse([ + 'links' => [] + ]); + $this->assertMessage($exp, $this->req("api&links")); + } } From c783ec4357fd5864a9f2932a4f1bd427b4b93d2b Mon Sep 17 00:00:00 2001 From: "J. King" Date: Mon, 8 Apr 2019 20:58:45 -0400 Subject: [PATCH 20/32] Prototype XML output for Fever --- lib/REST/Fever/API.php | 38 +++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/lib/REST/Fever/API.php b/lib/REST/Fever/API.php index 8c14d69..d1ad2b3 100644 --- a/lib/REST/Fever/API.php +++ b/lib/REST/Fever/API.php @@ -21,6 +21,7 @@ use JKingWeb\Arsse\REST\Exception405; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\ResponseInterface; use Zend\Diactoros\Response\JsonResponse; +use Zend\Diactoros\Response\XmlResponse; use Zend\Diactoros\Response\EmptyResponse; class API extends \JKingWeb\Arsse\REST\AbstractHandler { @@ -161,12 +162,47 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { protected function formatResponse(array $data, bool $xml): ResponseInterface { if ($xml) { - throw \Exception("Not implemented yet"); + $d = new \DOMDocument("1.0", "utf-8"); + $d->appendChild($this->makeXMLAssoc($data, $d->createElement("response"))); + return new XmlResponse($d->saveXML()); } else { return new JsonResponse($data, 200, [], \JSON_UNESCAPED_SLASHES | \JSON_UNESCAPED_UNICODE); } } + protected function makeXMLAssoc(array $data, \DOMElement $p): \DOMElement { + $d = $p->ownerDocument; + foreach ($data as $k => $v) { + if (!is_array($v)) { + $p->appendChild($d->createElement($k, $v)); + } elseif (isset($v[0])) { + // this is a very simplistic check for an indexed array + // it would not pass muster in the face of generic data, + // but we'll assume our code produces only well-ordered + // indexed arrays + $p->appendChild($this->makeXMLIndexed($v, $d->createElement($k), substr($k, 0, strlen($k) - 1))); + } else { + $p->appendChild($this->makeXMLAssoc($v, $d->createElement($k))); + } + } + return $p; + } + + protected function makeXMLIndexed(array $data, \DOMElement $p, string $k): \DOMElement { + $d = $p->ownerDocument; + foreach ($data as $v) { + if (!is_array($v)) { + $p->appendChild($d->createElement($k, $v)); + } elseif (isset($v[0])) { + $p->appendChild($this->makeXMLIndexed($v, $d->createElement($k), substr($k, 0, strlen($k) - 1))); + } else { + $p->appendChild($this->makeXMLAssoc($v, $d->createElement($k))); + } + } + return $p; + + } + protected function logIn(string $hash): bool { // if HTTP authentication was successful and sessions are not enforced, proceed unconditionally if (isset(Arsse::$user->id) && !Arsse::$conf->userSessionEnforced) { From 15915a4393b6029f757e13c7928c2d6baad98f93 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Mon, 8 Apr 2019 23:31:22 -0400 Subject: [PATCH 21/32] Initial implementation of simple marks --- lib/REST/Fever/API.php | 75 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 73 insertions(+), 2 deletions(-) diff --git a/lib/REST/Fever/API.php b/lib/REST/Fever/API.php index d1ad2b3..29ca3c2 100644 --- a/lib/REST/Fever/API.php +++ b/lib/REST/Fever/API.php @@ -119,6 +119,18 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { } protected function processRequest(array $out, array $G, array $P): array { + $listUnread = false; + $listSaved = false; + if ($P['unread_recently_read']) { + $this->setUnread(); + $listUnread = true; + } + if ($P['mark']) { + // depending on which mark are being made, + // either an 'unread_item_ids' or a + // 'saved_item_ids' entry will be added later + $listSaved = $this->setMarks($P, $listUnread); + } if ($G['feeds'] || $G['groups']) { if ($G['groups']) { $out['groups'] = $this->getGroups(); @@ -139,10 +151,10 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { // TODO: implement hot links $out['links'] = []; } - if ($G['unread_item_ids']) { + if ($G['unread_item_ids'] || $listUnread) { $out['unread_item_ids'] = $this->getItemIds((new Context)->unread(true)); } - if ($G['saved_item_ids']) { + if ($G['saved_item_ids'] || $listSaved) { $out['saved_item_ids'] = $this->getItemIds((new Context)->starred(true)); } return $out; @@ -219,6 +231,65 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { return true; } + protected function setMarks(array $P, &$listUnread): bool { + $listSaved = false; + $c = new Context; + $id = $P['id']; + if ($P['before']) { + $c->notMarkedSince($P['before']); + } + switch ($P['mark']) { + case "item": + $c->article($id); + break; + case "group": + if ($id > 0) { + // group zero is the "Kindling" supergroup i.e. all feeds + $c->tag($id); + } elseif ($id < 0) { + // group negative-one is the "Sparks" supergroup i.e. no feeds + $c->not->folder(0); + } + break; + case "feed": + $c->subscription($id); + break; + default: + return $listSaved; + } + switch ($P['as']) { + case "read": + $data = ['read' => true]; + $listUnread = true; + break; + case "unread": + // this option is undocumented, but valid + $data = ['read' => false]; + $listUnread = true; + break; + case "saved": + $data = ['starred' => true]; + $listSaved = true; + break; + case "unsaved": + $data = ['starred' => false]; + $listSaved = true; + break; + default: + return $listSaved; + } + try { + Arsse::$db->articleMark(Arsse::$user->id, $data, $c); + } catch (ExceptionInput $e) { + // ignore any errors + } + return $listSaved; + } + + protected function setUnread() { + // stub + } + protected function getRefreshTime() { return Date::transform(Arsse::$db->subscriptionRefreshed(Arsse::$user->id), "unix"); } From 61abf7ee7c876ae8b67a0f343a5ab036f1feffdd Mon Sep 17 00:00:00 2001 From: "J. King" Date: Tue, 9 Apr 2019 16:15:36 -0400 Subject: [PATCH 22/32] Upgrade to Diactoros 2.x --- arsse.php | 2 +- composer.json | 11 +-- composer.lock | 204 ++++++++++++++++++++++++++++++++++++++++++++------ 3 files changed, 190 insertions(+), 27 deletions(-) diff --git a/arsse.php b/arsse.php index 407be03..0cfa0ae 100644 --- a/arsse.php +++ b/arsse.php @@ -25,7 +25,7 @@ if (\PHP_SAPI === "cli") { $conf = file_exists(BASE."config.php") ? new Conf(BASE."config.php") : new Conf; Arsse::load($conf); // handle Web requests - $emitter = new \Zend\Diactoros\Response\SapiEmitter(); + $emitter = new \Zend\HttpHandlerRunner\Emitter\SapiEmitter; $response = (new REST)->dispatch(); $emitter->emit($response); } diff --git a/composer.json b/composer.json index 0f5570c..1a607c3 100644 --- a/composer.json +++ b/composer.json @@ -18,15 +18,16 @@ ], "require": { - "php": "^7.0", + "php": "7.*", "ext-intl": "*", "ext-json": "*", "ext-hash": "*", "p3k/picofeed": "0.1.*", - "hosteurope/password-generator": "^1.0", - "docopt/docopt": "^1.0", - "jkingweb/druuid": "^3.0", - "zendframework/zend-diactoros": "^1.6" + "hosteurope/password-generator": "1.*", + "docopt/docopt": "1.*", + "jkingweb/druuid": "3.*", + "zendframework/zend-diactoros": "2.*", + "zendframework/zend-httphandlerrunner": "1.*" }, "require-dev": { "bamarni/composer-bin-plugin": "*" diff --git a/composer.lock b/composer.lock index b5e5d38..0a7f073 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "d7a6a00be3d97c11d09ec4d4e56d36e0", + "content-hash": "bd427d25f07432e40d396060907cf1e3", "packages": [ { "name": "docopt/docopt", @@ -190,6 +190,58 @@ "homepage": "https://github.com/miniflux/picoFeed", "time": "2017-11-30T00:16:58+00:00" }, + { + "name": "psr/http-factory", + "version": "1.0.0", + "source": { + "type": "git", + "url": "https://github.com/php-fig/http-factory.git", + "reference": "378bfe27931ecc54ff824a20d6f6bfc303bbd04c" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/php-fig/http-factory/zipball/378bfe27931ecc54ff824a20d6f6bfc303bbd04c", + "reference": "378bfe27931ecc54ff824a20d6f6bfc303bbd04c", + "shasum": "" + }, + "require": { + "php": ">=7.0.0", + "psr/http-message": "^1.0" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "1.0.x-dev" + } + }, + "autoload": { + "psr-4": { + "Psr\\Http\\Message\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "PHP-FIG", + "homepage": "http://www.php-fig.org/" + } + ], + "description": "Common interfaces for PSR-7 HTTP message factories", + "keywords": [ + "factory", + "http", + "message", + "psr", + "psr-17", + "psr-7", + "request", + "response" + ], + "time": "2018-07-30T21:54:04+00:00" + }, { "name": "psr/http-message", "version": "1.0.1", @@ -240,40 +292,96 @@ ], "time": "2016-08-06T14:39:51+00:00" }, + { + "name": "psr/http-server-handler", + "version": "1.0.1", + "source": { + "type": "git", + "url": "https://github.com/php-fig/http-server-handler.git", + "reference": "aff2f80e33b7f026ec96bb42f63242dc50ffcae7" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/php-fig/http-server-handler/zipball/aff2f80e33b7f026ec96bb42f63242dc50ffcae7", + "reference": "aff2f80e33b7f026ec96bb42f63242dc50ffcae7", + "shasum": "" + }, + "require": { + "php": ">=7.0", + "psr/http-message": "^1.0" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "1.0.x-dev" + } + }, + "autoload": { + "psr-4": { + "Psr\\Http\\Server\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "PHP-FIG", + "homepage": "http://www.php-fig.org/" + } + ], + "description": "Common interface for HTTP server-side request handler", + "keywords": [ + "handler", + "http", + "http-interop", + "psr", + "psr-15", + "psr-7", + "request", + "response", + "server" + ], + "time": "2018-10-30T16:46:14+00:00" + }, { "name": "zendframework/zend-diactoros", - "version": "1.8.6", + "version": "2.1.1", "source": { "type": "git", "url": "https://github.com/zendframework/zend-diactoros.git", - "reference": "20da13beba0dde8fb648be3cc19765732790f46e" + "reference": "c3c330192bc9cc51b7e9ce968ff721dc32ffa986" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/zendframework/zend-diactoros/zipball/20da13beba0dde8fb648be3cc19765732790f46e", - "reference": "20da13beba0dde8fb648be3cc19765732790f46e", + "url": "https://api.github.com/repos/zendframework/zend-diactoros/zipball/c3c330192bc9cc51b7e9ce968ff721dc32ffa986", + "reference": "c3c330192bc9cc51b7e9ce968ff721dc32ffa986", "shasum": "" }, "require": { - "php": "^5.6 || ^7.0", + "php": "^7.1", + "psr/http-factory": "^1.0", "psr/http-message": "^1.0" }, "provide": { + "psr/http-factory-implementation": "1.0", "psr/http-message-implementation": "1.0" }, "require-dev": { "ext-dom": "*", "ext-libxml": "*", + "http-interop/http-factory-tests": "^0.5.0", "php-http/psr7-integration-tests": "dev-master", - "phpunit/phpunit": "^5.7.16 || ^6.0.8 || ^7.2.7", - "zendframework/zend-coding-standard": "~1.0" + "phpunit/phpunit": "^7.0.2", + "zendframework/zend-coding-standard": "~1.0.0" }, "type": "library", "extra": { "branch-alias": { - "dev-master": "1.8.x-dev", - "dev-develop": "1.9.x-dev", - "dev-release-2.0": "2.0.x-dev" + "dev-master": "2.1.x-dev", + "dev-develop": "2.2.x-dev", + "dev-release-1.8": "1.8.x-dev" } }, "autoload": { @@ -293,16 +401,70 @@ }, "notification-url": "https://packagist.org/downloads/", "license": [ - "BSD-2-Clause" + "BSD-3-Clause" ], "description": "PSR HTTP Message implementations", - "homepage": "https://github.com/zendframework/zend-diactoros", "keywords": [ "http", "psr", "psr-7" ], - "time": "2018-09-05T19:29:37+00:00" + "time": "2019-01-05T20:13:32+00:00" + }, + { + "name": "zendframework/zend-httphandlerrunner", + "version": "1.1.0", + "source": { + "type": "git", + "url": "https://github.com/zendframework/zend-httphandlerrunner.git", + "reference": "75fb12751fe9d6e392cce1ee0d687dacae2db787" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/zendframework/zend-httphandlerrunner/zipball/75fb12751fe9d6e392cce1ee0d687dacae2db787", + "reference": "75fb12751fe9d6e392cce1ee0d687dacae2db787", + "shasum": "" + }, + "require": { + "php": "^7.1", + "psr/http-message": "^1.0", + "psr/http-message-implementation": "^1.0", + "psr/http-server-handler": "^1.0" + }, + "require-dev": { + "phpunit/phpunit": "^7.0.2", + "zendframework/zend-coding-standard": "~1.0.0", + "zendframework/zend-diactoros": "^1.7 || ^2.1.1" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "1.1.x-dev", + "dev-develop": "1.2.x-dev" + }, + "zf": { + "config-provider": "Zend\\HttpHandlerRunner\\ConfigProvider" + } + }, + "autoload": { + "psr-4": { + "Zend\\HttpHandlerRunner\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "BSD-3-Clause" + ], + "description": "Execute PSR-15 RequestHandlerInterface instances and emit responses they generate.", + "keywords": [ + "ZendFramework", + "components", + "expressive", + "psr-15", + "psr-7", + "zf" + ], + "time": "2019-02-19T18:20:34+00:00" }, { "name": "zendframework/zendxml", @@ -354,16 +516,16 @@ "packages-dev": [ { "name": "bamarni/composer-bin-plugin", - "version": "v1.2.0", + "version": "v1.3.0", "source": { "type": "git", "url": "https://github.com/bamarni/composer-bin-plugin.git", - "reference": "62fef740245a85f00665e81ea8f0aa0b72afe6e7" + "reference": "67f9d314dc7ecf7245b8637906e151ccc62b8d24" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/bamarni/composer-bin-plugin/zipball/62fef740245a85f00665e81ea8f0aa0b72afe6e7", - "reference": "62fef740245a85f00665e81ea8f0aa0b72afe6e7", + "url": "https://api.github.com/repos/bamarni/composer-bin-plugin/zipball/67f9d314dc7ecf7245b8637906e151ccc62b8d24", + "reference": "67f9d314dc7ecf7245b8637906e151ccc62b8d24", "shasum": "" }, "require": { @@ -371,7 +533,7 @@ }, "require-dev": { "composer/composer": "dev-master", - "symfony/console": "^2.5 || ^3.0" + "symfony/console": "^2.5 || ^3.0 || ^4.0" }, "type": "composer-plugin", "extra": { @@ -389,7 +551,7 @@ "license": [ "MIT" ], - "time": "2017-09-11T13:13:58+00:00" + "time": "2019-03-17T12:38:04+00:00" } ], "aliases": [], @@ -398,7 +560,7 @@ "prefer-stable": false, "prefer-lowest": false, "platform": { - "php": "^7.0", + "php": "7.*", "ext-intl": "*", "ext-json": "*", "ext-hash": "*" From 52bc5fbda6124aaed1cf19fc9e7f4e1f04fcb6e9 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Wed, 10 Apr 2019 09:48:28 -0400 Subject: [PATCH 23/32] Tests for simple marking --- lib/REST/Fever/API.php | 7 ++- tests/cases/REST/Fever/TestAPI.php | 68 ++++++++++++++++++++++++++---- 2 files changed, 64 insertions(+), 11 deletions(-) diff --git a/lib/REST/Fever/API.php b/lib/REST/Fever/API.php index 29ca3c2..d472b11 100644 --- a/lib/REST/Fever/API.php +++ b/lib/REST/Fever/API.php @@ -125,7 +125,7 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { $this->setUnread(); $listUnread = true; } - if ($P['mark']) { + if ($P['mark'] && $P['as'] && is_int($P['id'])) { // depending on which mark are being made, // either an 'unread_item_ids' or a // 'saved_item_ids' entry will be added later @@ -244,11 +244,14 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { break; case "group": if ($id > 0) { - // group zero is the "Kindling" supergroup i.e. all feeds + // concrete groups $c->tag($id); } elseif ($id < 0) { // group negative-one is the "Sparks" supergroup i.e. no feeds $c->not->folder(0); + } else { + // group zero is the "Kindling" supergroup i.e. all feeds + // nothing need to be done for this } break; case "feed": diff --git a/tests/cases/REST/Fever/TestAPI.php b/tests/cases/REST/Fever/TestAPI.php index 686ef22..9a899c0 100644 --- a/tests/cases/REST/Fever/TestAPI.php +++ b/tests/cases/REST/Fever/TestAPI.php @@ -7,16 +7,11 @@ declare(strict_types=1); namespace JKingWeb\Arsse\TestCase\REST\Fever; use JKingWeb\Arsse\Arsse; -use JKingWeb\Arsse\Conf; use JKingWeb\Arsse\User; use JKingWeb\Arsse\Database; -use JKingWeb\Arsse\Service; -use JKingWeb\Arsse\REST\Request; use JKingWeb\Arsse\Test\Result; -use JKingWeb\Arsse\Misc\Date; use JKingWeb\Arsse\Context\Context; use JKingWeb\Arsse\Db\ExceptionInput; -use JKingWeb\Arsse\User\Exception as UserException; use JKingWeb\Arsse\Db\Transaction; use JKingWeb\Arsse\REST\Fever\API; use Psr\Http\Message\ResponseInterface; @@ -161,9 +156,8 @@ class TestAPI extends \JKingWeb\Arsse\Test\AbstractTest { if (is_array($dataPost)) { $req = $req->withParsedBody($dataPost); } else { - $body = $req->getBody(); - $body->write($dataPost); - $req = $req->withBody($body); + parse_str($dataPost, $arr); + $req = $req->withParsedBody($arr); } if (isset($user)) { if (strlen($user)) { @@ -319,7 +313,7 @@ class TestAPI extends \JKingWeb\Arsse\Test\AbstractTest { ]); $act = $this->req("api&feeds"); $this->assertMessage($exp, $act); - } + } /** @dataProvider provideItemListContexts */ public function testListItems(string $url, Context $c, bool $desc) { @@ -374,4 +368,60 @@ class TestAPI extends \JKingWeb\Arsse\Test\AbstractTest { ]); $this->assertMessage($exp, $this->req("api&links")); } + + /** @dataProvider provideMarkingContexts */ + public function testSetMarks(string $post, Context $c, array $data, array $out) { + $saved = [['id' => 1],['id' => 2],['id' => 3]]; + $unread = [['id' => 4],['id' => 5],['id' => 6]]; + \Phake::when(Arsse::$db)->articleList($this->anything(), (new Context)->starred(true))->thenReturn(new Result($saved)); + \Phake::when(Arsse::$db)->articleList($this->anything(), (new Context)->unread(true))->thenReturn(new Result($unread)); + \Phake::when(Arsse::$db)->articleMark->thenReturn(0); + \Phake::when(Arsse::$db)->articleMark($this->anything(), $this->anything(), (new Context)->article(2112))->thenThrow(new \JKingWeb\Arsse\Db\ExceptionInput("subjectMissing")); + $exp = new JsonResponse($out); + $act = $this->req("api", $post); + $this->assertMessage($exp, $act); + if ($c && $data) { + \Phake::verify(Arsse::$db)->articleMark($this->anything(), $data, $c); + } else { + \Phake::verify(Arsse::$db, \Phake::times(0))->articleMark; + } + } + + public function provideMarkingContexts() { + $markRead = ['read' => true]; + $markUnread = ['read' => false]; + $markSaved = ['starred' => true]; + $markUnsaved = ['starred' => false]; + $listSaved = ['saved_item_ids' => "1,2,3"]; + $listUnread = ['unread_item_ids' => "4,5,6"]; + return [ + ["mark=item&as=read&id=5", (new Context)->article(5), $markRead, $listUnread], + ["mark=item&as=unread&id=42", (new Context)->article(42), $markUnread, $listUnread], + ["mark=item&as=read&id=2112", (new Context)->article(2112), $markRead, $listUnread], // article doesn't exist + ["mark=item&as=saved&id=5", (new Context)->article(5), $markSaved, $listSaved], + ["mark=item&as=unsaved&id=42", (new Context)->article(42), $markUnsaved, $listSaved], + ["mark=feed&as=read&id=5", (new Context)->subscription(5), $markRead, $listUnread], + ["mark=feed&as=unread&id=42", (new Context)->subscription(42), $markUnread, $listUnread], + ["mark=feed&as=saved&id=5", (new Context)->subscription(5), $markSaved, $listSaved], + ["mark=feed&as=unsaved&id=42", (new Context)->subscription(42), $markUnsaved, $listSaved], + ["mark=group&as=read&id=5", (new Context)->tag(5), $markRead, $listUnread], + ["mark=group&as=unread&id=42", (new Context)->tag(42), $markUnread, $listUnread], + ["mark=group&as=saved&id=5", (new Context)->tag(5), $markSaved, $listSaved], + ["mark=group&as=unsaved&id=42", (new Context)->tag(42), $markUnsaved, $listSaved], + ["mark=item&as=invalid&id=42", new Context, [], []], + ["mark=invalid&as=unread&id=42", new Context, [], []], + ["mark=group&as=read&id=0", (new Context), $markRead, $listUnread], + ["mark=group&as=unread&id=0", (new Context), $markUnread, $listUnread], + ["mark=group&as=saved&id=0", (new Context), $markSaved, $listSaved], + ["mark=group&as=unsaved&id=0", (new Context), $markUnsaved, $listSaved], + ["mark=group&as=read&id=-1", (new Context)->not->folder(0), $markRead, $listUnread], + ["mark=group&as=unread&id=-1", (new Context)->not->folder(0), $markUnread, $listUnread], + ["mark=group&as=saved&id=-1", (new Context)->not->folder(0), $markSaved, $listSaved], + ["mark=group&as=unsaved&id=-1", (new Context)->not->folder(0), $markUnsaved, $listSaved], + ["mark=group&as=read&id=-1&before=946684800", (new Context)->not->folder(0)->notMarkedSince("2000-01-01T00:00:00"), $markRead, $listUnread], + ["mark=item&as=unread", new Context, [], []], + ["mark=item&id=6", new Context, [], []], + ["as=unread&id=6", new Context, [], []], + ]; + } } From afb95e53b025554aade539e842dc6c0c8b164011 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Wed, 10 Apr 2019 10:21:14 -0400 Subject: [PATCH 24/32] Initial implementation of read-undo --- lib/REST/Fever/API.php | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/REST/Fever/API.php b/lib/REST/Fever/API.php index d472b11..2d0f984 100644 --- a/lib/REST/Fever/API.php +++ b/lib/REST/Fever/API.php @@ -290,7 +290,19 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { } protected function setUnread() { - // stub + $lastUnread = Arsse::$db->articleList(Arsse::$user->id, (new Context)->limit(1), ["marked_date"], ["marked_date desc"])->getValue(); + if (!$lastUnread) { + // there are no articles + return; + } + // Fever takes the date of the last read article less fifteen seconds as a cut-off. + // We take the date of last mark (whether it be read, unread, saved, unsaved), which + // not actually signify a mark, but we'll otherwise also count back fifteen seconds + $c = new Context; + $lastUnread = Date::normalize($lastUnread, "sql"); + $since = Date::sub("DT15S", $lastUnread); + $c->unread(false)->markedSince($since); + Arsse::$db->articleMark(Arsse::$user->id, ['read' => false], $c); } protected function getRefreshTime() { From 8532c581a8c638a747e01e0655cb12bda52687ed Mon Sep 17 00:00:00 2001 From: "J. King" Date: Wed, 10 Apr 2019 10:51:02 -0400 Subject: [PATCH 25/32] Handle OPTIONS requests in Fever --- lib/REST/Fever/API.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/REST/Fever/API.php b/lib/REST/Fever/API.php index 2d0f984..baa501f 100644 --- a/lib/REST/Fever/API.php +++ b/lib/REST/Fever/API.php @@ -63,8 +63,10 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { } switch ($req->getMethod()) { case "OPTIONS": - // do stuff - break; + return new EmptyResponse(204, [ + 'Allow' => "POST", + 'Accept' => "application/x-www-form-urlencoded", + ]); case "POST": if (strlen($req->getHeaderLine("Content-Type")) && $req->getHeaderLine("Content-Type") !== "application/x-www-form-urlencoded") { return new EmptyResponse(415, ['Accept' => "application/x-www-form-urlencoded"]); @@ -297,7 +299,7 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { } // Fever takes the date of the last read article less fifteen seconds as a cut-off. // We take the date of last mark (whether it be read, unread, saved, unsaved), which - // not actually signify a mark, but we'll otherwise also count back fifteen seconds + // may not actually signify a mark, but we'll otherwise also count back fifteen seconds $c = new Context; $lastUnread = Date::normalize($lastUnread, "sql"); $since = Date::sub("DT15S", $lastUnread); From efd8492573f84b89a991cb4ca4d2750872f3ca2f Mon Sep 17 00:00:00 2001 From: "J. King" Date: Wed, 10 Apr 2019 15:07:34 -0400 Subject: [PATCH 26/32] Tests for various invalid requests --- tests/cases/REST/Fever/TestAPI.php | 38 ++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/tests/cases/REST/Fever/TestAPI.php b/tests/cases/REST/Fever/TestAPI.php index 9a899c0..e077197 100644 --- a/tests/cases/REST/Fever/TestAPI.php +++ b/tests/cases/REST/Fever/TestAPI.php @@ -141,14 +141,15 @@ class TestAPI extends \JKingWeb\Arsse\Test\AbstractTest { return $value; } - protected function req($dataGet, $dataPost = "", string $method = "POST", string $type = null, string $url = "", string $user = null): ResponseInterface { + protected function req($dataGet, $dataPost = "", string $method = "POST", string $type = null, string $url = "", string $user = null): ServerRequest { $url = "/fever/".$url; + $type = $type ?? "application/x-www-form-urlencoded"; $server = [ 'REQUEST_METHOD' => $method, 'REQUEST_URI' => $url, - 'HTTP_CONTENT_TYPE' => $type ?? "application/x-www-form-urlencoded", + 'HTTP_CONTENT_TYPE' => $type, ]; - $req = new ServerRequest($server, [], $url, $method, "php://memory"); + $req = new ServerRequest($server, [], $url, $method, "php://memory", ['Content-Type' => $type]); if (!is_array($dataGet)) { parse_str($dataGet, $dataGet); } @@ -166,7 +167,7 @@ class TestAPI extends \JKingWeb\Arsse\Test\AbstractTest { $req = $req->withAttribute("authenticationFailed", true); } } - return $this->h->dispatch($req); + return $req; } public function setUp() { @@ -205,7 +206,7 @@ class TestAPI extends \JKingWeb\Arsse\Test\AbstractTest { \Phake::when($this->h)->processRequest->thenReturnCallback(function($out, $G, $P) { return $out; }); - $act = $this->req($dataGet, $dataPost, "POST", null, "", $httpUser); + $act = $this->h->dispatch($this->req($dataGet, $dataPost, "POST", null, "", $httpUser)); $this->assertMessage($exp, $act); } @@ -284,7 +285,7 @@ class TestAPI extends \JKingWeb\Arsse\Test\AbstractTest { ['group_id' => 2, 'feed_ids' => "1,3"], ], ]); - $act = $this->req("api&groups"); + $act = $this->h->dispatch($this->req("api&groups")); $this->assertMessage($exp, $act); } @@ -311,7 +312,7 @@ class TestAPI extends \JKingWeb\Arsse\Test\AbstractTest { ['group_id' => 2, 'feed_ids' => "1,3"], ], ]); - $act = $this->req("api&feeds"); + $act = $this->h->dispatch($this->req("api&feeds")); $this->assertMessage($exp, $act); } @@ -325,7 +326,7 @@ class TestAPI extends \JKingWeb\Arsse\Test\AbstractTest { 'items' => $this->articles['rest'], 'total_items' => 1024, ]); - $act = $this->req("api&$url"); + $act = $this->h->dispatch($this->req("api&$url")); $this->assertMessage($exp, $act); \Phake::verify(Arsse::$db)->articleList($this->anything(), $c, $fields, $order); } @@ -354,11 +355,11 @@ class TestAPI extends \JKingWeb\Arsse\Test\AbstractTest { $exp = new JsonResponse([ 'saved_item_ids' => "1,2,3" ]); - $this->assertMessage($exp, $this->req("api&saved_item_ids")); + $this->assertMessage($exp, $this->h->dispatch($this->req("api&saved_item_ids"))); $exp = new JsonResponse([ 'unread_item_ids' => "4,5,6" ]); - $this->assertMessage($exp, $this->req("api&unread_item_ids")); + $this->assertMessage($exp, $this->h->dispatch($this->req("api&unread_item_ids"))); } public function testListHotLinks() { @@ -366,7 +367,7 @@ class TestAPI extends \JKingWeb\Arsse\Test\AbstractTest { $exp = new JsonResponse([ 'links' => [] ]); - $this->assertMessage($exp, $this->req("api&links")); + $this->assertMessage($exp, $this->h->dispatch($this->req("api&links"))); } /** @dataProvider provideMarkingContexts */ @@ -378,7 +379,7 @@ class TestAPI extends \JKingWeb\Arsse\Test\AbstractTest { \Phake::when(Arsse::$db)->articleMark->thenReturn(0); \Phake::when(Arsse::$db)->articleMark($this->anything(), $this->anything(), (new Context)->article(2112))->thenThrow(new \JKingWeb\Arsse\Db\ExceptionInput("subjectMissing")); $exp = new JsonResponse($out); - $act = $this->req("api", $post); + $act = $this->h->dispatch($this->req("api", $post)); $this->assertMessage($exp, $act); if ($c && $data) { \Phake::verify(Arsse::$db)->articleMark($this->anything(), $data, $c); @@ -424,4 +425,17 @@ class TestAPI extends \JKingWeb\Arsse\Test\AbstractTest { ["as=unread&id=6", new Context, [], []], ]; } + + /** @dataProvider provideInvalidRequests */ + public function testSendInvalidRequests(ServerRequest $req, ResponseInterface $exp) { + $this->assertMessage($exp, $this->h->dispatch($req)); + } + + public function provideInvalidRequests() { + return [ + 'Not an API request' => [$this->req(""), new EmptyResponse(404)], + 'Wrong method' => [$this->req("api", "", "GET"), new EmptyResponse(405, ['Allow' => "OPTIONS,POST"])], + 'Wrong content type' => [$this->req("api", "", "POST", "application/json"), new EmptyResponse(415, ['Accept' => "application/x-www-form-urlencoded"])], + ]; + } } From c55a960b856f548c55b6cd3854dd502b9559e947 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Wed, 10 Apr 2019 15:14:45 -0400 Subject: [PATCH 27/32] Slight cleanup --- lib/REST/TinyTinyRSS/API.php | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/REST/TinyTinyRSS/API.php b/lib/REST/TinyTinyRSS/API.php index 26cf441..b5b610f 100644 --- a/lib/REST/TinyTinyRSS/API.php +++ b/lib/REST/TinyTinyRSS/API.php @@ -8,11 +8,9 @@ namespace JKingWeb\Arsse\REST\TinyTinyRSS; use JKingWeb\Arsse\Feed; use JKingWeb\Arsse\Arsse; -use JKingWeb\Arsse\Database; -use JKingWeb\Arsse\User; -use JKingWeb\Arsse\Service; -use JKingWeb\Arsse\Misc\Date; +use JKingWeb\Arsse\Service;; use JKingWeb\Arsse\Context\Context; +use JKingWeb\Arsse\Misc\Date; use JKingWeb\Arsse\Misc\ValueInfo; use JKingWeb\Arsse\AbstractException; use JKingWeb\Arsse\ExceptionType; @@ -23,7 +21,6 @@ use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\ResponseInterface; use Zend\Diactoros\Response\JsonResponse as Response; use Zend\Diactoros\Response\EmptyResponse; -use Robo\Task\Archive\Pack; class API extends \JKingWeb\Arsse\REST\AbstractHandler { const LEVEL = 14; // emulated API level From daeff63239d2235cd29fa8df9b77ab4127bc0688 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Wed, 10 Apr 2019 16:01:58 -0400 Subject: [PATCH 28/32] Test basic Fever responses --- tests/cases/REST/Fever/TestAPI.php | 47 ++++++++++++++++++++++++------ 1 file changed, 38 insertions(+), 9 deletions(-) diff --git a/tests/cases/REST/Fever/TestAPI.php b/tests/cases/REST/Fever/TestAPI.php index e077197..aa7bc75 100644 --- a/tests/cases/REST/Fever/TestAPI.php +++ b/tests/cases/REST/Fever/TestAPI.php @@ -18,6 +18,7 @@ use Psr\Http\Message\ResponseInterface; use Zend\Diactoros\ServerRequest; use Zend\Diactoros\Response\JsonResponse; use Zend\Diactoros\Response\EmptyResponse; +use PHPUnit\Util\Json; /** @covers \JKingWeb\Arsse\REST\Fever\API */ class TestAPI extends \JKingWeb\Arsse\Test\AbstractTest { @@ -321,14 +322,14 @@ class TestAPI extends \JKingWeb\Arsse\Test\AbstractTest { $fields = ["id", "subscription", "title", "author", "content", "url", "starred", "unread", "published_date"]; $order = [$desc ? "id desc" : "id"]; \Phake::when(Arsse::$db)->articleList->thenReturn(new Result($this->articles['db'])); - \Phake::when(Arsse::$db)->articleCount($this->anything())->thenReturn(1024); + \Phake::when(Arsse::$db)->articleCount(Arsse::$user->id)->thenReturn(1024); $exp = new JsonResponse([ 'items' => $this->articles['rest'], 'total_items' => 1024, ]); $act = $this->h->dispatch($this->req("api&$url")); $this->assertMessage($exp, $act); - \Phake::verify(Arsse::$db)->articleList($this->anything(), $c, $fields, $order); + \Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, $c, $fields, $order); } public function provideItemListContexts() { @@ -350,8 +351,8 @@ class TestAPI extends \JKingWeb\Arsse\Test\AbstractTest { public function testListItemIds() { $saved = [['id' => 1],['id' => 2],['id' => 3]]; $unread = [['id' => 4],['id' => 5],['id' => 6]]; - \Phake::when(Arsse::$db)->articleList($this->anything(), (new Context)->starred(true))->thenReturn(new Result($saved)); - \Phake::when(Arsse::$db)->articleList($this->anything(), (new Context)->unread(true))->thenReturn(new Result($unread)); + \Phake::when(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->starred(true))->thenReturn(new Result($saved)); + \Phake::when(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->unread(true))->thenReturn(new Result($unread)); $exp = new JsonResponse([ 'saved_item_ids' => "1,2,3" ]); @@ -374,15 +375,15 @@ class TestAPI extends \JKingWeb\Arsse\Test\AbstractTest { public function testSetMarks(string $post, Context $c, array $data, array $out) { $saved = [['id' => 1],['id' => 2],['id' => 3]]; $unread = [['id' => 4],['id' => 5],['id' => 6]]; - \Phake::when(Arsse::$db)->articleList($this->anything(), (new Context)->starred(true))->thenReturn(new Result($saved)); - \Phake::when(Arsse::$db)->articleList($this->anything(), (new Context)->unread(true))->thenReturn(new Result($unread)); + \Phake::when(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->starred(true))->thenReturn(new Result($saved)); + \Phake::when(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->unread(true))->thenReturn(new Result($unread)); \Phake::when(Arsse::$db)->articleMark->thenReturn(0); - \Phake::when(Arsse::$db)->articleMark($this->anything(), $this->anything(), (new Context)->article(2112))->thenThrow(new \JKingWeb\Arsse\Db\ExceptionInput("subjectMissing")); + \Phake::when(Arsse::$db)->articleMark(Arsse::$user->id, $this->anything(), (new Context)->article(2112))->thenThrow(new \JKingWeb\Arsse\Db\ExceptionInput("subjectMissing")); $exp = new JsonResponse($out); $act = $this->h->dispatch($this->req("api", $post)); $this->assertMessage($exp, $act); if ($c && $data) { - \Phake::verify(Arsse::$db)->articleMark($this->anything(), $data, $c); + \Phake::verify(Arsse::$db)->articleMark(Arsse::$user->id, $data, $c); } else { \Phake::verify(Arsse::$db, \Phake::times(0))->articleMark; } @@ -419,7 +420,7 @@ class TestAPI extends \JKingWeb\Arsse\Test\AbstractTest { ["mark=group&as=unread&id=-1", (new Context)->not->folder(0), $markUnread, $listUnread], ["mark=group&as=saved&id=-1", (new Context)->not->folder(0), $markSaved, $listSaved], ["mark=group&as=unsaved&id=-1", (new Context)->not->folder(0), $markUnsaved, $listSaved], - ["mark=group&as=read&id=-1&before=946684800", (new Context)->not->folder(0)->notMarkedSince("2000-01-01T00:00:00"), $markRead, $listUnread], + ["mark=group&as=read&id=-1&before=946684800", (new Context)->not->folder(0)->notMarkedSince("2000-01-01T00:00:00Z"), $markRead, $listUnread], ["mark=item&as=unread", new Context, [], []], ["mark=item&id=6", new Context, [], []], ["as=unread&id=6", new Context, [], []], @@ -438,4 +439,32 @@ class TestAPI extends \JKingWeb\Arsse\Test\AbstractTest { 'Wrong content type' => [$this->req("api", "", "POST", "application/json"), new EmptyResponse(415, ['Accept' => "application/x-www-form-urlencoded"])], ]; } + + public function testMakeABaseQuery() { + $this->h = \Phake::partialMock(API::class); + \Phake::when($this->h)->logIn->thenReturn(true); + \Phake::when(Arsse::$db)->subscriptionRefreshed(Arsse::$user->id)->thenReturn(new \DateTimeImmutable("2000-01-01T00:00:00Z")); + $exp = new JsonResponse([ + 'api_version' => API::LEVEL, + 'auth' => 1, + 'last_refreshed_on_time' => 946684800, + ]); + $act = $this->h->dispatch($this->req("api")); + $this->assertMessage($exp, $act); + \Phake::when(Arsse::$db)->subscriptionRefreshed(Arsse::$user->id)->thenReturn(null); // no subscriptions + $exp = new JsonResponse([ + 'api_version' => API::LEVEL, + 'auth' => 1, + 'last_refreshed_on_time' => null, + ]); + $act = $this->h->dispatch($this->req("api")); + $this->assertMessage($exp, $act); + \Phake::when($this->h)->logIn->thenReturn(false); + $exp = new JsonResponse([ + 'api_version' => API::LEVEL, + 'auth' => 0, + ]); + $act = $this->h->dispatch($this->req("api")); + $this->assertMessage($exp, $act); + } } From 2d18be959cc8b55f3dedd76202fdbdb029f1c67e Mon Sep 17 00:00:00 2001 From: "J. King" Date: Wed, 10 Apr 2019 18:27:57 -0400 Subject: [PATCH 29/32] Tests for undoing read marks --- lib/REST/Fever/API.php | 2 +- tests/cases/REST/Fever/TestAPI.php | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/lib/REST/Fever/API.php b/lib/REST/Fever/API.php index baa501f..643d1e8 100644 --- a/lib/REST/Fever/API.php +++ b/lib/REST/Fever/API.php @@ -302,7 +302,7 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { // may not actually signify a mark, but we'll otherwise also count back fifteen seconds $c = new Context; $lastUnread = Date::normalize($lastUnread, "sql"); - $since = Date::sub("DT15S", $lastUnread); + $since = Date::sub("PT15S", $lastUnread); $c->unread(false)->markedSince($since); Arsse::$db->articleMark(Arsse::$user->id, ['read' => false], $c); } diff --git a/tests/cases/REST/Fever/TestAPI.php b/tests/cases/REST/Fever/TestAPI.php index aa7bc75..4023122 100644 --- a/tests/cases/REST/Fever/TestAPI.php +++ b/tests/cases/REST/Fever/TestAPI.php @@ -467,4 +467,20 @@ class TestAPI extends \JKingWeb\Arsse\Test\AbstractTest { $act = $this->h->dispatch($this->req("api")); $this->assertMessage($exp, $act); } + + public function testUndoReadMarks() { + $unread = [['id' => 4],['id' => 5],['id' => 6]]; + $out = ['unread_item_ids' => "4,5,6"]; + \Phake::when(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->limit(1), ["marked_date"], ["marked_date desc"])->thenReturn(new Result([['marked_date' => "2000-01-01 00:00:00"]])); + \Phake::when(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->unread(true))->thenReturn(new Result($unread)); + \Phake::when(Arsse::$db)->articleMark->thenReturn(0); + $exp = new JsonResponse($out); + $act = $this->h->dispatch($this->req("api", ['unread_recently_read' => 1])); + $this->assertMessage($exp, $act); + \Phake::verify(Arsse::$db)->articleMark(Arsse::$user->id, ['read' => false], (new Context)->unread(false)->markedSince("1999-12-31T23:59:45Z")); + \Phake::when(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->limit(1), ["marked_date"], ["marked_date desc"])->thenReturn(new Result([])); + $act = $this->h->dispatch($this->req("api", ['unread_recently_read' => 1])); + $this->assertMessage($exp, $act); + \Phake::verify(Arsse::$db)->articleMark; // only called one time, above + } } From 0480465e7eeb18e9c20f2b8db21b129ad96ab37a Mon Sep 17 00:00:00 2001 From: "J. King" Date: Wed, 24 Jul 2019 09:10:13 -0400 Subject: [PATCH 30/32] Test Fever XML responses Fixes #158 --- lib/REST/Fever/API.php | 8 +++++--- tests/cases/REST/Fever/TestAPI.php | 15 ++++++++++++++- tests/lib/AbstractTest.php | 5 +++-- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/lib/REST/Fever/API.php b/lib/REST/Fever/API.php index 643d1e8..59f7a9f 100644 --- a/lib/REST/Fever/API.php +++ b/lib/REST/Fever/API.php @@ -188,7 +188,7 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { $d = $p->ownerDocument; foreach ($data as $k => $v) { if (!is_array($v)) { - $p->appendChild($d->createElement($k, $v)); + $p->appendChild($d->createElement($k, (string) $v)); } elseif (isset($v[0])) { // this is a very simplistic check for an indexed array // it would not pass muster in the face of generic data, @@ -206,9 +206,11 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { $d = $p->ownerDocument; foreach ($data as $v) { if (!is_array($v)) { - $p->appendChild($d->createElement($k, $v)); + // this case is never encountered with Fever's output + $p->appendChild($d->createElement($k, (string) $v)); // @codeCoverageIgnore } elseif (isset($v[0])) { - $p->appendChild($this->makeXMLIndexed($v, $d->createElement($k), substr($k, 0, strlen($k) - 1))); + // this case is never encountered with Fever's output + $p->appendChild($this->makeXMLIndexed($v, $d->createElement($k), substr($k, 0, strlen($k) - 1))); // @codeCoverageIgnore } else { $p->appendChild($this->makeXMLAssoc($v, $d->createElement($k))); } diff --git a/tests/cases/REST/Fever/TestAPI.php b/tests/cases/REST/Fever/TestAPI.php index 4023122..def389a 100644 --- a/tests/cases/REST/Fever/TestAPI.php +++ b/tests/cases/REST/Fever/TestAPI.php @@ -17,11 +17,14 @@ use JKingWeb\Arsse\REST\Fever\API; use Psr\Http\Message\ResponseInterface; use Zend\Diactoros\ServerRequest; use Zend\Diactoros\Response\JsonResponse; +use Zend\Diactoros\Response\XmlResponse; use Zend\Diactoros\Response\EmptyResponse; -use PHPUnit\Util\Json; /** @covers \JKingWeb\Arsse\REST\Fever\API */ class TestAPI extends \JKingWeb\Arsse\Test\AbstractTest { + /** @var \JKingWeb\Arsse\REST\Fever\API */ + protected $h; + protected $articles = [ 'db' => [ [ @@ -483,4 +486,14 @@ class TestAPI extends \JKingWeb\Arsse\Test\AbstractTest { $this->assertMessage($exp, $act); \Phake::verify(Arsse::$db)->articleMark; // only called one time, above } + + public function testOutputToXml() { + \Phake::when($this->h)->processRequest->thenReturn([ + 'items' => $this->articles['rest'], + 'total_items' => 1024, + ]); + $exp = new XmlResponse("1018Article title 1<p>Article content 1</p>http://example.com/1009466848001028Article title 2<p>Article content 2</p>http://example.com/2019467712001039Article title 3<p>Article content 3</p>http://example.com/3109468576001049Article title 4<p>Article content 4</p>http://example.com/41194694400010510Article title 5<p>Article content 5</p>http://example.com/5009470304001024"); + $act = $this->h->dispatch($this->req("api=xml")); + $this->assertMessage($exp, $act); + } } diff --git a/tests/lib/AbstractTest.php b/tests/lib/AbstractTest.php index f55ca9b..cef0a8a 100644 --- a/tests/lib/AbstractTest.php +++ b/tests/lib/AbstractTest.php @@ -9,14 +9,13 @@ namespace JKingWeb\Arsse\Test; use JKingWeb\Arsse\Exception; use JKingWeb\Arsse\Arsse; use JKingWeb\Arsse\Conf; -use JKingWeb\Arsse\CLI; use JKingWeb\Arsse\Misc\Date; use Psr\Http\Message\MessageInterface; use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\ResponseInterface; use Zend\Diactoros\Response\JsonResponse; -use Zend\Diactoros\Response\EmptyResponse; +use Zend\Diactoros\Response\XmlResponse; /** @coversNothing */ abstract class AbstractTest extends \PHPUnit\Framework\TestCase { @@ -93,6 +92,8 @@ abstract class AbstractTest extends \PHPUnit\Framework\TestCase { if ($exp instanceof JsonResponse) { $this->assertEquals($exp->getPayload(), $act->getPayload(), $text); $this->assertSame($exp->getPayload(), $act->getPayload(), $text); + } elseif ($exp instanceof XmlResponse) { + $this->assertXmlStringEqualsXmlString((string) $exp->getBody(), (string) $act->getBody(), $text); } else { $this->assertEquals((string) $exp->getBody(), (string) $act->getBody(), $text); } From 61b942df70bba21827c9adda1a7abb3be5fa49f4 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Wed, 24 Jul 2019 12:27:50 -0400 Subject: [PATCH 31/32] Defer Fever favicons to a future release --- lib/REST/Fever/API.php | 13 +++++++++++-- tests/cases/REST/Fever/TestAPI.php | 10 ++++++++-- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/lib/REST/Fever/API.php b/lib/REST/Fever/API.php index 59f7a9f..83c3be8 100644 --- a/lib/REST/Fever/API.php +++ b/lib/REST/Fever/API.php @@ -26,6 +26,8 @@ use Zend\Diactoros\Response\EmptyResponse; class API extends \JKingWeb\Arsse\REST\AbstractHandler { const LEVEL = 3; + const GENERIC_ICON_TYPE = "image/png;base64"; + const GENERIC_ICON_DATA = "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAAAXNSR0IArs4c6QAAAARnQU1BAACxjwv8YQUAAAAJcEhZcwAADsMAAA7DAcdvqGQAAAAZdEVYdFNvZnR3YXJlAHBhaW50Lm5ldCA0LjAuMjHxIGmVAAAADUlEQVQYV2NgYGBgAAAABQABijPjAAAAAABJRU5ErkJggg=="; // GET parameters for which we only check presence: these will be converted to booleans const PARAM_BOOL = ["groups", "feeds", "items", "favicons", "links", "unread_item_ids", "saved_item_ids"]; @@ -143,7 +145,14 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { $out['feeds_groups'] = $this->getRelationships(); } if ($G['favicons']) { - # deal with favicons + // TODO: implement favicons properly + // we provide a single blank favicon for now + $out['favicons'] = [ + [ + 'id' => 0, + 'data' => self::GENERIC_ICON_TYPE.",".self::GENERIC_ICON_DATA, + ], + ]; } if ($G['items']) { $out['items'] = $this->getItems($G); @@ -318,7 +327,7 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { foreach (arsse::$db->subscriptionList(Arsse::$user->id) as $sub) { $out[] = [ 'id' => (int) $sub['id'], - 'favicon_id' => (int) ($sub['favicon'] ? $sub['feed'] : 0), + 'favicon_id' => 0, // TODO: implement favicons 'title' => (string) $sub['title'], 'url' => $sub['url'], 'site_url' => $sub['source'], diff --git a/tests/cases/REST/Fever/TestAPI.php b/tests/cases/REST/Fever/TestAPI.php index def389a..b81bdc3 100644 --- a/tests/cases/REST/Fever/TestAPI.php +++ b/tests/cases/REST/Fever/TestAPI.php @@ -307,9 +307,9 @@ class TestAPI extends \JKingWeb\Arsse\Test\AbstractTest { ])); $exp = new JsonResponse([ 'feeds' => [ - ['id' => 1, 'favicon_id' => 5, 'title' => "Ankh-Morpork News", 'url' => "http://example.com/feed", 'site_url' => "http://example.com/", 'is_spark' => 0, 'last_updated_on_time' => strtotime("2019-01-01T21:12:00Z")], + ['id' => 1, 'favicon_id' => 0, 'title' => "Ankh-Morpork News", 'url' => "http://example.com/feed", 'site_url' => "http://example.com/", 'is_spark' => 0, 'last_updated_on_time' => strtotime("2019-01-01T21:12:00Z")], ['id' => 2, 'favicon_id' => 0, 'title' => "Ook, Ook Eek Ook!", 'url' => "http://example.net/feed", 'site_url' => "http://example.net/", 'is_spark' => 0, 'last_updated_on_time' => strtotime("1988-06-24T12:21:00Z")], - ['id' => 3, 'favicon_id' => 1, 'title' => "The Last Soul", 'url' => "http://example.org/feed", 'site_url' => "http://example.org/", 'is_spark' => 0, 'last_updated_on_time' => strtotime("1991-08-12T03:22:00Z")], + ['id' => 3, 'favicon_id' => 0, 'title' => "The Last Soul", 'url' => "http://example.org/feed", 'site_url' => "http://example.org/", 'is_spark' => 0, 'last_updated_on_time' => strtotime("1991-08-12T03:22:00Z")], ], 'feeds_groups' => [ ['group_id' => 1, 'feed_ids' => "1,2"], @@ -496,4 +496,10 @@ class TestAPI extends \JKingWeb\Arsse\Test\AbstractTest { $act = $this->h->dispatch($this->req("api=xml")); $this->assertMessage($exp, $act); } + + public function testListFeedIcons() { + $act = $this->h->dispatch($this->req("api&favicons")); + $exp = new JsonResponse(['favicons' => [['id' => 0, 'data' => API::GENERIC_ICON_TYPE.",".API::GENERIC_ICON_DATA]]]); + $this->assertMessage($exp, $act); + } } From 56bb4608205d9b903116ac10d8822f7256272e71 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Wed, 24 Jul 2019 12:32:00 -0400 Subject: [PATCH 32/32] Test answering OPTIONS requests in Fever --- tests/cases/REST/Fever/TestAPI.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/cases/REST/Fever/TestAPI.php b/tests/cases/REST/Fever/TestAPI.php index b81bdc3..3393dde 100644 --- a/tests/cases/REST/Fever/TestAPI.php +++ b/tests/cases/REST/Fever/TestAPI.php @@ -502,4 +502,13 @@ class TestAPI extends \JKingWeb\Arsse\Test\AbstractTest { $exp = new JsonResponse(['favicons' => [['id' => 0, 'data' => API::GENERIC_ICON_TYPE.",".API::GENERIC_ICON_DATA]]]); $this->assertMessage($exp, $act); } + + public function testAnswerOptionsRequest() { + $act = $this->h->dispatch($this->req("api", "", "OPTIONS")); + $exp = new EmptyResponse(204, [ + 'Allow' => "POST", + 'Accept' => "application/x-www-form-urlencoded", + ]); + $this->assertMessage($exp, $act); + } }