From f565789e7dc0cabffe1d80147f3e969646ce298f Mon Sep 17 00:00:00 2001 From: Hideyuki MORI Date: Sat, 23 May 2026 01:55:14 +0900 Subject: [PATCH] feat(schema): SchemaDiffer + cli/schemaDiff.php + composer schema:diff (#419, ADR-0009 impl) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-0009 (#416) で Option C 採用、FT17 (#417) で実装した schema-diff CLI を framework に port。商用 feasibility report (#401) の "最大の実務 懸念" (migration 機構の欠如) に対する operator-applied resolution。 ### Code - class/xion/SchemaDiffer.php 新規 (pure-static、DB-agnostic) - diff(\$live, \$definition, \$driver): array{newTables, newColumns, warnings, inSync} - cli/schemaDiff.php 新規 (PDO + stdout/stderr 分離 CLI) - introspectMysql / introspectSqlite で driver 自動検出 - stdout = applied SQL only、stderr = annotation + warning - --dsn / --user / --pass / --help - class/xion/SchemaCompiler.php - 4 helper を private → public (mysqlCreateTable / sqliteCreateTable / mysqlColumn / sqliteColumn) - composer.json に "schema:diff" alias ### Tests tests/Unit/Xion/SchemaDifferTest.php 10 cases: - in-sync / new-table (MySQL+SQLite) / new-column (MySQL+SQLite) / bool default propagation / extra-column warning / extra-table warning / multi-warnings accumulate / inSync flag ### ADR cross-link docs/adr/0009-schema-migration-story.md の "Implementation tracking" section を更新: 本 PR # と F-5 follow-up issue 番号を明記。 ### Out of scope (per ADR-0009) - Drop / type-change / rename / constraint-change → warning のみ、SQL emit しない (operator hand-write) - Auto-apply (operator が常に実行する flow を維持) - Add-index path (本 PR に含めない、別 follow-up #421 で) ### Verification - composer test 138/138 (128 → 138、+10) - composer test:http 24/24 (1 expected skip) - composer analyze (Phan) exit 0 - composer format:check exit 0 - composer schema:diff -- --dsn=mysql:host=... で in-sync 確認、column drop → ALTER TABLE 出力、table drop → CREATE TABLE 出力 を live verify Closes #419. Co-Authored-By: Claude Opus 4.7 (1M context) --- class/xion/SchemaCompiler.php | 8 +- class/xion/SchemaDiffer.php | 126 +++++++++++++++ cli/schemaDiff.php | 191 ++++++++++++++++++++++ composer.json | 1 + docs/adr/0009-schema-migration-story.md | 6 +- tests/Unit/Xion/SchemaDifferTest.php | 202 ++++++++++++++++++++++++ 6 files changed, 528 insertions(+), 6 deletions(-) create mode 100644 class/xion/SchemaDiffer.php create mode 100644 cli/schemaDiff.php create mode 100644 tests/Unit/Xion/SchemaDifferTest.php diff --git a/class/xion/SchemaCompiler.php b/class/xion/SchemaCompiler.php index 325198c..d7f0132 100644 --- a/class/xion/SchemaCompiler.php +++ b/class/xion/SchemaCompiler.php @@ -100,7 +100,7 @@ public static function sqliteTriggerStatements(): array * @param string $name Table name. * @param array $table Table definition. */ - private static function mysqlCreateTable(string $name, array $table): string + public static function mysqlCreateTable(string $name, array $table): string { $lines = []; foreach ($table['columns'] as $columnName => $column) { @@ -133,7 +133,7 @@ private static function mysqlCreateTable(string $name, array $table): string * @param string $name Table name. * @param array $table Table definition. */ - private static function sqliteCreateTable(string $name, array $table): string + public static function sqliteCreateTable(string $name, array $table): string { $lines = []; foreach ($table['columns'] as $columnName => $column) { @@ -155,7 +155,7 @@ private static function sqliteCreateTable(string $name, array $table): string /** * @param array $column */ - private static function mysqlColumn(string $name, array $column): string + public static function mysqlColumn(string $name, array $column): string { $type = (string)($column['type'] ?? ''); return match (true) { @@ -173,7 +173,7 @@ private static function mysqlColumn(string $name, array $column): string /** * @param array $column */ - private static function sqliteColumn(string $name, array $column): string + public static function sqliteColumn(string $name, array $column): string { $type = (string)($column['type'] ?? ''); return match (true) { diff --git a/class/xion/SchemaDiffer.php b/class/xion/SchemaDiffer.php new file mode 100644 index 0000000..048a098 --- /dev/null +++ b/class/xion/SchemaDiffer.php @@ -0,0 +1,126 @@ + ['columns' => ['id' => [...], ...]], + * 'todos' => ['columns' => [...]], + * ] + * + * `$definitionTables` shape: `SchemaDefinition::tables()` output. + * + * @param array> $liveTables Introspected live schema. + * @param array> $definitionTables Source of truth. + * @param string $driver `mysql` or `sqlite`. + * + * @return array{ + * newTables: array, + * newColumns: array, + * warnings: array, + * inSync: bool + * } + */ + public static function diff(array $liveTables, array $definitionTables, string $driver): array + { + $newTables = []; + $newColumns = []; + $warnings = []; + + foreach ($definitionTables as $tableName => $tableSpec) { + if (!isset($liveTables[$tableName])) { + $newTables[$tableName] = self::createTableSql($tableName, $tableSpec, $driver); + continue; + } + $liveCols = $liveTables[$tableName]['columns'] ?? []; + $defCols = $tableSpec['columns'] ?? []; + foreach ($defCols as $colName => $colSpec) { + if (!isset($liveCols[$colName])) { + $newColumns[] = [ + 'table' => $tableName, + 'column' => $colName, + 'sql' => self::addColumnSql($tableName, $colName, $colSpec, $driver), + ]; + } + } + foreach (array_keys($liveCols) as $liveCol) { + if (!isset($defCols[$liveCol])) { + $warnings[] = sprintf( + 'column `%s.%s` exists in the live database but not in SchemaDefinition — drop SQL must be hand-written (ADR-0009 destructive-op rule).', + $tableName, + $liveCol + ); + } + } + } + foreach (array_keys($liveTables) as $liveTable) { + if (!isset($definitionTables[$liveTable])) { + $warnings[] = sprintf( + 'table `%s` exists in the live database but not in SchemaDefinition — drop SQL must be hand-written (ADR-0009 destructive-op rule).', + $liveTable + ); + } + } + + return [ + 'newTables' => $newTables, + 'newColumns' => $newColumns, + 'warnings' => $warnings, + 'inSync' => $newTables === [] && $newColumns === [], + ]; + } + + /** + * @param array $tableSpec + */ + private static function createTableSql(string $name, array $tableSpec, string $driver): string + { + return match ($driver) { + self::DRIVER_SQLITE => SchemaCompiler::sqliteCreateTable($name, $tableSpec) . ';', + default => SchemaCompiler::mysqlCreateTable($name, $tableSpec) . ';', + }; + } + + /** + * @param array $colSpec + */ + private static function addColumnSql(string $table, string $column, array $colSpec, string $driver): string + { + $columnDdl = $driver === self::DRIVER_SQLITE + ? SchemaCompiler::sqliteColumn($column, $colSpec) + : SchemaCompiler::mysqlColumn($column, $colSpec); + return 'ALTER TABLE ' . $table . ' ADD COLUMN ' . $columnDdl . ';'; + } +} diff --git a/cli/schemaDiff.php b/cli/schemaDiff.php new file mode 100644 index 0000000..7b7771b --- /dev/null +++ b/cli/schemaDiff.php @@ -0,0 +1,191 @@ + $options */ +$options = getopt('', ['dsn:', 'user:', 'pass:', 'help']) ?: []; + +if (isset($options['help'])) { + fwrite(STDOUT, file_get_contents(__FILE__) ? "See the docblock at the top of cli/schemaDiff.php for full usage.\n" : ''); + exit(0); +} + +$dsn = isset($options['dsn']) ? (string)$options['dsn'] : ''; +if ($dsn === '') { + fwrite(STDERR, "error: --dsn=… is required. Run `php cli/schemaDiff.php --help` for usage.\n"); + exit(1); +} + +$user = isset($options['user']) ? (string)$options['user'] : null; +$pass = isset($options['pass']) ? (string)$options['pass'] : null; + +try { + $pdo = new PDO($dsn, $user, $pass, [ + PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, + PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC, + ]); +} catch (PDOException $exception) { + fwrite(STDERR, 'error: database connection failed: ' . $exception->getMessage() . "\n"); + exit(2); +} + +$driver = $pdo->getAttribute(PDO::ATTR_DRIVER_NAME); +if ($driver !== SchemaDiffer::DRIVER_MYSQL && $driver !== SchemaDiffer::DRIVER_SQLITE) { + fwrite(STDERR, "error: unsupported driver `$driver`. Supported: mysql, sqlite.\n"); + exit(1); +} + +try { + $live = introspect($pdo, $driver); +} catch (PDOException $exception) { + fwrite(STDERR, 'error: schema introspection failed: ' . $exception->getMessage() . "\n"); + exit(2); +} + +$diff = SchemaDiffer::diff($live, SchemaDefinition::tables(), $driver); + +if ($diff['inSync']) { + fwrite(STDERR, "-- schema is in sync with SchemaDefinition (driver=$driver)\n"); + foreach ($diff['warnings'] as $warning) { + fwrite(STDERR, "-- warning: $warning\n"); + } + exit(0); +} + +fwrite(STDERR, "-- schema diff for driver=$driver (review before applying)\n"); +foreach ($diff['warnings'] as $warning) { + fwrite(STDERR, "-- warning: $warning\n"); +} +fwrite(STDOUT, "-- generated by cli/schemaDiff.php (ADR-0009)\n"); +fwrite(STDOUT, "-- driver: $driver\n"); +fwrite(STDOUT, "-- review every statement before applying; this tool only emits add-only changes.\n"); +foreach ($diff['newTables'] as $tableName => $sql) { + fwrite(STDOUT, "\n-- new table: $tableName\n"); + fwrite(STDOUT, $sql . "\n"); +} +foreach ($diff['newColumns'] as $entry) { + fwrite(STDOUT, "\n-- new column: {$entry['table']}.{$entry['column']}\n"); + fwrite(STDOUT, $entry['sql'] . "\n"); +} + +exit(0); + +/** + * @return array> + */ +function introspect(PDO $pdo, string $driver): array +{ + if ($driver === SchemaDiffer::DRIVER_SQLITE) { + return introspectSqlite($pdo); + } + return introspectMysql($pdo); +} + +/** + * @return array> + */ +function introspectMysql(PDO $pdo): array +{ + $database = (string)$pdo->query('SELECT DATABASE()')->fetchColumn(); + if ($database === '') { + throw new RuntimeException('MySQL DSN did not specify a database (no DATABASE() in session).'); + } + + $tableStmt = $pdo->prepare( + 'SELECT TABLE_NAME FROM INFORMATION_SCHEMA.TABLES ' + . 'WHERE TABLE_SCHEMA = :schema AND TABLE_TYPE = "BASE TABLE"' + ); + $tableStmt->execute([':schema' => $database]); + /** @var array $tables */ + $tables = $tableStmt->fetchAll(PDO::FETCH_COLUMN); + + $columnStmt = $pdo->prepare( + 'SELECT COLUMN_NAME, DATA_TYPE FROM INFORMATION_SCHEMA.COLUMNS ' + . 'WHERE TABLE_SCHEMA = :schema AND TABLE_NAME = :table' + ); + + $result = []; + foreach ($tables as $table) { + $columnStmt->execute([':schema' => $database, ':table' => $table]); + $columns = []; + foreach ($columnStmt->fetchAll() as $row) { + $name = (string)$row['COLUMN_NAME']; + $columns[$name] = ['data_type' => (string)$row['DATA_TYPE']]; + } + $result[$table] = ['columns' => $columns]; + } + return $result; +} + +/** + * @return array> + */ +function introspectSqlite(PDO $pdo): array +{ + $tableStmt = $pdo->query('SELECT name FROM sqlite_master WHERE type = "table" AND name NOT LIKE "sqlite_%"'); + /** @var array $tables */ + $tables = $tableStmt->fetchAll(PDO::FETCH_COLUMN); + + $result = []; + foreach ($tables as $table) { + $columnStmt = $pdo->query('PRAGMA table_info(' . $table . ')'); + $columns = []; + foreach ($columnStmt->fetchAll() as $row) { + $name = (string)$row['name']; + $columns[$name] = ['data_type' => (string)$row['type']]; + } + $result[$table] = ['columns' => $columns]; + } + return $result; +} diff --git a/composer.json b/composer.json index cac7e33..306df62 100644 --- a/composer.json +++ b/composer.json @@ -49,6 +49,7 @@ "setup": "@php cli/setupDatabase.php --env=.env --yes", "schema:generate": "@php cli/generateSchemaSql.php", "schema:check": "@php cli/generateSchemaSql.php --check", + "schema:diff": "@php cli/schemaDiff.php", "test": "phpunit --testsuite unit", "test:http": [ "@php tools/test-http-preflight.php", diff --git a/docs/adr/0009-schema-migration-story.md b/docs/adr/0009-schema-migration-story.md index 7d64f64..7550927 100644 --- a/docs/adr/0009-schema-migration-story.md +++ b/docs/adr/0009-schema-migration-story.md @@ -82,6 +82,8 @@ Neutral: ## Implementation tracking -- This ADR is **drafted in parallel with #409** (the eval-report-derived issue) but the **implementation lands as a separate trial / PR**, not in this ADR's branch. The next FT (FT17 candidate: "schema-diff CLI") picks up the implementation when the trial-clone bandwidth is available. +- **Implemented by FT17** (`docs/field-trials/2026-05-field-trial-17.md`, Trial Issue #417). The implementation PR is #419 (feat) — adds `Nene\Xion\SchemaDiffer`, `cli/schemaDiff.php`, promotes four `SchemaCompiler` helpers to `public static`, and ships `composer schema:diff` as the operator-facing entry point. +- The docs / workflow side ships in #420 — `docs/development/schema-migrations.md`. +- The **add-index path** is the only ADR-0009 scope item not in #419; it ships as the small follow-up Issue #421 (no new trial needed). - A future ADR may extend this surface to (a) include Phinx as an optional dep, or (b) move to Option B (auto-generated `ALTER TABLE` via versioned `SchemaDefinition`) if the operator-side friction surfaces again. -- ADR-0005's Consequences section gets a cross-link to ADR-0009 ("Schema migrations: addressed by ADR-0009"). +- ADR-0005's Consequences section already cross-links to this ADR ("Schema migrations: addressed by ADR-0009"). diff --git a/tests/Unit/Xion/SchemaDifferTest.php b/tests/Unit/Xion/SchemaDifferTest.php new file mode 100644 index 0000000..6ccad12 --- /dev/null +++ b/tests/Unit/Xion/SchemaDifferTest.php @@ -0,0 +1,202 @@ + ['columns' => [ + 'id' => ['type' => 'pk-bigint'], + 'title' => ['type' => 'varchar:255'], + ]], + ]; + $live = [ + 'todos' => ['columns' => [ + 'id' => ['data_type' => 'bigint'], + 'title' => ['data_type' => 'varchar'], + ]], + ]; + + $diff = SchemaDiffer::diff($live, $tables, SchemaDiffer::DRIVER_MYSQL); + + self::assertTrue($diff['inSync']); + self::assertSame([], $diff['newTables']); + self::assertSame([], $diff['newColumns']); + self::assertSame([], $diff['warnings']); + } + + public function testNewTableEmitsMysqlCreateTable(): void + { + $tables = [ + 'audit_log' => ['columns' => [ + 'id' => ['type' => 'pk-bigint'], + 'created_at' => ['type' => 'datetime-now'], + 'message' => ['type' => 'text'], + ]], + ]; + $diff = SchemaDiffer::diff([], $tables, SchemaDiffer::DRIVER_MYSQL); + + self::assertFalse($diff['inSync']); + self::assertArrayHasKey('audit_log', $diff['newTables']); + self::assertStringContainsString('CREATE TABLE IF NOT EXISTS audit_log', $diff['newTables']['audit_log']); + self::assertStringContainsString('ENGINE=InnoDB', $diff['newTables']['audit_log']); + self::assertStringEndsWith(';', $diff['newTables']['audit_log']); + } + + public function testNewTableEmitsSqliteCreateTable(): void + { + $tables = [ + 'audit_log' => ['columns' => [ + 'id' => ['type' => 'pk-bigint'], + 'message' => ['type' => 'text'], + ]], + ]; + $diff = SchemaDiffer::diff([], $tables, SchemaDiffer::DRIVER_SQLITE); + + self::assertStringContainsString('CREATE TABLE IF NOT EXISTS audit_log', $diff['newTables']['audit_log']); + self::assertStringContainsString('INTEGER PRIMARY KEY AUTOINCREMENT', $diff['newTables']['audit_log']); + self::assertStringNotContainsString('ENGINE=InnoDB', $diff['newTables']['audit_log']); + } + + public function testNewColumnEmitsMysqlAlterTable(): void + { + $tables = [ + 'todos' => ['columns' => [ + 'id' => ['type' => 'pk-bigint'], + 'is_completed' => ['type' => 'bool', 'default' => 0], + ]], + ]; + $live = [ + 'todos' => ['columns' => [ + 'id' => ['data_type' => 'bigint'], + ]], + ]; + $diff = SchemaDiffer::diff($live, $tables, SchemaDiffer::DRIVER_MYSQL); + + self::assertCount(1, $diff['newColumns']); + $entry = $diff['newColumns'][0]; + self::assertSame('todos', $entry['table']); + self::assertSame('is_completed', $entry['column']); + self::assertSame( + 'ALTER TABLE todos ADD COLUMN is_completed TINYINT(1) NOT NULL DEFAULT 0;', + $entry['sql'] + ); + } + + public function testNewColumnEmitsSqliteAlterTable(): void + { + $tables = [ + 'todos' => ['columns' => [ + 'id' => ['type' => 'pk-bigint'], + 'title' => ['type' => 'varchar:255'], + ]], + ]; + $live = [ + 'todos' => ['columns' => [ + 'id' => ['data_type' => 'INTEGER'], + ]], + ]; + $diff = SchemaDiffer::diff($live, $tables, SchemaDiffer::DRIVER_SQLITE); + + self::assertCount(1, $diff['newColumns']); + self::assertSame( + 'ALTER TABLE todos ADD COLUMN title TEXT NOT NULL;', + $diff['newColumns'][0]['sql'] + ); + } + + public function testBoolDefaultPropagatesToNewColumn(): void + { + // Regression: the `default` key on a bool column must be + // reflected in the ALTER TABLE — operators who add a new bool + // with default=1 must not get a NULL-after-default-0 surprise. + $tables = [ + 'todos' => ['columns' => [ + 'id' => ['type' => 'pk-bigint'], + 'is_archived' => ['type' => 'bool', 'default' => 1], + ]], + ]; + $live = [ + 'todos' => ['columns' => ['id' => ['data_type' => 'bigint']]], + ]; + $diff = SchemaDiffer::diff($live, $tables, SchemaDiffer::DRIVER_MYSQL); + + self::assertSame( + 'ALTER TABLE todos ADD COLUMN is_archived TINYINT(1) NOT NULL DEFAULT 1;', + $diff['newColumns'][0]['sql'] + ); + } + + public function testExtraColumnInLiveEmitsWarningNotSql(): void + { + $tables = [ + 'users' => ['columns' => [ + 'id' => ['type' => 'pk-bigint'], + ]], + ]; + $live = [ + 'users' => ['columns' => [ + 'id' => ['data_type' => 'bigint'], + 'legacy_phone' => ['data_type' => 'varchar'], + ]], + ]; + $diff = SchemaDiffer::diff($live, $tables, SchemaDiffer::DRIVER_MYSQL); + + self::assertTrue($diff['inSync']); + self::assertCount(1, $diff['warnings']); + self::assertStringContainsString('users.legacy_phone', $diff['warnings'][0]); + self::assertStringContainsString('hand-written', $diff['warnings'][0]); + } + + public function testExtraTableInLiveEmitsWarningNotSql(): void + { + $tables = []; + $live = [ + 'legacy_audit' => ['columns' => ['id' => ['data_type' => 'bigint']]], + ]; + $diff = SchemaDiffer::diff($live, $tables, SchemaDiffer::DRIVER_MYSQL); + + self::assertTrue($diff['inSync']); + self::assertCount(1, $diff['warnings']); + self::assertStringContainsString('table `legacy_audit`', $diff['warnings'][0]); + } + + public function testMultipleWarningsAccumulate(): void + { + $tables = [ + 'users' => ['columns' => ['id' => ['type' => 'pk-bigint']]], + ]; + $live = [ + 'users' => ['columns' => [ + 'id' => ['data_type' => 'bigint'], + 'extra1' => ['data_type' => 'varchar'], + 'extra2' => ['data_type' => 'varchar'], + ]], + 'orphan' => ['columns' => ['id' => ['data_type' => 'bigint']]], + ]; + $diff = SchemaDiffer::diff($live, $tables, SchemaDiffer::DRIVER_MYSQL); + + self::assertCount(3, $diff['warnings']); + } + + public function testInSyncFlagIsFalseWhenAnyAddIsPresent(): void + { + $tables = ['todos' => ['columns' => ['id' => ['type' => 'pk-bigint']]]]; + $live = []; + $diff = SchemaDiffer::diff($live, $tables, SchemaDiffer::DRIVER_MYSQL); + self::assertFalse($diff['inSync']); + } +}