From bd37c0d3678397327c8b9279925f3b9749a5faa1 Mon Sep 17 00:00:00 2001 From: Chris Minett <1084019+chrisminett@users.noreply.github.com> Date: Tue, 11 Feb 2025 11:05:11 +0000 Subject: [PATCH 1/2] Use MySQL native prepares by default Add reconnect behaviour to `prepare()`. InvalidQueryException allows no 'bind' for exceptions during 'prepare'. --- CHANGELOG.md | 2 + src/Adapter.php | 15 ++-- src/Adapter/Config.php | 1 + src/AdapterInterface.php | 4 +- src/Exception/InvalidQueryException.php | 10 ++- tests/Adapter/ConfigTest.php | 14 ++++ tests/AdapterTest.php | 79 ++++++++++++++++++- tests/Exception/InvalidQueryExceptionTest.php | 38 +++++++++ 8 files changed, 149 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b5f9a11..521c9e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Added - Add PHP v8 type declarations. ### Changed +- Set default PDO config `PDO::ATTR_EMULATE_PREPARES = 0` + to disable emulated prepares, i.e. use MySQL native prepares. - 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()`. diff --git a/src/Adapter.php b/src/Adapter.php index 6c42111..80e4680 100644 --- a/src/Adapter.php +++ b/src/Adapter.php @@ -183,15 +183,14 @@ public function lastInsertId(?string $tablename = null): string return $this->getConnection()->lastInsertId($tablename); } - public function prepare(string $statement): \PDOStatement + public function prepare(string $sql): \PDOStatement { - // the prepare method is emulated by PDO, so no point in detected disconnection - return $this->getConnection()->prepare($statement); + return $this->doQuery($sql, null); } - public function execute(string $statement, array $bind = []): int + public function execute(string $sql, array $bind = []): int { - $stmt = $this->query($statement, $bind); + $stmt = $this->query($sql, $bind); return $stmt->rowCount(); } @@ -200,11 +199,13 @@ public function query(string $sql, array $bind = []): \PDOStatement return $this->doQuery($sql, $bind); } - private function doQuery(string $sql, array $bind, bool $hasCaughtException = false): \PDOStatement + private function doQuery(string $sql, ?array $bind, bool $hasCaughtException = false): \PDOStatement { try { $stmt = $this->getConnection()->prepare($sql); - $stmt->execute($bind); + if ($bind !== null) { + $stmt->execute($bind); + } return $stmt; } catch (\PDOException $exception) { if (InvalidQueryException::isInvalidSyntax($exception)) { diff --git a/src/Adapter/Config.php b/src/Adapter/Config.php index c173b06..1b8a921 100644 --- a/src/Adapter/Config.php +++ b/src/Adapter/Config.php @@ -60,6 +60,7 @@ public function getOptions(): array \PDO::ATTR_TIMEOUT => $timeout, \PDO::ATTR_ERRMODE => \PDO::ERRMODE_EXCEPTION, \PDO::ATTR_DEFAULT_FETCH_MODE => \PDO::FETCH_ASSOC, + \PDO::ATTR_EMULATE_PREPARES => 0, ]; } diff --git a/src/AdapterInterface.php b/src/AdapterInterface.php index 3aa2c08..9a255aa 100644 --- a/src/AdapterInterface.php +++ b/src/AdapterInterface.php @@ -42,9 +42,9 @@ public function ping(): bool; */ public function lastInsertId(?string $tablename = null): string; - public function prepare(string $statement): \PDOStatement; + public function prepare(string $sql): \PDOStatement; - public function execute(string $statement, array $bind = []): int; + public function execute(string $sql, array $bind = []): int; public function query(string $sql, array $bind = []): \PDOStatement; diff --git a/src/Exception/InvalidQueryException.php b/src/Exception/InvalidQueryException.php index 8742d49..05e2c01 100644 --- a/src/Exception/InvalidQueryException.php +++ b/src/Exception/InvalidQueryException.php @@ -13,7 +13,7 @@ public static function isInvalidSyntax(\PDOException $exception): bool public function __construct( private readonly string $query, - private readonly array $bind = [], + private readonly ?array $bind = null, ?\PDOException $previous = null, ) { $message = 'You have an error in your SQL syntax.'; @@ -22,7 +22,11 @@ public function __construct( $message = $previous->getMessage(); $code = (int)$previous->getCode(); } - $message .= ' SQL: ' . $this->query . ' Bind: ' . var_export($this->bind, true); + $message .= '; SQL: ' . $this->query; + + if ($this->bind !== null) { + $message .= '; Bind: ' . var_export($this->bind, true); + } parent::__construct($message, $code, $previous); } @@ -32,7 +36,7 @@ public function getQuery(): string return $this->query; } - public function getBindData(): array + public function getBindData(): ?array { return $this->bind; } diff --git a/tests/Adapter/ConfigTest.php b/tests/Adapter/ConfigTest.php index a2d5986..3b98a08 100644 --- a/tests/Adapter/ConfigTest.php +++ b/tests/Adapter/ConfigTest.php @@ -134,6 +134,20 @@ public static function getOptionsDataProvider(): array \PDO::ATTR_DEFAULT_FETCH_MODE, \PDO::FETCH_COLUMN, ], + 'emulate-default' => [ + [], + \PDO::ATTR_EMULATE_PREPARES, + 0, + ], + 'emulate-override' => [ + [ + 'attributes' => [ + \PDO::ATTR_EMULATE_PREPARES => 1, + ], + ], + \PDO::ATTR_EMULATE_PREPARES, + 1, + ], 'attributes-one' => [ [ 'attributes' => [ diff --git a/tests/AdapterTest.php b/tests/AdapterTest.php index 54ce99b..6dc2b0d 100644 --- a/tests/AdapterTest.php +++ b/tests/AdapterTest.php @@ -238,7 +238,10 @@ public function testLastInsertIdWithTableName(): void public function testPrepare(): void { $pdoStatement = $this->createMock(\PDOStatement::class); - $sql = 'SELECT * FROM table'; + $pdoStatement->expects(static::never()) + ->method('execute'); + + $sql = 'SELECT * FROM foo WHERE bar = ?'; $this->pdo->expects(static::once()) ->method('prepare') ->with($sql) @@ -249,6 +252,71 @@ public function testPrepare(): void static::assertSame($pdoStatement, $adapter->prepare($sql)); } + public function testPrepareWithInvalidSql(): void + { + $sql = 'SELEECT * FORM foo WHERE bar = ?'; + + $this->expectException(InvalidQueryException::class); + $this->expectExceptionMessage('You have an error in your SQL syntax; SQL: ' . $sql); + // There are no 'Bind' details to include + $this->expectExceptionMessageMatches('/^((?!Bind).)*$/i'); + + $exception = new \PDOException('You have an error in your SQL syntax'); + $this->pdo->method('prepare') + ->willThrowException($exception); + + $adapter = new Adapter(); + $adapter->setConnection($this->pdo); + $adapter->prepare($sql); + } + + public function testPrepareReconnectsWhenMysqlHasGoneAway(): void + { + $exception = new \PDOException('MySQL server has gone away'); + + // First attempt throws 'gone away' exception to trigger reconnect + $this->pdo->expects(static::once()) + ->method('prepare') + ->willThrowException($exception); + + $adapter = new Adapter(); + $adapter->setConnection($this->pdo); + $adapter->setConnectionFactory(function (): \PDO { + // New PDO for reconnect; second attempt successful + $statement2 = $this->createMock(\PDOStatement::class); + $statement2->expects(static::never()) + ->method('execute'); + $pdo2 = $this->createMock(\PDO::class); + $pdo2->method('prepare') + ->willReturn($statement2); + return $pdo2; + }); + $adapter->prepare('SELECT * FROM foo WHERE bar = ?'); + } + + public function testPrepareFailsAfterSuccessfulReconnect(): void + { + $this->expectException(RuntimeException::class); + + $exception = new \PDOException('MySQL server has gone away'); + + // First attempt throws 'gone away' exception to trigger reconnect + $this->pdo->method('prepare') + ->willThrowException($exception); + + $adapter = new Adapter(); + $adapter->setConnection($this->pdo); + $adapter->setConnectionFactory(function (): \PDO { + // New PDO for reconnect; second attempt also fails + $exception = new PDOExceptionStub('failed for some random reason', 1234); + $pdo2 = $this->createMock(\PDO::class); + $pdo2->method('prepare') + ->willThrowException($exception); + return $pdo2; + }); + $adapter->prepare('SELECT * FROM foo WHERE bar = ?'); + } + public function testExecute(): void { $sql = 'dummy sql'; @@ -339,7 +407,14 @@ public function testQueryWithBind(): void public function testQueryWithInvalidSql(): void { + $sql = 'SELEECT * FORM foo'; + $this->expectException(InvalidQueryException::class); + $this->expectExceptionMessage( + 'You have an error in your SQL syntax; SQL: ' . + $sql . + "; Bind: array (\n)", + ); $exception = new \PDOException('You have an error in your SQL syntax'); $statement = $this->createMock(\PDOStatement::class); @@ -350,7 +425,7 @@ public function testQueryWithInvalidSql(): void $adapter = new Adapter(); $adapter->setConnection($this->pdo); - $adapter->query('SELEECT * FORM foo'); + $adapter->query($sql); } public function testQueryReconnectsWhenMysqlHasGoneAway(): void diff --git a/tests/Exception/InvalidQueryExceptionTest.php b/tests/Exception/InvalidQueryExceptionTest.php index 9d7a6b8..9c81742 100644 --- a/tests/Exception/InvalidQueryExceptionTest.php +++ b/tests/Exception/InvalidQueryExceptionTest.php @@ -5,6 +5,7 @@ namespace Phlib\Db\Tests\Exception; use Phlib\Db\Exception\InvalidQueryException; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; class InvalidQueryExceptionTest extends TestCase @@ -26,6 +27,37 @@ public function testConstructorUsesPreviousException(): void static::assertStringStartsWith($message, $exception->getMessage()); } + public static function dataMessageIncludesQueryAndBind(): array + { + return [ + 'withBind' => [[ + 'one' => sha1(uniqid('one')), + 'two' => sha1(uniqid('two')), + ]], + 'emptyBind' => [[]], + 'withoutBind' => [null], + ]; + } + + #[DataProvider('dataMessageIncludesQueryAndBind')] + public function testMessageIncludesQueryAndBind(?array $bind): void + { + $sql = 'SELECT * FRM foo'; + $code = rand(1000, 9999); + $message = sha1(uniqid('message')); + $pdoException = new PDOExceptionStub($message, $code); + + $exception = new InvalidQueryException($sql, $bind, $pdoException); + + $expected = $message . + '; SQL: ' . $sql; + if ($bind !== null) { + $expected .= '; Bind: ' . var_export($bind, true); + } + + static::assertSame($expected, $exception->getMessage()); + } + public function testGetQuery(): void { $query = 'SELECT * FRM foo'; @@ -40,6 +72,12 @@ public function testGetBindData(): void static::assertSame($bind, $exception->getBindData()); } + public function testGetBindDataNotSet(): void + { + $exception = new InvalidQueryException('', null); + static::assertNull($exception->getBindData()); + } + public function testSuccessfullyDetectsInvalidSyntaxException(): void { $code = '42000'; From 5a8e3bd5a791f2a6c7b873346e05cb20422eb93f Mon Sep 17 00:00:00 2001 From: Chris Minett <1084019+chrisminett@users.noreply.github.com> Date: Tue, 11 Feb 2025 11:47:57 +0000 Subject: [PATCH 2/2] Separate and reuse the reconnect behaviour --- src/Adapter.php | 50 ++++++++++++++++++++++++++++--------------------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/src/Adapter.php b/src/Adapter.php index 80e4680..beea64f 100644 --- a/src/Adapter.php +++ b/src/Adapter.php @@ -199,25 +199,40 @@ public function query(string $sql, array $bind = []): \PDOStatement return $this->doQuery($sql, $bind); } - private function doQuery(string $sql, ?array $bind, bool $hasCaughtException = false): \PDOStatement + private function doQuery(string $sql, ?array $bind): \PDOStatement { try { - $stmt = $this->getConnection()->prepare($sql); - if ($bind !== null) { - $stmt->execute($bind); - } - return $stmt; + return $this->tryOrReconnect(function () use ($sql, $bind) { + $stmt = $this->getConnection()->prepare($sql); + if ($bind !== null) { + $stmt->execute($bind); + } + return $stmt; + }); } catch (\PDOException $exception) { if (InvalidQueryException::isInvalidSyntax($exception)) { throw new InvalidQueryException($sql, $bind, $exception); - } elseif (RuntimeException::hasServerGoneAway($exception) && !$hasCaughtException) { - $this->reconnect(); - return $this->doQuery($sql, $bind, true); } throw RuntimeException::createFromException($exception); } } + /** + * @return mixed Value from $closure + */ + private function tryOrReconnect(\Closure $closure): mixed + { + try { + return $closure(); + } catch (\PDOException $exception) { + if (RuntimeException::hasServerGoneAway($exception)) { + $this->reconnect(); + return $closure(); + } + throw $exception; + } + } + private function connect(): self { if (!isset($this->connection)) { @@ -233,20 +248,13 @@ public function cloneConnection(): \PDO } public function beginTransaction(): bool - { - return $this->doBeginTransaction(); - } - - private function doBeginTransaction(bool $hasCaughtException = false): bool { try { - return $this->getConnection()->beginTransaction(); - } catch (\PDOException $exception) { - if (RuntimeException::hasServerGoneAway($exception) && !$hasCaughtException) { - $this->reconnect(); - return $this->doBeginTransaction(true); - } - throw RuntimeException::createFromException($exception); + return $this->tryOrReconnect( + fn() => $this->getConnection()->beginTransaction(), + ); + } catch (\PDOException $e) { + throw RuntimeException::createFromException($e); } }