From bcb9e7858a3d357e9c5db5bf1f321f8f2c81bc61 Mon Sep 17 00:00:00 2001 From: Chris Minett <1084019+chrisminett@users.noreply.github.com> Date: Mon, 10 Feb 2025 12:22:51 +0000 Subject: [PATCH] Add type checks for `\Stringable` to quoter The PDO default quote method value param has a string type. Add a placeholder for dynamic SQL statements to preserve the parentheses quote behaviour. Simplify the quote type logic for the expected types. --- CHANGELOG.md | 5 ++ src/Adapter.php | 2 +- src/Adapter/QuoteHandler.php | 75 ++++++++++++++++-------------- src/SqlFragment.php | 2 +- src/SqlStatement.php | 13 ++++++ tests/Adapter/QuoteHandlerTest.php | 45 ++++++++++++++++++ 6 files changed, 104 insertions(+), 38 deletions(-) create mode 100644 src/SqlStatement.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 183a0b9..b5f9a11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] ### Added - Add PHP v8 type declarations. +### Changed +- An object passed to a quote method now specifically checks for an + implementation of `\Stringable`. + This is equivalent to the previous behaviour of checking for `__toString()`. + Arbitrary objects will be blocked by type declarations. ### Removed - **BC break**: Removed support for PHP versions <= v8.0 as they are no longer [actively supported](https://php.net/supported-versions.php) by the PHP project. diff --git a/src/Adapter.php b/src/Adapter.php index 689e4bd..6c42111 100644 --- a/src/Adapter.php +++ b/src/Adapter.php @@ -41,7 +41,7 @@ public function __construct(array $config = []) public function quote(): Adapter\QuoteHandler { if (!isset($this->quoter)) { - $this->quoter = new Adapter\QuoteHandler(function ($value): string { + $this->quoter = new Adapter\QuoteHandler(function (string $value): string { return $this->getConnection()->quote($value); }); } diff --git a/src/Adapter/QuoteHandler.php b/src/Adapter/QuoteHandler.php index c34d9c0..5994184 100644 --- a/src/Adapter/QuoteHandler.php +++ b/src/Adapter/QuoteHandler.php @@ -5,12 +5,13 @@ namespace Phlib\Db\Adapter; use Phlib\Db\Exception\InvalidArgumentException; +use Phlib\Db\SqlStatement; class QuoteHandler { /** * @param \Closure{ - * value: mixed, + * value: string, * }:string $quoteFn */ public function __construct( @@ -22,13 +23,11 @@ public function __construct( /** * Quote a value for the database */ - public function value(mixed $value): string - { + public function value( + string|\Stringable|array|bool|int|float|null $value, + ): string { switch (true) { - case is_object($value): - if (!method_exists($value, '__toString')) { - throw new InvalidArgumentException('Object can not be converted to string value.'); - } + case $value instanceof \Stringable: return (string)$value; case is_bool($value): return (string)(int)$value; @@ -44,16 +43,21 @@ public function value(mixed $value): string return $this->value($value); }, $value); return implode(', ', $value); - default: + case is_string($value): return ($this->quoteFn)($value); + default: + // Not reachable due to type declarations + throw new InvalidArgumentException('Value can not be converted to string for quoting'); } } /** * Quote a value to replace the `?` placeholder */ - public function into(string $text, mixed $value): string - { + public function into( + string $text, + string|\Stringable|array|bool|int|float|null $value, + ): string { return str_replace('?', $this->value($value), $text); } @@ -61,7 +65,7 @@ public function into(string $text, mixed $value): string * Quote a column identifier and alias */ public function columnAs( - string|array|object $ident, + string|\Stringable|array $ident, string $alias, bool $auto = false, ): string { @@ -72,7 +76,7 @@ public function columnAs( * Quote a table identifier and alias */ public function tableAs( - string|array|object $ident, + string|\Stringable|array $ident, string $alias, bool $auto = false, ): string { @@ -83,45 +87,42 @@ public function tableAs( * Quote an identifier */ public function identifier( - string|array|object $ident, + string|\Stringable|array $ident, bool $auto = false, ): string { return $this->quoteIdentifierAs($ident, null, $auto); } private function quoteIdentifierAs( - string|array|object $ident, + string|\Stringable|array $ident, ?string $alias = null, bool $auto = false, string $as = ' AS ', ): string { - if (is_object($ident) && method_exists($ident, 'assemble')) { - $quoted = '(' . $ident->assemble() . ')'; - } elseif (is_object($ident)) { - if (!method_exists($ident, '__toString')) { - throw new InvalidArgumentException('Object can not be converted to string identifier.'); - } + if ($ident instanceof \Stringable) { $quoted = (string)$ident; + if ($ident instanceof SqlStatement) { + $quoted = '(' . $quoted . ')'; + } } else { if (is_string($ident)) { $ident = explode('.', $ident); } - if (is_array($ident)) { - $segments = []; - foreach ($ident as $segment) { - if (is_object($segment)) { - $segments[] = (string)$segment; - } else { - $segments[] = $this->performQuoteIdentifier($segment, $auto); - } - } - if ($alias !== null && end($ident) === $alias) { - $alias = null; + + $segments = []; + foreach ($ident as $segment) { + if ($segment instanceof \Stringable) { + $segments[] = (string)$segment; + } elseif (is_string($segment)) { + $segments[] = $this->performQuoteIdentifier($segment, $auto); + } else { + throw new InvalidArgumentException('Ident array values must be stringable'); } - $quoted = implode('.', $segments); - } else { - $quoted = $this->performQuoteIdentifier($ident, $auto); } + if ($alias !== null && end($ident) === $alias) { + $alias = null; + } + $quoted = implode('.', $segments); } if ($alias !== null) { @@ -131,8 +132,10 @@ private function quoteIdentifierAs( return $quoted; } - private function performQuoteIdentifier(string $value, bool $auto = false): string - { + private function performQuoteIdentifier( + string $value, + bool $auto = false, + ): string { if ($auto === false || $this->autoQuoteIdentifiers === true) { $q = '`'; return $q . str_replace("{$q}", "{$q}{$q}", $value) . $q; diff --git a/src/SqlFragment.php b/src/SqlFragment.php index 1f52fcd..9b2258b 100644 --- a/src/SqlFragment.php +++ b/src/SqlFragment.php @@ -8,7 +8,7 @@ * This class will hold a SQL fragment and return it when cast to a string by Db::quote() * to avoid the string otherwise being quoted as a value */ -class SqlFragment +class SqlFragment implements \Stringable { public function __construct( private readonly string $value, diff --git a/src/SqlStatement.php b/src/SqlStatement.php new file mode 100644 index 0000000..af7dffa --- /dev/null +++ b/src/SqlStatement.php @@ -0,0 +1,13 @@ +statement; + } + }; + + $actual = $this->handler->tableAs($sqlStatement, $alias); + + // SQL Statement should be wrapped in parentheses + $expected = "({$statement}) AS `{$alias}`"; + static::assertSame($expected, $actual); + } + #[DataProvider('identifierData')] public function testIdentifier(string $expected, string|array|SqlFragment $ident, ?bool $auto): void { @@ -126,4 +152,23 @@ public static function identifierData(): array ['`table1`.`*`', 'table1.*', true], ]; } + + public static function dataIdentifierWithInvalidArrayValue(): array + { + return [ + 'int' => [rand()], + 'float' => [rand() / 100], + 'bool' => [true], + 'objectNotStringable' => [new \stdClass()], + ]; + } + + #[DataProvider('dataIdentifierWithInvalidArrayValue')] + public function testIdentifierWithInvalidArrayValue(mixed $value): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Ident array values must be stringable'); + + $this->handler->identifier([$value]); + } }