From f72c85c9f64f0015e3b5e37e4c2303ac050b4c58 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Thu, 4 Apr 2019 11:22:50 -0400 Subject: [PATCH] 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]],