From b950ac066f153fc4db108c0da62a872183000d9d Mon Sep 17 00:00:00 2001 From: "J. King" Date: Mon, 25 Feb 2019 22:41:12 -0500 Subject: [PATCH] Restrict options in not-context and hopefully make it easier to use --- lib/Context/Context.php | 101 +++++++++++++++++ .../ExclusionContext.php} | 102 ++---------------- lib/Database.php | 28 ++++- lib/Misc/Query.php | 26 ++++- lib/REST/NextCloudNews/V1_2.php | 2 +- lib/REST/TinyTinyRSS/API.php | 2 +- tests/cases/Database/SeriesArticle.php | 2 +- tests/cases/Database/SeriesLabel.php | 2 +- tests/cases/Misc/TestContext.php | 13 ++- tests/cases/REST/NextCloudNews/TestV1_2.php | 2 +- tests/cases/REST/TinyTinyRSS/TestAPI.php | 2 +- 11 files changed, 170 insertions(+), 112 deletions(-) create mode 100644 lib/Context/Context.php rename lib/{Misc/Context.php => Context/ExclusionContext.php} (56%) diff --git a/lib/Context/Context.php b/lib/Context/Context.php new file mode 100644 index 0000000..d997773 --- /dev/null +++ b/lib/Context/Context.php @@ -0,0 +1,101 @@ +not = new ExclusionContext; + } + + public function __clone() { + // clone the exclusion context as well + $this->not = clone $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); + } + + public function offset(int $spec = null) { + return $this->act(__FUNCTION__, func_num_args(), $spec); + } + + public function unread(bool $spec = null) { + return $this->act(__FUNCTION__, func_num_args(), $spec); + } + + public function starred(bool $spec = null) { + return $this->act(__FUNCTION__, func_num_args(), $spec); + } + + public function labelled(bool $spec = null) { + return $this->act(__FUNCTION__, func_num_args(), $spec); + } + + public function annotated(bool $spec = null) { + return $this->act(__FUNCTION__, func_num_args(), $spec); + } + + public function latestArticle(int $spec = null) { + return $this->act(__FUNCTION__, func_num_args(), $spec); + } + + public function oldestArticle(int $spec = null) { + return $this->act(__FUNCTION__, func_num_args(), $spec); + } + + public function latestEdition(int $spec = null) { + return $this->act(__FUNCTION__, func_num_args(), $spec); + } + + public function oldestEdition(int $spec = null) { + return $this->act(__FUNCTION__, func_num_args(), $spec); + } + + public function modifiedSince($spec = null) { + $spec = Date::normalize($spec); + return $this->act(__FUNCTION__, func_num_args(), $spec); + } + + public function notModifiedSince($spec = null) { + $spec = Date::normalize($spec); + 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); + } +} diff --git a/lib/Misc/Context.php b/lib/Context/ExclusionContext.php similarity index 56% rename from lib/Misc/Context.php rename to lib/Context/ExclusionContext.php index 9263fa1..5a2a9cf 100644 --- a/lib/Misc/Context.php +++ b/lib/Context/ExclusionContext.php @@ -4,49 +4,27 @@ * See LICENSE and AUTHORS files for details */ declare(strict_types=1); -namespace JKingWeb\Arsse\Misc; +namespace JKingWeb\Arsse\Context; -use JKingWeb\Arsse\Misc\Date; use JKingWeb\Arsse\Misc\ValueInfo; -class Context { - public $not = null; - public $reverse = false; - public $limit = 0; - public $offset = 0; +class ExclusionContext { public $folder; public $folderShallow; public $subscription; - public $oldestArticle; - public $latestArticle; - public $oldestEdition; - public $latestEdition; - public $unread = null; - public $starred = null; - public $modifiedSince; - public $notModifiedSince; - public $markedSince; - public $notMarkedSince; public $edition; public $article; public $editions; public $articles; public $label; public $labelName; - public $labelled = null; - public $annotated = null; - public $annotationTerms = null; - public $searchTerms = null; - public $titleTerms = null; - public $authorTerms = null; + public $annotationTerms; + public $searchTerms; + public $titleTerms; + public $authorTerms; protected $props = []; - public function __clone() { - // clone the negation context, if any - $this->not = $this->not ? clone $this->not : null; - } - protected function act(string $prop, int $set, $value) { if ($set) { if (is_null($value)) { @@ -87,18 +65,6 @@ class Context { return array_values($spec); } - 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); - } - - public function offset(int $spec = null) { - return $this->act(__FUNCTION__, func_num_args(), $spec); - } - public function folder(int $spec = null) { return $this->act(__FUNCTION__, func_num_args(), $spec); } @@ -111,50 +77,6 @@ class Context { return $this->act(__FUNCTION__, func_num_args(), $spec); } - public function latestArticle(int $spec = null) { - return $this->act(__FUNCTION__, func_num_args(), $spec); - } - - public function oldestArticle(int $spec = null) { - return $this->act(__FUNCTION__, func_num_args(), $spec); - } - - public function latestEdition(int $spec = null) { - return $this->act(__FUNCTION__, func_num_args(), $spec); - } - - public function oldestEdition(int $spec = null) { - return $this->act(__FUNCTION__, func_num_args(), $spec); - } - - public function unread(bool $spec = null) { - return $this->act(__FUNCTION__, func_num_args(), $spec); - } - - public function starred(bool $spec = null) { - return $this->act(__FUNCTION__, func_num_args(), $spec); - } - - public function modifiedSince($spec = null) { - $spec = Date::normalize($spec); - return $this->act(__FUNCTION__, func_num_args(), $spec); - } - - public function notModifiedSince($spec = null) { - $spec = Date::normalize($spec); - 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); } @@ -185,14 +107,6 @@ class Context { return $this->act(__FUNCTION__, func_num_args(), $spec); } - public function labelled(bool $spec = null) { - return $this->act(__FUNCTION__, func_num_args(), $spec); - } - - public function annotated(bool $spec = null) { - return $this->act(__FUNCTION__, func_num_args(), $spec); - } - public function annotationTerms(array $spec = null) { if (isset($spec)) { $spec = $this->cleanStringArray($spec); @@ -220,8 +134,4 @@ class Context { } return $this->act(__FUNCTION__, func_num_args(), $spec); } - - public function not(self $spec = null) { - return $this->act(__FUNCTION__, func_num_args(), $spec); - } } diff --git a/lib/Database.php b/lib/Database.php index 353e7b9..6f7a9b4 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -9,7 +9,8 @@ namespace JKingWeb\Arsse; use JKingWeb\DrUUID\UUID; use JKingWeb\Arsse\Db\Statement; use JKingWeb\Arsse\Misc\Query; -use JKingWeb\Arsse\Misc\Context; +use JKingWeb\Arsse\Context\Context; +use JKingWeb\Arsse\Context\ExclusionContext; use JKingWeb\Arsse\Misc\Date; use JKingWeb\Arsse\Misc\ValueInfo; @@ -1178,8 +1179,9 @@ class Database { // if there are no output columns requested we're getting a count and should not group, but otherwise we should $q->setGroup("arsse_articles.id", "arsse_marks.note", "arsse_enclosures.url", "arsse_enclosures.type", "arsse_subscriptions.title", "arsse_feeds.title", "arsse_subscriptions.id", "arsse_marks.modified", "arsse_label_members.modified", "arsse_marks.read", "arsse_marks.starred", "latest_editions.edition"); } + $excContext = new ExclusionContext; // handle the simple context options - foreach ([ + $options = [ // each context array consists of a column identifier (see $colDefs above), a comparison operator, a data type, and an upper bound if the value is an array "edition" => ["edition", "=", "int", 1], "editions" => ["edition", "in", "int", self::LIMIT_ARTICLES], @@ -1197,7 +1199,8 @@ class Database { "subscription" => ["subscription", "=", "int", 1], "unread" => ["unread", "=", "bool", 1], "starred" => ["starred", "=", "bool", 1], - ] as $m => list($col, $op, $type, $max)) { + ]; + foreach ($options as $m => list($col, $op, $type, $max)) { if (!$context->$m()) { // context is not being used continue; @@ -1213,6 +1216,25 @@ class Database { $q->setWhere("{$colDefs[$col]} $op ?", $type, $context->$m); } } + if ($context->not != $excContext) { + // further handle exclusionary options if specified + foreach ($options as $m => list($col, $op, $type, $max)) { + if (!method_exists($context->not, $m) || !$context->not->$m()) { + // context option is not being used + continue; + } elseif (is_array($context->not->$m)) { + if (!$context->not->$m) { + // for exclusions we don't care if the array is empty + } elseif (sizeof($context->not->$m) > $max) { + throw new Db\ExceptionInput("tooLong", ['field' => $m, 'action' => $this->caller(), 'max' => $max]); // @codeCoverageIgnore + } + list($clause, $types, $values) = $this->generateIn($context->$m, $type); + $q->setWhereNot("{$colDefs[$col]} $op ($clause)", $types, $values); + } else { + $q->setWhereNot("{$colDefs[$col]} $op ?", $type, $context->$m); + } + } + } // handle complex context options if ($context->labelled()) { // any label (true) or no label (false) diff --git a/lib/Misc/Query.php b/lib/Misc/Query.php index d7a2c7f..458b7ed 100644 --- a/lib/Misc/Query.php +++ b/lib/Misc/Query.php @@ -20,6 +20,9 @@ class Query { protected $qWhere = []; // WHERE clause components protected $tWhere = []; // WHERE clause type bindings protected $vWhere = []; // WHERE clause binding values + protected $qWhereNot = []; // WHERE NOT clause components + protected $tWhereNot = []; // WHERE NOT clause type bindings + protected $vWhereNot = []; // WHERE NOT clause binding values protected $group = []; // GROUP BY clause components protected $order = []; // ORDER BY clause components protected $limit = 0; @@ -69,6 +72,15 @@ class Query { return true; } + public function setWhereNot(string $where, $types = null, $values = null): bool { + $this->qWhereNot[] = $where; + if (!is_null($types)) { + $this->tWhereNot[] = $types; + $this->vWhereNot[] = $values; + } + return true; + } + public function setGroup(string ...$column): bool { foreach ($column as $col) { $this->group[] = $col; @@ -94,7 +106,7 @@ class Query { public function pushCTE(string $tableSpec, string $join = ''): bool { // this function takes the query body and converts it to a common table expression, putting it at the bottom of the existing CTE stack // all WHERE, ORDER BY, and LIMIT parts belong to the new CTE and are removed from the main query - $this->setCTE($tableSpec, $this->buildQueryBody(), [$this->tBody, $this->tWhere], [$this->vBody, $this->vWhere]); + $this->setCTE($tableSpec, $this->buildQueryBody(), [$this->tBody, $this->tWhere, $this->tWhereNot], [$this->vBody, $this->vWhere, $this->vWhereNot]); $this->jCTE = []; $this->tBody = []; $this->vBody = []; @@ -129,11 +141,11 @@ class Query { } public function getTypes(): array { - return [$this->tCTE, $this->tBody, $this->tJoin, $this->tWhere]; + return [$this->tCTE, $this->tBody, $this->tJoin, $this->tWhere, $this->tWhereNot]; } public function getValues(): array { - return [$this->vCTE, $this->vBody, $this->vJoin, $this->vWhere]; + return [$this->vCTE, $this->vBody, $this->vJoin, $this->vWhere, $this->vWhereNot]; } public function getJoinTypes(): array { @@ -173,8 +185,12 @@ class Query { $out .= " ".implode(" ", $this->qJoin); } // add any WHERE terms - if (sizeof($this->qWhere)) { - $out .= " WHERE ".implode(" AND ", $this->qWhere); + if (sizeof($this->qWhere) || sizeof($this->qWhereNot)) { + $where = implode(" AND ", $this->qWhere); + $whereNot = implode(" OR ", $this->qWhereNot); + $whereNot = strlen($whereNot) ? "NOT ($whereNot)" : ""; + $where = implode(" AND ", array_filter([$where, $whereNot])); + $out .= " WHERE $where"; } // add any GROUP BY terms if (sizeof($this->group)) { diff --git a/lib/REST/NextCloudNews/V1_2.php b/lib/REST/NextCloudNews/V1_2.php index 84beda3..7f4301c 100644 --- a/lib/REST/NextCloudNews/V1_2.php +++ b/lib/REST/NextCloudNews/V1_2.php @@ -10,7 +10,7 @@ use JKingWeb\Arsse\Arsse; use JKingWeb\Arsse\Database; use JKingWeb\Arsse\User; use JKingWeb\Arsse\Service; -use JKingWeb\Arsse\Misc\Context; +use JKingWeb\Arsse\Context\Context; use JKingWeb\Arsse\Misc\ValueInfo; use JKingWeb\Arsse\AbstractException; use JKingWeb\Arsse\Db\ExceptionInput; diff --git a/lib/REST/TinyTinyRSS/API.php b/lib/REST/TinyTinyRSS/API.php index 4ddea6f..f126324 100644 --- a/lib/REST/TinyTinyRSS/API.php +++ b/lib/REST/TinyTinyRSS/API.php @@ -12,7 +12,7 @@ use JKingWeb\Arsse\Database; use JKingWeb\Arsse\User; use JKingWeb\Arsse\Service; use JKingWeb\Arsse\Misc\Date; -use JKingWeb\Arsse\Misc\Context; +use JKingWeb\Arsse\Context\Context; use JKingWeb\Arsse\Misc\ValueInfo; use JKingWeb\Arsse\AbstractException; use JKingWeb\Arsse\ExceptionType; diff --git a/tests/cases/Database/SeriesArticle.php b/tests/cases/Database/SeriesArticle.php index 8530099..37a5311 100644 --- a/tests/cases/Database/SeriesArticle.php +++ b/tests/cases/Database/SeriesArticle.php @@ -8,7 +8,7 @@ namespace JKingWeb\Arsse\TestCase\Database; use JKingWeb\Arsse\Database; use JKingWeb\Arsse\Arsse; -use JKingWeb\Arsse\Misc\Context; +use JKingWeb\Arsse\Context\Context; use JKingWeb\Arsse\Misc\Date; use Phake; diff --git a/tests/cases/Database/SeriesLabel.php b/tests/cases/Database/SeriesLabel.php index 8347ce5..e6fc426 100644 --- a/tests/cases/Database/SeriesLabel.php +++ b/tests/cases/Database/SeriesLabel.php @@ -7,7 +7,7 @@ declare(strict_types=1); namespace JKingWeb\Arsse\TestCase\Database; use JKingWeb\Arsse\Arsse; -use JKingWeb\Arsse\Misc\Context; +use JKingWeb\Arsse\Context\Context; use JKingWeb\Arsse\Misc\Date; use Phake; diff --git a/tests/cases/Misc/TestContext.php b/tests/cases/Misc/TestContext.php index 12a9969..db088b4 100644 --- a/tests/cases/Misc/TestContext.php +++ b/tests/cases/Misc/TestContext.php @@ -6,10 +6,10 @@ declare(strict_types=1); namespace JKingWeb\Arsse\TestCase\Misc; -use JKingWeb\Arsse\Misc\Context; +use JKingWeb\Arsse\Context\Context; use JKingWeb\Arsse\Misc\ValueInfo; -/** @covers \JKingWeb\Arsse\Misc\Context */ +/** @covers \JKingWeb\Arsse\Context\Context */ class TestContext extends \JKingWeb\Arsse\Test\AbstractTest { public function testVerifyInitialState() { $c = new Context; @@ -96,4 +96,13 @@ class TestContext extends \JKingWeb\Arsse\Test\AbstractTest { $this->assertSame($out, $c->$method($in)->$method, "Context method $method did not return the expected results"); } } + + public function testCloneAContext() { + $c1 = new Context; + $c2 = clone $c1; + $this->assertEquals($c1, $c2); + $this->assertEquals($c1->not, $c2->not); + $this->assertNotSame($c1, $c2); + $this->assertNotSame($c1->not, $c2->not); + } } diff --git a/tests/cases/REST/NextCloudNews/TestV1_2.php b/tests/cases/REST/NextCloudNews/TestV1_2.php index f35e21e..664db4e 100644 --- a/tests/cases/REST/NextCloudNews/TestV1_2.php +++ b/tests/cases/REST/NextCloudNews/TestV1_2.php @@ -13,7 +13,7 @@ use JKingWeb\Arsse\Database; use JKingWeb\Arsse\Service; use JKingWeb\Arsse\Test\Result; use JKingWeb\Arsse\Misc\Date; -use JKingWeb\Arsse\Misc\Context; +use JKingWeb\Arsse\Context\Context; use JKingWeb\Arsse\Db\ExceptionInput; use JKingWeb\Arsse\Db\Transaction; use JKingWeb\Arsse\REST\NextCloudNews\V1_2; diff --git a/tests/cases/REST/TinyTinyRSS/TestAPI.php b/tests/cases/REST/TinyTinyRSS/TestAPI.php index bf35a30..4b497a7 100644 --- a/tests/cases/REST/TinyTinyRSS/TestAPI.php +++ b/tests/cases/REST/TinyTinyRSS/TestAPI.php @@ -14,7 +14,7 @@ use JKingWeb\Arsse\Service; use JKingWeb\Arsse\REST\Request; use JKingWeb\Arsse\Test\Result; use JKingWeb\Arsse\Misc\Date; -use JKingWeb\Arsse\Misc\Context; +use JKingWeb\Arsse\Context\Context; use JKingWeb\Arsse\Db\ExceptionInput; use JKingWeb\Arsse\Db\Transaction; use JKingWeb\Arsse\REST\TinyTinyRSS\API;