diff --git a/lib/AbstractException.php b/lib/AbstractException.php index a79cd62..5c7ec40 100644 --- a/lib/AbstractException.php +++ b/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, diff --git a/lib/Database.php b/lib/Database.php index 366b880..88d8679 100644 --- a/lib/Database.php +++ b/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)),'') - ) as modified_date, + 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_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 = [ - // (id, subscription, feed, modified, unread, starred, edition): always included - "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", - "(select note from arsse_marks where article is arsse_articles.id and subscription in (select sub from subscribed_feeds)) as note" - ]; + $columns = []; + 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", + "guid", + "published as published_date", + "edited as edited_date", + "url_title_hash||':'||url_content_hash||':'||title_content_hash as fingerprint", + ]); + case self::AL_MINIMAL: // base metadata (always included: required for context matching) + // 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 diff --git a/lib/Misc/Context.php b/lib/Misc/Context.php index 8ebf454..edc9fb4 100644 --- a/lib/Misc/Context.php +++ b/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); } diff --git a/lib/REST/NextCloudNews/V1_2.php b/lib/REST/NextCloudNews/V1_2.php index 2a50b99..1261fb5 100644 --- a/lib/REST/NextCloudNews/V1_2.php +++ b/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 { diff --git a/locale/en.php b/locale/en.php index 286a2ff..f2f2148 100644 --- a/locale/en.php +++ b/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}', diff --git a/tests/Misc/TestContext.php b/tests/Misc/TestContext.php index 598789c..6626bf5 100644 --- a/tests/Misc/TestContext.php +++ b/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()) { diff --git a/tests/REST/NextCloudNews/TestNCNV1_2.php b/tests/REST/NextCloudNews/TestNCNV1_2.php index 9680fb8..df126ab 100644 --- a/tests/REST/NextCloudNews/TestNCNV1_2.php +++ b/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)); } diff --git a/tests/lib/Database/SeriesArticle.php b/tests/lib/Database/SeriesArticle.php index 5bf36bd..7b3d6bd 100644 --- a/tests/lib/Database/SeriesArticle.php +++ b/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, 20,0,1,'2010-01-01 00:00:00',''], + [5, 19,1,0,'2016-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'], @@ -229,20 +229,21 @@ trait SeriesArticle { ], 'arsse_label_members' => [ 'columns' => [ - 'label' => "int", - 'article' => "int", + 'label' => "int", + 'article' => "int", 'subscription' => "int", - 'assigned' => "bool", + 'assigned' => "bool", + 'modified' => "datetime", ], 'rows' => [ - [1, 1,1,1], - [2, 1,1,1], - [1,19,5,1], - [2,20,5,1], - [1, 5,3,0], - [2, 5,3,1], - [4, 7,4,0], - [4, 8,4,1], + [1, 1,1,1,'2000-01-01 00:00:00'], + [2, 1,1,1,'2000-01-01 00:00:00'], + [1,19,5,1,'2000-01-01 00:00:00'], + [2,20,5,1,'2000-01-01 00:00:00'], + [1, 5,3,0,'2000-01-01 00:00:00'], + [2, 5,3,1,'2000-01-01 00:00:00'], + [4, 7,4,0,'2000-01-01 00:00:00'], + [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() { - Arsse::$db->articleMark($this->user, ['starred'=>true], (new Context)->modifiedSince('2017-01-01T00:00:00Z')); + public function testMarkByLastMarked() { + 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() { - Arsse::$db->articleMark($this->user, ['starred'=>true], (new Context)->notModifiedSince('2000-01-01T00:00:00Z')); + public function testMarkByNotLastMarked() { + 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,''];