From d00d45aa87854d21db258ea7b69c7b53cbe3e966 Mon Sep 17 00:00:00 2001 From: "Alexander M. Turek" Date: Sun, 15 Oct 2023 11:51:20 +0200 Subject: [PATCH] Add deprecation layer and upgrade path for QueryBuilder::reset*() methods --- UPGRADE.md | 17 ++++ phpstan.neon.dist | 6 ++ psalm.xml.dist | 8 ++ src/Query/QueryBuilder.php | 103 +++++++++++++++++++- tests/Query/QueryBuilderTest.php | 160 ++++++++++++++++++++++++++++--- 5 files changed, 275 insertions(+), 19 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 084d4a68cb5..c27231a450c 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -8,6 +8,23 @@ awareness about deprecated code. # Upgrade to 3.8 +## Deprecated reset methods from `QueryBuilder` + +`QueryBuilder::resetQueryParts()` has been deprecated. + +Resetting individual query parts through the generic `resetQueryPart()` method has been deprecated as well. +However, several replacements have been put in place depending on the `$queryPartName` parameter: + +| `$queryPartName` | suggested replacement | +|------------------|--------------------------------------------| +| `'select'` | Call `select()` with a new set of columns. | +| `'distinct'` | `distinct(false)` | +| `'where'` | `resetWhere()` | +| `'groupBy'` | `resetGroupBy()` | +| `'having'` | `resetHaving()` | +| `'orderBy'` | `resetOrderBy()` | +| `'values'` | Call `values()` with a new set of values. | + ## Deprecated getting query parts from `QueryBuilder` The usage of `QueryBuilder::getQueryPart()` and `::getQueryParts()` is deprecated. The query parts diff --git a/phpstan.neon.dist b/phpstan.neon.dist index ebda6402cbb..3074c054646 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -112,6 +112,12 @@ parameters: paths: - src/Platforms/AbstractPlatform.php + # Deprecated method, will be removed in 4.0.0 + - + message: '~^Variable method call on \$this\(Doctrine\\DBAL\\Query\\QueryBuilder\)\.$~' + paths: + - src/Query/QueryBuilder.php + # There is no way to make this assertion in the code, # and the API doesn't support parametrization of returned column types. - diff --git a/psalm.xml.dist b/psalm.xml.dist index 88948561586..bfeea5a7222 100644 --- a/psalm.xml.dist +++ b/psalm.xml.dist @@ -504,6 +504,8 @@ --> + + @@ -658,6 +660,12 @@ + + + + + + diff --git a/src/Query/QueryBuilder.php b/src/Query/QueryBuilder.php index c0d18d5f2ff..28ad02585a0 100644 --- a/src/Query/QueryBuilder.php +++ b/src/Query/QueryBuilder.php @@ -17,14 +17,17 @@ use function array_keys; use function array_unshift; use function count; +use function func_get_arg; use function func_get_args; use function func_num_args; use function implode; use function is_array; use function is_object; use function key; +use function method_exists; use function strtoupper; use function substr; +use function ucfirst; /** * QueryBuilder class is responsible to dynamically create SQL queries. @@ -35,6 +38,8 @@ * The query builder does no validation whatsoever if certain features even work with the * underlying database vendor. Limit queries and joins are NOT applied to UPDATE and DELETE statements * even if some vendors such as MySQL support it. + * + * @method $this distinct(bool $distinct = true) Adds or removes DISTINCT to/from the query. */ class QueryBuilder { @@ -677,7 +682,7 @@ public function select($select = null/*, string ...$selects*/) } /** - * Adds DISTINCT to the query. + * Adds or removes DISTINCT to/from the query. * * * $qb = $conn->createQueryBuilder() @@ -688,9 +693,10 @@ public function select($select = null/*, string ...$selects*/) * * @return $this This QueryBuilder instance. */ - public function distinct(): self + public function distinct(/* bool $distinct = true */): self { - $this->sqlParts['distinct'] = true; + $this->sqlParts['distinct'] = func_num_args() < 1 || func_get_arg(0); + $this->state = self::STATE_DIRTY; return $this; } @@ -1335,30 +1341,77 @@ public function getQueryParts() /** * Resets SQL parts. * + * @deprecated Use the dedicated reset*() methods instead. + * * @param string[]|null $queryPartNames * * @return $this This QueryBuilder instance. */ public function resetQueryParts($queryPartNames = null) { + Deprecation::trigger( + 'doctrine/dbal', + 'TODO', + '%s() is deprecated, instead use dedicated reset methods for the parts that shall be reset.', + __METHOD__, + ); + $queryPartNames ??= array_keys($this->sqlParts); foreach ($queryPartNames as $queryPartName) { - $this->resetQueryPart($queryPartName); + $this->sqlParts[$queryPartName] = self::SQL_PARTS_DEFAULTS[$queryPartName]; } + $this->state = self::STATE_DIRTY; + return $this; } /** * Resets a single SQL part. * + * @deprecated Use the dedicated reset*() methods instead. + * * @param string $queryPartName * * @return $this This QueryBuilder instance. */ public function resetQueryPart($queryPartName) { + if ($queryPartName === 'distinct') { + Deprecation::trigger( + 'doctrine/dbal', + 'TODO', + 'Calling %s() with "distinct" is deprecated, call distinct(false) instead.', + __METHOD__, + ); + + return $this->distinct(false); + } + + $newMethodName = 'reset' . ucfirst($queryPartName); + if (array_key_exists($queryPartName, self::SQL_PARTS_DEFAULTS) && method_exists($this, $newMethodName)) { + Deprecation::trigger( + 'doctrine/dbal', + 'TODO', + 'Calling %s() with "%s" is deprecated, call %s() instead.', + __METHOD__, + $queryPartName, + $newMethodName, + ); + + return $this->$newMethodName(); + } + + Deprecation::trigger( + 'doctrine/dbal', + 'TODO', + 'Calling %s() with "%s" is deprecated without replacement.', + __METHOD__, + $queryPartName, + $newMethodName, + ); + $this->sqlParts[$queryPartName] = self::SQL_PARTS_DEFAULTS[$queryPartName]; $this->state = self::STATE_DIRTY; @@ -1366,6 +1419,48 @@ public function resetQueryPart($queryPartName) return $this; } + /** + * Resets the WHERE conditions for the query. + * + * @return $this This QueryBuilder instance. + */ + public function resetWhere(): self + { + $this->sqlParts['where'] = self::SQL_PARTS_DEFAULTS['where']; + + $this->state = self::STATE_DIRTY; + + return $this; + } + + /** + * Resets the grouping for the query. + * + * @return $this This QueryBuilder instance. + */ + public function resetGroupBy(): self + { + $this->sqlParts['groupBy'] = self::SQL_PARTS_DEFAULTS['groupBy']; + + $this->state = self::STATE_DIRTY; + + return $this; + } + + /** + * Resets the HAVING conditions for the query. + * + * @return $this This QueryBuilder instance. + */ + public function resetHaving(): self + { + $this->sqlParts['having'] = self::SQL_PARTS_DEFAULTS['having']; + + $this->state = self::STATE_DIRTY; + + return $this; + } + /** * Resets the ordering for the query. * diff --git a/tests/Query/QueryBuilderTest.php b/tests/Query/QueryBuilderTest.php index 76fe76bdb7f..acd061f7c53 100644 --- a/tests/Query/QueryBuilderTest.php +++ b/tests/Query/QueryBuilderTest.php @@ -609,37 +609,167 @@ public function testSetFirstResult(): void self::assertEquals(10, $qb->getFirstResult()); } - public function testResetQueryPart(): void + private function prepareQueryBuilderToReset(): QueryBuilder { - $qb = new QueryBuilder($this->conn); + $qb = (new QueryBuilder($this->conn)) + ->select('u.*') + ->distinct() + ->from('users', 'u') + ->where('u.name = ?') + ->orderBy('u.name', 'ASC'); - $qb->select('u.*')->from('users', 'u')->where('u.name = ?'); + self::assertEquals('SELECT DISTINCT u.* FROM users u WHERE u.name = ? ORDER BY u.name ASC', (string) $qb); - self::assertEquals('SELECT u.* FROM users u WHERE u.name = ?', (string) $qb); - $qb->resetQueryPart('where'); - self::assertEquals('SELECT u.* FROM users u', (string) $qb); + return $qb; } public function testResetQueryParts(): void { - $qb = new QueryBuilder($this->conn); + $qb = $this->prepareQueryBuilderToReset(); - $qb->select('u.*')->from('users', 'u')->where('u.name = ?')->orderBy('u.name'); + $this->expectDeprecationWithIdentifier('TODO'); + $qb->resetQueryParts(['distinct', 'where', 'orderBy']); - self::assertEquals('SELECT u.* FROM users u WHERE u.name = ? ORDER BY u.name ASC', (string) $qb); - $qb->resetQueryParts(['where', 'orderBy']); self::assertEquals('SELECT u.* FROM users u', (string) $qb); } - public function testResetOrderBy(): void + public function testLegacyResetSelect(): void { - $qb = new QueryBuilder($this->conn); + $qb = $this->prepareQueryBuilderToReset(); + + $this->expectDeprecationWithIdentifier('TODO'); + $qb->resetQueryPart('select')->addSelect('u.name'); + + self::assertEquals('SELECT DISTINCT u.name FROM users u WHERE u.name = ? ORDER BY u.name ASC', (string) $qb); + } + + public function testLegacyResetDistinct(): void + { + $qb = $this->prepareQueryBuilderToReset(); - $qb->select('u.*')->from('users', 'u')->orderBy('u.name'); + $this->expectDeprecationWithIdentifier('TODO'); + $qb->resetQueryPart('distinct'); + + self::assertEquals('SELECT u.* FROM users u WHERE u.name = ? ORDER BY u.name ASC', (string) $qb); + } - self::assertEquals('SELECT u.* FROM users u ORDER BY u.name ASC', (string) $qb); + public function testResetDistinct(): void + { + $qb = $this->prepareQueryBuilderToReset(); + + $this->expectNoDeprecationWithIdentifier('TODO'); + $qb->distinct(false); + + self::assertEquals('SELECT u.* FROM users u WHERE u.name = ? ORDER BY u.name ASC', (string) $qb); + } + + public function testLegacyResetWhere(): void + { + $qb = $this->prepareQueryBuilderToReset(); + + $this->expectDeprecationWithIdentifier('TODO'); + $qb->resetQueryPart('where'); + + self::assertEquals('SELECT DISTINCT u.* FROM users u ORDER BY u.name ASC', (string) $qb); + } + + public function testResetWhere(): void + { + $qb = $this->prepareQueryBuilderToReset(); + + $this->expectNoDeprecationWithIdentifier('TODO'); + $qb->resetWhere(); + + self::assertEquals('SELECT DISTINCT u.* FROM users u ORDER BY u.name ASC', (string) $qb); + } + + public function testLegacyResetOrderBy(): void + { + $qb = $this->prepareQueryBuilderToReset(); + + $this->expectDeprecationWithIdentifier('TODO'); + $qb->resetQueryPart('orderBy'); + + self::assertEquals('SELECT DISTINCT u.* FROM users u WHERE u.name = ?', (string) $qb); + } + + public function testResetOrderBy(): void + { + $qb = $this->prepareQueryBuilderToReset(); + + $this->expectNoDeprecationWithIdentifier('TODO'); $qb->resetOrderBy(); - self::assertEquals('SELECT u.* FROM users u', (string) $qb); + + self::assertEquals('SELECT DISTINCT u.* FROM users u WHERE u.name = ?', (string) $qb); + } + + private function prepareGroupedQueryBuilderToReset(): QueryBuilder + { + $qb = (new QueryBuilder($this->conn)) + ->select('u.country', 'COUNT(*)') + ->from('users', 'u') + ->groupBy('u.country') + ->having('COUNT(*) > ?') + ->orderBy('COUNT(*)', 'DESC'); + + self::assertEquals( + 'SELECT u.country, COUNT(*) FROM users u GROUP BY u.country HAVING COUNT(*) > ? ORDER BY COUNT(*) DESC', + (string) $qb, + ); + + return $qb; + } + + public function testLegacyResetHaving(): void + { + $qb = $this->prepareGroupedQueryBuilderToReset(); + + $this->expectDeprecationWithIdentifier('TODO'); + $qb->resetQueryPart('having'); + + self::assertEquals( + 'SELECT u.country, COUNT(*) FROM users u GROUP BY u.country ORDER BY COUNT(*) DESC', + (string) $qb, + ); + } + + public function testResetHaving(): void + { + $qb = $this->prepareGroupedQueryBuilderToReset(); + + $this->expectNoDeprecationWithIdentifier('TODO'); + $qb->resetHaving(); + + self::assertEquals( + 'SELECT u.country, COUNT(*) FROM users u GROUP BY u.country ORDER BY COUNT(*) DESC', + (string) $qb, + ); + } + + public function testLegacyResetGroupBy(): void + { + $qb = $this->prepareGroupedQueryBuilderToReset(); + + $this->expectDeprecationWithIdentifier('TODO'); + $qb->resetQueryPart('groupBy'); + + self::assertEquals( + 'SELECT u.country, COUNT(*) FROM users u HAVING COUNT(*) > ? ORDER BY COUNT(*) DESC', + (string) $qb, + ); + } + + public function testGroupBy(): void + { + $qb = $this->prepareGroupedQueryBuilderToReset(); + + $this->expectNoDeprecationWithIdentifier('TODO'); + $qb->resetGroupBy(); + + self::assertEquals( + 'SELECT u.country, COUNT(*) FROM users u HAVING COUNT(*) > ? ORDER BY COUNT(*) DESC', + (string) $qb, + ); } public function testCreateNamedParameter(): void