diff --git a/lib/Database.php b/lib/Database.php index f661e14..ceff7ff 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -11,6 +11,7 @@ use JKingWeb\Arsse\Misc\ValueInfo; class Database { const SCHEMA_VERSION = 2; + const LIMIT_ARTICLES = 50; /** @var Db\Driver */ public $db; @@ -855,8 +856,8 @@ class Database { // if multiple specific editions have been requested, prepare a CTE to list them and their articles if (!$context->editions) { throw new Db\ExceptionInput("tooShort", ['field' => "editions", 'action' => __FUNCTION__, 'min' => 1]); // must have at least one array element - } elseif (sizeof($context->editions) > 50) { - throw new Db\ExceptionInput("tooLong", ['field' => "editions", 'action' => __FUNCTION__, 'max' => 50]); // must not have more than 50 array elements + } elseif (sizeof($context->editions) > self::LIMIT_ARTICLES) { + throw new Db\ExceptionInput("tooLong", ['field' => "editions", 'action' => __FUNCTION__, 'max' => self::LIMIT_ARTICLES]); // @codeCoverageIgnore } list($inParams, $inTypes) = $this->generateIn($context->editions, "int"); $q->setCTE("requested_articles(id,edition)", @@ -869,8 +870,8 @@ class Database { // if multiple specific articles have been requested, prepare a CTE to list them and their articles if (!$context->articles) { throw new Db\ExceptionInput("tooShort", ['field' => "articles", 'action' => __FUNCTION__, 'min' => 1]); // must have at least one array element - } elseif (sizeof($context->articles) > 50) { - throw new Db\ExceptionInput("tooLong", ['field' => "articles", 'action' => __FUNCTION__, 'max' => 50]); // must not have more than 50 array elements + } elseif (sizeof($context->articles) > self::LIMIT_ARTICLES) { + throw new Db\ExceptionInput("tooLong", ['field' => "articles", 'action' => __FUNCTION__, 'max' => self::LIMIT_ARTICLES]); // @codeCoverageIgnore } list($inParams, $inTypes) = $this->generateIn($context->articles, "int"); $q->setCTE("requested_articles(id,edition)", @@ -917,27 +918,62 @@ class Database { return $q; } + protected function articleChunk(Context $context): array { + $exception = ""; + if ($context->editions()) { + // editions take precedence over articles + if (sizeof($context->editions) > self::LIMIT_ARTICLES) { + $exception = "editions"; + } + } elseif ($context->articles()) { + if (sizeof($context->articles) > self::LIMIT_ARTICLES) { + $exception = "articles"; + } + } + if ($exception) { + $out = []; + $list = array_chunk($context->$exception, self::LIMIT_ARTICLES); + foreach ($list as $chunk) { + $out[] = (clone $context)->$exception($chunk); + } + return $out; + } else { + return []; + } + } + public function articleList(string $user, Context $context = null): Db\Result { if (!Arsse::$user->authorize($user, __FUNCTION__)) { throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); } $context = $context ?? new Context; - $columns = [ - "arsse_articles.url as url", - "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", - ]; - $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 - return $this->db->prepare($q->getQuery(), $q->getTypes())->run($q->getValues()); + // if the context has more articles or editions than we can process in one query, perform a series of queries and return an aggregate result + if ($contexts = $this->articleChunk($context)) { + $out = []; + $tr = $this->begin(); + foreach ($contexts as $context) { + $out[] = $this->articleList($user, $context); + } + $tr->commit(); + return new Db\ResultAggregate(...$out); + } else { + $columns = [ + "arsse_articles.url as url", + "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", + ]; + $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 + return $this->db->prepare($q->getQuery(), $q->getTypes())->run($q->getValues()); + } } public function articleCount(string $user, Context $context = null): int { @@ -945,10 +981,21 @@ class Database { throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); } $context = $context ?? new Context; - $q = $this->articleQuery($user, $context); - $q->pushCTE("selected_articles"); - $q->setBody("SELECT count(*) from selected_articles"); - return $this->db->prepare($q->getQuery(), $q->getTypes())->run($q->getValues())->getValue(); + // if the context has more articles or editions than we can process in one query, perform a series of queries and return an aggregate result + if ($contexts = $this->articleChunk($context)) { + $out = 0; + $tr = $this->begin(); + foreach ($contexts as $context) { + $out += $this->articleCount($user, $context); + } + $tr->commit(); + return $out; + } else { + $q = $this->articleQuery($user, $context); + $q->pushCTE("selected_articles"); + $q->setBody("SELECT count(*) from selected_articles"); + return $this->db->prepare($q->getQuery(), $q->getTypes())->run($q->getValues())->getValue(); + } } public function articleMark(string $user, array $data, Context $context = null): int { @@ -956,62 +1003,73 @@ class Database { throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); } $context = $context ?? new Context; - // sanitize input - $values = [ - isset($data['read']) ? $data['read'] : null, - isset($data['starred']) ? $data['starred'] : null, - ]; - // the two queries we want to execute to make the requested changes - $queries = [ - "UPDATE arsse_marks - set - read = case when (select honour_read from target_articles where target_articles.id is article) is 1 then (select read from target_values) else read end, - starred = coalesce((select starred from target_values),starred), - modified = CURRENT_TIMESTAMP - WHERE - subscription in (select sub from subscribed_feeds) - and article in (select id from target_articles where to_insert is 0 and (honour_read is 1 or honour_star is 1))", - "INSERT INTO arsse_marks(subscription,article,read,starred) - select - (select id from arsse_subscriptions join user on user is owner where arsse_subscriptions.feed is target_articles.feed), - id, - coalesce((select read from target_values) * honour_read,0), - coalesce((select starred from target_values),0) - from target_articles where to_insert is 1 and (honour_read is 1 or honour_star is 1)" - ]; - $out = 0; - // wrap this UPDATE and INSERT together into a transaction - $tr = $this->begin(); - // if an edition context is specified, make sure it's valid - if ($context->edition()) { - // make sure the edition exists - $edition = $this->articleValidateEdition($user, $context->edition); - // if the edition is not the latest, do not mark the read flag - if (!$edition['current']) { - $values[0] = null; + // if the context has more articles or editions than we can process in one query, perform a series of queries and return an aggregate result + if ($contexts = $this->articleChunk($context)) { + $out = 0; + $tr = $this->begin(); + foreach ($contexts as $context) { + $out += $this->articleMark($user, $data, $context); } - } elseif ($context->article()) { - // otherwise if an article context is specified, make sure it's valid - $this->articleValidateId($user, $context->article); - } - // execute each query in sequence - foreach ($queries as $query) { - // first build the query which will select the target articles; we will later turn this into a CTE for the actual query that manipulates the articles - $q = $this->articleQuery($user, $context, [ - "(not exists(select article from arsse_marks where article is arsse_articles.id and subscription in (select sub from subscribed_feeds))) as to_insert", - "((select read from target_values) is not null and (select read from target_values) is not (coalesce((select read from arsse_marks where article is arsse_articles.id and subscription in (select sub from subscribed_feeds)),0)) and (not exists(select * from requested_articles) or (select max(id) from arsse_editions where article is arsse_articles.id) in (select edition from requested_articles))) as honour_read", - "((select starred from target_values) is not null and (select starred from target_values) is not (coalesce((select starred from arsse_marks where article is arsse_articles.id and subscription in (select sub from subscribed_feeds)),0))) as honour_star", - ]); - // common table expression with the values to set - $q->setCTE("target_values(read,starred)", "SELECT ?,?", ["bool","bool"], $values); - // push the current query onto the CTE stack and execute the query we're actually interested in - $q->pushCTE("target_articles"); - $q->setBody($query); - $out += $this->db->prepare($q->getQuery(), $q->getTypes())->run($q->getValues())->changes(); + $tr->commit(); + return $out; + } else { + // sanitize input + $values = [ + isset($data['read']) ? $data['read'] : null, + isset($data['starred']) ? $data['starred'] : null, + ]; + // the two queries we want to execute to make the requested changes + $queries = [ + "UPDATE arsse_marks + set + read = case when (select honour_read from target_articles where target_articles.id is article) is 1 then (select read from target_values) else read end, + starred = coalesce((select starred from target_values),starred), + modified = CURRENT_TIMESTAMP + WHERE + subscription in (select sub from subscribed_feeds) + and article in (select id from target_articles where to_insert is 0 and (honour_read is 1 or honour_star is 1))", + "INSERT INTO arsse_marks(subscription,article,read,starred) + select + (select id from arsse_subscriptions join user on user is owner where arsse_subscriptions.feed is target_articles.feed), + id, + coalesce((select read from target_values) * honour_read,0), + coalesce((select starred from target_values),0) + from target_articles where to_insert is 1 and (honour_read is 1 or honour_star is 1)" + ]; + $out = 0; + // wrap this UPDATE and INSERT together into a transaction + $tr = $this->begin(); + // if an edition context is specified, make sure it's valid + if ($context->edition()) { + // make sure the edition exists + $edition = $this->articleValidateEdition($user, $context->edition); + // if the edition is not the latest, do not mark the read flag + if (!$edition['current']) { + $values[0] = null; + } + } elseif ($context->article()) { + // otherwise if an article context is specified, make sure it's valid + $this->articleValidateId($user, $context->article); + } + // execute each query in sequence + foreach ($queries as $query) { + // first build the query which will select the target articles; we will later turn this into a CTE for the actual query that manipulates the articles + $q = $this->articleQuery($user, $context, [ + "(not exists(select article from arsse_marks where article is arsse_articles.id and subscription in (select sub from subscribed_feeds))) as to_insert", + "((select read from target_values) is not null and (select read from target_values) is not (coalesce((select read from arsse_marks where article is arsse_articles.id and subscription in (select sub from subscribed_feeds)),0)) and (not exists(select * from requested_articles) or (select max(id) from arsse_editions where article is arsse_articles.id) in (select edition from requested_articles))) as honour_read", + "((select starred from target_values) is not null and (select starred from target_values) is not (coalesce((select starred from arsse_marks where article is arsse_articles.id and subscription in (select sub from subscribed_feeds)),0))) as honour_star", + ]); + // common table expression with the values to set + $q->setCTE("target_values(read,starred)", "SELECT ?,?", ["bool","bool"], $values); + // push the current query onto the CTE stack and execute the query we're actually interested in + $q->pushCTE("target_articles"); + $q->setBody($query); + $out += $this->db->prepare($q->getQuery(), $q->getTypes())->run($q->getValues())->changes(); + } + // commit the transaction + $tr->commit(); + return $out; } - // commit the transaction - $tr->commit(); - return $out; } public function articleStarred(string $user): array { diff --git a/lib/Db/AbstractResult.php b/lib/Db/AbstractResult.php index dce2926..59592f9 100644 --- a/lib/Db/AbstractResult.php +++ b/lib/Db/AbstractResult.php @@ -9,17 +9,19 @@ abstract class AbstractResult implements Result { // actual public methods public function getValue() { - $this->next(); if ($this->valid()) { - $keys = array_keys($this->cur); - return $this->cur[array_shift($keys)]; + $out = array_shift($this->cur); + $this->next(); + return $out; } + $this->next(); return null; } public function getRow() { + $out = ($this->valid() ? $this->cur : null); $this->next(); - return ($this->valid() ? $this->cur : null); + return $out; } public function getAll(): array { diff --git a/lib/Db/ResultAggregate.php b/lib/Db/ResultAggregate.php new file mode 100644 index 0000000..2244bd9 --- /dev/null +++ b/lib/Db/ResultAggregate.php @@ -0,0 +1,46 @@ +data, function($sum, $value) {return $sum + $value->changes();}, 0); + } + + public function lastId() { + return $this->data[sizeof($this->data) - 1]->lastId(); + } + + // constructor/destructor + + public function __construct(Result ...$result) { + $this->data = $result; + } + + public function __destruct() { + $max = sizeof($this->data); + for ($a = 0; $a < $max; $a++) { + unset($this->data[$a]); + } + } + + // PHP iterator methods + + public function valid() { + while (!$this->cur && isset($this->data[$this->index])) { + $this->cur = $this->data[$this->index]->getRow(); + if (!$this->cur) { + $this->index++; + } + } + return (bool) $this->cur; + } +} diff --git a/lib/Db/SQLite3/Result.php b/lib/Db/SQLite3/Result.php index 9aed7a9..d0fdacd 100644 --- a/lib/Db/SQLite3/Result.php +++ b/lib/Db/SQLite3/Result.php @@ -7,7 +7,6 @@ use JKingWeb\Arsse\Db\Exception; class Result extends \JKingWeb\Arsse\Db\AbstractResult { protected $st; protected $set; - protected $pos = 0; protected $cur = null; protected $rows = 0; protected $id = 0; diff --git a/tests/Db/TestResultAggregate.php b/tests/Db/TestResultAggregate.php new file mode 100644 index 0000000..433ef42 --- /dev/null +++ b/tests/Db/TestResultAggregate.php @@ -0,0 +1,101 @@ + */ +class TestResultAggregate extends Test\AbstractTest { + + public function testGetChangeCountAndLastInsertId() { + $in = [ + new Result([], 3, 4), + new Result([], 27, 10), + new Result([], 12, 2112), + ]; + $r = new Db\ResultAggregate(...$in); + $this->assertEquals(42, $r->changes()); + $this->assertEquals(2112, $r->lastId()); + } + + public function testIterateOverResults() { + $in = [ + new Result([['col' => 1]]), + new Result([['col' => 2]]), + new Result([['col' => 3]]), + ]; + $rows = []; + foreach (new Db\ResultAggregate(...$in) as $index => $row) { + $rows[$index] = $row['col']; + } + $this->assertEquals([0 => 1, 1 => 2, 2 => 3], $rows); + } + + public function testIterateOverResultsTwice() { + $in = [ + new Result([['col' => 1]]), + new Result([['col' => 2]]), + new Result([['col' => 3]]), + ]; + $rows = []; + $test = new Db\ResultAggregate(...$in); + foreach ($test as $row) { + $rows[] = $row['col']; + } + $this->assertEquals([1,2,3], $rows); + $this->assertException("resultReused", "Db"); + foreach ($test as $row) { + $rows[] = $row['col']; + } + } + + public function testGetSingleValues() { + $test = new Db\ResultAggregate(...[ + new Result([['year' => 1867]]), + new Result([['year' => 1970]]), + new Result([['year' => 2112]]), + ]); + $this->assertEquals(1867, $test->getValue()); + $this->assertEquals(1970, $test->getValue()); + $this->assertEquals(2112, $test->getValue()); + $this->assertSame(null, $test->getValue()); + } + + public function testGetFirstValuesOnly() { + $test = new Db\ResultAggregate(...[ + new Result([['year' => 1867, 'century' => 19]]), + new Result([['year' => 1970, 'century' => 20]]), + new Result([['year' => 2112, 'century' => 22]]), + ]); + $this->assertEquals(1867, $test->getValue()); + $this->assertEquals(1970, $test->getValue()); + $this->assertEquals(2112, $test->getValue()); + $this->assertSame(null, $test->getValue()); + } + + public function testGetRows() { + $test = new Db\ResultAggregate(...[ + new Result([['album' => '2112', 'track' => '2112']]), + new Result([['album' => 'Clockwork Angels', 'track' => 'The Wreckers']]), + ]); + $rows = [ + ['album' => '2112', 'track' => '2112'], + ['album' => 'Clockwork Angels', 'track' => 'The Wreckers'], + ]; + $this->assertEquals($rows[0], $test->getRow()); + $this->assertEquals($rows[1], $test->getRow()); + $this->assertSame(null, $test->getRow()); + } + + public function testGetAllRows() { + $test = new Db\ResultAggregate(...[ + new Result([['album' => '2112', 'track' => '2112']]), + new Result([['album' => 'Clockwork Angels', 'track' => 'The Wreckers']]), + ]); + $rows = [ + ['album' => '2112', 'track' => '2112'], + ['album' => 'Clockwork Angels', 'track' => 'The Wreckers'], + ]; + $this->assertEquals($rows, $test->getAll()); + } +} \ No newline at end of file diff --git a/tests/REST/TinyTinyRSS/TestTinyTinyAPI.php b/tests/REST/TinyTinyRSS/TestTinyTinyAPI.php index 1012d24..d92df3e 100644 --- a/tests/REST/TinyTinyRSS/TestTinyTinyAPI.php +++ b/tests/REST/TinyTinyRSS/TestTinyTinyAPI.php @@ -929,7 +929,7 @@ class TestTinyTinyAPI extends Test\AbstractTest { $this->setUp(); Phake::when(Arsse::$db)->articleMark->thenReturn(42); Phake::when(Arsse::$db)->subscriptionList->thenReturn(new Result($this->subscriptions)); - Phake::when(Arsse::$db)->subscriptionList($this->anything(), null, false)->thenReturn(new Result(array_filter($this->subscriptions, function($value) {return is_null($value['folder']);}))); + Phake::when(Arsse::$db)->subscriptionList($this->anything(), null, false)->thenReturn(new Result($this->filterSubs(null))); Phake::when(Arsse::$db)->labelList->thenReturn(new Result($this->labels)); Phake::when(Arsse::$db)->labelList($this->anything(), false)->thenReturn(new Result($this->usedLabels)); // verify the complex contexts diff --git a/tests/lib/Database/SeriesArticle.php b/tests/lib/Database/SeriesArticle.php index 83c4d80..9731f4f 100644 --- a/tests/lib/Database/SeriesArticle.php +++ b/tests/lib/Database/SeriesArticle.php @@ -2,6 +2,7 @@ declare(strict_types=1); namespace JKingWeb\Arsse\Test\Database; +use JKingWeb\Arsse\Database; use JKingWeb\Arsse\Arsse; use JKingWeb\Arsse\Misc\Context; use JKingWeb\Arsse\Misc\Date; @@ -346,6 +347,7 @@ trait SeriesArticle { // get all items for user $exp = [1,2,3,4,5,6,7,8,19,20]; $this->compareIds($exp, new Context); + $this->compareIds($exp, (new Context)->articles(range(1, Database::LIMIT_ARTICLES * 3))); // get items from a folder tree $exp = [5,6,7,8]; $this->compareIds($exp, (new Context)->folder(1)); @@ -610,8 +612,7 @@ trait SeriesArticle { } public function testMarkTooManyMultipleArticles() { - $this->assertException("tooLong", "Db", "ExceptionInput"); - Arsse::$db->articleMark($this->user, ['read'=>false,'starred'=>true], (new Context)->articles(range(1, 51))); + $this->assertSame(7, Arsse::$db->articleMark($this->user, ['read'=>false,'starred'=>true], (new Context)->articles(range(1, Database::LIMIT_ARTICLES * 3)))); } public function testMarkAMissingArticle() { @@ -676,8 +677,7 @@ trait SeriesArticle { } public function testMarkTooManyMultipleEditions() { - $this->assertException("tooLong", "Db", "ExceptionInput"); - Arsse::$db->articleMark($this->user, ['read'=>false,'starred'=>true], (new Context)->editions(range(1, 51))); + $this->assertSame(7, Arsse::$db->articleMark($this->user, ['read'=>false,'starred'=>true], (new Context)->editions(range(1, 51)))); } public function testMarkAStaleEditionUnread() { @@ -769,6 +769,7 @@ trait SeriesArticle { $this->assertSame(2, Arsse::$db->articleCount("john.doe@example.com", (new Context)->starred(true))); $this->assertSame(4, Arsse::$db->articleCount("john.doe@example.com", (new Context)->folder(1))); $this->assertSame(0, Arsse::$db->articleCount("jane.doe@example.com", (new Context)->starred(true))); + $this->assertSame(10, Arsse::$db->articleCount("john.doe@example.com", (new Context)->articles(range(1, Database::LIMIT_ARTICLES *3)))); } public function testCountArticlesWithoutAuthority() { diff --git a/tests/lib/Result.php b/tests/lib/Result.php index 3bf3316..6c44d82 100644 --- a/tests/lib/Result.php +++ b/tests/lib/Result.php @@ -13,21 +13,24 @@ class Result implements \JKingWeb\Arsse\Db\Result { // actual public methods public function getValue() { - $arr = $this->next(); if ($this->valid()) { - $keys = array_keys($arr); - return $arr[array_shift($keys)]; + $keys = array_keys($this->current()); + $out = $this->current()[array_shift($keys)]; + $this->next(); + return $out; } + $this->next(); return null; } public function getRow() { - $arr = $this->next(); - return ($this->valid() ? $arr : null); + $out = ($this->valid() ? $this->current() : null); + $this->next(); + return $out; } public function getAll(): array { - return $this->set; + return iterator_to_array($this, false); } public function changes() { @@ -52,22 +55,22 @@ class Result implements \JKingWeb\Arsse\Db\Result { // PHP iterator methods public function valid() { - return !is_null(key($this->set)); + return $this->pos < sizeof($this->set); } public function next() { - return next($this->set); + $this->pos++; } public function current() { - return current($this->set); + return $this->set[$this->key()]; } public function key() { - return key($this->set); + return array_keys($this->set)[$this->pos]; } public function rewind() { - reset($this->set); + $this->pos = 0; } } diff --git a/tests/phpunit.xml b/tests/phpunit.xml index f12fe40..68213eb 100644 --- a/tests/phpunit.xml +++ b/tests/phpunit.xml @@ -47,6 +47,7 @@ Db/TestTransaction.php + Db/TestResultAggregate.php Db/SQLite3/TestDbResultSQLite3.php Db/SQLite3/TestDbStatementSQLite3.php Db/SQLite3/TestDbDriverCreationSQLite3.php