Hopefully working but maybe broken custom sorting

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);
     }

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,7 +1257,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"], array $sort = []): Query {
+    protected function articleQuery(string $user, Context $context, array $cols = ["id"]): Query {
         // validate input
         if ($context->subscription()) {
             $this->subscriptionValidateId($user, $context->subscription);
@@ -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 = $this->articleColumns();
-        $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",
-        ];
         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" : ""));
+        // make an ORDER BY column list
-        $q->setOrder("latest_editions.edition".($context->reverse ? " desc" : ""));
+        $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());
     }

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']) {
+        $reverse = !$data['oldestFirst'];
-            $c->reverse(false);
-        } else {
-            $c->reverse(true);
-        }
         // 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);

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
+                // sort most recently marked for special feeds, newest first otherwise 
-                $c->reverse(true);
+                $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);
     }
 }

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]],