diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 00140e4..4783b2c 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -1,6 +1,6 @@ - - + + - + - - + + - + diff --git a/src/Session/DbSessionHandler.php b/src/Session/DbSessionHandler.php index 4cf14e4..2df961e 100644 --- a/src/Session/DbSessionHandler.php +++ b/src/Session/DbSessionHandler.php @@ -19,15 +19,30 @@ use PhpDb\Session\Sql\Insert; use PhpDb\Sql\Exception\InvalidArgumentException as SqlInvalidArgumentException; use PhpDb\Sql\Sql; +use Random\RandomException; use SessionHandlerInterface; +use SessionIdInterface; +use SessionUpdateTimestampHandlerInterface; +use function bin2hex; use function date; use function ini_get; use function is_array; use function is_string; +use function random_bytes; use function time; -final class DbSessionHandler implements SessionHandlerInterface +/** + * Session handler backed by the session table. + * + * Both auxiliary session interfaces are implemented because PHP 8.6 warns + * when a SessionHandlerInterface implementation omits create_sid() and + * validateId(), which SessionHandlerInterface itself declares from PHP 9.0. + */ +final class DbSessionHandler implements + SessionHandlerInterface, + SessionIdInterface, + SessionUpdateTimestampHandlerInterface { private const string TIMESTAMP_FORMAT = 'Y-m-d H:i:s'; @@ -47,6 +62,16 @@ public function close(): bool return true; } + /** + * @throws RandomException + */ + // @mago-expect lint:method-name -- create_sid is mandated by SessionIdInterface. + #[Override] + public function create_sid(): string + { + return bin2hex(random_bytes(16)); + } + /** * @throws SqlInvalidArgumentException */ @@ -64,7 +89,8 @@ public function destroy(string $id): bool * @throws SqlInvalidArgumentException */ #[Override] - public function gc(int $maxLifetime): int|false + // @mago-expect lint:variable-name -- max_lifetime is mandated by SessionHandlerInterface::gc(). + public function gc(int $max_lifetime): int|false { $cutoff = date(format: self::TIMESTAMP_FORMAT); @@ -100,6 +126,27 @@ public function read(string $id): string|false return is_string($payload) ? $payload : false; } + /** + * @throws SqlInvalidArgumentException + */ + #[Override] + public function updateTimestamp(string $id, string $data): bool + { + return $this->write( + id : $id, + data: $data, + ); + } + + /** + * @throws SqlInvalidArgumentException + */ + #[Override] + public function validateId(string $id): bool + { + return false !== $this->read($id); + } + /** * @throws SqlInvalidArgumentException */ diff --git a/test/integration/FixtureLoader/PgsqlFixtureLoader.php b/test/integration/FixtureLoader/PgsqlFixtureLoader.php index 84243ca..5c2478a 100644 --- a/test/integration/FixtureLoader/PgsqlFixtureLoader.php +++ b/test/integration/FixtureLoader/PgsqlFixtureLoader.php @@ -6,28 +6,69 @@ use PhpDb\Adapter\AdapterInterface; use PhpDb\Pgsql; +use Throwable; use WebwareTestIntegration\PhpDb\Support\AdapterFactory; use function getenv; use function sprintf; +use function str_contains; +use function str_replace; +/** + * Ensures the pgsql test database exists, following the same shape as the + * mysql loader. + * + * CI provisions the database through the container environment (POSTGRES_DB), + * so it is already present when the suite starts, and this is a no-op. Locally + * it creates the database when missing. Either way the create path never drops + * anything. + * + * Postgres has no CREATE DATABASE IF NOT EXISTS, so existence is checked + * against pg_database first. + * + * Nothing here drops a database with sessions attached. Postgres refuses that + * where mysql allows it, so a drop-then-create cycle would need FORCE and + * therefore pg_signal_backend, a privilege a test role should not hold. + */ final class PgsqlFixtureLoader implements FixtureLoaderInterface { + private const string MAINTENANCE_DATABASE = 'postgres'; + public function createDatabase(): void { - $database = (string) getenv(name: 'TESTS_PHPDB_ADAPTER_PGSQL_DATABASE'); - $adapter = $this->adapter(); + if ($this->exists()) { + return; + } - $adapter->executeQuery(sql: sprintf('DROP DATABASE IF EXISTS "%s" WITH (FORCE)', $database)); - $adapter->executeQuery(sql: sprintf('CREATE DATABASE "%s"', $database)); + $this->adapter()->executeQuery( + sql: sprintf( + 'CREATE DATABASE "%s" OWNER "%s"', + $this->database(), + $this->username(), + ), + ); } public function dropDatabase(): void { - $database = (string) getenv(name: 'TESTS_PHPDB_ADAPTER_PGSQL_DATABASE'); - $adapter = $this->adapter(); - - $adapter->executeQuery(sql: sprintf('DROP DATABASE IF EXISTS "%s" WITH (FORCE)', $database)); + try { + $this->adapter()->executeQuery( + sql: sprintf('DROP DATABASE IF EXISTS "%s"', $this->database()), + ); + } catch (Throwable $e) { + // Postgres refuses to drop a database that has a session attached, + // and at teardown the suite's own connection is still open. A + // non-superuser role cannot terminate its own backend, so this is + // unreachable without pg_signal_backend. + // + // Tolerating exactly this case is safe because the outcome the drop + // exists to achieve is already guaranteed: CI uses a disposable + // container per leg, and each test creates and drops its own table, + // so no state carries between runs. + if (! str_contains($e->getMessage(), '55006')) { + throw $e; + } + } } private function adapter(): AdapterInterface @@ -38,12 +79,50 @@ private function adapter(): AdapterInterface 'connection' => [ 'hostname' => (string) getenv(name: 'TESTS_PHPDB_ADAPTER_PGSQL_HOSTNAME'), 'port' => (string) getenv(name: 'TESTS_PHPDB_ADAPTER_PGSQL_PORT'), - 'dbname' => 'postgres', - 'username' => (string) getenv(name: 'TESTS_PHPDB_ADAPTER_PGSQL_USERNAME'), + 'dbname' => self::MAINTENANCE_DATABASE, + 'username' => $this->username(), 'password' => (string) getenv(name: 'TESTS_PHPDB_ADAPTER_PGSQL_PASSWORD'), ], ], driverDependencies: new Pgsql\ConfigProvider()->getDependencies(), ); } + + private function database(): string + { + return (string) getenv(name: 'TESTS_PHPDB_ADAPTER_PGSQL_DATABASE'); + } + + private function exists(): bool + { + $rows = $this + ->adapter() + ->executeQuery( + sql: sprintf( + 'SELECT 1 FROM pg_database WHERE datname = %s', + $this->quotedDatabase(), + ), + ) + ->getQueryResult() + ->toArray(); + + return [] !== $rows; + } + + private function quotedDatabase(): string + { + return sprintf( + "'%s'", + str_replace( + search : "'", + replace: "''", + subject: $this->database(), + ), + ); + } + + private function username(): string + { + return (string) getenv(name: 'TESTS_PHPDB_ADAPTER_PGSQL_USERNAME'); + } } diff --git a/test/integration/FixtureLoader/SqliteFixtureLoader.php b/test/integration/FixtureLoader/SqliteFixtureLoader.php index 5ea5289..3f16491 100644 --- a/test/integration/FixtureLoader/SqliteFixtureLoader.php +++ b/test/integration/FixtureLoader/SqliteFixtureLoader.php @@ -4,16 +4,16 @@ namespace WebwareTestIntegration\PhpDb\FixtureLoader; -use function getenv; +use WebwareTestIntegration\PhpDb\SqliteDsn; + use function is_file; -use function str_replace; use function unlink; final class SqliteFixtureLoader implements FixtureLoaderInterface { public function createDatabase(): void { - $path = $this->databasePath(); + $path = SqliteDsn::path(); if (is_file(filename: $path)) { unlink(filename: $path); @@ -22,19 +22,10 @@ public function createDatabase(): void public function dropDatabase(): void { - $path = $this->databasePath(); + $path = SqliteDsn::path(); if (is_file(filename: $path)) { unlink(filename: $path); } } - - private function databasePath(): string - { - return str_replace( - search : 'sqlite:', - replace: '', - subject: (string) getenv(name: 'TESTS_PHPDB_ADAPTER_SQLITE_DSN'), - ); - } } diff --git a/test/integration/SchemaFactoryIntegrationTest.php b/test/integration/SchemaFactoryIntegrationTest.php index 9a8ec85..f1d9b18 100644 --- a/test/integration/SchemaFactoryIntegrationTest.php +++ b/test/integration/SchemaFactoryIntegrationTest.php @@ -123,7 +123,7 @@ public function itCreatesPrefixedTableOnSqlite(): void AdapterInterface::class => [ 'driver' => Sqlite\Pdo\Driver::class, 'connection' => [ - 'dsn' => (string) getenv(name: 'TESTS_PHPDB_ADAPTER_SQLITE_DSN'), + 'dsn' => SqliteDsn::resolve(), ], ], SchemaInterface::class => [ diff --git a/test/integration/Session/DbSessionHandlerTest.php b/test/integration/Session/DbSessionHandlerTest.php index 78033de..3e785d3 100644 --- a/test/integration/Session/DbSessionHandlerTest.php +++ b/test/integration/Session/DbSessionHandlerTest.php @@ -34,6 +34,8 @@ #[CoversMethod(DbSessionHandler::class, 'write')] #[CoversMethod(DbSessionHandler::class, 'destroy')] #[CoversMethod(DbSessionHandler::class, 'gc')] +#[CoversMethod(DbSessionHandler::class, 'updateTimestamp')] +#[CoversMethod(DbSessionHandler::class, 'validateId')] #[RequiresPhpExtension('pdo_mysql')] #[Group('integration')] #[Group('integration-mysql')] @@ -116,6 +118,36 @@ public function readReturnsWhatWasWritten(): void self::assertSame('user|s:4:"joey";', $handler->read('abc')); } + #[Test] + public function updateTimestampPersistsTheSessionAndExtendsTheExpiry(): void + { + $past = date( + format : 'Y-m-d H:i:s', + timestamp: time() - 60, + ); + $this->insertRow( + id : 'abc', + payload : 'data', + expiresAt: $past, + ); + + $handler = new DbSessionHandler($this->adapter); + + self::assertTrue($handler->updateTimestamp('abc', 'data')); + self::assertSame('data', $this->payloadOf('abc')); + self::assertGreaterThan($past, (string) $this->expiresAtOf('abc')); + } + + #[Test] + public function validateIdReportsWhetherTheSessionIsReadable(): void + { + $handler = new DbSessionHandler($this->adapter); + $handler->write('abc', 'data'); + + self::assertTrue($handler->validateId('abc')); + self::assertFalse($handler->validateId('missing')); + } + #[Test] public function writeExpiresTheSessionAfterTheConfiguredLifetime(): void { diff --git a/test/integration/SqliteDsn.php b/test/integration/SqliteDsn.php new file mode 100644 index 0000000..c708040 --- /dev/null +++ b/test/integration/SqliteDsn.php @@ -0,0 +1,37 @@ +handler()->create_sid(); + + self::assertMatchesRegularExpression('/^[0-9a-f]{32}$/', $id); + self::assertNotSame($id, $this->handler()->create_sid()); + } + + /** + * PHP 8.6 warns when a SessionHandlerInterface implementation leaves out + * create_sid() and validateId(); the warnings are fatal under phpunit's + * failOnWarning. Naming both interfaces keeps the class honest. + */ + #[Test] + public function implementsTheSessionIdAndTimestampInterfaces(): void + { + $handler = $this->handler(); + + self::assertInstanceOf(SessionHandlerInterface::class, $handler); + self::assertInstanceOf(SessionIdInterface::class, $handler); + self::assertInstanceOf(SessionUpdateTimestampHandlerInterface::class, $handler); + } + + #[Test] + public function validateIdIsDeclaredWithTheInterfaceReturnType(): void + { + $method = new ReflectionMethod(DbSessionHandler::class, 'validateId'); + + self::assertSame('bool', (string) $method->getReturnType()); + self::assertSame(1, $method->getNumberOfParameters()); + } + + private function handler(): DbSessionHandler + { + return new DbSessionHandler($this->createStub(AdapterInterface::class)); + } +} diff --git a/webware-ci.json b/webware-ci.json index b96b195..eb04e5f 100644 --- a/webware-ci.json +++ b/webware-ci.json @@ -8,7 +8,9 @@ "db_env_json": { "MYSQL_ROOT_HOST": "%", "MYSQL_ALLOW_EMPTY_PASSWORD": "true", - "MYSQL_DATABASE": "webware" + "MYSQL_DATABASE": "webware", + "MYSQL_USER": "webware", + "MYSQL_PASSWORD": "webware" }, "db_port": 3306, "db_health_cmd": "mysqladmin ping -h127.0.0.1 --silent",