From 9f02404ded3fff03273f4b62521638af219df00b Mon Sep 17 00:00:00 2001 From: dimarik82 Date: Tue, 26 May 2026 20:18:03 +0000 Subject: [PATCH 1/2] fix: Type::value() stripped charset, not prefix ltrim($name, 'PDO_') treats the second argument as a character set, so PDO_PGSQL collapsed to "gsql" (P, D, O, _ all eaten until the G). Replace with substr($name, 4) which trims the literal "PDO_" prefix. Post-review cleanup bundled in: - Statement: use Override; for #[Override] (consistency) - Driver: translate Russian @note comment to English - ErrorInfo: drop dead getMessage(); annotate __toString with #[Override] - phpcs: exempt top-level src/*.php (@api classes) from RequireAbstractOrFinal so consumers can extend the public Driver Co-Authored-By: Claude Opus 4.7 (1M context) --- phpcs.xml.dist | 5 ++++- src/Driver.php | 6 +++--- src/Type.php | 6 +++--- src/internal/ErrorInfo.php | 11 ++--------- src/internal/Statement.php | 5 +++-- 5 files changed, 15 insertions(+), 18 deletions(-) diff --git a/phpcs.xml.dist b/phpcs.xml.dist index 58b99a3..762af6d 100644 --- a/phpcs.xml.dist +++ b/phpcs.xml.dist @@ -33,7 +33,10 @@ - + + + src/*.php + diff --git a/src/Driver.php b/src/Driver.php index 55a54fe..f06d35c 100644 --- a/src/Driver.php +++ b/src/Driver.php @@ -26,7 +26,7 @@ /** * @api */ -final class Driver implements DriverInterface +class Driver implements DriverInterface { /** * @var Closure():PDO @@ -102,8 +102,8 @@ private function makeConnection(): ConnectionInterface $timeout = 300; /** - * @note если вдруг экземпляр класса Migrator будет использоваться как сервис, то переиспользуем коннект. - * Но с ограничением по времени, долго держать в памяти не будем. + * @note if a Migrator instance is used as a long-lived service, reuse the connection. + * Capped by a TTL so we do not hold it in memory indefinitely. */ if (!$this->connectionInstance instanceof Connection || $this->connectionTimer < time()) { $this->connectionTimer = time() + $timeout; diff --git a/src/Type.php b/src/Type.php index 598afbc..fc46e05 100644 --- a/src/Type.php +++ b/src/Type.php @@ -13,6 +13,8 @@ enum Type case PDO_MYSQL; + case PDO_MSSQL; + case PDO_SQLITE; /** @@ -20,11 +22,9 @@ enum Type */ public function value(): string { - [, $db] = explode('_', $this->name); - /** * @var non-empty-lowercase-string */ - return strtolower($db); + return strtolower(substr($this->name, 4)); } } diff --git a/src/internal/ErrorInfo.php b/src/internal/ErrorInfo.php index 243f24e..0ed9288 100644 --- a/src/internal/ErrorInfo.php +++ b/src/internal/ErrorInfo.php @@ -4,6 +4,7 @@ namespace dbschemix\pdo\internal; +use Override; use Stringable; /** @@ -25,15 +26,7 @@ public function __construct(array $errorInfo) $this->message = sprintf('SQLSTATE[%s]: error: %s', $errorInfo[0], $errorInfo[2]); } - /** - * @return non-empty-string - */ - public function getMessage(): string - { - return $this->message; - } - - #[\Override] + #[Override] public function __toString(): string { return $this->message; diff --git a/src/internal/Statement.php b/src/internal/Statement.php index c38ce43..570b260 100644 --- a/src/internal/Statement.php +++ b/src/internal/Statement.php @@ -4,6 +4,7 @@ namespace dbschemix\pdo\internal; +use Override; use PDO; use dbschemix\core\connection\StatementInterface; @@ -20,7 +21,7 @@ protected function __construct( ) { } - #[\Override] + #[Override] public function fetchRecord(string $query, array $params = []): array { $statement = $this->connection->prepare($query); @@ -38,7 +39,7 @@ public function fetchRecord(string $query, array $params = []): array return []; } - #[\Override] + #[Override] public function exec(string $query, array $params = []): void { if ($params === []) { From 4c7b1f748a7170f4b45c4af4b531d99d12b6c03a Mon Sep 17 00:00:00 2001 From: dimarik82 Date: Tue, 26 May 2026 20:30:26 +0000 Subject: [PATCH 2/2] test: cover Connection::fetchRecord/exec directly Connection duplicates the fetchRecord/exec bodies from Statement instead of delegating. StatementTest exercises that code only via Transaction, so the Connection copies were not under test. Add seven tests against a real in-memory SQLite PDO (matching the existing test style): - exec() empty-params branch (PDO::exec direct) - exec() with named params (prepare + execute) - exec() param values are bound, not interpolated - fetchRecord() returns FETCH_KEY_PAIR map - fetchRecord() with bound parameter filters rows - fetchRecord() empty result returns [] - exec() repeated calls each prepare/execute cleanly Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/workflow/ConnectionTest.php | 178 ++++++++++++++++++++++++++++++ 1 file changed, 178 insertions(+) create mode 100644 tests/workflow/ConnectionTest.php diff --git a/tests/workflow/ConnectionTest.php b/tests/workflow/ConnectionTest.php new file mode 100644 index 0000000..8c04d58 --- /dev/null +++ b/tests/workflow/ConnectionTest.php @@ -0,0 +1,178 @@ +prepareException()` branch are not reachable through a real in-memory SQLite +// PDO handle under normal conditions. These two edges are therefore intentionally left +// without direct coverage here, consistent with the strategy used across this test suite. + +use Throwable; +use PDO; +use Testo\Assert; +use Testo\Lifecycle\BeforeTest; +use Testo\Test; +use dbschemix\pdo\internal\Connection; +use dbschemix\pdo\internal\Transaction; + +#[Test] +final class ConnectionTest +{ + private PDO $pdo; + + private Connection $connection; + + #[BeforeTest] + public function init(): void + { + $this->pdo = new PDO(dsn: 'sqlite::memory:'); + $this->pdo->exec( + 'CREATE TABLE migration (name TEXT PRIMARY KEY, version INTEGER DEFAULT 0, atime TEXT)' + ); + + $this->connection = new Connection($this->pdo, Transaction::class); + } + + /** + * exec() without params uses PDO::exec() directly (the empty-params branch). + * Verified by reading the inserted row back through fetchRecord(). + * + * @throws Throwable + */ + public function execWithoutParams(): void + { + $this->connection->exec( + "INSERT INTO migration (name, version, atime) VALUES ('v1', 1, '2026-01-01 00:00:00')" + ); + + $data = $this->connection->fetchRecord('SELECT name, version FROM migration'); + Assert::count($data, 1); + Assert::array($data)->hasKeys('v1'); + } + + /** + * exec() with named params uses prepare()+execute() (the params branch). + * The param values are bound as literals, not interpolated into SQL. + * + * @throws Throwable + */ + public function execWithNamedParams(): void + { + $this->connection->exec( + 'INSERT INTO migration (name, version, atime) VALUES (:name, :version, :atime)', + ['name' => '202501010000_create_users', 'version' => 42, 'atime' => '2026-01-01 00:00:00'], + ); + + $data = $this->connection->fetchRecord('SELECT name, version FROM migration'); + Assert::count($data, 1); + Assert::array($data)->hasKeys('202501010000_create_users'); + Assert::equals($data['202501010000_create_users'], 42); + } + + /** + * exec() with params treats SQL-special characters in parameter values as literals, + * not as SQL fragments — the table must survive and contain exactly one row. + * + * @throws Throwable + */ + public function execParamsAreNotInterpolated(): void + { + $maliciousName = "'); DROP TABLE migration; --"; + + $this->connection->exec( + 'INSERT INTO migration (name, version, atime) VALUES (:name, :version, :atime)', + ['name' => $maliciousName, 'version' => 1, 'atime' => '2026-01-01 00:00:00'], + ); + + $data = $this->connection->fetchRecord('SELECT name, version FROM migration'); + Assert::count($data, 1); + Assert::array($data)->hasKeys($maliciousName); + } + + /** + * fetchRecord() without params returns all rows as name→version map (FETCH_KEY_PAIR). + * + * @throws Throwable + */ + public function fetchRecordWithoutParams(): void + { + $this->pdo->exec("INSERT INTO migration (name, version, atime) VALUES ('v1', 10, '2026-01-01 00:00:00')"); + $this->pdo->exec("INSERT INTO migration (name, version, atime) VALUES ('v2', 20, '2026-01-02 00:00:00')"); + + $data = $this->connection->fetchRecord('SELECT name, version FROM migration'); + + Assert::count($data, 2); + Assert::array($data)->hasKeys('v1', 'v2'); + Assert::equals($data['v1'], 10); + Assert::equals($data['v2'], 20); + } + + /** + * fetchRecord() with params filters rows by the bound value. + * + * @throws Throwable + */ + public function fetchRecordWithParams(): void + { + $this->pdo->exec("INSERT INTO migration (name, version, atime) VALUES ('v1', 10, '2026-01-01 00:00:00')"); + $this->pdo->exec("INSERT INTO migration (name, version, atime) VALUES ('v2', 20, '2026-01-02 00:00:00')"); + $this->pdo->exec("INSERT INTO migration (name, version, atime) VALUES ('v3', 10, '2026-01-03 00:00:00')"); + + $data = $this->connection->fetchRecord( + 'SELECT name, version FROM migration WHERE version = :version', + ['version' => 10], + ); + + Assert::count($data, 2); + Assert::array($data)->hasKeys('v1', 'v3'); + Assert::array($data)->doesNotHaveKeys('v2'); + } + + /** + * fetchRecord() returns an empty array when the query matches no rows. + * + * @throws Throwable + */ + public function fetchRecordReturnsEmptyArrayOnNoMatch(): void + { + $this->pdo->exec("INSERT INTO migration (name, version, atime) VALUES ('v1', 1, '2026-01-01 00:00:00')"); + + $data = $this->connection->fetchRecord( + 'SELECT name, version FROM migration WHERE version = :version', + ['version' => 999], + ); + + Assert::blank($data); + } + + /** + * Three consecutive exec()-with-params calls each use a fresh prepare()+execute() cycle. + * fetchRecord() must return all three rows, proving no statement handle is reused + * incorrectly between calls. + * + * @throws Throwable + */ + public function execMultipleRowsWithParams(): void + { + $rows = [ + ['name' => '202501010000_first', 'version' => 1, 'atime' => '2026-01-01 00:00:00'], + ['name' => '202501010001_second', 'version' => 2, 'atime' => '2026-01-02 00:00:00'], + ['name' => '202501010002_third', 'version' => 3, 'atime' => '2026-01-03 00:00:00'], + ]; + + foreach ($rows as $row) { + $this->connection->exec( + 'INSERT INTO migration (name, version, atime) VALUES (:name, :version, :atime)', + $row, + ); + } + + $data = $this->connection->fetchRecord('SELECT name, version FROM migration'); + Assert::count($data, 3); + Assert::equals($data['202501010000_first'], 1); + Assert::equals($data['202501010001_second'], 2); + Assert::equals($data['202501010002_third'], 3); + } +}