From c4c4a9deb6c35d32e706093035a6c4d16f7dcebb Mon Sep 17 00:00:00 2001 From: Sergei Tigrov Date: Mon, 27 Nov 2023 01:46:06 +0700 Subject: [PATCH] Refactor `AbstractCommand::getRawSql()` (#785) * Refactor `AbstractCommand::getRawSql()` * Apply fixes from StyleCI * Update test * Remove unnecessary psalm annotation * Update type annotation * Add line to CHANGELOG.md * Update test * Improve * minor improvement --------- Co-authored-by: StyleCI Bot Co-authored-by: Sergei Predvoditelev --- CHANGELOG.md | 2 ++ src/Command/AbstractCommand.php | 58 ++++++++++++++---------------- tests/Provider/CommandProvider.php | 13 +++++++ tests/Support/Stringable.php | 17 +++++++++ 4 files changed, 59 insertions(+), 31 deletions(-) create mode 100644 tests/Support/Stringable.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 549983ded..91c1c94bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ `DbArrayHelper::populate()` methods to `array[]` (@vjik) - Enh #779: Specify populate closure type in `BatchQueryResultInterface` (@vjik) - Enh #778: Deprecate unnecessary argument `$rawSql` of `AbstractCommand::internalExecute()` (@Tigrov) +- Enh #785: Refactor `AbstractCommand::getRawSql()` (@Tigrov) +- Bug #785: Fix bug of `AbstractCommand::getRawSql()` when a param value is `Stringable` object (@Tigrov) - Enh #786: Refactor `AbstractSchema::getDataType()` (@Tigrov) - Enh #784: Specify result type of `ConstraintSchemaInterface::getTableIndexes()` method to `IndexConstraint[]` (@vjik) - Enh #784: Remove unused code in `AbstractSchema::getTableIndexes()` (@vjik) diff --git a/src/Command/AbstractCommand.php b/src/Command/AbstractCommand.php index 0561e3bd1..dae234e46 100644 --- a/src/Command/AbstractCommand.php +++ b/src/Command/AbstractCommand.php @@ -16,15 +16,13 @@ use function explode; use function get_resource_type; use function is_array; -use function is_bool; use function is_int; -use function is_object; use function is_resource; use function is_scalar; use function is_string; use function preg_replace_callback; +use function str_starts_with; use function stream_get_contents; -use function strncmp; /** * Represents an SQL statement to execute in a database. @@ -110,12 +108,12 @@ abstract class AbstractCommand implements CommandInterface * @var string|null Transaction isolation level. */ protected string|null $isolationLevel = null; + /** - * @var array Parameters to use. - * - * @psalm-var ParamInterface[] + * @var ParamInterface[] Parameters to use. */ protected array $params = []; + /** * @var string|null Name of the table to refresh schema for. Null means not to refresh the schema. */ @@ -343,44 +341,42 @@ public function getRawSql(): string } $params = []; + $quoter = $this->getQueryBuilder()->quoter(); - /** @psalm-var mixed $value */ - foreach ($this->params as $name => $value) { - if (is_string($name) && strncmp(':', $name, 1)) { + foreach ($this->params as $name => $param) { + if (is_string($name) && !str_starts_with($name, ':')) { $name = ':' . $name; } - if ($value instanceof ParamInterface) { - /** @psalm-var mixed $value */ - $value = $value->getValue(); - } - - if (is_string($value)) { - /** @psalm-var mixed */ - $params[$name] = $this->getQueryBuilder()->quoter()->quoteValue($value); - } elseif (is_bool($value)) { - /** @psalm-var string */ - $params[$name] = $value ? 'TRUE' : 'FALSE'; - } elseif ($value === null) { - $params[$name] = 'NULL'; - } elseif ((!is_object($value) && !is_resource($value)) || $value instanceof Expression) { - /** @psalm-var mixed */ - $params[$name] = $value; - } + $value = $param->getValue(); + + $params[$name] = match ($param->getType()) { + DataType::INTEGER => (string)$value, + DataType::STRING, DataType::LOB => match (true) { + $value instanceof Expression => (string)$value, + is_resource($value) => $name, + default => $quoter->quoteValue((string)$value), + }, + DataType::BOOLEAN => $value ? 'TRUE' : 'FALSE', + DataType::NULL => 'NULL', + default => $name, + }; } + /** @var string[] $params */ if (!isset($params[0])) { - return preg_replace_callback('#(:\w+)#', static function (array $matches) use ($params): string { - $m = $matches[1]; - return (string)($params[$m] ?? $m); - }, $this->sql); + return preg_replace_callback( + '#(:\w+)#', + static fn (array $matches): string => $params[$matches[1]] ?? $matches[1], + $this->sql + ); } // Support unnamed placeholders should be dropped $sql = ''; foreach (explode('?', $this->sql) as $i => $part) { - $sql .= $part . (string)($params[$i] ?? ''); + $sql .= $part . ($params[$i] ?? ''); } return $sql; diff --git a/tests/Provider/CommandProvider.php b/tests/Provider/CommandProvider.php index 9f7408b73..96156b8ae 100644 --- a/tests/Provider/CommandProvider.php +++ b/tests/Provider/CommandProvider.php @@ -8,6 +8,7 @@ use Yiisoft\Db\Query\Query; use Yiisoft\Db\Schema\SchemaInterface; use Yiisoft\Db\Tests\Support\DbHelper; +use Yiisoft\Db\Tests\Support\Stringable; use Yiisoft\Db\Tests\Support\TestTrait; class CommandProvider @@ -637,6 +638,18 @@ public static function rawSql(): array static::$driverName, ), ], + [ + << new Stringable('Alfa')], + DbHelper::replaceQuotes( + <<value; + } +}