From 60edee21a35c9858ea82d26e5e04eccdeeba566e Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Sun, 17 Mar 2019 09:32:55 -0700 Subject: [PATCH] Changed the type of `$char` in `AbstractPlatform::getTrimExpression()` from `string|false` to `?string` --- UPGRADE.md | 4 ++ .../DBAL/Platforms/AbstractPlatform.php | 37 +++++++++++++------ .../DBAL/Platforms/SQLAnywherePlatform.php | 15 ++++++-- .../DBAL/Platforms/SQLServerPlatform.php | 24 +++++++----- .../DBAL/Platforms/SqlitePlatform.php | 25 ++++++++++--- .../Tests/DBAL/Functional/DataAccessTest.php | 23 ++++++++---- .../Platforms/SQLAnywherePlatformTest.php | 5 --- 7 files changed, 89 insertions(+), 44 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 7e9448ce3b5..2bd77b73166 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,5 +1,9 @@ # Upgrade to 3.0 +## BC BREAK The type of `$char` in `AbstractPlatform::getTrimExpression()` changed from `string|false` to `?string` + +The default value of `$char` is now `null`, not `false`. Additionally, the method will throw an `InvalidArgumentException` in an invalid value of `$mode` is passed. + ## BC BREAK `Statement::quote()` only accepts strings. `Statement::quote()` and `ExpressionBuilder::literal()` no longer accept arguments of an arbitrary type and and don't implement type-specific handling. Only strings can be quoted. diff --git a/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php b/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php index 52543b17881..4ba4dee13d6 100644 --- a/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php +++ b/lib/Doctrine/DBAL/Platforms/AbstractPlatform.php @@ -753,37 +753,50 @@ public function getModExpression($expression1, $expression2) * * @param string $str The expression to apply the trim to. * @param int $mode The position of the trim (leading/trailing/both). - * @param string|bool $char The char to trim, has to be quoted already. Defaults to space. + * @param string|null $char The char to trim, has to be quoted already. Defaults to space. * - * @return string + * @throws InvalidArgumentException */ - public function getTrimExpression($str, $mode = TrimMode::UNSPECIFIED, $char = false) + public function getTrimExpression(string $str, int $mode = TrimMode::UNSPECIFIED, ?string $char = null) : string { - $expression = ''; + $tokens = []; switch ($mode) { + case TrimMode::UNSPECIFIED: + break; + case TrimMode::LEADING: - $expression = 'LEADING '; + $tokens[] = 'LEADING'; break; case TrimMode::TRAILING: - $expression = 'TRAILING '; + $tokens[] = 'TRAILING'; break; case TrimMode::BOTH: - $expression = 'BOTH '; + $tokens[] = 'BOTH'; break; + + default: + throw new InvalidArgumentException( + sprintf( + 'The value of $mode is expected to be one of the TrimMode constants, %d given', + $mode + ) + ); } - if ($char !== false) { - $expression .= $char . ' '; + if ($char !== null) { + $tokens[] = $char; } - if ($mode || $char !== false) { - $expression .= 'FROM '; + if (count($tokens) > 0) { + $tokens[] = 'FROM'; } - return 'TRIM(' . $expression . $str . ')'; + $tokens[] = $str; + + return sprintf('TRIM(%s)', implode(' ', $tokens)); } /** diff --git a/lib/Doctrine/DBAL/Platforms/SQLAnywherePlatform.php b/lib/Doctrine/DBAL/Platforms/SQLAnywherePlatform.php index 77e550de2f9..d89bf8aa138 100644 --- a/lib/Doctrine/DBAL/Platforms/SQLAnywherePlatform.php +++ b/lib/Doctrine/DBAL/Platforms/SQLAnywherePlatform.php @@ -24,6 +24,7 @@ use function func_get_args; use function get_class; use function implode; +use function in_array; use function is_string; use function preg_match; use function sprintf; @@ -1121,10 +1122,16 @@ public function getTimeTypeDeclarationSQL(array $fieldDeclaration) /** * {@inheritdoc} */ - public function getTrimExpression($str, $pos = TrimMode::UNSPECIFIED, $char = false) + public function getTrimExpression(string $str, int $mode = TrimMode::UNSPECIFIED, ?string $char = null) : string { - if (! $char) { - switch ($pos) { + if (! in_array($mode, [TrimMode::UNSPECIFIED, TrimMode::LEADING, TrimMode::TRAILING, TrimMode::BOTH], true)) { + throw new InvalidArgumentException( + sprintf('The value of $mode is expected to be one of the TrimMode constants, %d given', $mode) + ); + } + + if ($char === null) { + switch ($mode) { case TrimMode::LEADING: return $this->getLtrimExpression($str); case TrimMode::TRAILING: @@ -1136,7 +1143,7 @@ public function getTrimExpression($str, $pos = TrimMode::UNSPECIFIED, $char = fa $pattern = "'%[^' + " . $char . " + ']%'"; - switch ($pos) { + switch ($mode) { case TrimMode::LEADING: return 'SUBSTR(' . $str . ', PATINDEX(' . $pattern . ', ' . $str . '))'; case TrimMode::TRAILING: diff --git a/lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php b/lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php index 8f608391223..a1d426d5c5e 100644 --- a/lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php +++ b/lib/Doctrine/DBAL/Platforms/SQLServerPlatform.php @@ -21,6 +21,7 @@ use function explode; use function func_get_args; use function implode; +use function in_array; use function is_array; use function is_bool; use function is_numeric; @@ -1031,23 +1032,26 @@ public function getModExpression($expression1, $expression2) /** * {@inheritDoc} */ - public function getTrimExpression($str, $pos = TrimMode::UNSPECIFIED, $char = false) + public function getTrimExpression(string $str, int $mode = TrimMode::UNSPECIFIED, ?string $char = null) : string { - if (! $char) { - switch ($pos) { + if (! in_array($mode, [TrimMode::UNSPECIFIED, TrimMode::LEADING, TrimMode::TRAILING, TrimMode::BOTH], true)) { + throw new InvalidArgumentException( + sprintf('The value of $mode is expected to be one of the TrimMode constants, %d given', $mode) + ); + } + + if ($char === null) { + switch ($mode) { case TrimMode::LEADING: - $trimFn = 'LTRIM'; - break; + return 'LTRIM(' . $str . ')'; case TrimMode::TRAILING: - $trimFn = 'RTRIM'; + return 'RTRIM(' . $str . ')'; break; default: return 'LTRIM(RTRIM(' . $str . '))'; } - - return $trimFn . '(' . $str . ')'; } /** Original query used to get those expressions @@ -1061,11 +1065,11 @@ public function getTrimExpression($str, $pos = TrimMode::UNSPECIFIED, $char = fa */ $pattern = "'%[^' + " . $char . " + ']%'"; - if ($pos === TrimMode::LEADING) { + if ($mode === TrimMode::LEADING) { return 'stuff(' . $str . ', 1, patindex(' . $pattern . ', ' . $str . ') - 1, null)'; } - if ($pos === TrimMode::TRAILING) { + if ($mode === TrimMode::TRAILING) { return 'reverse(stuff(reverse(' . $str . '), 1, patindex(' . $pattern . ', reverse(' . $str . ')) - 1, null))'; } diff --git a/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php b/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php index 1aadb10d56e..b3ad00cd7a9 100644 --- a/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php +++ b/lib/Doctrine/DBAL/Platforms/SqlitePlatform.php @@ -12,6 +12,7 @@ use Doctrine\DBAL\Schema\TableDiff; use Doctrine\DBAL\TransactionIsolationLevel; use Doctrine\DBAL\Types; +use InvalidArgumentException; use function array_merge; use function array_unique; use function array_values; @@ -59,11 +60,14 @@ public function getNowExpression($type = 'timestamp') /** * {@inheritDoc} */ - public function getTrimExpression($str, $pos = TrimMode::UNSPECIFIED, $char = false) + public function getTrimExpression(string $str, int $mode = TrimMode::UNSPECIFIED, ?string $char = null) : string { - $trimChar = $char !== false ? (', ' . $char) : ''; + switch ($mode) { + case TrimMode::UNSPECIFIED: + case TrimMode::BOTH: + $trimFn = 'TRIM'; + break; - switch ($pos) { case TrimMode::LEADING: $trimFn = 'LTRIM'; break; @@ -73,10 +77,21 @@ public function getTrimExpression($str, $pos = TrimMode::UNSPECIFIED, $char = fa break; default: - $trimFn = 'TRIM'; + throw new InvalidArgumentException( + sprintf( + 'The value of $mode is expected to be one of the TrimMode constants, %d given', + $mode + ) + ); + } + + $arguments = [$str]; + + if ($char !== null) { + $arguments[] = $char; } - return $trimFn . '(' . $str . $trimChar . ')'; + return sprintf('%s(%s)', $trimFn, implode(', ', $arguments)); } /** diff --git a/tests/Doctrine/Tests/DBAL/Functional/DataAccessTest.php b/tests/Doctrine/Tests/DBAL/Functional/DataAccessTest.php index 196385c4d01..8f60abffff5 100644 --- a/tests/Doctrine/Tests/DBAL/Functional/DataAccessTest.php +++ b/tests/Doctrine/Tests/DBAL/Functional/DataAccessTest.php @@ -18,6 +18,7 @@ use Doctrine\DBAL\Statement; use Doctrine\DBAL\Types\Type; use Doctrine\Tests\DbalFunctionalTestCase; +use InvalidArgumentException; use PDO; use const CASE_LOWER; use const PHP_EOL; @@ -508,10 +509,10 @@ public function testTrimExpression($value, $position, $char, $expectedResult) public function getTrimExpressionData() { return [ - ['test_string', TrimMode::UNSPECIFIED, false, 'foo'], - ['test_string', TrimMode::LEADING, false, 'foo'], - ['test_string', TrimMode::TRAILING, false, 'foo'], - ['test_string', TrimMode::BOTH, false, 'foo'], + ['test_string', TrimMode::UNSPECIFIED, null, 'foo'], + ['test_string', TrimMode::LEADING, null, 'foo'], + ['test_string', TrimMode::TRAILING, null, 'foo'], + ['test_string', TrimMode::BOTH, null, 'foo'], ['test_string', TrimMode::UNSPECIFIED, "'f'", 'oo'], ['test_string', TrimMode::UNSPECIFIED, "'o'", 'f'], ['test_string', TrimMode::UNSPECIFIED, "'.'", 'foo'], @@ -524,10 +525,10 @@ public function getTrimExpressionData() ['test_string', TrimMode::BOTH, "'f'", 'oo'], ['test_string', TrimMode::BOTH, "'o'", 'f'], ['test_string', TrimMode::BOTH, "'.'", 'foo'], - ["' foo '", TrimMode::UNSPECIFIED, false, 'foo'], - ["' foo '", TrimMode::LEADING, false, 'foo '], - ["' foo '", TrimMode::TRAILING, false, ' foo'], - ["' foo '", TrimMode::BOTH, false, 'foo'], + ["' foo '", TrimMode::UNSPECIFIED, null, 'foo'], + ["' foo '", TrimMode::LEADING, null, 'foo '], + ["' foo '", TrimMode::TRAILING, null, ' foo'], + ["' foo '", TrimMode::BOTH, null, 'foo'], ["' foo '", TrimMode::UNSPECIFIED, "'f'", ' foo '], ["' foo '", TrimMode::UNSPECIFIED, "'o'", ' foo '], ["' foo '", TrimMode::UNSPECIFIED, "'.'", ' foo '], @@ -547,6 +548,12 @@ public function getTrimExpressionData() ]; } + public function testTrimExpressionInvalidMode() : void + { + $this->expectException(InvalidArgumentException::class); + $this->connection->getDatabasePlatform()->getTrimExpression('Trim me!', 0xBEEF); + } + /** * @group DDC-1014 */ diff --git a/tests/Doctrine/Tests/DBAL/Platforms/SQLAnywherePlatformTest.php b/tests/Doctrine/Tests/DBAL/Platforms/SQLAnywherePlatformTest.php index 8122a64302c..51989653425 100644 --- a/tests/Doctrine/Tests/DBAL/Platforms/SQLAnywherePlatformTest.php +++ b/tests/Doctrine/Tests/DBAL/Platforms/SQLAnywherePlatformTest.php @@ -665,11 +665,6 @@ public function testGeneratesSQLSnippets() "REVERSE(SUBSTR(REVERSE(column), PATINDEX('%[^' + c + ']%', REVERSE(column))))", $this->platform->getTrimExpression('column', TrimMode::TRAILING, 'c') ); - self::assertEquals( - "REVERSE(SUBSTR(REVERSE(SUBSTR(column, PATINDEX('%[^' + c + ']%', column))), PATINDEX('%[^' + c + ']%', " . - "REVERSE(SUBSTR(column, PATINDEX('%[^' + c + ']%', column))))))", - $this->platform->getTrimExpression('column', null, 'c') - ); self::assertEquals( "REVERSE(SUBSTR(REVERSE(SUBSTR(column, PATINDEX('%[^' + c + ']%', column))), PATINDEX('%[^' + c + ']%', " . "REVERSE(SUBSTR(column, PATINDEX('%[^' + c + ']%', column))))))",