Skip to content

Commit 9ec5142

Browse files
committed
\assert() 利用を排除し設計レベルの型保証に置換
\assert() は production で no-op になり silent pass を許す可能性があるため、 全 6 箇所を CLAUDE.md「設計や言語機能で型エラーを解決する」方針に沿って 置き換えた。 src 側: - Connection::query: fetchAll の row 型 narrowing を if + throw に変更 (FETCH_ASSOC コントラクト違反時 UnexpectedValueException で fast-fail) - Connection::dialect / serverVersion: ??= による lazy init に再設計 ensureDialectDetected() を probeServerVersion() に置換し、nullable narrowing を不要にした。SELECT VERSION() の実行回数は変わらない (どちらの getter を先に呼んでも 1 回) test 側: - ConnectionTest::countUsers: テストヘルパーの \assert() を PHPUnit assertion (assertNotFalse / assertIsArray / assertIsInt) に変更 - 防御コードと挙動保証のテスト 3 件を追加: - testServerVersionCachesResultAcrossCalls (キャッシュ対称性) - testDialectAndServerVersionShareSingleSelectVersionQuery (??= リファクタの単一 SELECT VERSION() 実行不変条件) - testQueryThrowsWhenPdoReturnsNonArrayRow (FETCH_ASSOC 防御コード) 機能変更なし。公開 API 変更なし。
1 parent 1eb810a commit 9ec5142

2 files changed

Lines changed: 81 additions & 32 deletions

File tree

src/Sloop/Database/Connection.php

Lines changed: 15 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Sloop\Database\Exception\ExceptionFactory;
1616
use Sloop\Database\Exception\LockWaitTimeoutException;
1717
use Throwable;
18+
use UnexpectedValueException;
1819

