From 2329e295a9df98c833463c979327bd00748197c7 Mon Sep 17 00:00:00 2001 From: Greg Bowler Date: Thu, 2 Apr 2026 09:13:43 +0100 Subject: [PATCH 1/6] feature: implement DevMigrator closes #283 --- composer.json | 1 - src/Cli/ExecuteCommand.php | 142 ++++++++++- src/Migration/DevMigrator.php | 267 +++++++++++++++++++++ src/Migration/Migrator.php | 4 + test/phpunit/Cli/ExecuteCommandTest.php | 161 +++++++++++-- test/phpunit/Migration/DevMigratorTest.php | 118 +++++++++ 6 files changed, 667 insertions(+), 26 deletions(-) create mode 100644 src/Migration/DevMigrator.php create mode 100644 test/phpunit/Migration/DevMigratorTest.php diff --git a/composer.json b/composer.json index a81ef3c..416114c 100644 --- a/composer.json +++ b/composer.json @@ -51,7 +51,6 @@ "scripts": { "phpunit": "vendor/bin/phpunit --configuration phpunit.xml", - "phpunit:coverage": "XDEBUG_MODE=coverage vendor/bin/phpunit --configuration phpunit.xml --coverage-text", "phpstan": "vendor/bin/phpstan analyse --level 6 --memory-limit 1G src", "phpcs": "vendor/bin/phpcs src --standard=phpcs.xml", "phpmd": "vendor/bin/phpmd src/ text phpmd.xml", diff --git a/src/Cli/ExecuteCommand.php b/src/Cli/ExecuteCommand.php index dfdb210..d98288d 100644 --- a/src/Cli/ExecuteCommand.php +++ b/src/Cli/ExecuteCommand.php @@ -4,8 +4,10 @@ use Gt\Cli\Argument\ArgumentValueList; use Gt\Cli\Command\Command; use Gt\Cli\Parameter\Parameter; +use Gt\Config\Config; use Gt\Config\ConfigFactory; use Gt\Database\Connection\Settings; +use Gt\Database\Migration\DevMigrator; use Gt\Database\Migration\MigrationIntegrityException; use Gt\Database\Migration\Migrator; use Gt\Database\StatementExecutionException; @@ -19,6 +21,7 @@ public function run(?ArgumentValueList $arguments = null):void { $settings = $this->buildSettingsFromConfig($config, $repoBasePath, $arguments); [$migrationPath, $migrationTable] = $this->getMigrationLocation($config, $repoBasePath, $arguments); + [$devMigrationPath, $devMigrationTable] = $this->getDevMigrationLocation($config, $repoBasePath, $arguments); $migrator = new Migrator($settings, $migrationPath, $migrationTable); $migrator->setOutput( @@ -38,6 +41,15 @@ public function run(?ArgumentValueList $arguments = null):void { $runFrom = $this->calculateResetNumber($arguments, $migrationFileList, $migrator, $migrationCount); $this->executeMigrations($migrator, $migrationFileList, $runFrom); + + if($this->isDevMerge($arguments)) { + $this->mergeDevMigrations($settings, $migrator, $migrationPath, $devMigrationPath, $devMigrationTable); + return; + } + + if($this->isDev($arguments)) { + $this->executeDevMigrations($settings, $devMigrationPath, $devMigrationTable); + } } /** Determine whether the --force flag was provided. */ @@ -45,9 +57,17 @@ private function isForced(?ArgumentValueList $arguments):bool { return $arguments?->contains("force") ?? false; } + private function isDev(?ArgumentValueList $arguments):bool { + return $arguments?->contains("dev") ?? false; + } + + private function isDevMerge(?ArgumentValueList $arguments):bool { + return $arguments?->contains("dev-merge") ?? false; + } + /** Build Settings from config for the current repository. */ protected function buildSettingsFromConfig( - \Gt\Config\Config $config, + Config $config, string $repoBasePath, ?ArgumentValueList $arguments = null ): Settings { @@ -110,7 +130,7 @@ protected function buildSettingsFromConfig( * @return list */ protected function getMigrationLocation( - \Gt\Config\Config $config, + Config $config, string $repoBasePath, ?ArgumentValueList $arguments = null ): array { @@ -129,6 +149,34 @@ protected function getMigrationLocation( return [$migrationPath, $migrationTable]; } + /** + * Return [devMigrationPath, devMigrationTable] derived from config. + * + * @return list + */ + protected function getDevMigrationLocation( + Config $config, + string $repoBasePath, + ?ArgumentValueList $arguments = null + ): array { + $queryPath = $this->getOverrideOrConfigValue( + $config, + $arguments, + "base-directory", + "database.query_path", + "query" + ); + $devMigrationPath = implode(DIRECTORY_SEPARATOR, [ + $this->resolvePath($repoBasePath, $queryPath), + $config->get("database.dev_migration_path") ?? implode(DIRECTORY_SEPARATOR, [ + "_migration", + "dev", + ]), + ]); + $devMigrationTable = $config->get("database.dev_migration_table") ?? "_migration_dev"; + return [$devMigrationPath, $devMigrationTable]; + } + /** * Calculate the migration start point from --reset or current migration count. * @@ -158,7 +206,11 @@ private function calculateResetNumber( * * @param list $migrationFileList */ - private function executeMigrations(Migrator $migrator, array $migrationFileList, int $runFrom): void { + private function executeMigrations( + Migrator $migrator, + array $migrationFileList, + int $runFrom, + ): void { try { $migrator->checkIntegrity($migrationFileList, $runFrom); $migrator->performMigration($migrationFileList, $runFrom); @@ -180,6 +232,72 @@ private function executeMigrations(Migrator $migrator, array $migrationFileList, } } + private function executeDevMigrations( + Settings $settings, + string $devMigrationPath, + string $devMigrationTable + ): void { + $devMigrator = new DevMigrator( + $settings, + $devMigrationPath, + $devMigrationTable, + ); + $devMigrator->setOutput( + $this->stream->getOutStream(), + $this->stream->getErrorStream() + ); + + $devMigrator->createMigrationTable(); + $devMigrationFileList = $devMigrator->getMigrationFileList(); + + try { + $devMigrator->checkFileListOrder($devMigrationFileList); + $devMigrator->checkIntegrity($devMigrationFileList); + $devMigrator->performMigration($devMigrationFileList); + } + catch(MigrationIntegrityException $exception) { + $this->writeLine( + "There was an integrity error migrating dev file '" + . $exception->getMessage() + . "' - this dev migration is recorded to have been run already, " + . "but the contents of the file has changed." + ); + } + catch(StatementPreparationException|StatementExecutionException $exception) { + $this->writeLine( + "There was an error executing dev migration file: " + . $exception->getMessage() + . "'" + ); + } + } + + private function mergeDevMigrations( + Settings $settings, + Migrator $migrator, + string $migrationPath, + string $devMigrationPath, + string $devMigrationTable + ): void { + $devMigrator = new DevMigrator($settings, $devMigrationPath, $devMigrationTable); + $devMigrator->setOutput( + $this->stream->getOutStream(), + $this->stream->getErrorStream() + ); + $devMigrator->createMigrationTable(); + + try { + $devMigrator->mergeIntoMainMigrationDirectory($migrator, $migrationPath); + } + catch(MigrationIntegrityException $exception) { + $this->writeLine( + "There was an integrity error merging dev migration file '" + . $exception->getMessage() + . "' - ensure the dev migration has already been run and not edited since." + ); + } + } + public function getName():string { return "execute"; } @@ -244,6 +362,18 @@ public function getOptionalParameterList():array { null, "Override database.password for this command" ), + new Parameter( + false, + "dev", + null, + "Run branch-local migrations from the dev migration directory" + ), + new Parameter( + false, + "dev-merge", + null, + "Promote branch-local dev migrations into canonical migrations" + ), new Parameter( false, "force", @@ -280,9 +410,9 @@ private function getDefaultPath(string $repoBasePath):?string { /** * @param bool|string $repoBasePath * @param string|null $defaultPath - * @return \Gt\Config\Config + * @return Config */ - protected function getConfig(bool|string $repoBasePath, ?string $defaultPath):\Gt\Config\Config { + protected function getConfig(bool|string $repoBasePath, ?string $defaultPath):Config { $config = ConfigFactory::createForProject($repoBasePath); $default = $defaultPath @@ -296,7 +426,7 @@ protected function getConfig(bool|string $repoBasePath, ?string $defaultPath):\G } protected function getOverrideOrConfigValue( - \Gt\Config\Config $config, + Config $config, ?ArgumentValueList $arguments, string $argumentKey, string $configKey, diff --git a/src/Migration/DevMigrator.php b/src/Migration/DevMigrator.php new file mode 100644 index 0000000..0592548 --- /dev/null +++ b/src/Migration/DevMigrator.php @@ -0,0 +1,267 @@ +driver = $settings->getDriver(); + $this->path = $path; + $this->tableName = $tableName; + + if($this->driver !== Settings::DRIVER_SQLITE) { + $settings = $settings->withoutSchema(); + } + + $this->dbClient = new Database($settings); + } + + public function setOutput( + SplFileObject $out, + ?SplFileObject $error = null + ):void { + $this->streamOut = $out; + $this->streamError = $error; + } + + public function createMigrationTable():void { + $this->dbClient->executeSql(implode("\n", [ + "create table if not exists `{$this->tableName}` (", + "`" . self::COLUMN_FILE_NAME . "` varchar(255) primary key,", + "`" . self::COLUMN_QUERY_HASH . "` varchar(32) not null,", + "`" . self::COLUMN_MIGRATED_AT . "` datetime not null )", + ])); + } + + /** @return array */ + public function getMigrationFileList():array { + if(!is_dir($this->path)) { + return []; + } + + $fileList = glob("$this->path/*.sql"); + sort($fileList); + return $fileList; + } + + /** @param array $fileList */ + public function checkFileListOrder(array $fileList):void { + $previousNumber = null; + + foreach($fileList as $file) { + $migrationNumber = $this->extractNumberFromFilename($file); + + if(!is_null($previousNumber)) { + if($migrationNumber === $previousNumber) { + throw new MigrationSequenceOrderException("Duplicate: $migrationNumber"); + } + if($migrationNumber < $previousNumber) { + throw new MigrationSequenceOrderException("Out of order: $migrationNumber before $previousNumber"); + } + } + + $previousNumber = $migrationNumber; + } + } + + /** @param array $migrationFileList */ + public function checkIntegrity(array $migrationFileList):void { + foreach($migrationFileList as $file) { + $fileName = basename($file); + $md5 = md5_file($file); + $hashInDb = $this->getStoredHash($fileName); + + if($hashInDb && $hashInDb !== $md5) { + throw new MigrationIntegrityException($file); + } + } + } + + public function extractNumberFromFilename(string $pathName):int { + $file = new SplFileInfo($pathName); + $filename = $file->getFilename(); + preg_match("/(\d+)-?.*\.sql/", $filename, $matches); + + if(!isset($matches[1])) { + throw new MigrationFileNameFormatException($filename); + } + + return (int)$matches[1]; + } + + /** @param array $migrationFileList */ + public function performMigration(array $migrationFileList):int { + $numCompleted = 0; + $sqlStatementSplitter = new SqlStatementSplitter(); + + foreach($migrationFileList as $file) { + $fileName = basename($file); + if($this->hasMigrationBeenApplied($fileName)) { + continue; + } + + $fileNumber = $this->extractNumberFromFilename($file); + $this->output("Dev migration $fileNumber: `$file`."); + $md5 = md5_file($file); + + foreach($sqlStatementSplitter->split(file_get_contents($file)) as $sql) { + $this->dbClient->executeSql($sql); + } + + $this->recordMigrationSuccess($fileName, $md5); + $numCompleted++; + } + + if($numCompleted === 0) { + $this->output("Dev migrations are already up to date."); + } + else { + $this->output("$numCompleted dev migrations were completed successfully."); + } + + return $numCompleted; + } + + public function mergeIntoMainMigrationDirectory( + Migrator $migrator, + string $targetPath + ):int { + $migrationFileList = $this->getMigrationFileList(); + $this->checkFileListOrder($migrationFileList); + $this->checkIntegrity($migrationFileList); + + if(empty($migrationFileList)) { + $this->output("No dev migrations to merge."); + return 0; + } + + if(!is_dir($targetPath)) { + mkdir($targetPath, 0775, true); + } + + $nextNumber = $this->getNextMainMigrationNumber($migrator); + $numMerged = 0; + + foreach($migrationFileList as $file) { + $fileName = basename($file); + $storedHash = $this->getStoredHash($fileName); + if(!$storedHash) { + throw new MigrationIntegrityException($file); + } + + $targetName = $this->createMergedFilename($fileName, $nextNumber); + $targetFile = implode(DIRECTORY_SEPARATOR, [$targetPath, $targetName]); + if(file_exists($targetFile)) { + throw new MigrationSequenceOrderException("Duplicate: $targetName"); + } + + rename($file, $targetFile); + $migrator->markMigrationApplied($nextNumber, $storedHash); + $this->deleteMigrationRecord($fileName); + $this->output("Merged dev migration `$fileName` to `$targetName`."); + $nextNumber++; + $numMerged++; + } + + $this->output("$numMerged dev migrations were merged successfully."); + return $numMerged; + } + + protected function hasMigrationBeenApplied(string $fileName):bool { + return (bool)$this->getStoredHash($fileName); + } + + protected function getStoredHash(string $fileName):?string { + $result = $this->dbClient->executeSql(implode("\n", [ + "select `" . self::COLUMN_QUERY_HASH . "`", + "from `{$this->tableName}`", + "where `" . self::COLUMN_FILE_NAME . "` = ?", + "limit 1", + ]), [$fileName]); + + return ($result->fetch())?->getString(self::COLUMN_QUERY_HASH); + } + + protected function getNextMainMigrationNumber(Migrator $migrator):int { + $currentHighest = $migrator->getMigrationCount(); + $mainMigrationFileList = $migrator->getMigrationFileList(); + + foreach($mainMigrationFileList as $file) { + $currentHighest = max( + $currentHighest, + $migrator->extractNumberFromFilename($file) + ); + } + + return $currentHighest + 1; + } + + protected function createMergedFilename(string $fileName, int $number):string { + preg_match("/^\d+(-?.*\.sql)$/", $fileName, $matches); + $suffix = $matches[1] ?? ".sql"; + return str_pad((string)$number, 4, "0", STR_PAD_LEFT) . $suffix; + } + + protected function recordMigrationSuccess(string $fileName, string $hash):void { + $now = "now()"; + + if($this->driver === Settings::DRIVER_SQLITE) { + $now = "datetime('now')"; + } + + $this->dbClient->executeSql(implode("\n", [ + "insert into `{$this->tableName}` (", + "`" . self::COLUMN_FILE_NAME . "`, ", + "`" . self::COLUMN_QUERY_HASH . "`, ", + "`" . self::COLUMN_MIGRATED_AT . "` ", + ") values (", + "?, ?, $now", + ")", + ]), [$fileName, $hash]); + } + + protected function deleteMigrationRecord(string $fileName):void { + $this->dbClient->executeSql(implode("\n", [ + "delete from `{$this->tableName}`", + "where `" . self::COLUMN_FILE_NAME . "` = ?", + ]), [$fileName]); + } + + protected function output( + string $message, + string $streamName = self::STREAM_OUT + ):void { + $stream = $this->streamOut ?? null; + if($streamName === self::STREAM_ERROR) { + $stream = $this->streamError; + } + + if(is_null($stream)) { + return; + } + + $stream->fwrite($message . PHP_EOL); + } +} diff --git a/src/Migration/Migrator.php b/src/Migration/Migrator.php index 938ad9a..a0a26ee 100644 --- a/src/Migration/Migrator.php +++ b/src/Migration/Migrator.php @@ -273,6 +273,10 @@ protected function recordMigrationSuccess(int $number, ?string $hash):void { ]), [$number, $hash]); } + public function markMigrationApplied(int $number, string $hash):void { + $this->recordMigrationSuccess($number, $hash); + } + /** * @param int $numberToForce A null-hashed migration will be marked as * successful with this number. This will allow the next number to be diff --git a/test/phpunit/Cli/ExecuteCommandTest.php b/test/phpunit/Cli/ExecuteCommandTest.php index 14a8388..614fbc7 100644 --- a/test/phpunit/Cli/ExecuteCommandTest.php +++ b/test/phpunit/Cli/ExecuteCommandTest.php @@ -1,4 +1,4 @@ -getConfig($repoBasePath, $defaultPath); } }; @@ -157,7 +181,7 @@ public function testExecuteWithResetWithoutNumber():void { $project = $this->createProjectDir(); $sqlitePath = str_replace("\\", "/", $project . DIRECTORY_SEPARATOR . "cli-test.db"); $this->writeConfigIni($project, $sqlitePath); - $migrations = $this->createMigrations($project, 4); + $this->createMigrations($project, 4); // Prepare base state: create table so we can skip first migration safely. $settings = new Settings($project . DIRECTORY_SEPARATOR . "query", Settings::DRIVER_SQLITE, $sqlitePath); @@ -191,6 +215,7 @@ public function testExecuteWithResetWithNumber():void { $sqlitePath = str_replace("\\", "/", $project . DIRECTORY_SEPARATOR . "cli-test.db"); $this->writeConfigIni($project, $sqlitePath); $migrations = $this->createMigrations($project, 5); + self::assertCount(5, $migrations); // Prepare base state when skipping the initial migrations. $settings = new Settings($project . DIRECTORY_SEPARATOR . "query", Settings::DRIVER_SQLITE, $sqlitePath); @@ -236,10 +261,81 @@ public function testOptionalParameterListContainsCliOverrides():void { self::assertContains("port", $parameterNames); self::assertContains("username", $parameterNames); self::assertContains("password", $parameterNames); + self::assertContains("dev", $parameterNames); + self::assertContains("dev-merge", $parameterNames); self::assertContains("force", $parameterNames); self::assertContains("reset", $parameterNames); } + public function testExecuteWithDevRunsCanonicalAndDevMigrations():void { + $project = $this->createProjectDir(); + $sqlitePath = str_replace("\\", "/", $project . DIRECTORY_SEPARATOR . "cli-test.db"); + $this->writeConfigIni($project, $sqlitePath); + $this->createMigrations($project, 1); + $this->createDevMigrations($project, 2); + + $cwdBackup = getcwd(); + chdir($project); + try { + $cmd = new ExecuteCommand(); + $streams = $this->makeStreamFiles(); + $cmd->setStream($streams["stream"]); + + $args = new ArgumentValueList(); + $args->set("dev"); + $cmd->run($args); + + list("out" => $out) = $this->readFromFiles($streams["out"], $streams["err"]); + self::assertStringContainsString("1 migrations were completed successfully.", $out); + self::assertStringContainsString("2 dev migrations were completed successfully.", $out); + + $settings = new Settings($project . DIRECTORY_SEPARATOR . "query", Settings::DRIVER_SQLITE, $sqlitePath); + $db = new Database($settings); + $result = $db->executeSql("PRAGMA table_info(test);"); + self::assertCount(4, $result->fetchAll()); + } + finally { + chdir($cwdBackup); + } + } + + public function testExecuteWithDevMergePromotesDevMigrations():void { + $project = $this->createProjectDir(); + $sqlitePath = str_replace("\\", "/", $project . DIRECTORY_SEPARATOR . "cli-test.db"); + $this->writeConfigIni($project, $sqlitePath); + $this->createMigrations($project, 1); + $devFiles = $this->createDevMigrations($project, 2); + + $cwdBackup = getcwd(); + chdir($project); + try { + $cmd = new ExecuteCommand(); + $streams = $this->makeStreamFiles(); + $cmd->setStream($streams["stream"]); + + $args = new ArgumentValueList(); + $args->set("dev"); + $cmd->run($args); + + $mergeStreams = $this->makeStreamFiles(); + $cmd->setStream($mergeStreams["stream"]); + $mergeArgs = new ArgumentValueList(); + $mergeArgs->set("dev-merge"); + $cmd->run($mergeArgs); + + list("out" => $out) = $this->readFromFiles($mergeStreams["out"], $mergeStreams["err"]); + self::assertStringContainsString("Merged dev migration", $out); + self::assertStringContainsString("2 dev migrations were merged successfully.", $out); + self::assertFileDoesNotExist($devFiles[0]); + self::assertFileDoesNotExist($devFiles[1]); + $mergedName = preg_replace("/^\d+/", "0002", basename($devFiles[0])); + self::assertFileExists($project . DIRECTORY_SEPARATOR . "query" . DIRECTORY_SEPARATOR . "_migration" . DIRECTORY_SEPARATOR . $mergedName); + } + finally { + chdir($cwdBackup); + } + } + public function testCliArgumentsOverrideConfigValuesWhenBuildingSettings():void { $repoBasePath = "/tmp/project-root"; $config = new Config( @@ -295,13 +391,32 @@ public function testBaseDirectoryOverrideIsUsedForMigrationLocation():void { self::assertSame("migration_log", $migrationTable); } + public function testBaseDirectoryOverrideIsUsedForDevMigrationLocation():void { + $repoBasePath = "/tmp/project-root"; + $config = new Config( + new ConfigSection("database", [ + "query_path" => "query", + "dev_migration_path" => "_migration/dev", + "dev_migration_table" => "migration_dev_log", + ]) + ); + $args = new ArgumentValueList(); + $args->set("base-directory", "alt-query"); + + $command = $this->createCommandProbe(); + [$migrationPath, $migrationTable] = $command->getDevMigrationLocationForTest($config, $repoBasePath, $args); + + self::assertSame("/tmp/project-root/alt-query/_migration/dev", $migrationPath); + self::assertSame("migration_dev_log", $migrationTable); + } + private function createCommandProbe():ExecuteCommand { return new class extends ExecuteCommand { public function buildSettingsForTest( Config $config, string $repoBasePath, ?ArgumentValueList $arguments = null - ): Settings { + ):Settings { return $this->buildSettingsFromConfig($config, $repoBasePath, $arguments); } @@ -310,10 +425,18 @@ public function getMigrationLocationForTest( Config $config, string $repoBasePath, ?ArgumentValueList $arguments = null - ): array { + ):array { return $this->getMigrationLocation($config, $repoBasePath, $arguments); } + + /** @return list */ + public function getDevMigrationLocationForTest( + Config $config, + string $repoBasePath, + ?ArgumentValueList $arguments = null + ):array { + return $this->getDevMigrationLocation($config, $repoBasePath, $arguments); + } }; } - } diff --git a/test/phpunit/Migration/DevMigratorTest.php b/test/phpunit/Migration/DevMigratorTest.php new file mode 100644 index 0000000..c571544 --- /dev/null +++ b/test/phpunit/Migration/DevMigratorTest.php @@ -0,0 +1,118 @@ + */ + private function createMigrationFiles(string $path, string $prefix, int $count):array { + mkdir($path, 0775, true); + $fileList = []; + + for($i = 1; $i <= $count; $i++) { + $fileName = str_pad((string)$i, 3, "0", STR_PAD_LEFT) . "-$prefix-$i.sql"; + $sql = match($i) { + 1 => "create table `test` (`id` int primary key, `name` text)", + default => "alter table `test` add `{$prefix}_{$i}` text", + }; + $filePath = implode(DIRECTORY_SEPARATOR, [$path, $fileName]); + file_put_contents($filePath, $sql); + array_push($fileList, $filePath); + } + + return $fileList; + } + + public function testPerformDevMigrationRunsOnlyPendingFiles():void { + $project = $this->createProjectDir(); + $databasePath = $project . DIRECTORY_SEPARATOR . "dev.sqlite"; + $settings = $this->createSettings($project, $databasePath); + $devPath = $project . DIRECTORY_SEPARATOR . "query" . DIRECTORY_SEPARATOR . "_migration" . DIRECTORY_SEPARATOR . "dev"; + $files = $this->createMigrationFiles($devPath, "feature", 2); + + $devMigrator = new DevMigrator($settings, $devPath); + $devMigrator->createMigrationTable(); + + self::assertSame(2, $devMigrator->performMigration($files)); + self::assertSame(0, $devMigrator->performMigration($files)); + + $db = new Database($settings); + $result = $db->executeSql("pragma table_info(test)"); + self::assertCount(3, $result->fetchAll()); + } + + public function testDevMigrationIntegrityFailsWhenAppliedFileChanges():void { + $project = $this->createProjectDir(); + $databasePath = $project . DIRECTORY_SEPARATOR . "dev.sqlite"; + $settings = $this->createSettings($project, $databasePath); + $devPath = $project . DIRECTORY_SEPARATOR . "query" . DIRECTORY_SEPARATOR . "_migration" . DIRECTORY_SEPARATOR . "dev"; + $files = $this->createMigrationFiles($devPath, "feature", 1); + + $devMigrator = new DevMigrator($settings, $devPath); + $devMigrator->createMigrationTable(); + $devMigrator->performMigration($files); + + file_put_contents($files[0], "create table `test` (`id` int primary key, `changed` text)"); + + $this->expectException(MigrationIntegrityException::class); + $devMigrator->checkIntegrity($files); + } + + public function testMergeIntoMainMigrationDirectoryPromotesAppliedDevMigrations():void { + $project = $this->createProjectDir(); + $databasePath = $project . DIRECTORY_SEPARATOR . "dev.sqlite"; + $settings = $this->createSettings($project, $databasePath); + $mainPath = $project . DIRECTORY_SEPARATOR . "query" . DIRECTORY_SEPARATOR . "_migration"; + $devPath = $mainPath . DIRECTORY_SEPARATOR . "dev"; + + $mainFiles = $this->createMigrationFiles($mainPath, "main", 1); + mkdir($devPath, 0775, true); + $devFiles = [ + $devPath . DIRECTORY_SEPARATOR . "001-feature-1.sql", + $devPath . DIRECTORY_SEPARATOR . "002-feature-2.sql", + ]; + file_put_contents($devFiles[0], "alter table `test` add `feature_1` text"); + file_put_contents($devFiles[1], "alter table `test` add `feature_2` text"); + + $migrator = new Migrator($settings, $mainPath); + $migrator->createMigrationTable(); + $migrator->performMigration($mainFiles); + + $devMigrator = new DevMigrator($settings, $devPath); + $devMigrator->createMigrationTable(); + $devMigrator->performMigration($devFiles); + + self::assertSame(2, $devMigrator->mergeIntoMainMigrationDirectory($migrator, $mainPath)); + + $mergedFileList = $migrator->getMigrationFileList(); + $mergedNames = array_map("basename", $mergedFileList); + + self::assertContains("0002-feature-1.sql", $mergedNames); + self::assertContains("0003-feature-2.sql", $mergedNames); + self::assertFileDoesNotExist($devFiles[0]); + self::assertFileDoesNotExist($devFiles[1]); + self::assertSame([], $devMigrator->getMigrationFileList()); + self::assertSame(3, $migrator->getMigrationCount()); + } +} From e015e862ce62f10568c42c2a89169e51d6fa05b1 Mon Sep 17 00:00:00 2001 From: Greg Bowler Date: Thu, 2 Apr 2026 10:41:44 +0100 Subject: [PATCH 2/6] feature: reject gaps or duplicates in the sequence --- src/Migration/DevMigrator.php | 6 +++ src/Migration/Migrator.php | 8 +++- test/phpunit/Migration/DevMigratorTest.php | 31 +++++++++++++ test/phpunit/Migration/MigratorTest.php | 53 ++++------------------ 4 files changed, 51 insertions(+), 47 deletions(-) diff --git a/src/Migration/DevMigrator.php b/src/Migration/DevMigrator.php index 0592548..a38e59c 100644 --- a/src/Migration/DevMigrator.php +++ b/src/Migration/DevMigrator.php @@ -80,6 +80,12 @@ public function checkFileListOrder(array $fileList):void { if($migrationNumber < $previousNumber) { throw new MigrationSequenceOrderException("Out of order: $migrationNumber before $previousNumber"); } + if($migrationNumber !== $previousNumber + 1) { + throw new MigrationSequenceOrderException("Gap: $previousNumber before $migrationNumber"); + } + } + elseif($migrationNumber !== 1) { + throw new MigrationSequenceOrderException("Gap: expected 1, got $migrationNumber"); } $previousNumber = $migrationNumber; diff --git a/src/Migration/Migrator.php b/src/Migration/Migrator.php index a0a26ee..a08cff0 100644 --- a/src/Migration/Migrator.php +++ b/src/Migration/Migrator.php @@ -125,11 +125,9 @@ public function getMigrationFileList():array { /** @param array $fileList */ public function checkFileListOrder(array $fileList):void { $previousNumber = null; - $sequence = []; foreach($fileList as $file) { $migrationNumber = $this->extractNumberFromFilename($file); - $sequence []= $migrationNumber; if(!is_null($previousNumber)) { if($migrationNumber === $previousNumber) { @@ -138,6 +136,12 @@ public function checkFileListOrder(array $fileList):void { if($migrationNumber < $previousNumber) { throw new MigrationSequenceOrderException("Out of order: $migrationNumber before $previousNumber"); } + if($migrationNumber !== $previousNumber + 1) { + throw new MigrationSequenceOrderException("Gap: $previousNumber before $migrationNumber"); + } + } + elseif($migrationNumber !== 1) { + throw new MigrationSequenceOrderException("Gap: expected 1, got $migrationNumber"); } $previousNumber = $migrationNumber; diff --git a/test/phpunit/Migration/DevMigratorTest.php b/test/phpunit/Migration/DevMigratorTest.php index c571544..563b526 100644 --- a/test/phpunit/Migration/DevMigratorTest.php +++ b/test/phpunit/Migration/DevMigratorTest.php @@ -5,6 +5,7 @@ use Gt\Database\Database; use Gt\Database\Migration\DevMigrator; use Gt\Database\Migration\MigrationIntegrityException; +use Gt\Database\Migration\MigrationSequenceOrderException; use Gt\Database\Migration\Migrator; use Gt\Database\Test\Helper\Helper; use PHPUnit\Framework\TestCase; @@ -115,4 +116,34 @@ public function testMergeIntoMainMigrationDirectoryPromotesAppliedDevMigrations( self::assertSame([], $devMigrator->getMigrationFileList()); self::assertSame(3, $migrator->getMigrationCount()); } + + public function testDevMigratorThrowsOnGapsInSequence():void { + $project = $this->createProjectDir(); + $databasePath = $project . DIRECTORY_SEPARATOR . "dev.sqlite"; + $settings = $this->createSettings($project, $databasePath); + $devPath = $project . DIRECTORY_SEPARATOR . "query" . DIRECTORY_SEPARATOR . "_migration" . DIRECTORY_SEPARATOR . "dev"; + mkdir($devPath, 0775, true); + file_put_contents($devPath . DIRECTORY_SEPARATOR . "001-feature.sql", "select 1"); + file_put_contents($devPath . DIRECTORY_SEPARATOR . "003-feature.sql", "select 1"); + + $devMigrator = new DevMigrator($settings, $devPath); + + $this->expectException(MigrationSequenceOrderException::class); + $devMigrator->checkFileListOrder($devMigrator->getMigrationFileList()); + } + + public function testDevMigratorThrowsOnDuplicateSequenceNumbers():void { + $project = $this->createProjectDir(); + $databasePath = $project . DIRECTORY_SEPARATOR . "dev.sqlite"; + $settings = $this->createSettings($project, $databasePath); + $devPath = $project . DIRECTORY_SEPARATOR . "query" . DIRECTORY_SEPARATOR . "_migration" . DIRECTORY_SEPARATOR . "dev"; + mkdir($devPath, 0775, true); + file_put_contents($devPath . DIRECTORY_SEPARATOR . "001-feature-a.sql", "select 1"); + file_put_contents($devPath . DIRECTORY_SEPARATOR . "001-feature-b.sql", "select 1"); + + $devMigrator = new DevMigrator($settings, $devPath); + + $this->expectException(MigrationSequenceOrderException::class); + $devMigrator->checkFileListOrder($devMigrator->getMigrationFileList()); + } } diff --git a/test/phpunit/Migration/MigratorTest.php b/test/phpunit/Migration/MigratorTest.php index 92c0c1c..ff2cec8 100644 --- a/test/phpunit/Migration/MigratorTest.php +++ b/test/phpunit/Migration/MigratorTest.php @@ -114,12 +114,9 @@ public function testCheckFileListOrderMissing(array $fileList) { $settings = $this->createSettings($path); $migrator = new Migrator($settings, $path); $actualFileList = $migrator->getMigrationFileList(); - $exception = null; - try { - $migrator->checkFileListOrder($actualFileList); - } - catch (Exception $exception) {} - self::assertNull($exception, "No exception should be thrown for missing sequence numbers as long as order is increasing and non-duplicated"); + + $this->expectException(MigrationSequenceOrderException::class); + $migrator->checkFileListOrder($actualFileList); } /** @dataProvider dataMigrationFileListDuplicate */ @@ -822,7 +819,7 @@ protected function createSettings(string $path):Settings { } /** - * New tests for migrating from a specific file number and handling gaps + * New tests for migrating from a specific file number. */ /** @dataProvider dataMigrationFileList */ public function testPerformMigrationFromSpecificNumber(array $fileList) { @@ -866,9 +863,9 @@ public function testPerformMigrationFromSpecificNumber(array $fileList) { } /** @dataProvider dataMigrationFileListMissing */ - public function testPerformMigrationFromSpecificNumberWithGaps(array $fileList) { + public function testCheckFileListOrderThrowsOnGapsWhenMigratingFromSpecificNumber(array $fileList) { $path = $this->getMigrationDirectory(); - $this->createMigrationFiles($fileList, $path); + $this->createFiles($fileList, $path); $settings = $this->createSettings($path); $migrator = new Migrator($settings, $path); @@ -876,42 +873,8 @@ public function testPerformMigrationFromSpecificNumberWithGaps(array $fileList) return implode(DIRECTORY_SEPARATOR, [ $path, $file ]); }, $fileList); - // Build the list of actual migration numbers present (with gaps allowed) - $numbers = array_map(function($file) use ($migrator) { - return $migrator->extractNumberFromFilename($file); - }, $absoluteFileList); - sort($numbers); - - // Pick a start number from the set (not the last one) - $startNumber = $numbers[(int)floor(count($numbers) / 2)]; - $from = $startNumber - 1; - - $migrator->createMigrationTable(); - if($from >= 1) { - $db = new Database($settings); - $db->executeSql(self::MIGRATION_CREATE); - } - - $streamOut = new SplFileObject("php://memory", "w"); - $migrator->setOutput($streamOut); - - $executed = $migrator->performMigration($absoluteFileList, $from); - - $expected = 0; - foreach($numbers as $n) { - if($n >= $startNumber) { - $expected++; - } - } - - $streamOut->rewind(); - $output = $streamOut->fread(4096); - self::assertMatchesRegularExpression("/Migration\\s+{$startNumber}:/", $output); - for($n = 1; $n < $startNumber; $n++) { - self::assertStringNotContainsString("Migration $n:", $output); - } - self::assertStringContainsString("$expected migrations were completed successfully.", $output); - self::assertSame($expected, $executed); + $this->expectException(MigrationSequenceOrderException::class); + $migrator->checkFileListOrder($absoluteFileList); } protected function createFiles(array $files, string $path):void { From 18740ba9a183e0e22ec3cdcaf1253b0183e5fcc3 Mon Sep 17 00:00:00 2001 From: Greg Bowler Date: Thu, 2 Apr 2026 10:45:01 +0100 Subject: [PATCH 3/6] feature: only execute numbered files closes #357 --- src/Migration/DevMigrator.php | 5 ++- src/Migration/Migrator.php | 5 ++- test/phpunit/Migration/DevMigratorTest.php | 22 +++++++++++ test/phpunit/Migration/MigratorTest.php | 46 ++++++++++++++++++++++ 4 files changed, 76 insertions(+), 2 deletions(-) diff --git a/src/Migration/DevMigrator.php b/src/Migration/DevMigrator.php index a38e59c..c9df3bb 100644 --- a/src/Migration/DevMigrator.php +++ b/src/Migration/DevMigrator.php @@ -62,6 +62,9 @@ public function getMigrationFileList():array { } $fileList = glob("$this->path/*.sql"); + $fileList = array_values(array_filter($fileList, function(string $file):bool { + return preg_match("/^\d+.*\.sql$/", basename($file)) === 1; + })); sort($fileList); return $fileList; } @@ -108,7 +111,7 @@ public function checkIntegrity(array $migrationFileList):void { public function extractNumberFromFilename(string $pathName):int { $file = new SplFileInfo($pathName); $filename = $file->getFilename(); - preg_match("/(\d+)-?.*\.sql/", $filename, $matches); + preg_match("/^(\d+)-?.*\.sql$/", $filename, $matches); if(!isset($matches[1])) { throw new MigrationFileNameFormatException($filename); diff --git a/src/Migration/Migrator.php b/src/Migration/Migrator.php index a08cff0..6d47d26 100644 --- a/src/Migration/Migrator.php +++ b/src/Migration/Migrator.php @@ -118,6 +118,9 @@ public function getMigrationFileList():array { } $fileList = glob("$this->path/*.sql"); + $fileList = array_values(array_filter($fileList, function(string $file):bool { + return preg_match("/^\d+.*\.sql$/", basename($file)) === 1; + })); sort($fileList); return $fileList; } @@ -186,7 +189,7 @@ public function checkIntegrity( public function extractNumberFromFilename(string $pathName):int { $file = new SplFileInfo($pathName); $filename = $file->getFilename(); - preg_match("/(\d+)-?.*\.sql/", $filename, $matches); + preg_match("/^(\d+)-?.*\.sql$/", $filename, $matches); if(!isset($matches[1])) { throw new MigrationFileNameFormatException($filename); diff --git a/test/phpunit/Migration/DevMigratorTest.php b/test/phpunit/Migration/DevMigratorTest.php index 563b526..9cd5530 100644 --- a/test/phpunit/Migration/DevMigratorTest.php +++ b/test/phpunit/Migration/DevMigratorTest.php @@ -146,4 +146,26 @@ public function testDevMigratorThrowsOnDuplicateSequenceNumbers():void { $this->expectException(MigrationSequenceOrderException::class); $devMigrator->checkFileListOrder($devMigrator->getMigrationFileList()); } + + public function testDevMigratorIgnoresNonNumericFilesAndThrowsOnResultingGap():void { + $project = $this->createProjectDir(); + $databasePath = $project . DIRECTORY_SEPARATOR . "dev.sqlite"; + $settings = $this->createSettings($project, $databasePath); + $devPath = $project . DIRECTORY_SEPARATOR . "query" . DIRECTORY_SEPARATOR . "_migration" . DIRECTORY_SEPARATOR . "dev"; + mkdir($devPath, 0775, true); + file_put_contents($devPath . DIRECTORY_SEPARATOR . "001-first.sql", "select 1"); + file_put_contents($devPath . DIRECTORY_SEPARATOR . "a002-second.sql", "select 1"); + file_put_contents($devPath . DIRECTORY_SEPARATOR . "003-third.sql", "select 1"); + + $devMigrator = new DevMigrator($settings, $devPath); + $fileList = $devMigrator->getMigrationFileList(); + + self::assertSame([ + $devPath . DIRECTORY_SEPARATOR . "001-first.sql", + $devPath . DIRECTORY_SEPARATOR . "003-third.sql", + ], $fileList); + + $this->expectException(MigrationSequenceOrderException::class); + $devMigrator->checkFileListOrder($fileList); + } } diff --git a/test/phpunit/Migration/MigratorTest.php b/test/phpunit/Migration/MigratorTest.php index ff2cec8..ca22d92 100644 --- a/test/phpunit/Migration/MigratorTest.php +++ b/test/phpunit/Migration/MigratorTest.php @@ -154,6 +154,28 @@ public function testCheckFileListOrderOutOfOrder():void { $migrator->checkFileListOrder($outOfOrder); } + public function testCheckFileListOrderIgnoresNonNumericFilesAndThrowsOnResultingGap():void { + $path = $this->getMigrationDirectory(); + $files = [ + "001-first.sql", + "a002-second.sql", + "003-third.sql", + ]; + $this->createFiles($files, $path); + + $settings = $this->createSettings($path); + $migrator = new Migrator($settings, $path); + $actualFileList = $migrator->getMigrationFileList(); + + self::assertSame([ + $path . DIRECTORY_SEPARATOR . "001-first.sql", + $path . DIRECTORY_SEPARATOR . "003-third.sql", + ], $actualFileList); + + $this->expectException(MigrationSequenceOrderException::class); + $migrator->checkFileListOrder($actualFileList); + } + /** @dataProvider dataMigrationFileList */ public function testCheckIntegrityGood(array $fileList) { $path = $this->getMigrationDirectory(); @@ -531,6 +553,30 @@ public function testNonSqlExtensions(array $fileList) { self::assertNull($exception); } + public function testPerformMigrationIgnoresNonNumericPrefixedSqlFiles():void { + $path = $this->getMigrationDirectory(); + file_put_contents($path . DIRECTORY_SEPARATOR . "0001-create-test.sql", self::MIGRATION_CREATE); + file_put_contents( + $path . DIRECTORY_SEPARATOR . "a0002-ignored.sql", + "alter table `test` add `ignored_column` varchar(32)" + ); + + $settings = $this->createSettings($path); + $migrator = new Migrator($settings, $path); + $migrator->createMigrationTable(); + + $actualFileList = $migrator->getMigrationFileList(); + self::assertSame([ + $path . DIRECTORY_SEPARATOR . "0001-create-test.sql", + ], $actualFileList); + + $migrator->performMigration($actualFileList); + + $db = new Database($settings); + $result = $db->executeSql("PRAGMA table_info(test);"); + self::assertCount(2, $result->fetchAll()); + } + /** @dataProvider dataMigrationFileList */ public function testMigrationThrowsExceptionWhenNoMigrationTable(array $fileList) { $path = $this->getMigrationDirectory(); From 70ea3badfaed5b3cea14f3e21db0c57c4a1b088d Mon Sep 17 00:00:00 2001 From: Greg Bowler Date: Thu, 2 Apr 2026 12:00:41 +0100 Subject: [PATCH 4/6] fix: sqlite can be migrated with `--force` (#395) closes #352 --- src/Migration/Migrator.php | 10 ++++++ test/phpunit/Cli/ExecuteCommandTest.php | 43 +++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/src/Migration/Migrator.php b/src/Migration/Migrator.php index 6d47d26..03c6ff2 100644 --- a/src/Migration/Migrator.php +++ b/src/Migration/Migrator.php @@ -27,12 +27,14 @@ class Migrator { protected string $tableName; protected string $charset; protected string $collate; + protected Settings $settings; public function __construct( Settings $settings, string $path, string $tableName = "_migration" ) { + $this->settings = clone $settings; $this->schema = $settings->getSchema(); $this->path = $path; $this->tableName = $tableName; @@ -298,6 +300,14 @@ public function resetMigrationSequence(int $numberToForce):void { */ public function deleteAndRecreateSchema():void { if($this->driver === Settings::DRIVER_SQLITE) { + unset($this->dbClient); + + if($this->schema !== Settings::SCHEMA_IN_MEMORY + && is_file($this->schema)) { + unlink($this->schema); + } + + $this->dbClient = new Database($this->settings); return; } diff --git a/test/phpunit/Cli/ExecuteCommandTest.php b/test/phpunit/Cli/ExecuteCommandTest.php index 614fbc7..30a49ca 100644 --- a/test/phpunit/Cli/ExecuteCommandTest.php +++ b/test/phpunit/Cli/ExecuteCommandTest.php @@ -247,6 +247,49 @@ public function testExecuteWithResetWithNumber():void { } } + public function testExecuteWithForceResetsSqliteDatabaseAndRerunsFromMigrationOne():void { + $project = $this->createProjectDir(); + $sqlitePath = str_replace("\\", "/", $project . DIRECTORY_SEPARATOR . "cli-test.db"); + $this->writeConfigIni($project, $sqlitePath); + $this->createMigrations($project, 2); + + $cwdBackup = getcwd(); + chdir($project); + try { + $cmd = new ExecuteCommand(); + $initialStreams = $this->makeStreamFiles(); + $cmd->setStream($initialStreams["stream"]); + $cmd->run(new ArgumentValueList()); + + $settings = new Settings($project . DIRECTORY_SEPARATOR . "query", Settings::DRIVER_SQLITE, $sqlitePath); + $db = new Database($settings); + $db->executeSql("insert into `test` (`id`, `name`, `new_column_2`) values (1, 'before-force', 'value')"); + $rowCountBeforeForce = $db->executeSql("select count(*) as c from `test`")->fetch()?->getInt("c"); + self::assertSame(1, $rowCountBeforeForce); + + unset($db); + + $forceStreams = $this->makeStreamFiles(); + $cmd->setStream($forceStreams["stream"]); + $args = new ArgumentValueList(); + $args->set("force"); + $cmd->run($args); + + list("out" => $out) = $this->readFromFiles($forceStreams["out"], $forceStreams["err"]); + self::assertStringContainsString("Migration 1:", $out); + self::assertStringContainsString("2 migrations were completed successfully.", $out); + + $dbAfterForce = new Database($settings); + $rowCountAfterForce = $dbAfterForce->executeSql("select count(*) as c from `test`")->fetch()?->getInt("c"); + self::assertSame(0, $rowCountAfterForce); + $migrationCount = $dbAfterForce->executeSql("select count(*) as c from `_migration`")->fetch()?->getInt("c"); + self::assertSame(2, $migrationCount); + } + finally { + chdir($cwdBackup); + } + } + public function testOptionalParameterListContainsCliOverrides():void { $command = new ExecuteCommand(); $parameterNames = array_map( From 1fb0bbfe0ae1580a5e3abf7c3299838d23319cb1 Mon Sep 17 00:00:00 2001 From: Greg Bowler Date: Thu, 2 Apr 2026 12:08:31 +0100 Subject: [PATCH 5/6] maintenance: remove duplication in migrator classes --- src/Migration/AbstractMigrator.php | 135 +++++++++++++++++++++++++++++ src/Migration/DevMigrator.php | 124 ++------------------------ src/Migration/Migrator.php | 126 +++------------------------ 3 files changed, 150 insertions(+), 235 deletions(-) create mode 100644 src/Migration/AbstractMigrator.php diff --git a/src/Migration/AbstractMigrator.php b/src/Migration/AbstractMigrator.php new file mode 100644 index 0000000..5d9fd79 --- /dev/null +++ b/src/Migration/AbstractMigrator.php @@ -0,0 +1,135 @@ +settings = clone $settings; + $this->driver = $settings->getDriver(); + $this->path = $path; + $this->tableName = $tableName ?? $this->getDefaultTableName(); + + if($this->driver !== Settings::DRIVER_SQLITE) { + $settings = $settings->withoutSchema(); + } + + $this->dbClient = new Database($settings); + } + + abstract protected function getDefaultTableName():string; + + public function setOutput( + SplFileObject $out, + ?SplFileObject $error = null + ):void { + $this->streamOut = $out; + $this->streamError = $error; + } + + /** @return array */ + public function getMigrationFileList():array { + if(!is_dir($this->path)) { + return []; + } + + $fileList = glob("$this->path/*.sql"); + $fileList = array_values(array_filter($fileList, function(string $file):bool { + return preg_match("/^\d+.*\.sql$/", basename($file)) === 1; + })); + sort($fileList); + return $fileList; + } + + /** @param array $fileList */ + public function checkFileListOrder(array $fileList):void { + $previousNumber = null; + + foreach($fileList as $file) { + $migrationNumber = $this->extractNumberFromFilename($file); + + if(!is_null($previousNumber)) { + if($migrationNumber === $previousNumber) { + throw new MigrationSequenceOrderException("Duplicate: $migrationNumber"); + } + if($migrationNumber < $previousNumber) { + throw new MigrationSequenceOrderException("Out of order: $migrationNumber before $previousNumber"); + } + if($migrationNumber !== $previousNumber + 1) { + throw new MigrationSequenceOrderException("Gap: $previousNumber before $migrationNumber"); + } + } + elseif($migrationNumber !== 1) { + throw new MigrationSequenceOrderException("Gap: expected 1, got $migrationNumber"); + } + + $previousNumber = $migrationNumber; + } + } + + public function extractNumberFromFilename(string $pathName):int { + $file = new SplFileInfo($pathName); + $filename = $file->getFilename(); + preg_match("/^(\d+)-?.*\.sql$/", $filename, $matches); + + if(!isset($matches[1])) { + throw new MigrationFileNameFormatException($filename); + } + + return (int)$matches[1]; + } + + protected function executeSqlFile(string $file):string { + $md5 = md5_file($file); + $sqlStatementSplitter = new SqlStatementSplitter(); + + foreach($sqlStatementSplitter->split(file_get_contents($file)) as $sql) { + $this->dbClient->executeSql($sql); + } + + return $md5; + } + + protected function nowExpression():string { + if($this->driver === Settings::DRIVER_SQLITE) { + return "datetime('now')"; + } + + return "now()"; + } + + protected function output( + string $message, + string $streamName = self::STREAM_OUT + ):void { + $stream = $this->streamOut ?? null; + if($streamName === self::STREAM_ERROR) { + $stream = $this->streamError; + } + + if(is_null($stream)) { + return; + } + + $stream->fwrite($message . PHP_EOL); + } +} diff --git a/src/Migration/DevMigrator.php b/src/Migration/DevMigrator.php index c9df3bb..1f11aca 100644 --- a/src/Migration/DevMigrator.php +++ b/src/Migration/DevMigrator.php @@ -1,49 +1,13 @@ driver = $settings->getDriver(); - $this->path = $path; - $this->tableName = $tableName; - - if($this->driver !== Settings::DRIVER_SQLITE) { - $settings = $settings->withoutSchema(); - } - - $this->dbClient = new Database($settings); - } - - public function setOutput( - SplFileObject $out, - ?SplFileObject $error = null - ):void { - $this->streamOut = $out; - $this->streamError = $error; + protected function getDefaultTableName():string { + return "_migration_dev"; } public function createMigrationTable():void { @@ -55,46 +19,6 @@ public function createMigrationTable():void { ])); } - /** @return array */ - public function getMigrationFileList():array { - if(!is_dir($this->path)) { - return []; - } - - $fileList = glob("$this->path/*.sql"); - $fileList = array_values(array_filter($fileList, function(string $file):bool { - return preg_match("/^\d+.*\.sql$/", basename($file)) === 1; - })); - sort($fileList); - return $fileList; - } - - /** @param array $fileList */ - public function checkFileListOrder(array $fileList):void { - $previousNumber = null; - - foreach($fileList as $file) { - $migrationNumber = $this->extractNumberFromFilename($file); - - if(!is_null($previousNumber)) { - if($migrationNumber === $previousNumber) { - throw new MigrationSequenceOrderException("Duplicate: $migrationNumber"); - } - if($migrationNumber < $previousNumber) { - throw new MigrationSequenceOrderException("Out of order: $migrationNumber before $previousNumber"); - } - if($migrationNumber !== $previousNumber + 1) { - throw new MigrationSequenceOrderException("Gap: $previousNumber before $migrationNumber"); - } - } - elseif($migrationNumber !== 1) { - throw new MigrationSequenceOrderException("Gap: expected 1, got $migrationNumber"); - } - - $previousNumber = $migrationNumber; - } - } - /** @param array $migrationFileList */ public function checkIntegrity(array $migrationFileList):void { foreach($migrationFileList as $file) { @@ -108,22 +32,9 @@ public function checkIntegrity(array $migrationFileList):void { } } - public function extractNumberFromFilename(string $pathName):int { - $file = new SplFileInfo($pathName); - $filename = $file->getFilename(); - preg_match("/^(\d+)-?.*\.sql$/", $filename, $matches); - - if(!isset($matches[1])) { - throw new MigrationFileNameFormatException($filename); - } - - return (int)$matches[1]; - } - /** @param array $migrationFileList */ public function performMigration(array $migrationFileList):int { $numCompleted = 0; - $sqlStatementSplitter = new SqlStatementSplitter(); foreach($migrationFileList as $file) { $fileName = basename($file); @@ -133,12 +44,7 @@ public function performMigration(array $migrationFileList):int { $fileNumber = $this->extractNumberFromFilename($file); $this->output("Dev migration $fileNumber: `$file`."); - $md5 = md5_file($file); - - foreach($sqlStatementSplitter->split(file_get_contents($file)) as $sql) { - $this->dbClient->executeSql($sql); - } - + $md5 = $this->executeSqlFile($file); $this->recordMigrationSuccess($fileName, $md5); $numCompleted++; } @@ -234,11 +140,7 @@ protected function createMergedFilename(string $fileName, int $number):string { } protected function recordMigrationSuccess(string $fileName, string $hash):void { - $now = "now()"; - - if($this->driver === Settings::DRIVER_SQLITE) { - $now = "datetime('now')"; - } + $now = $this->nowExpression(); $this->dbClient->executeSql(implode("\n", [ "insert into `{$this->tableName}` (", @@ -257,20 +159,4 @@ protected function deleteMigrationRecord(string $fileName):void { "where `" . self::COLUMN_FILE_NAME . "` = ?", ]), [$fileName]); } - - protected function output( - string $message, - string $streamName = self::STREAM_OUT - ):void { - $stream = $this->streamOut ?? null; - if($streamName === self::STREAM_ERROR) { - $stream = $this->streamError; - } - - if(is_null($stream)) { - return; - } - - $stream->fwrite($message . PHP_EOL); - } } diff --git a/src/Migration/Migrator.php b/src/Migration/Migrator.php index 03c6ff2..9304d6f 100644 --- a/src/Migration/Migrator.php +++ b/src/Migration/Migrator.php @@ -1,61 +1,33 @@ settings = clone $settings; + parent::__construct($settings, $path, $tableName); $this->schema = $settings->getSchema(); - $this->path = $path; - $this->tableName = $tableName; - $this->driver = $settings->getDriver(); - $this->charset = $settings->getCharset(); $this->collate = $settings->getCollation(); - - if($this->driver !== Settings::DRIVER_SQLITE) { - $settings = $settings->withoutSchema(); // @codeCoverageIgnore - } - - $this->dbClient = new Database($settings); } - public function setOutput( - SplFileObject $out, - ?SplFileObject $error = null - ):void { - $this->streamOut = $out; - $this->streamError = $error; + protected function getDefaultTableName():string { + return "_migration"; } public function checkMigrationTableExists():bool { @@ -119,38 +91,7 @@ public function getMigrationFileList():array { ); } - $fileList = glob("$this->path/*.sql"); - $fileList = array_values(array_filter($fileList, function(string $file):bool { - return preg_match("/^\d+.*\.sql$/", basename($file)) === 1; - })); - sort($fileList); - return $fileList; - } - - /** @param array $fileList */ - public function checkFileListOrder(array $fileList):void { - $previousNumber = null; - - foreach($fileList as $file) { - $migrationNumber = $this->extractNumberFromFilename($file); - - if(!is_null($previousNumber)) { - if($migrationNumber === $previousNumber) { - throw new MigrationSequenceOrderException("Duplicate: $migrationNumber"); - } - if($migrationNumber < $previousNumber) { - throw new MigrationSequenceOrderException("Out of order: $migrationNumber before $previousNumber"); - } - if($migrationNumber !== $previousNumber + 1) { - throw new MigrationSequenceOrderException("Gap: $previousNumber before $migrationNumber"); - } - } - elseif($migrationNumber !== 1) { - throw new MigrationSequenceOrderException("Gap: expected 1, got $migrationNumber"); - } - - $previousNumber = $migrationNumber; - } + return parent::getMigrationFileList(); } /** @param array $migrationFileList */ @@ -159,18 +100,15 @@ public function checkIntegrity( ?int $migrationStartFrom = null ):int { $fileNumber = 0; - + foreach($migrationFileList as $file) { $fileNumber = $this->extractNumberFromFilename($file); - // If a start point is provided, skip files at or before that number - // and only verify files AFTER the provided migration count. if(!is_null($migrationStartFrom) && $fileNumber <= $migrationStartFrom) { continue; } $md5 = md5_file($file); - $result = $this->dbClient->executeSql(implode("\n", [ "select `" . self::COLUMN_QUERY_HASH . "`", "from `{$this->tableName}`", @@ -188,26 +126,13 @@ public function checkIntegrity( return $fileNumber; } - public function extractNumberFromFilename(string $pathName):int { - $file = new SplFileInfo($pathName); - $filename = $file->getFilename(); - preg_match("/^(\d+)-?.*\.sql$/", $filename, $matches); - - if(!isset($matches[1])) { - throw new MigrationFileNameFormatException($filename); - } - - return (int)$matches[1]; - } - /** @param array $migrationFileList */ public function performMigration( array $migrationFileList, int $existingFileNumber = 0 ):int { $numCompleted = 0; - $sqlStatementSplitter = new SqlStatementSplitter(); - + foreach($migrationFileList as $file) { $fileNumber = $this->extractNumberFromFilename($file); if($fileNumber <= $existingFileNumber) { @@ -215,12 +140,7 @@ public function performMigration( } $this->output("Migration $fileNumber: `$file`."); - $md5 = md5_file($file); - - foreach($sqlStatementSplitter->split(file_get_contents($file)) as $sql) { - $this->dbClient->executeSql($sql); - } - + $md5 = $this->executeSqlFile($file); $this->recordMigrationSuccess($fileNumber, $md5); $numCompleted++; } @@ -239,7 +159,6 @@ public function performMigration( * @codeCoverageIgnore */ public function selectSchema():void { -// SQLITE databases represent their own schema. if($this->driver === Settings::DRIVER_SQLITE) { return; } @@ -265,11 +184,7 @@ public function selectSchema():void { } protected function recordMigrationSuccess(int $number, ?string $hash):void { - $now = "now()"; - - if($this->driver === Settings::DRIVER_SQLITE) { - $now = "datetime('now')"; - } + $now = $this->nowExpression(); $this->dbClient->executeSql(implode("\n", [ "insert into `{$this->tableName}` (", @@ -286,11 +201,6 @@ public function markMigrationApplied(int $number, string $hash):void { $this->recordMigrationSuccess($number, $hash); } - /** - * @param int $numberToForce A null-hashed migration will be marked as - * successful with this number. This will allow the next number to be - * executed out of sequence. - */ public function resetMigrationSequence(int $numberToForce):void { $this->recordMigrationSuccess($numberToForce, null); } @@ -333,20 +243,4 @@ public function deleteAndRecreateSchema():void { throw $exception; } } - - protected function output( - string $message, - string $streamName = self::STREAM_OUT - ):void { - $stream = $this->streamOut ?? null; - if($streamName === self::STREAM_ERROR) { - $stream = $this->streamError; - } - - if(is_null($stream)) { - return; - } - - $stream->fwrite($message . PHP_EOL); - } } From cbfc3e707cf9df1438221a2532c0ff1db795edf3 Mon Sep 17 00:00:00 2001 From: Greg Bowler Date: Thu, 2 Apr 2026 12:26:14 +0100 Subject: [PATCH 6/6] test: improve coverage --- test/phpunit/Cli/ExecuteCommandTest.php | 114 ++++++++++++++++ .../Migration/AbstractMigratorTest.php | 127 ++++++++++++++++++ test/phpunit/Migration/MigratorTest.php | 1 - .../Migration/SqlStatementSplitterTest.php | 33 +++++ 4 files changed, 274 insertions(+), 1 deletion(-) create mode 100644 test/phpunit/Migration/AbstractMigratorTest.php create mode 100644 test/phpunit/Migration/SqlStatementSplitterTest.php diff --git a/test/phpunit/Cli/ExecuteCommandTest.php b/test/phpunit/Cli/ExecuteCommandTest.php index 30a49ca..5750412 100644 --- a/test/phpunit/Cli/ExecuteCommandTest.php +++ b/test/phpunit/Cli/ExecuteCommandTest.php @@ -145,6 +145,29 @@ public function getConfigPublic(string $repoBasePath, ?string $defaultPath):Conf self::assertSame("sqlite", $config->get("database.driver")); } + public function testGetDefaultPathReturnsNullWhenVendorDefaultDoesNotExist():void { + $project = $this->createProjectDir(); + $command = new ExecuteCommand(); + $reflectionMethod = new \ReflectionMethod($command, "getDefaultPath"); + + self::assertNull($reflectionMethod->invoke($command, $project)); + } + + public function testGetDefaultPathFindsVendorConfigDefaultIni():void { + $project = $this->createProjectDir(); + $vendorConfigDir = $project . DIRECTORY_SEPARATOR . "vendor" + . DIRECTORY_SEPARATOR . "phpgt" + . DIRECTORY_SEPARATOR . "webengine"; + mkdir($vendorConfigDir, 0775, true); + $defaultConfig = $vendorConfigDir . DIRECTORY_SEPARATOR . "config.default.ini"; + file_put_contents($defaultConfig, "[database]\ndriver = sqlite\n"); + + $command = new ExecuteCommand(); + $reflectionMethod = new \ReflectionMethod($command, "getDefaultPath"); + + self::assertSame($defaultConfig, $reflectionMethod->invoke($command, $project)); + } + public function testExecuteMigratesAll():void { $project = $this->createProjectDir(); $sqlitePath = str_replace("\\", "/", $project . DIRECTORY_SEPARATOR . "cli-test.db"); @@ -379,6 +402,97 @@ public function testExecuteWithDevMergePromotesDevMigrations():void { } } + public function testExecuteWithDevReportsIntegrityErrorWhenAppliedDevFileChanges():void { + $project = $this->createProjectDir(); + $sqlitePath = str_replace("\\", "/", $project . DIRECTORY_SEPARATOR . "cli-test.db"); + $this->writeConfigIni($project, $sqlitePath); + $this->createMigrations($project, 1); + $devFiles = $this->createDevMigrations($project, 1); + + $cwdBackup = getcwd(); + chdir($project); + try { + $cmd = new ExecuteCommand(); + $streams = $this->makeStreamFiles(); + $cmd->setStream($streams["stream"]); + + $args = new ArgumentValueList(); + $args->set("dev"); + $cmd->run($args); + + file_put_contents($devFiles[0], "alter table `test` add `changed_dev_column` varchar(32)"); + + $errorStreams = $this->makeStreamFiles(); + $cmd->setStream($errorStreams["stream"]); + $cmd->run($args); + + list("out" => $out) = $this->readFromFiles($errorStreams["out"], $errorStreams["err"]); + self::assertStringContainsString("integrity error migrating dev file", $out); + } + finally { + chdir($cwdBackup); + } + } + + public function testExecuteWithDevMergeReportsIntegrityErrorWhenAppliedDevFileChanges():void { + $project = $this->createProjectDir(); + $sqlitePath = str_replace("\\", "/", $project . DIRECTORY_SEPARATOR . "cli-test.db"); + $this->writeConfigIni($project, $sqlitePath); + $this->createMigrations($project, 1); + $devFiles = $this->createDevMigrations($project, 1); + + $cwdBackup = getcwd(); + chdir($project); + try { + $cmd = new ExecuteCommand(); + $devStreams = $this->makeStreamFiles(); + $cmd->setStream($devStreams["stream"]); + + $devArgs = new ArgumentValueList(); + $devArgs->set("dev"); + $cmd->run($devArgs); + + file_put_contents($devFiles[0], "alter table `test` add `changed_dev_column` varchar(32)"); + + $mergeStreams = $this->makeStreamFiles(); + $cmd->setStream($mergeStreams["stream"]); + $mergeArgs = new ArgumentValueList(); + $mergeArgs->set("dev-merge"); + $cmd->run($mergeArgs); + + list("out" => $out) = $this->readFromFiles($mergeStreams["out"], $mergeStreams["err"]); + self::assertStringContainsString("integrity error merging dev migration file", $out); + } + finally { + chdir($cwdBackup); + } + } + + public function testExecuteWithDevMergeReportsNoDevMigrationsToMerge():void { + $project = $this->createProjectDir(); + $sqlitePath = str_replace("\\", "/", $project . DIRECTORY_SEPARATOR . "cli-test.db"); + $this->writeConfigIni($project, $sqlitePath); + $this->createMigrations($project, 1); + + $cwdBackup = getcwd(); + chdir($project); + try { + $cmd = new ExecuteCommand(); + $streams = $this->makeStreamFiles(); + $cmd->setStream($streams["stream"]); + + $args = new ArgumentValueList(); + $args->set("dev-merge"); + $cmd->run($args); + + list("out" => $out) = $this->readFromFiles($streams["out"], $streams["err"]); + self::assertStringContainsString("No dev migrations to merge.", $out); + } + finally { + chdir($cwdBackup); + } + } + public function testCliArgumentsOverrideConfigValuesWhenBuildingSettings():void { $repoBasePath = "/tmp/project-root"; $config = new Config( diff --git a/test/phpunit/Migration/AbstractMigratorTest.php b/test/phpunit/Migration/AbstractMigratorTest.php new file mode 100644 index 0000000..01e6189 --- /dev/null +++ b/test/phpunit/Migration/AbstractMigratorTest.php @@ -0,0 +1,127 @@ +tableName; + } + + public function nowExpressionPublic():string { + return $this->nowExpression(); + } + + public function setDriverPublic(string $driver):void { + $this->driver = $driver; + } + + public function executeSqlFilePublic(string $file):string { + return $this->executeSqlFile($file); + } + + public function outputPublic(string $message, string $streamName = self::STREAM_OUT):void { + $this->output($message, $streamName); + } + }; + } + + public function testConstructorUsesDefaultTableNameWhenNotProvided():void { + $dir = $this->createWorkspace(); + $settings = $this->createSettings($dir, Settings::DRIVER_SQLITE, $dir . "/probe.sqlite"); + $probe = $this->createProbe($settings, $dir); + + self::assertSame("_probe", $probe->getTableNamePublic()); + } + + public function testGetMigrationFileListReturnsEmptyForMissingDirectory():void { + $dir = $this->createWorkspace(); + $settings = $this->createSettings($dir, Settings::DRIVER_SQLITE, $dir . "/probe.sqlite"); + $probe = $this->createProbe($settings, $dir . "/missing-dir"); + + self::assertSame([], $probe->getMigrationFileList()); + } + + public function testNowExpressionUsesDriverSpecificValue():void { + $dir = $this->createWorkspace(); + $sqliteProbe = $this->createProbe( + $this->createSettings($dir, Settings::DRIVER_SQLITE, $dir . "/probe.sqlite"), + $dir + ); + $mysqlProbe = $this->createProbe( + $this->createSettings($dir, Settings::DRIVER_SQLITE, $dir . "/probe-mysql.sqlite"), + $dir + ); + $mysqlProbe->setDriverPublic(Settings::DRIVER_MYSQL); + + self::assertSame("datetime('now')", $sqliteProbe->nowExpressionPublic()); + self::assertSame("now()", $mysqlProbe->nowExpressionPublic()); + } + + public function testExecuteSqlFileRunsEachStatementAndReturnsHash():void { + $dir = Helper::getTmpDir() . DIRECTORY_SEPARATOR . uniqid("probe-"); + mkdir($dir, 0775, true); + $databasePath = $dir . DIRECTORY_SEPARATOR . "probe.sqlite"; + $sqlFile = $dir . DIRECTORY_SEPARATOR . "001-probe.sql"; + file_put_contents($sqlFile, implode("\n", [ + "create table test(id int primary key, name text);", + "insert into test values(1, 'alpha');", + ])); + + $probe = $this->createProbe( + $this->createSettings($dir, Settings::DRIVER_SQLITE, $databasePath), + $dir + ); + + $hash = $probe->executeSqlFilePublic($sqlFile); + self::assertSame(md5_file($sqlFile), $hash); + + $db = new \Gt\Database\Database( + $this->createSettings($dir, Settings::DRIVER_SQLITE, $databasePath) + ); + $row = $db->executeSql("select name from test where id = 1")->fetch(); + self::assertSame("alpha", $row?->getString("name")); + } + + public function testOutputWritesToErrorStreamWhenRequested():void { + $dir = $this->createWorkspace(); + $probe = $this->createProbe( + $this->createSettings($dir, Settings::DRIVER_SQLITE, $dir . "/probe.sqlite"), + $dir + ); + + $out = new SplFileObject("php://memory", "w+"); + $err = new SplFileObject("php://memory", "w+"); + $probe->setOutput($out, $err); + $probe->outputPublic("problem", AbstractMigrator::STREAM_ERROR); + + $out->rewind(); + $err->rewind(); + self::assertSame("", $out->fread(128)); + self::assertSame("problem\n", $err->fread(128)); + } +} diff --git a/test/phpunit/Migration/MigratorTest.php b/test/phpunit/Migration/MigratorTest.php index ca22d92..c1cfa08 100644 --- a/test/phpunit/Migration/MigratorTest.php +++ b/test/phpunit/Migration/MigratorTest.php @@ -420,7 +420,6 @@ public function testResetMigration(array $fileList) { /** * @dataProvider dataMigrationFileList - * @runInSeparateProcess */ public function testPerformMigrationGood(array $fileList):void { $path = $this->getMigrationDirectory(); diff --git a/test/phpunit/Migration/SqlStatementSplitterTest.php b/test/phpunit/Migration/SqlStatementSplitterTest.php new file mode 100644 index 0000000..10d9a6f --- /dev/null +++ b/test/phpunit/Migration/SqlStatementSplitterTest.php @@ -0,0 +1,33 @@ +split(implode("\n", [ + "create table test(id int);", + "insert into test values(1);", + "update test set id = 2 where id = 1;", + ])); + + self::assertSame([ + "create table test(id int)", + "insert into test values(1)", + "update test set id = 2 where id = 1", + ], $statements); + } + + public function testSplitIgnoresEmptyStatements():void { + $splitter = new SqlStatementSplitter(); + + $statements = $splitter->split(" ; ;\nselect 1;; "); + + self::assertSame([ + "select 1", + ], $statements); + } +}