diff --git a/lib/Database.php b/lib/Database.php index 43e6a2e5..72a06e45 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -43,6 +43,12 @@ class Database { const LIMIT_SET_SIZE = 25; /** The length of a string in an embedded set beyond which a parameter placeholder will be used for the string */ const LIMIT_SET_STRING_LENGTH = 200; + /** Makes tag/label association change operations remove members */ + const ASSOC_REMOVE = 0; + /** Makes tag/label association change operations add members */ + const ASSOC_ADD = 1; + /** Makes tag/label association change operations replace members */ + const ASSOC_REPLACE = 2; /** A map database driver short-names and their associated class names */ const DRIVER_NAMES = [ 'sqlite3' => \JKingWeb\Arsse\Db\SQLite3\Driver::class, @@ -1955,37 +1961,61 @@ class Database { * @param string $user The owner of the label * @param integer|string $id The numeric identifier or name of the label * @param Context $context The query context matching the desired articles - * @param boolean $remove Whether to remove (true) rather than add (true) an association with the articles matching the context + * @param int $mode Whether to add (ASSOC_ADD), remove (ASSOC_REMOVE), or replace with (ASSOC_REPLACE) the matching associations * @param boolean $byName Whether to interpret the $id parameter as the label's name (true) or identifier (false) */ - public function labelArticlesSet(string $user, $id, Context $context = null, bool $remove = false, bool $byName = false): int { + public function labelArticlesSet(string $user, $id, Context $context, int $mode = self::ASSOC_ADD, bool $byName = false): int { + if (!in_array($mode, [self::ASSOC_ADD, self::ASSOC_REMOVE, self::ASSOC_REPLACE])) { + throw new Exception("constantUnknown", $mode); // @codeCoverageIgnore + } if (!Arsse::$user->authorize($user, __FUNCTION__)) { throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); } - // validate the label ID, and get the numeric ID if matching by name + // validate the tag ID, and get the numeric ID if matching by name $id = $this->labelValidateId($user, $id, $byName, true)['id']; - $context = $context ?? new Context; - // prepare either one or two queries - // first update any existing entries with the removal or re-addition of their association - $q1 = $this->articleQuery($user, $context); - $q1->pushCTE("target_articles"); - $q1->setBody("UPDATE arsse_label_members set assigned = ?, modified = CURRENT_TIMESTAMP where label = ? and assigned <> ? and article in (select id from target_articles)", ["bool","int","bool"], [!$remove, $id, !$remove]); - $v1 = $q1->getValues(); - $q1 = $this->db->prepare($q1->getQuery(), $q1->getTypes()); - // next, if we're not removing, add any new entries that need to be added - if (!$remove) { - $q2 = $this->articleQuery($user, $context, ["id", "subscription"]); - $q2->pushCTE("target_articles"); - $q2->setBody("SELECT ?,id,subscription from target_articles where id not in (select article from arsse_label_members where label = ?)", ["int", "int"], [$id, $id]); - $v2 = $q2->getValues(); - $q2 = $this->db->prepare("INSERT INTO arsse_label_members(label,article,subscription) ".$q2->getQuery(), $q2->getTypes()); + // get the list of articles matching the context + $articles = iterator_to_array($this->articleList($user, $context ?? new Context)); + // an empty article list is a special case + if (!sizeof($articles)) { + if ($mode == self::ASSOC_REPLACE) { + // replacing with an empty set means setting everything to zero + return $this->db->prepare("UPDATE arsse_label_members set assigned = 0, modified = CURRENT_TIMESTAMP where label = ? and assigned = 1", "int")->run($id)->changes(); + } else { + // adding or removing is a no-op + return 0; + } + } else { + $articles = array_column($articles, "id"); + } + // prepare up to three queries: removing requires one, adding two, and replacing three + list($inClause, $inTypes, $inValues) = $this->generateIn($articles, "int"); + $updateQ = "UPDATE arsse_label_members set assigned = ?, modified = CURRENT_TIMESTAMP where label = ? and assigned <> ? and article %in% ($inClause)"; + $updateT = ["bool", "int", "bool", $inTypes]; + $insertQ = "INSERT INTO arsse_label_members(label,article,subscription) SELECT ?,a.id,s.id from arsse_articles as a join arsse_subscriptions as s on a.feed = s.feed where s.owner = ? and a.id not in (select article from arsse_label_members where label = ?) and a.id in ($inClause)"; + $insertT = ["int", "str", "int", $inTypes]; + $clearQ = str_replace("%in%", "not in", $updateQ); + $clearT = $updateT; + $updateQ = str_replace("%in%", "in", $updateQ); + $qList = []; + switch ($mode) { + case self::ASSOC_REMOVE: + $qList[] = [$updateQ, $updateT, [false, $id, false, $inValues]]; // soft-delete any existing associations + break; + case self::ASSOC_ADD: + $qList[] = [$updateQ, $updateT, [true, $id, true, $inValues]]; // re-enable any previously soft-deleted association + $qList[] = [$insertQ, $insertT, [$id, $user, $id, $inValues]]; // insert any newly-required associations + break; + case self::ASSOC_REPLACE: + $qList[] = [$clearQ, $clearT, [false, $id, false, $inValues]]; // soft-delete any existing associations for articles not in the list + $qList[] = [$updateQ, $updateT, [true, $id, true, $inValues]]; // re-enable any previously soft-deleted association + $qList[] = [$insertQ, $insertT, [$id, $user, $id, $inValues]]; // insert any newly-required associations + break; } // execute them in a transaction $out = 0; $tr = $this->begin(); - $out += $q1->run($v1)->changes(); - if (!$remove) { - $out += $q2->run($v2)->changes(); + foreach ($qList as list($q, $t, $v)) { + $out += $this->db->prepare($q, ...$t)->run(...$v)->changes(); } $tr->commit(); return $out; @@ -2235,47 +2265,58 @@ class Database { * * @param string $user The owner of the tag * @param integer|string $id The numeric identifier or name of the tag - * @param integer[] $context The query context matching the desired subscriptions - * @param boolean $remove Whether to remove (true) rather than add (true) an association with the subscriptions matching the context + * @param integer[] $subscriptions An array listing the desired subscriptions + * @param int $mode Whether to add (ASSOC_ADD), remove (ASSOC_REMOVE), or replace with (ASSOC_REPLACE) the listed associations * @param boolean $byName Whether to interpret the $id parameter as the tag's name (true) or identifier (false) */ - public function tagSubscriptionsSet(string $user, $id, array $subscriptions, bool $remove = false, bool $byName = false): int { + public function tagSubscriptionsSet(string $user, $id, array $subscriptions, int $mode = self::ASSOC_ADD, bool $byName = false): int { + if (!in_array($mode, [self::ASSOC_ADD, self::ASSOC_REMOVE, self::ASSOC_REPLACE])) { + throw new Exception("constantUnknown", $mode); // @codeCoverageIgnore + } if (!Arsse::$user->authorize($user, __FUNCTION__)) { throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); } // validate the tag ID, and get the numeric ID if matching by name $id = $this->tagValidateId($user, $id, $byName, true)['id']; - // prepare either one or two queries + // an empty subscription list is a special case + if (!sizeof($subscriptions)) { + if ($mode == self::ASSOC_REPLACE) { + // replacing with an empty set means setting everything to zero + return $this->db->prepare("UPDATE arsse_tag_members set assigned = 0, modified = CURRENT_TIMESTAMP where tag = ? and assigned = 1", "int")->run($id)->changes(); + } else { + // adding or removing is a no-op + return 0; + } + } + // prepare up to three queries: removing requires one, adding two, and replacing three list($inClause, $inTypes, $inValues) = $this->generateIn($subscriptions, "int"); - // first update any existing entries with the removal or re-addition of their association - $q1 = $this->db->prepare( - "UPDATE arsse_tag_members - set assigned = ?, modified = CURRENT_TIMESTAMP - where tag = ? and assigned <> ? and subscription in (select id from arsse_subscriptions where owner = ? and id in ($inClause))", - "bool", - "int", - "bool", - "str", - $inTypes - ); - $v1 = [!$remove, $id, !$remove, $user, $inValues]; - // next, if we're not removing, add any new entries that need to be added - if (!$remove) { - $q2 = $this->db->prepare( - "INSERT INTO arsse_tag_members(tag,subscription) SELECT ?,id from arsse_subscriptions where id not in (select subscription from arsse_tag_members where tag = ?) and owner = ? and id in ($inClause)", - "int", - "int", - "str", - $inTypes - ); - $v2 = [$id, $id, $user, $inValues]; + $updateQ = "UPDATE arsse_tag_members set assigned = ?, modified = CURRENT_TIMESTAMP where tag = ? and assigned <> ? and subscription in (select id from arsse_subscriptions where owner = ? and id %in% ($inClause))"; + $updateT = ["bool", "int", "bool", "str", $inTypes]; + $insertQ = "INSERT INTO arsse_tag_members(tag,subscription) SELECT ?,id from arsse_subscriptions where id not in (select subscription from arsse_tag_members where tag = ?) and owner = ? and id in ($inClause)"; + $insertT = ["int", "int", "str", $inTypes]; + $clearQ = str_replace("%in%", "not in", $updateQ); + $clearT = $updateT; + $updateQ = str_replace("%in%", "in", $updateQ); + $qList = []; + switch ($mode) { + case self::ASSOC_REMOVE: + $qList[] = [$updateQ, $updateT, [0, $id, 0, $user, $inValues]]; // soft-delete any existing associations + break; + case self::ASSOC_ADD: + $qList[] = [$updateQ, $updateT, [1, $id, 1, $user, $inValues]]; // re-enable any previously soft-deleted association + $qList[] = [$insertQ, $insertT, [$id, $id, $user, $inValues]]; // insert any newly-required associations + break; + case self::ASSOC_REPLACE: + $qList[] = [$clearQ, $clearT, [0, $id, 0, $user, $inValues]]; // soft-delete any existing associations for subscriptions not in the list + $qList[] = [$updateQ, $updateT, [1, $id, 1, $user, $inValues]]; // re-enable any previously soft-deleted association + $qList[] = [$insertQ, $insertT, [$id, $id, $user, $inValues]]; // insert any newly-required associations + break; } // execute them in a transaction $out = 0; $tr = $this->begin(); - $out += $q1->run($v1)->changes(); - if (!$remove) { - $out += $q2->run($v2)->changes(); + foreach ($qList as list($q, $t, $v)) { + $out += $this->db->prepare($q, ...$t)->run(...$v)->changes(); } $tr->commit(); return $out; diff --git a/lib/REST/TinyTinyRSS/API.php b/lib/REST/TinyTinyRSS/API.php index 8bf85bcc..d29c0cf4 100644 --- a/lib/REST/TinyTinyRSS/API.php +++ b/lib/REST/TinyTinyRSS/API.php @@ -1017,6 +1017,7 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { $label = $this->labelIn($data['label_id']); $articles = explode(",", (string) $data['article_ids']); $assign = $data['assign'] ?? false; + $assign = $assign ? Database::ASSOC_ADD : Database::ASSOC_REMOVE; $out = 0; $in = array_chunk($articles, 50); for ($a = 0; $a < sizeof($in); $a++) { @@ -1024,7 +1025,7 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { $c = new Context; $c->articles($in[$a]); try { - $out += Arsse::$db->labelArticlesSet(Arsse::$user->id, $label, $c, !$assign); + $out += Arsse::$db->labelArticlesSet(Arsse::$user->id, $label, $c, $assign); } catch (ExceptionInput $e) { } } diff --git a/tests/cases/Database/SeriesLabel.php b/tests/cases/Database/SeriesLabel.php index 9ffc01bc..1f11004d 100644 --- a/tests/cases/Database/SeriesLabel.php +++ b/tests/cases/Database/SeriesLabel.php @@ -7,6 +7,7 @@ declare(strict_types=1); namespace JKingWeb\Arsse\TestCase\Database; use JKingWeb\Arsse\Arsse; +use JKingWeb\Arsse\Database; use JKingWeb\Arsse\Context\Context; use JKingWeb\Arsse\Misc\Date; use Phake; @@ -490,14 +491,14 @@ trait SeriesLabel { } public function testClearALabelFromArticles() { - Arsse::$db->labelArticlesSet("john.doe@example.com", 1, (new Context)->articles([1,5]), true); + Arsse::$db->labelArticlesSet("john.doe@example.com", 1, (new Context)->articles([1,5]), Database::ASSOC_REMOVE); $state = $this->primeExpectations($this->data, $this->checkMembers); $state['arsse_label_members']['rows'][0][3] = 0; $this->compareExpectations($state); } public function testApplyALabelToArticlesByName() { - Arsse::$db->labelArticlesSet("john.doe@example.com", "Interesting", (new Context)->articles([2,5]), false, true); + Arsse::$db->labelArticlesSet("john.doe@example.com", "Interesting", (new Context)->articles([2,5]), Database::ASSOC_ADD, true); $state = $this->primeExpectations($this->data, $this->checkMembers); $state['arsse_label_members']['rows'][4][3] = 1; $state['arsse_label_members']['rows'][] = [1,2,1,1]; @@ -505,12 +506,42 @@ trait SeriesLabel { } public function testClearALabelFromArticlesByName() { - Arsse::$db->labelArticlesSet("john.doe@example.com", "Interesting", (new Context)->articles([1,5]), true, true); + Arsse::$db->labelArticlesSet("john.doe@example.com", "Interesting", (new Context)->articles([1,5]), Database::ASSOC_REMOVE, true); $state = $this->primeExpectations($this->data, $this->checkMembers); $state['arsse_label_members']['rows'][0][3] = 0; $this->compareExpectations($state); } + public function testApplyALabelToNoArticles() { + Arsse::$db->labelArticlesSet("john.doe@example.com", 1, (new Context)->articles([10000])); + $state = $this->primeExpectations($this->data, $this->checkMembers); + $this->compareExpectations($state); + } + + public function testClearALabelFromNoArticles() { + Arsse::$db->labelArticlesSet("john.doe@example.com", 1, (new Context)->articles([10000]), Database::ASSOC_REMOVE); + $state = $this->primeExpectations($this->data, $this->checkMembers); + $this->compareExpectations($state); + } + + public function testReplaceArticlesOfALabel() { + Arsse::$db->labelArticlesSet("john.doe@example.com", 1, (new Context)->articles([2,5]), Database::ASSOC_REPLACE); + $state = $this->primeExpectations($this->data, $this->checkMembers); + $state['arsse_label_members']['rows'][0][3] = 0; + $state['arsse_label_members']['rows'][2][3] = 0; + $state['arsse_label_members']['rows'][4][3] = 1; + $state['arsse_label_members']['rows'][] = [1,2,1,1]; + $this->compareExpectations($state); + } + + public function testPurgeArticlesOfALabel() { + Arsse::$db->labelArticlesSet("john.doe@example.com", 1, (new Context)->articles([10000]), Database::ASSOC_REPLACE); + $state = $this->primeExpectations($this->data, $this->checkMembers); + $state['arsse_label_members']['rows'][0][3] = 0; + $state['arsse_label_members']['rows'][2][3] = 0; + $this->compareExpectations($state); + } + public function testApplyALabelToArticlesWithoutAuthority() { Phake::when(Arsse::$user)->authorize->thenReturn(false); $this->assertException("notAuthorized", "User", "ExceptionAuthz"); diff --git a/tests/cases/Database/SeriesTag.php b/tests/cases/Database/SeriesTag.php index 404e2f1b..b3ff4e48 100644 --- a/tests/cases/Database/SeriesTag.php +++ b/tests/cases/Database/SeriesTag.php @@ -7,6 +7,7 @@ declare(strict_types=1); namespace JKingWeb\Arsse\TestCase\Database; use JKingWeb\Arsse\Arsse; +use JKingWeb\Arsse\Database; use JKingWeb\Arsse\Misc\Date; use Phake; @@ -311,7 +312,7 @@ trait SeriesTag { Arsse::$db->tagPropertiesSet("john.doe@example.com", 1, ['name' => "Exciting"]); } - public function testListTagledSubscriptions() { + public function testListTaggedSubscriptions() { $exp = [1,5]; $this->assertEquals($exp, Arsse::$db->tagSubscriptionsGet("john.doe@example.com", 1)); $this->assertEquals($exp, Arsse::$db->tagSubscriptionsGet("john.doe@example.com", "Interesting", true)); @@ -323,17 +324,17 @@ trait SeriesTag { $this->assertEquals($exp, Arsse::$db->tagSubscriptionsGet("john.doe@example.com", "Lonely", true)); } - public function testListTagledSubscriptionsForAMissingTag() { + public function testListTaggedSubscriptionsForAMissingTag() { $this->assertException("subjectMissing", "Db", "ExceptionInput"); Arsse::$db->tagSubscriptionsGet("john.doe@example.com", 3); } - public function testListTagledSubscriptionsForAnInvalidTag() { + public function testListTaggedSubscriptionsForAnInvalidTag() { $this->assertException("typeViolation", "Db", "ExceptionInput"); Arsse::$db->tagSubscriptionsGet("john.doe@example.com", -1); } - public function testListTagledSubscriptionsWithoutAuthority() { + public function testListTaggedSubscriptionsWithoutAuthority() { Phake::when(Arsse::$user)->authorize->thenReturn(false); $this->assertException("notAuthorized", "User", "ExceptionAuthz"); Arsse::$db->tagSubscriptionsGet("john.doe@example.com", 1); @@ -348,14 +349,14 @@ trait SeriesTag { } public function testClearATagFromSubscriptions() { - Arsse::$db->tagSubscriptionsSet("john.doe@example.com", 1, [1,3], true); + Arsse::$db->tagSubscriptionsSet("john.doe@example.com", 1, [1,3], Database::ASSOC_REMOVE); $state = $this->primeExpectations($this->data, $this->checkMembers); $state['arsse_tag_members']['rows'][0][2] = 0; $this->compareExpectations($state); } public function testApplyATagToSubscriptionsByName() { - Arsse::$db->tagSubscriptionsSet("john.doe@example.com", "Interesting", [3,4], false, true); + Arsse::$db->tagSubscriptionsSet("john.doe@example.com", "Interesting", [3,4], Database::ASSOC_ADD, true); $state = $this->primeExpectations($this->data, $this->checkMembers); $state['arsse_tag_members']['rows'][1][2] = 1; $state['arsse_tag_members']['rows'][] = [1,4,1]; @@ -363,12 +364,42 @@ trait SeriesTag { } public function testClearATagFromSubscriptionsByName() { - Arsse::$db->tagSubscriptionsSet("john.doe@example.com", "Interesting", [1,3], true, true); + Arsse::$db->tagSubscriptionsSet("john.doe@example.com", "Interesting", [1,3], Database::ASSOC_REMOVE, true); $state = $this->primeExpectations($this->data, $this->checkMembers); $state['arsse_tag_members']['rows'][0][2] = 0; $this->compareExpectations($state); } + public function testApplyATagToNoSubscriptionsByName() { + Arsse::$db->tagSubscriptionsSet("john.doe@example.com", "Interesting", [], Database::ASSOC_ADD, true); + $state = $this->primeExpectations($this->data, $this->checkMembers); + $this->compareExpectations($state); + } + + public function testClearATagFromNoSubscriptionsByName() { + Arsse::$db->tagSubscriptionsSet("john.doe@example.com", "Interesting", [], Database::ASSOC_REMOVE, true); + $state = $this->primeExpectations($this->data, $this->checkMembers); + $this->compareExpectations($state); + } + + public function testReplaceSubscriptionsOfATag() { + Arsse::$db->tagSubscriptionsSet("john.doe@example.com", 1, [3,4], Database::ASSOC_REPLACE); + $state = $this->primeExpectations($this->data, $this->checkMembers); + $state['arsse_tag_members']['rows'][0][2] = 0; + $state['arsse_tag_members']['rows'][1][2] = 1; + $state['arsse_tag_members']['rows'][2][2] = 0; + $state['arsse_tag_members']['rows'][] = [1,4,1]; + $this->compareExpectations($state); + } + + public function testPurgeSubscriptionsOfATag() { + Arsse::$db->tagSubscriptionsSet("john.doe@example.com", 1, [], Database::ASSOC_REPLACE); + $state = $this->primeExpectations($this->data, $this->checkMembers); + $state['arsse_tag_members']['rows'][0][2] = 0; + $state['arsse_tag_members']['rows'][2][2] = 0; + $this->compareExpectations($state); + } + public function testApplyATagToSubscriptionsWithoutAuthority() { Phake::when(Arsse::$user)->authorize->thenReturn(false); $this->assertException("notAuthorized", "User", "ExceptionAuthz"); diff --git a/tests/cases/REST/TinyTinyRSS/TestAPI.php b/tests/cases/REST/TinyTinyRSS/TestAPI.php index 91b370cf..5cab9964 100644 --- a/tests/cases/REST/TinyTinyRSS/TestAPI.php +++ b/tests/cases/REST/TinyTinyRSS/TestAPI.php @@ -1289,18 +1289,18 @@ LONG_STRING; ]; Phake::when(Arsse::$db)->labelArticlesSet(Arsse::$user->id, $this->anything(), (new Context)->articles([]), $this->anything())->thenThrow(new ExceptionInput("tooShort")); // data model function requires one valid integer for multiples Phake::when(Arsse::$db)->labelArticlesSet(Arsse::$user->id, $this->anything(), (new Context)->articles($list[0]), $this->anything())->thenThrow(new ExceptionInput("tooLong")); // data model function limited to 50 items for multiples - Phake::when(Arsse::$db)->labelArticlesSet(Arsse::$user->id, 1088, (new Context)->articles($list[1]), true)->thenReturn(42); - Phake::when(Arsse::$db)->labelArticlesSet(Arsse::$user->id, 1088, (new Context)->articles($list[2]), true)->thenReturn(47); - Phake::when(Arsse::$db)->labelArticlesSet(Arsse::$user->id, 1088, (new Context)->articles($list[1]), false)->thenReturn(5); - Phake::when(Arsse::$db)->labelArticlesSet(Arsse::$user->id, 1088, (new Context)->articles($list[2]), false)->thenReturn(2); + Phake::when(Arsse::$db)->labelArticlesSet(Arsse::$user->id, 1088, (new Context)->articles($list[1]), Database::ASSOC_REMOVE)->thenReturn(42); + Phake::when(Arsse::$db)->labelArticlesSet(Arsse::$user->id, 1088, (new Context)->articles($list[2]), Database::ASSOC_REMOVE)->thenReturn(47); + Phake::when(Arsse::$db)->labelArticlesSet(Arsse::$user->id, 1088, (new Context)->articles($list[1]), Database::ASSOC_ADD)->thenReturn(5); + Phake::when(Arsse::$db)->labelArticlesSet(Arsse::$user->id, 1088, (new Context)->articles($list[2]), Database::ASSOC_ADD)->thenReturn(2); $exp = $this->respGood(['status' => "OK", 'updated' => 89]); $this->assertMessage($exp, $this->req($in[0])); - Phake::verify(Arsse::$db)->labelArticlesSet(Arsse::$user->id, 1088, (new Context)->articles($list[1]), true); - Phake::verify(Arsse::$db)->labelArticlesSet(Arsse::$user->id, 1088, (new Context)->articles($list[2]), true); + Phake::verify(Arsse::$db)->labelArticlesSet(Arsse::$user->id, 1088, (new Context)->articles($list[1]), Database::ASSOC_REMOVE); + Phake::verify(Arsse::$db)->labelArticlesSet(Arsse::$user->id, 1088, (new Context)->articles($list[2]), Database::ASSOC_REMOVE); $exp = $this->respGood(['status' => "OK", 'updated' => 7]); $this->assertMessage($exp, $this->req($in[1])); - Phake::verify(Arsse::$db)->labelArticlesSet(Arsse::$user->id, 1088, (new Context)->articles($list[1]), false); - Phake::verify(Arsse::$db)->labelArticlesSet(Arsse::$user->id, 1088, (new Context)->articles($list[2]), false); + Phake::verify(Arsse::$db)->labelArticlesSet(Arsse::$user->id, 1088, (new Context)->articles($list[1]), Database::ASSOC_ADD); + Phake::verify(Arsse::$db)->labelArticlesSet(Arsse::$user->id, 1088, (new Context)->articles($list[2]), Database::ASSOC_ADD); $exp = $this->respGood(['status' => "OK", 'updated' => 0]); $this->assertMessage($exp, $this->req($in[2])); $exp = $this->respErr("INCORRECT_USAGE");