1920
/**
2021
* PDO wrapper exposing sloop's database API.
@@ -107,19 +108,20 @@ public static function open(
107108
/**
108109
* Execute a SELECT-style statement and return the fetched rows.
109110
*
110-
* @param string $sql SQL statement returning a result set
111-
* @param array<int|string, mixed> $bindings Parameters to bind
112-
* @return Result Fetched rows
113-
* @throws DatabaseException When the statement fails
111+
* @param string $sql SQL statement returning a result set
112+
* @param array<int|string, mixed> $bindings Parameters to bind
113+
* @return Result Fetched rows
114+
* @throws DatabaseException When the statement fails
115+
* @throws UnexpectedValueException When PDO returns a non-array row under FETCH_ASSOC (driver contract violation)
114116
*/
115117
public function query(string $sql, array $bindings = []): Result
116118
{
117119
$stmt = $this->prepareAndExecute($sql, $bindings);
118-
// PDOStatement::fetchAll's stub only exposes `array`, so per-row
119-
// assert is required to narrow to Result's parameter type.
120120
$rows = [];
121121
foreach ($stmt->fetchAll(PDO::FETCH_ASSOC) as $row) {
122-
\assert(\is_array($row));
122+
if (!\is_array($row)) {
123+
throw new UnexpectedValueException('PDO returned non-array row from FETCH_ASSOC');
124+
}
123125
$rows[] = $row;
124126
}
125127

@@ -294,11 +296,7 @@ public function transaction(
294296
*/
295297
public function dialect(): Dialect
296298
{
297-
$this->ensureDialectDetected();
298-
299-
\assert($this->dialect !== null);
300-
301-
return $this->dialect;
299+
return $this->dialect ??= Dialect::detect($this->serverVersion());
302300
}
303301

304302
/**
@@ -311,11 +309,7 @@ public function dialect(): Dialect
311309
*/
312310
public function serverVersion(): string
313311
{
314-
$this->ensureDialectDetected();
315-
316-
\assert($this->serverVersion !== null);
317-
318-
return $this->serverVersion;
312+
return $this->serverVersion ??= $this->probeServerVersion();
319313
}
320314

321315
/**
@@ -387,17 +381,13 @@ private function sleepBackoff(int $backoffMs): void
387381
}
388382

389383
/**
390-
* Probe the server version on first use and cache the result.
384+
* Probe the server version via `SELECT VERSION()`; called lazily by serverVersion().
391385
*
392-
* @return void
386+
* @return string Raw `SELECT VERSION()` output, or '' if the driver returned false instead of throwing
393387
* @throws DatabaseException When `SELECT VERSION()` fails
394388
*/
395-
private function ensureDialectDetected(): void
389+
private function probeServerVersion(): string
396390
{
397-
if ($this->dialect !== null) {
398-
return;
399-
}
400-
401391
try {
402392
$statement = $this->pdo->query('SELECT VERSION()');
403393
} catch (PDOException $e) {
@@ -406,10 +396,7 @@ private function ensureDialectDetected(): void
406396

407397
// Defensive: unreachable under ERRMODE_EXCEPTION (contractual for open()),
408398
// but tolerate callers that inject a PDO with a different error mode.
409-
$value = $statement === false ? '' : (string) $statement->fetchColumn();
410-
411-
$this->serverVersion = $value;
412-
$this->dialect = Dialect::detect($value);
399+
return $statement === false ? '' : (string) $statement->fetchColumn();
413400
}
414401

415402
/**

tests/Unit/Database/ConnectionTest.php

Lines changed: 66 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use LogicException;
88
use PDO;
99
use Pdo\Sqlite;
10+
use PDOStatement;
1011
use PHPUnit\Framework\TestCase;
1112
use RuntimeException;
1213
use Sloop\Database\Connection;
@@ -16,6 +17,7 @@
1617
use Sloop\Database\Exception\LockWaitTimeoutException;
1718
use Sloop\Database\Exception\QueryException;
1819
use Sloop\Database\IsolationLevel;
20+
use UnexpectedValueException;
1921

2022
/*
2123
* Policy: several tests below omit a trailing $this->fail() inside the try
@@ -55,13 +57,15 @@ protected function setUp(): void
5557
private function countUsers(): int
5658
{
5759
$stmt = $this->pdo->query('SELECT COUNT(*) AS c FROM users');
58-
\assert($stmt !== false);
60+
$this->assertNotFalse($stmt);
61+
5962
$row = $stmt->fetch(PDO::FETCH_ASSOC);
60-
\assert(\is_array($row));
63+
$this->assertIsArray($row);
64+
6165
$count = $row['c'] ?? 0;
62-
\assert(\is_int($count) || \is_string($count));
66+
$this->assertIsInt($count);
6367

64-
return (int) $count;
68+
return $count;
6569
}
6670

6771
// -------------------------------------------------------
@@ -136,6 +140,31 @@ public function testQueryReturnsEmptyResultWhenNoRowsMatch(): void
136140
$this->assertSame([], $result->toArray());
137141
}
138142

143+
public function testQueryThrowsWhenPdoReturnsNonArrayRow(): void
144+
{
145+
// Defensive guard: FETCH_ASSOC contractually returns associative arrays,
146+
// but the throw is the type-narrowing fallback when a non-conformant
147+
// driver violates that contract.
148+
$statement = $this->createStub(PDOStatement::class);
149+
$statement->method('execute')->willReturn(true);
150+
$statement->method('fetchAll')->willReturn([['valid' => 1], 'invalid-row']);
151+
152+
$pdo = $this->createStub(PDO::class);
153+
$pdo->method('prepare')->willReturn($statement);
154+
155+
$connection = new Connection($pdo, 'test');
156+
157+
try {
158+
$connection->query('SELECT 1');
159+
$this->fail('Expected UnexpectedValueException');
160+
} catch (UnexpectedValueException $e) {
161+
$this->assertSame(
162+
'PDO returned non-array row from FETCH_ASSOC',
163+
$e->getMessage(),
164+
);
165+
}
166+
}
167+
139168
public function testStatementReturnsAffectedRowCount(): void
140169
{
141170
$affected = $this->connection->statement(
@@ -490,6 +519,39 @@ public function testDialectCachesResultAcrossCalls(): void
490519
$this->assertSame($first, $second);
491520
}
492521

522+
public function testServerVersionCachesResultAcrossCalls(): void
523+
{
524+
$first = $this->connection->serverVersion();
525+
$second = $this->connection->serverVersion();
526+
527+
$this->assertSame($first, $second);
528+
}
529+
530+
public function testDialectAndServerVersionShareSingleSelectVersionQuery(): void
531+
{
532+
// Both getters are independently lazy via ??= but must share a single
533+
// `SELECT VERSION()` execution regardless of which is called first or
534+
// how many times callers interleave them.
535+
$statement = $this->createStub(PDOStatement::class);
536+
$statement->method('fetchColumn')->willReturn('10.11.11-MariaDB');
537+
538+
$pdo = $this->createMock(PDO::class);
539+
$pdo->expects($this->once())
540+
->method('query')
541+
->with('SELECT VERSION()')
542+
->willReturn($statement);
543+
544+
$connection = new Connection($pdo, 'test');
545+
546+
$connection->serverVersion();
547+
$connection->dialect();
548+
$connection->serverVersion();
549+
$connection->dialect();
550+
551+
$this->assertSame(Dialect::MariaDB, $connection->dialect());
552+
$this->assertSame('10.11.11-MariaDB', $connection->serverVersion());
553+
}
554+
493555
public function testDialectFallsBackToMysqlWhenVersionLacksMariadbMarker(): void
494556
{
495557
$sqlite = new Sqlite('sqlite::memory:', null, null, [

0 commit comments

Comments
 (0)