From b6dd8ab20d86a57abb1af84f34a1de72315f39cf Mon Sep 17 00:00:00 2001 From: "J. King" Date: Fri, 18 Oct 2019 13:11:03 -0400 Subject: [PATCH] Improvements to and proper tests for query builder --- lib/Misc/Query.php | 46 +++---- tests/cases/Db/MySQL/TestDatabase.php | 1 - tests/cases/Db/MySQLPDO/TestDatabase.php | 1 - tests/cases/Db/PostgreSQL/TestDatabase.php | 1 - tests/cases/Db/PostgreSQLPDO/TestDatabase.php | 1 - tests/cases/Db/SQLite3/TestDatabase.php | 1 - tests/cases/Db/SQLite3PDO/TestDatabase.php | 1 - tests/cases/Misc/TestQuery.php | 115 ++++++++++++++++++ tests/phpunit.dist.xml | 1 + 9 files changed, 140 insertions(+), 28 deletions(-) create mode 100644 tests/cases/Misc/TestQuery.php diff --git a/lib/Misc/Query.php b/lib/Misc/Query.php index 55d2cac..95b5c2c 100644 --- a/lib/Misc/Query.php +++ b/lib/Misc/Query.php @@ -29,61 +29,63 @@ class Query { $this->setBody($body, $types, $values); } - public function setBody(string $body = "", $types = null, $values = null): bool { + public function setBody(string $body = "", $types = null, $values = null): self { $this->qBody = $body; if (!is_null($types)) { $this->tBody[] = $types; $this->vBody[] = $values; } - return true; + return $this; } - public function setCTE(string $tableSpec, string $body, $types = null, $values = null): bool { + public function setCTE(string $tableSpec, string $body, $types = null, $values = null): self { $this->qCTE[] = "$tableSpec as ($body)"; if (!is_null($types)) { $this->tCTE[] = $types; $this->vCTE[] = $values; } - return true; + return $this; } - public function setWhere(string $where, $types = null, $values = null): bool { + public function setWhere(string $where, $types = null, $values = null): self { $this->qWhere[] = $where; if (!is_null($types)) { $this->tWhere[] = $types; $this->vWhere[] = $values; } - return true; + return $this; } - public function setWhereNot(string $where, $types = null, $values = null): bool { + public function setWhereNot(string $where, $types = null, $values = null): self { $this->qWhereNot[] = $where; if (!is_null($types)) { $this->tWhereNot[] = $types; $this->vWhereNot[] = $values; } - return true; + return $this; } - public function setGroup(string ...$column): bool { + public function setGroup(string ...$column): self { foreach ($column as $col) { $this->group[] = $col; } - return true; + return $this; } - public function setOrder(string $order): bool { - $this->order[] = $order; - return true; + public function setOrder(string ...$order): self { + foreach ($order as $o) { + $this->order[] = $o; + } + return $this; } - public function setLimit(int $limit, int $offset = 0): bool { + public function setLimit(int $limit, int $offset = 0): self { $this->limit = $limit; $this->offset = $offset; - return true; + return $this; } - public function pushCTE(string $tableSpec): bool { + public function pushCTE(string $tableSpec): self { // 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->tWhereNot], [$this->vBody, $this->vWhere, $this->vWhereNot]); @@ -98,7 +100,7 @@ class Query { $this->order = []; $this->group = []; $this->setLimit(0, 0); - return true; + return $this; } public function __toString(): string { @@ -117,11 +119,11 @@ class Query { } public function getTypes(): array { - return [$this->tCTE, $this->tBody, $this->tWhere, $this->tWhereNot]; + return ValueInfo::flatten([$this->tCTE, $this->tBody, $this->tWhere, $this->tWhereNot]); } public function getValues(): array { - return [$this->vCTE, $this->vBody, $this->vWhere, $this->vWhereNot]; + return ValueInfo::flatten([$this->vCTE, $this->vBody, $this->vWhere, $this->vWhereNot]); } protected function buildQueryBody(): string { @@ -144,9 +146,9 @@ class Query { if (sizeof($this->order)) { $out .= " ORDER BY ".implode(", ", $this->order); } - // add LIMIT and OFFSET if the former is specified - if ($this->limit > 0) { - $out .= " LIMIT ".$this->limit; + // add LIMIT and OFFSET if either is specified + if ($this->limit > 0 || $this->offset > 0) { + $out .= " LIMIT ".($this->limit < 1 ? -1 : $this->limit); if ($this->offset > 0) { $out .= " OFFSET ".$this->offset; } diff --git a/tests/cases/Db/MySQL/TestDatabase.php b/tests/cases/Db/MySQL/TestDatabase.php index 4364170..9ad47ed 100644 --- a/tests/cases/Db/MySQL/TestDatabase.php +++ b/tests/cases/Db/MySQL/TestDatabase.php @@ -10,7 +10,6 @@ namespace JKingWeb\Arsse\TestCase\Db\MySQL; * @group slow * @group coverageOptional * @covers \JKingWeb\Arsse\Database - * @covers \JKingWeb\Arsse\Misc\Query */ class TestDatabase extends \JKingWeb\Arsse\TestCase\Database\AbstractTest { use \JKingWeb\Arsse\Test\DatabaseDrivers\MySQL; diff --git a/tests/cases/Db/MySQLPDO/TestDatabase.php b/tests/cases/Db/MySQLPDO/TestDatabase.php index 6a7550e..e734555 100644 --- a/tests/cases/Db/MySQLPDO/TestDatabase.php +++ b/tests/cases/Db/MySQLPDO/TestDatabase.php @@ -11,7 +11,6 @@ namespace JKingWeb\Arsse\TestCase\Db\MySQLPDO; * @group optional * @group coverageOptional * @covers \JKingWeb\Arsse\Database - * @covers \JKingWeb\Arsse\Misc\Query */ class TestDatabase extends \JKingWeb\Arsse\TestCase\Database\AbstractTest { use \JKingWeb\Arsse\Test\DatabaseDrivers\MySQLPDO; diff --git a/tests/cases/Db/PostgreSQL/TestDatabase.php b/tests/cases/Db/PostgreSQL/TestDatabase.php index 9fda4d9..e72c2a2 100644 --- a/tests/cases/Db/PostgreSQL/TestDatabase.php +++ b/tests/cases/Db/PostgreSQL/TestDatabase.php @@ -10,7 +10,6 @@ namespace JKingWeb\Arsse\TestCase\Db\PostgreSQL; * @group slow * @group coverageOptional * @covers \JKingWeb\Arsse\Database - * @covers \JKingWeb\Arsse\Misc\Query */ class TestDatabase extends \JKingWeb\Arsse\TestCase\Database\AbstractTest { use \JKingWeb\Arsse\Test\DatabaseDrivers\PostgreSQL; diff --git a/tests/cases/Db/PostgreSQLPDO/TestDatabase.php b/tests/cases/Db/PostgreSQLPDO/TestDatabase.php index 6f8ef2a..22afa5d 100644 --- a/tests/cases/Db/PostgreSQLPDO/TestDatabase.php +++ b/tests/cases/Db/PostgreSQLPDO/TestDatabase.php @@ -11,7 +11,6 @@ namespace JKingWeb\Arsse\TestCase\Db\PostgreSQLPDO; * @group optional * @group coverageOptional * @covers \JKingWeb\Arsse\Database - * @covers \JKingWeb\Arsse\Misc\Query */ class TestDatabase extends \JKingWeb\Arsse\TestCase\Database\AbstractTest { use \JKingWeb\Arsse\Test\DatabaseDrivers\PostgreSQLPDO; diff --git a/tests/cases/Db/SQLite3/TestDatabase.php b/tests/cases/Db/SQLite3/TestDatabase.php index eab0970..ea30221 100644 --- a/tests/cases/Db/SQLite3/TestDatabase.php +++ b/tests/cases/Db/SQLite3/TestDatabase.php @@ -9,7 +9,6 @@ namespace JKingWeb\Arsse\TestCase\Db\SQLite3; /** * @group optional * @covers \JKingWeb\Arsse\Database - * @covers \JKingWeb\Arsse\Misc\Query */ class TestDatabase extends \JKingWeb\Arsse\TestCase\Database\AbstractTest { use \JKingWeb\Arsse\Test\DatabaseDrivers\SQLite3; diff --git a/tests/cases/Db/SQLite3PDO/TestDatabase.php b/tests/cases/Db/SQLite3PDO/TestDatabase.php index 079bcc1..751647a 100644 --- a/tests/cases/Db/SQLite3PDO/TestDatabase.php +++ b/tests/cases/Db/SQLite3PDO/TestDatabase.php @@ -8,7 +8,6 @@ namespace JKingWeb\Arsse\TestCase\Db\SQLite3PDO; /** * @covers \JKingWeb\Arsse\Database - * @covers \JKingWeb\Arsse\Misc\Query */ class TestDatabase extends \JKingWeb\Arsse\TestCase\Database\AbstractTest { use \JKingWeb\Arsse\Test\DatabaseDrivers\SQLite3PDO; diff --git a/tests/cases/Misc/TestQuery.php b/tests/cases/Misc/TestQuery.php new file mode 100644 index 0000000..a1588f1 --- /dev/null +++ b/tests/cases/Misc/TestQuery.php @@ -0,0 +1,115 @@ +assertSame("select * from table where a = ?", $q->getQuery()); + $this->assertSame(["int"], $q->getTypes()); + $this->assertSame([3], $q->getValues()); + } + + public function testWhereQuery() { + // simple where clause + $q = (new Query("select * from table"))->setWhere("a = ?", "int", 3); + $this->assertSame("select * from table WHERE a = ?", $q->getQuery()); + $this->assertSame(["int"], $q->getTypes()); + $this->assertSame([3], $q->getValues()); + // compound where clause + $q = (new Query("select * from table"))->setWhere("a = ?", "int", 3)->setWhere("b = ?", "str", 4); + $this->assertSame("select * from table WHERE a = ? AND b = ?", $q->getQuery()); + $this->assertSame(["int", "str"], $q->getTypes()); + $this->assertSame([3, 4], $q->getValues()); + // negative where clause + $q = (new Query("select * from table"))->setWhereNot("a = ?", "int", 3); + $this->assertSame("select * from table WHERE NOT (a = ?)", $q->getQuery()); + $this->assertSame(["int"], $q->getTypes()); + $this->assertSame([3], $q->getValues()); + // compound negative where clause + $q = (new Query("select * from table"))->setWhereNot("a = ?", "int", 3)->setWhereNot("b = ?", "str", 4); + $this->assertSame("select * from table WHERE NOT (a = ? OR b = ?)", $q->getQuery()); + $this->assertSame(["int", "str"], $q->getTypes()); + $this->assertSame([3, 4], $q->getValues()); + // mixed where clause + $q = (new Query("select * from table"))->setWhereNot("a = ?", "int", 1)->setWhere("b = ?", "str", 2)->setWhereNot("c = ?", "int", 3)->setWhere("d = ?", "str", 4); + $this->assertSame("select * from table WHERE b = ? AND d = ? AND NOT (a = ? OR c = ?)", $q->getQuery()); + $this->assertSame(["str", "str", "int", "int"], $q->getTypes()); + $this->assertSame([2, 4, 1, 3], $q->getValues()); + } + + public function testGroupedQuery() { + $q = (new Query("select col1, col2, count(*) as count from table"))->setGroup("col1", "col2"); + $this->assertSame("select col1, col2, count(*) as count from table GROUP BY col1, col2", $q->getQuery()); + $this->assertSame([], $q->getTypes()); + $this->assertSame([], $q->getValues()); + } + + public function testOrderedQuery() { + $q = (new Query("select col1, col2, col3 from table"))->setOrder("col1 desc", "col2")->setOrder("col3 asc"); + $this->assertSame("select col1, col2, col3 from table ORDER BY col1 desc, col2, col3 asc", $q->getQuery()); + $this->assertSame([], $q->getTypes()); + $this->assertSame([], $q->getValues()); + } + + public function testLimitedQuery() { + // no offset + $q = (new Query("select * from table"))->setLimit(5); + $this->assertSame("select * from table LIMIT 5", $q->getQuery()); + $this->assertSame([], $q->getTypes()); + $this->assertSame([], $q->getValues()); + // with offset + $q = (new Query("select * from table"))->setLimit(5, 10); + $this->assertSame("select * from table LIMIT 5 OFFSET 10", $q->getQuery()); + $this->assertSame([], $q->getTypes()); + $this->assertSame([], $q->getValues()); + // no limit with offset + $q = (new Query("select * from table"))->setLimit(0, 10); + $this->assertSame("select * from table LIMIT -1 OFFSET 10", $q->getQuery()); + $this->assertSame([], $q->getTypes()); + $this->assertSame([], $q->getValues()); + } + + public function testQueryWithCommonTableExpression() { + $q = (new Query("select * from table where a in (select * from cte where a = ?)", "int", 1))->setCTE("cte", "select * from other_table where a = ? and b = ?", ["str", "str"], [2, 3]); + $this->assertSame("WITH RECURSIVE cte as (select * from other_table where a = ? and b = ?) select * from table where a in (select * from cte where a = ?)", $q->getQuery()); + $this->assertSame(["str", "str", "int"], $q->getTypes()); + $this->assertSame([2, 3, 1], $q->getValues()); + // multiple CTEs + $q = (new Query("select * from table where a in (select * from cte1 join cte2 using (a) where a = ?)", "int", 1))->setCTE("cte1", "select * from other_table where a = ? and b = ?", ["str", "str"], [2, 3])->setCTE("cte2", "select * from other_table where c between ? and ?", ["datetime", "datetime"], [4, 5]); + $this->assertSame("WITH RECURSIVE cte1 as (select * from other_table where a = ? and b = ?), cte2 as (select * from other_table where c between ? and ?) select * from table where a in (select * from cte1 join cte2 using (a) where a = ?)", $q->getQuery()); + $this->assertSame(["str", "str", "datetime", "datetime", "int"], $q->getTypes()); + $this->assertSame([2, 3, 4, 5, 1], $q->getValues()); + } + + public function testQueryWithPushedCommonTableExpression() { + $q = (new Query("select * from table1"))->setWhere("a between ? and ?", ["datetime", "datetime"], [1, 2]) + ->setCTE("cte1", "select * from table2 where a = ? and b = ?", ["str", "str"], [3, 4]) + ->pushCTE("cte2") + ->setBody("select * from table3 join cte1 using (a) join cte2 using (a) where a = ?", "int", 5); + $this->assertSame("WITH RECURSIVE cte1 as (select * from table2 where a = ? and b = ?), cte2 as (select * from table1 WHERE a between ? and ?) select * from table3 join cte1 using (a) join cte2 using (a) where a = ?", $q->getQuery()); + $this->assertSame(["str", "str", "datetime", "datetime", "int"], $q->getTypes()); + $this->assertSame([3, 4, 1, 2, 5], $q->getValues()); + } + + public function testComplexQuery() { + $q = (new query("select *, ? as const from table", "datetime", 1)) + ->setWhereNot("b = ?", "bool", 2) + ->setGroup("col1", "col2") + ->setWhere("a = ?", "str", 3) + ->setLimit(4, 5) + ->setOrder("col3") + ->setCTE("cte", "select ? as const", "int", 6); + $this->assertSame("WITH RECURSIVE cte as (select ? as const) select *, ? as const from table WHERE a = ? AND NOT (b = ?) GROUP BY col1, col2 ORDER BY col3 LIMIT 4 OFFSET 5", $q->getQuery()); + $this->assertSame(["int", "datetime", "str", "bool"], $q->getTypes()); + $this->assertSame([6, 1, 3, 2], $q->getValues()); + } +} diff --git a/tests/phpunit.dist.xml b/tests/phpunit.dist.xml index 5489f94..e24e0d0 100644 --- a/tests/phpunit.dist.xml +++ b/tests/phpunit.dist.xml @@ -45,6 +45,7 @@ cases/Misc/TestValueInfo.php cases/Misc/TestDate.php + cases/Misc/TestQuery.php cases/Misc/TestContext.php cases/Misc/TestURL.php