Make distinction between modified and marked for articles

Also added various sets of fields to include when listing articles.

lib/AbstractException.php

@@ -7,7 +7,8 @@ declare(strict_types=1);
 namespace JKingWeb\Arsse;
 
 abstract class AbstractException extends \Exception {
-    const CODES = [        "Exception.uncoded"                           => -1,
+    const CODES = [        
+        "Exception.uncoded"                           => -1,
         "Exception.unknown"                           => 10000,
         "ExceptionType.strictFailure"                 => 10011,
         "ExceptionType.typeUnknown"                   => 10012,
@@ -39,7 +40,8 @@ abstract class AbstractException extends \Exception {
         "Db/Exception.savepointStatusUnknown"         => 10225,
         "Db/Exception.savepointInvalid"               => 10226,
         "Db/Exception.savepointStale"                 => 10227,
-        "Db/Exception.resultReused"                   => 10227,
+        "Db/Exception.resultReused"                   => 10228,
+        "Db/Exception.constantUnknown"                => 10229,
         "Db/ExceptionInput.missing"                   => 10231,
         "Db/ExceptionInput.whitespace"                => 10232,
         "Db/ExceptionInput.tooLong"                   => 10233,

lib/Database.php

@@ -16,6 +16,11 @@ use JKingWeb\Arsse\Misc\ValueInfo;
 class Database {
     const SCHEMA_VERSION = 2;
     const LIMIT_ARTICLES = 50;
+    // articleList verbosity levels
+    const AL_MINIMAL      = 0; // only that metadata which is required for context matching
+    const AL_CONSERVATIVE = 1; // base metadata plus anything that is not potentially large text
+    const AL_TYPICAL      = 2; // conservative, with the addition of content
+    const AL_FULL         = 3; // all possible fields
     
     /** @var Db\Driver */
     public $db;
@@ -824,10 +829,12 @@ class Database {
                 $extraColumns
                 arsse_articles.id as id,
                 arsse_articles.feed as feed,
+                arsse_articles.modified as modified_date,
                 max(
                     arsse_articles.modified, 
-                    coalesce((select modified from arsse_marks where article is arsse_articles.id and subscription in (select sub from subscribed_feeds)),'')
+                    coalesce((select modified from arsse_marks where article is arsse_articles.id and subscription in (select sub from subscribed_feeds)),''),
-                ) as modified_date,
+                    coalesce((select modified from arsse_label_members where article is arsse_articles.id and subscription in (select sub from subscribed_feeds)),'')
+                ) as marked_date,
                 NOT (select count(*) from arsse_marks where article is arsse_articles.id and read is 1 and subscription in (select sub from subscribed_feeds)) as unread,
                 (select count(*) from arsse_marks where article is arsse_articles.id and starred is 1 and subscription in (select sub from subscribed_feeds)) as starred,
                 (select max(id) from arsse_editions where article is arsse_articles.id) as edition,
@@ -917,13 +924,19 @@ class Database {
         if ($context->latestEdition()) {
             $q->setWhere("edition <= ?", "int", $context->latestEdition);
         }
-        // filter based on lastmod time
+        // filter based on time at which an article was changed by feed updates (modified), or by user action (marked)
         if ($context->modifiedSince()) {
             $q->setWhere("modified_date >= ?", "datetime", $context->modifiedSince);
         }
         if ($context->notModifiedSince()) {
             $q->setWhere("modified_date <= ?", "datetime", $context->notModifiedSince);
         }
+        if ($context->markedSince()) {
+            $q->setWhere("marked_date >= ?", "datetime", $context->markedSince);
+        }
+        if ($context->notMarkedSince()) {
+            $q->setWhere("marked_date <= ?", "datetime", $context->notMarkedSince);
+        }
         // filter for un/read and un/starred status if specified
         if ($context->unread()) {
             $q->setWhere("unread is ?", "bool", $context->unread);
@@ -959,7 +972,7 @@ class Database {
         }
     }
 
-    public function articleList(string $user, Context $context = null): Db\Result {
+    public function articleList(string $user, Context $context = null, int $fields = self::AL_FULL): Db\Result {
         if (!Arsse::$user->authorize($user, __FUNCTION__)) {
             throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]);
         }
@@ -969,26 +982,41 @@ class Database {
             $out = [];
             $tr = $this->begin();
             foreach ($contexts as $context) {
-                $out[] = $this->articleList($user, $context);
+                $out[] = $this->articleList($user, $context, $fields);
             }
             $tr->commit();
             return new Db\ResultAggregate(...$out);
         } else {
-            $columns = [
+            $columns = [];
-                // (id, subscription, feed, modified, unread, starred, edition): always included
+            switch ($fields) {
+                // NOTE: the cases all cascade into each other: a given verbosity level is always a superset of the previous one
+                case self::AL_FULL: // everything
+                    $columns = array_merge($columns,[
+                        "(select note from arsse_marks where article is arsse_articles.id and subscription in (select sub from subscribed_feeds)) as note",
+                    ]);
+                case self::AL_TYPICAL: // conservative, plus content
+                    $columns = array_merge($columns,[
+                        "content",
+                        "arsse_enclosures.url as media_url", // enclosures are potentially large due to data: URLs
+                        "arsse_enclosures.type as media_type", // FIXME: enclosures should eventually have their own fetch method
+                    ]);
+                case self::AL_CONSERVATIVE: // base metadata, plus anything that is not likely to be large text
+                    $columns = array_merge($columns,[
                         "arsse_articles.url as url",
                         "arsse_articles.title as title",
                         "(select coalesce(arsse_subscriptions.title,arsse_feeds.title) from arsse_feeds join arsse_subscriptions on arsse_subscriptions.feed is arsse_feeds.id where arsse_feeds.id is arsse_articles.feed) as subscription_title",
                         "author",
-                "content",
                         "guid",
                         "published as published_date",
                         "edited as edited_date",
                         "url_title_hash||':'||url_content_hash||':'||title_content_hash as fingerprint",
-                "arsse_enclosures.url as media_url",
+                    ]);
-                "arsse_enclosures.type as media_type",
+                case self::AL_MINIMAL: // base metadata (always included: required for context matching)
-                "(select note from arsse_marks where article is arsse_articles.id and subscription in (select sub from subscribed_feeds)) as note"
+                    // id, subscription, feed, modified_date, marked_date, unread, starred, edition
-            ];
+                    break;
+                default:
+                    throw new Db\Exception("constantUnknown", $fields);
+            }
             $q = $this->articleQuery($user, $context, $columns);
             $q->setJoin("left join arsse_enclosures on arsse_enclosures.article is arsse_articles.id");
             // perform the query and return results

lib/Misc/Context.php

@@ -22,6 +22,8 @@ class Context {
     public $starred = null;
     public $modifiedSince;
     public $notModifiedSince;
+    public $markedSince;
+    public $notMarkedSince;
     public $edition;
     public $article;
     public $editions;
@@ -104,6 +106,16 @@ class Context {
         return $this->act(__FUNCTION__, func_num_args(), $spec);
     }
     
+    public function markedSince($spec = null) {
+        $spec = Date::normalize($spec);
+        return $this->act(__FUNCTION__, func_num_args(), $spec);
+    }
+    
+    public function notMarkedSince($spec = null) {
+        $spec = Date::normalize($spec);
+        return $this->act(__FUNCTION__, func_num_args(), $spec);
+    }
+    
     public function edition(int $spec = null) {
         return $this->act(__FUNCTION__, func_num_args(), $spec);
     }

lib/REST/NextCloudNews/V1_2.php

@@ -506,7 +506,7 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler {
         }
         // whether to return only updated items
         if ($data['lastModified']) {
-            $c->modifiedSince($data['lastModified']);
+            $c->markedSince($data['lastModified']);
         }
         // perform the fetch
         try {

locale/en.php

@@ -142,6 +142,7 @@ return [
     'Exception.JKingWeb/Arsse/Db/Exception.savepointInvalid'               => 'Tried to {action} invalid savepoint {index}',
     'Exception.JKingWeb/Arsse/Db/Exception.savepointStale'                 => 'Tried to {action} stale savepoint {index}',
     'Exception.JKingWeb/Arsse/Db/Exception.resultReused'                   => 'Result set already iterated',
+    'Exception.JKingWeb/Arsse/Db/Exception.constantUnknown'                => 'Supplied constant value ({0}) is unknown or invalid in the context in which it was used',
     'Exception.JKingWeb/Arsse/Db/ExceptionInput.missing'                   => 'Required field "{field}" missing while performing action "{action}"',
     'Exception.JKingWeb/Arsse/Db/ExceptionInput.whitespace'                => 'Field "{field}" of action "{action}" may not contain only whitespace',
     'Exception.JKingWeb/Arsse/Db/ExceptionInput.tooLong'                   => 'Field "{field}" of action "{action}" has a maximum length of {max}',

tests/Misc/TestContext.php

@@ -28,6 +28,7 @@ class TestContext extends Test\AbstractTest {
             'limit' => 10,
             'offset' => 5,
             'folder' => 42,
+            'folderShallow' => 42,
             'subscription' => 2112,
             'article' => 255,
             'edition' => 65535,
@@ -37,12 +38,15 @@ class TestContext extends Test\AbstractTest {
             'starred' => true,
             'modifiedSince' => new \DateTime(),
             'notModifiedSince' => new \DateTime(),
+            'markedSince' => new \DateTime(),
+            'notMarkedSince' => new \DateTime(),
             'editions' => [1,2],
             'articles' => [1,2],
             'label' => 2112,
             'labelName' => "Rush",
+            'labelled' => true,
         ];
-        $times = ['modifiedSince','notModifiedSince'];
+        $times = ['modifiedSince','notModifiedSince','markedSince','notMarkedSince'];
         $c = new Context;
         foreach ((new \ReflectionObject($c))->getMethods(\ReflectionMethod::IS_PUBLIC) as $m) {
             if ($m->isConstructor() || $m->isStatic()) {

tests/REST/NextCloudNews/TestNCNV1_2.php

@@ -700,7 +700,7 @@ class TestNCNV1_2 extends Test\AbstractTest {
         Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(false)->limit(10)->oldestEdition(6)); // offset is one more than specified
         Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->limit(5)->latestEdition(4));   // offset is one less than specified
         Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->unread(true));
-        Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->modifiedSince($t));
+        Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->markedSince($t));
         Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->limit(5));
     }
 

tests/lib/Database/SeriesArticle.php

@@ -201,8 +201,8 @@ trait SeriesArticle {
             ],
             'rows' => [
                 [1,   1,1,1,'2000-01-01 00:00:00',''],
-                [5,  19,1,0,'2000-01-01 00:00:00',''],
+                [5,  19,1,0,'2016-01-01 00:00:00',''],
-                [5,  20,0,1,'2010-01-01 00:00:00',''],
+                [5,  20,0,1,'2005-01-01 00:00:00',''],
                 [7,  20,1,0,'2010-01-01 00:00:00',''],
                 [8, 102,1,0,'2000-01-02 02:00:00','Note 2'],
                 [9, 103,0,1,'2000-01-03 03:00:00','Note 3'],
@@ -233,16 +233,17 @@ trait SeriesArticle {
                 'article'      => "int",
                 'subscription' => "int",
                 'assigned'     => "bool",
+                'modified'     => "datetime",
             ],
             'rows' => [
-                [1, 1,1,1],
+                [1, 1,1,1,'2000-01-01 00:00:00'],
-                [2, 1,1,1],
+                [2, 1,1,1,'2000-01-01 00:00:00'],
-                [1,19,5,1],
+                [1,19,5,1,'2000-01-01 00:00:00'],
-                [2,20,5,1],
+                [2,20,5,1,'2000-01-01 00:00:00'],
-                [1, 5,3,0],
+                [1, 5,3,0,'2000-01-01 00:00:00'],
-                [2, 5,3,1],
+                [2, 5,3,1,'2000-01-01 00:00:00'],
-                [4, 7,4,0],
+                [4, 7,4,0,'2000-01-01 00:00:00'],
-                [4, 8,4,1],
+                [4, 8,4,1,'2015-01-01 00:00:00'],
             ],
         ],
     ];
@@ -348,6 +349,26 @@ trait SeriesArticle {
             'note' => "",
         ],
     ];
+    protected $fields = [
+        Database::AL_MINIMAL => [
+            "id", "subscription", "feed", "modified_date", "marked_date", "unread", "starred", "edition", 
+        ],
+        Database::AL_CONSERVATIVE => [
+            "id", "subscription", "feed", "modified_date", "marked_date", "unread", "starred", "edition", 
+            "url", "title", "subscription_title", "author", "guid", "published_date", "edited_date", "fingerprint",
+        ],
+        Database::AL_TYPICAL => [
+            "id", "subscription", "feed", "modified_date", "marked_date", "unread", "starred", "edition", 
+            "url", "title", "subscription_title", "author", "guid", "published_date", "edited_date", "fingerprint",
+            "content", "media_url", "media_type",
+        ],
+        Database::AL_FULL => [
+            "id", "subscription", "feed", "modified_date", "marked_date", "unread", "starred", "edition", 
+            "url", "title", "subscription_title", "author", "guid", "published_date", "edited_date", "fingerprint",
+            "content", "media_url", "media_type",
+            "note",
+        ],
+    ];
 
     public function setUpSeries() {
         $this->checkTables = ['arsse_marks' => ["subscription","article","read","starred","modified","note"],];
@@ -389,13 +410,18 @@ trait SeriesArticle {
         $this->compareIds([19], (new Context)->subscription(5)->latestEdition(19));
         $this->compareIds([20], (new Context)->subscription(5)->oldestEdition(999));
         $this->compareIds([20], (new Context)->subscription(5)->oldestEdition(1001));
-        // get items relative to modification date
+        // get items relative to (feed) modification date
         $exp = [2,4,6,8,20];
         $this->compareIds($exp, (new Context)->modifiedSince("2005-01-01T00:00:00Z"));
         $this->compareIds($exp, (new Context)->modifiedSince("2010-01-01T00:00:00Z"));
         $exp = [1,3,5,7,19];
         $this->compareIds($exp, (new Context)->notModifiedSince("2005-01-01T00:00:00Z"));
         $this->compareIds($exp, (new Context)->notModifiedSince("2000-01-01T00:00:00Z"));
+        // get items relative to (user) modification date (both marks and labels apply)
+        $this->compareIds([8,19], (new Context)->markedSince("2014-01-01T00:00:00Z"));
+        $this->compareIds([2,4,6,8,19,20], (new Context)->markedSince("2010-01-01T00:00:00Z"));
+        $this->compareIds([1,2,3,4,5,6,7,20], (new Context)->notMarkedSince("2014-01-01T00:00:00Z"));
+        $this->compareIds([1,3,5,7], (new Context)->notMarkedSince("2005-01-01T00:00:00Z"));
         // paged results
         $this->compareIds([1], (new Context)->limit(1));
         $this->compareIds([2], (new Context)->limit(1)->oldestEdition(1+1));
@@ -406,15 +432,21 @@ trait SeriesArticle {
         $this->compareIds([19], (new Context)->reverse(true)->limit(1)->latestEdition(1001-1));
         $this->compareIds([8], (new Context)->reverse(true)->limit(1)->latestEdition(19-1));
         $this->compareIds([7,6], (new Context)->reverse(true)->limit(2)->latestEdition(8-1));
-        // label by ID
+        // get articles by label ID
         $this->compareIds([1,19], (new Context)->label(1));
         $this->compareIds([1,5,20], (new Context)->label(2));
-        // label by name
+        // get articles by label name
         $this->compareIds([1,19], (new Context)->labelName("Interesting"));
         $this->compareIds([1,5,20], (new Context)->labelName("Fascinating"));
-        // any or no label
+        // get articles with any or no label
         $this->compareIds([1,5,8,19,20], (new Context)->labelled(true));
         $this->compareIds([2,3,4,6,7], (new Context)->labelled(false));
+        // get a specific article or edition
+        $this->compareIds([20], (new Context)->article(20));
+        $this->compareIds([20], (new Context)->edition(1001));
+        // get multiple specific articles or editions
+        $this->compareIds([1,20], (new Context)->articles([1,20,50]));
+        $this->compareIds([1,20], (new Context)->editions([1,1001,50]));
     }
 
     public function testListArticlesOfAMissingFolder() {
@@ -430,6 +462,16 @@ trait SeriesArticle {
     public function testListArticlesCheckingProperties() {
         $this->user = "john.doe@example.org";
         $this->assertResult($this->matches, Arsse::$db->articleList($this->user));
+        // check that the different fieldset groups return the expected columns
+        foreach ($this->fields as $constant => $columns) {
+            $test = array_keys(Arsse::$db->articleList($this->user, (new Context)->article(101), $constant)->getRow());
+            sort($columns);
+            sort($test);
+            $this->assertEquals($columns, $test, "Fields do not match expectation for verbosity $constant");
+        }
+        // check that an unknown fieldset produces an exception
+        $this->assertException("constantUnknown", "Db", "Exception");
+        Arsse::$db->articleList($this->user, (new Context)->article(101), \PHP_INT_MAX);
     }
 
     public function testListArticlesWithoutAuthority() {
@@ -781,8 +823,8 @@ trait SeriesArticle {
         $this->compareExpectations($state);
     }
 
-    public function testMarkByLastModified() {
+    public function testMarkByLastMarked() {
-        Arsse::$db->articleMark($this->user, ['starred'=>true], (new Context)->modifiedSince('2017-01-01T00:00:00Z'));
+        Arsse::$db->articleMark($this->user, ['starred'=>true], (new Context)->markedSince('2017-01-01T00:00:00Z'));
         $now = Date::transform(time(), "sql");
         $state = $this->primeExpectations($this->data, $this->checkTables);
         $state['arsse_marks']['rows'][8][3] = 1;
@@ -792,8 +834,8 @@ trait SeriesArticle {
         $this->compareExpectations($state);
     }
 
-    public function testMarkByNotLastModified() {
+    public function testMarkByNotLastMarked() {
-        Arsse::$db->articleMark($this->user, ['starred'=>true], (new Context)->notModifiedSince('2000-01-01T00:00:00Z'));
+        Arsse::$db->articleMark($this->user, ['starred'=>true], (new Context)->notMarkedSince('2000-01-01T00:00:00Z'));
         $now = Date::transform(time(), "sql");
         $state = $this->primeExpectations($this->data, $this->checkTables);
         $state['arsse_marks']['rows'][] = [13,5,0,1,$now,''];