From 87a4e70ea80399a20f6f417ce4901902c3e9dcd2 Mon Sep 17 00:00:00 2001 From: Greg Bowler Date: Tue, 5 Aug 2025 09:43:11 +0100 Subject: [PATCH 1/4] wip: reset hashing for #114 --- src/Cli/ExecuteCommand.php | 20 +++++++++- src/Migration/Migrator.php | 39 +++++++++++++------ src/Query/QueryCollectionFactory.php | 1 - test/phpunit/Migration/MigratorTest.php | 52 +++++++++++++++++++++++++ 4 files changed, 97 insertions(+), 15 deletions(-) diff --git a/src/Cli/ExecuteCommand.php b/src/Cli/ExecuteCommand.php index cfa1245..d39b4be 100644 --- a/src/Cli/ExecuteCommand.php +++ b/src/Cli/ExecuteCommand.php @@ -57,9 +57,19 @@ public function run(?ArgumentValueList $arguments = null):void { $migrationCount = $migrator->getMigrationCount(); $migrationFileList = $migrator->getMigrationFileList(); + $resetNumber = null; + if($arguments->contains("reset")) { + $resetNumber = $arguments->get("reset")->get(); + if(!$resetNumber) { + $lastKey = array_key_last($migrationFileList); + $resetNumber = $migrator->extractNumberFromFilename($migrationFileList[$lastKey]); + } + $resetNumber = (int)$resetNumber; + } + try { - $migrator->checkIntegrity($migrationFileList, $migrationCount); - $migrator->performMigration($migrationFileList, $migrationCount); + $migrator->checkIntegrity($migrationFileList, $resetNumber ?? $migrationCount); + $migrator->performMigration($migrationFileList, $resetNumber ?? $migrationCount); } catch(MigrationIntegrityException $exception) { $this->writeLine( @@ -106,6 +116,12 @@ public function getOptionalParameterList():array { "force", "f", "Forcefully drop the current schema and run from migration 1" + ), + new Parameter( + true, + "reset", + "r", + "Reset the integrity checks to a specific migration number" ) ]; } diff --git a/src/Migration/Migrator.php b/src/Migration/Migrator.php index ed1095c..f874b81 100644 --- a/src/Migration/Migrator.php +++ b/src/Migration/Migrator.php @@ -89,7 +89,7 @@ public function createMigrationTable():void { $this->dbClient->executeSql(implode("\n", [ "create table if not exists `{$this->tableName}` (", "`" . self::COLUMN_QUERY_NUMBER . "` int primary key,", - "`" . self::COLUMN_QUERY_HASH . "` varchar(32) not null,", + "`" . self::COLUMN_QUERY_HASH . "` varchar(32) null,", "`" . self::COLUMN_MIGRATED_AT . "` datetime not null )", ])); } @@ -144,16 +144,22 @@ public function checkFileListOrder(array $fileList):void { /** @param array $migrationFileList */ public function checkIntegrity( array $migrationFileList, - ?int $migrationCount = null + ?int $migrationStartFrom = null ):int { $fileNumber = 0; foreach($migrationFileList as $i => $file) { - $fileNumber = $i + 1; + $fileNumber = $this->extractNumberFromFilename($file); $md5 = md5_file($file); - if(is_null($migrationCount) - || $fileNumber <= $migrationCount) { + if($migrationStartFrom) { + if($fileNumber < $migrationStartFrom) { + continue; + } + } + + if(is_null($migrationStartFrom) + || $fileNumber <= $migrationStartFrom) { $result = $this->dbClient->executeSql(implode("\n", [ "select `" . self::COLUMN_QUERY_HASH . "`", "from `{$this->tableName}`", @@ -161,9 +167,9 @@ public function checkIntegrity( "limit 1", ]), [$fileNumber]); - $hashInDb = ($result->fetch())->getString(self::COLUMN_QUERY_HASH); + $hashInDb = ($result->fetch())?->getString(self::COLUMN_QUERY_HASH); - if($hashInDb !== $md5) { + if($hashInDb && $hashInDb !== $md5) { throw new MigrationIntegrityException($file); } } @@ -172,7 +178,7 @@ public function checkIntegrity( return $fileNumber; } - protected function extractNumberFromFilename(string $pathName):int { + public function extractNumberFromFilename(string $pathName):int { $file = new SplFileInfo($pathName); $filename = $file->getFilename(); preg_match("/(\d+)-?.*\.sql/", $filename, $matches); @@ -187,15 +193,15 @@ protected function extractNumberFromFilename(string $pathName):int { /** @param array $migrationFileList */ public function performMigration( array $migrationFileList, - int $existingMigrationCount = 0 + int $existingFileNumber = 0 ):int { $fileNumber = 0; $numCompleted = 0; foreach($migrationFileList as $i => $file) { - $fileNumber = $i + 1; + $fileNumber = $this->extractNumberFromFilename($file); - if($fileNumber <= $existingMigrationCount) { + if($fileNumber <= $existingFileNumber) { continue; } @@ -277,7 +283,7 @@ public function selectSchema():void { } } - protected function recordMigrationSuccess(int $number, string $hash):void { + protected function recordMigrationSuccess(int $number, ?string $hash):void { $now = "now()"; if($this->driver === Settings::DRIVER_SQLITE) { @@ -295,6 +301,15 @@ protected function recordMigrationSuccess(int $number, string $hash):void { ]), [$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); + } + /** * @codeCoverageIgnore */ diff --git a/src/Query/QueryCollectionFactory.php b/src/Query/QueryCollectionFactory.php index c66a7cb..3044973 100644 --- a/src/Query/QueryCollectionFactory.php +++ b/src/Query/QueryCollectionFactory.php @@ -27,7 +27,6 @@ public function create(string $name):QueryCollection { } return $this->queryCollectionCache[$name]; - } public function directoryExists(string $name):bool { diff --git a/test/phpunit/Migration/MigratorTest.php b/test/phpunit/Migration/MigratorTest.php index baf7286..aa828e3 100644 --- a/test/phpunit/Migration/MigratorTest.php +++ b/test/phpunit/Migration/MigratorTest.php @@ -279,6 +279,58 @@ public function testForcedMigration(array $fileList) { self::assertNull($exception,"Exception should not be thrown"); } + /** + * This test needs an explanation because it's not immediately obvious. + * The fileList is generated as usual, but then to simulate a real + * production "messy" codebase, a new migration file is created with a + * much higher sequence (15 higher than the last in the fileList). + * + * Because of this, the migration will fail. However, we reset the + * migration sequence before performing the migration, and even though + * none of the files in fileList are migrated yet, we should only see + * 1 migration take place. + * + * @dataProvider dataMigrationFileList + */ + public function testResetMigration(array $fileList) { + $path = $this->getMigrationDirectory(); + $this->createMigrationFiles($fileList, $path); + $settings = $this->createSettings($path); + + $migrator = new Migrator( + $settings, + $path, + "_migration", + ); + + $newNumber = count($fileList) + 15; + + $newFileName = str_pad($newNumber, 4, "0", STR_PAD_LEFT); + $newFileName .= "-" . uniqid() . ".sql"; + $newFilePath = implode(DIRECTORY_SEPARATOR, [ + $path, + $newFileName, + ]); + array_push($fileList, $newFileName); + + $absoluteFileList = array_map(function($file)use($path) { + return implode(DIRECTORY_SEPARATOR, [ + $path, + $file, + ]); + },$fileList); + + $lastKey = array_key_last($absoluteFileList); + file_put_contents($absoluteFileList[$lastKey], "create table migrated_out_of_order ( id int primary key )"); + + $migrator->createMigrationTable(); + $migrator->resetMigrationSequence($newNumber - 1); + $migrationCount = $migrator->getMigrationCount(); + $migrator->checkIntegrity($absoluteFileList, $migrationCount); + $migrationsExecuted = $migrator->performMigration($absoluteFileList, $migrationCount); + self::assertSame(1, $migrationsExecuted); + } + /** * @dataProvider dataMigrationFileList * @runInSeparateProcess From 285b630c37630e5c348a95c019edb8830cc64866 Mon Sep 17 00:00:00 2001 From: Greg Bowler Date: Sat, 4 Oct 2025 14:22:37 +0100 Subject: [PATCH 2/4] feature: check integrity with optional skip-to point for #114 --- src/Cli/ExecuteCommand.php | 3 ++ src/Migration/Migrator.php | 32 +++++++++---------- test/phpunit/Migration/MigratorTest.php | 41 +++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 17 deletions(-) diff --git a/src/Cli/ExecuteCommand.php b/src/Cli/ExecuteCommand.php index d39b4be..45c0e5d 100644 --- a/src/Cli/ExecuteCommand.php +++ b/src/Cli/ExecuteCommand.php @@ -57,6 +57,9 @@ public function run(?ArgumentValueList $arguments = null):void { $migrationCount = $migrator->getMigrationCount(); $migrationFileList = $migrator->getMigrationFileList(); +// TODO: Expected functionality is to provide a number to reset like --reset=15, OR leave it blank like --reset +// If number is provided, ignore everything before this number. +// If number is not provided, ignore everything other than the latest file in the directory. $resetNumber = null; if($arguments->contains("reset")) { $resetNumber = $arguments->get("reset")->get(); diff --git a/src/Migration/Migrator.php b/src/Migration/Migrator.php index f874b81..1db85fa 100644 --- a/src/Migration/Migrator.php +++ b/src/Migration/Migrator.php @@ -150,28 +150,26 @@ public function checkIntegrity( foreach($migrationFileList as $i => $file) { $fileNumber = $this->extractNumberFromFilename($file); - $md5 = md5_file($file); - if($migrationStartFrom) { - if($fileNumber < $migrationStartFrom) { - continue; - } + // 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; } - if(is_null($migrationStartFrom) - || $fileNumber <= $migrationStartFrom) { - $result = $this->dbClient->executeSql(implode("\n", [ - "select `" . self::COLUMN_QUERY_HASH . "`", - "from `{$this->tableName}`", - "where `" . self::COLUMN_QUERY_NUMBER . "` = ?", - "limit 1", - ]), [$fileNumber]); + $md5 = md5_file($file); - $hashInDb = ($result->fetch())?->getString(self::COLUMN_QUERY_HASH); + $result = $this->dbClient->executeSql(implode("\n", [ + "select `" . self::COLUMN_QUERY_HASH . "`", + "from `{$this->tableName}`", + "where `" . self::COLUMN_QUERY_NUMBER . "` = ?", + "limit 1", + ]), [$fileNumber]); - if($hashInDb && $hashInDb !== $md5) { - throw new MigrationIntegrityException($file); - } + $hashInDb = ($result->fetch())?->getString(self::COLUMN_QUERY_HASH); + + if($hashInDb && $hashInDb !== $md5) { + throw new MigrationIntegrityException($file); } } diff --git a/test/phpunit/Migration/MigratorTest.php b/test/phpunit/Migration/MigratorTest.php index aa828e3..f380d8e 100644 --- a/test/phpunit/Migration/MigratorTest.php +++ b/test/phpunit/Migration/MigratorTest.php @@ -279,6 +279,47 @@ public function testForcedMigration(array $fileList) { self::assertNull($exception,"Exception should not be thrown"); } + /** @dataProvider dataMigrationFileList */ + public function testHashMismatchAfterEditingFirstFile(array $fileList) { + $path = $this->getMigrationDirectory(); + $this->createMigrationFiles($fileList, $path); + + $settings = $this->createSettings($path); + $migrator = new Migrator($settings, $path); + $absoluteFileList = array_map(function($file) use ($path) { + return implode(DIRECTORY_SEPARATOR, [ + $path, + $file, + ]); + }, $fileList); + +// Perform the initial full migration. + $migrator->createMigrationTable(); + $migrator->performMigration($absoluteFileList); + +// Now change the contents of the first migration file to break integrity. + $firstFile = $absoluteFileList[0]; + $originalSql = file_get_contents($firstFile); + file_put_contents($firstFile, $originalSql . "\n-- edited to break hash\n"); + +// First, when providing the current migration count (skipping +// already-migrated files), the integrity check should NOT throw an exception +// because it skips the altered first file. + $exception = null; + $migrationCount = $migrator->getMigrationCount(); + try { + $migrator->checkIntegrity($absoluteFileList, $migrationCount); + } + catch(Exception $exception) {} + + self::assertNull($exception); + +// However, checking integrity with no migration count should fail due to a hash +// mismatch in the first file. + self::expectException(MigrationIntegrityException::class); + $migrator->checkIntegrity($absoluteFileList); + } + /** * This test needs an explanation because it's not immediately obvious. * The fileList is generated as usual, but then to simulate a real From b6768d1a26162e0d7c68b39c277162a929d33570 Mon Sep 17 00:00:00 2001 From: Greg Bowler Date: Sat, 4 Oct 2025 15:44:30 +0100 Subject: [PATCH 3/4] test: cli tests for reset flag --- src/Cli/ExecuteCommand.php | 5 +- src/Migration/Migrator.php | 16 +- test/phpunit/Cli/ExecuteCommandTest.php | 192 ++++++++++++++++++++++++ test/phpunit/Migration/MigratorTest.php | 124 ++++++++++++++- 4 files changed, 328 insertions(+), 9 deletions(-) create mode 100644 test/phpunit/Cli/ExecuteCommandTest.php diff --git a/src/Cli/ExecuteCommand.php b/src/Cli/ExecuteCommand.php index 45c0e5d..ed934b7 100644 --- a/src/Cli/ExecuteCommand.php +++ b/src/Cli/ExecuteCommand.php @@ -65,7 +65,10 @@ public function run(?ArgumentValueList $arguments = null):void { $resetNumber = $arguments->get("reset")->get(); if(!$resetNumber) { $lastKey = array_key_last($migrationFileList); - $resetNumber = $migrator->extractNumberFromFilename($migrationFileList[$lastKey]); + $lastNumber = $migrator->extractNumberFromFilename($migrationFileList[$lastKey]); + // When no number provided, execute only the latest migration by + // setting the reset point to one less than the latest number. + $resetNumber = max(0, $lastNumber - 1); } $resetNumber = (int)$resetNumber; } diff --git a/src/Migration/Migrator.php b/src/Migration/Migrator.php index 1db85fa..e24b5a4 100644 --- a/src/Migration/Migrator.php +++ b/src/Migration/Migrator.php @@ -125,19 +125,23 @@ public function getMigrationFileList():array { /** @param array $fileList */ public function checkFileListOrder(array $fileList):void { - $counter = 0; + $previousNumber = null; $sequence = []; foreach($fileList as $file) { - $counter++; $migrationNumber = $this->extractNumberFromFilename($file); $sequence []= $migrationNumber; - if($counter !== $migrationNumber) { - throw new MigrationSequenceOrderException( - "Missing: $counter" - ); + 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; } } diff --git a/test/phpunit/Cli/ExecuteCommandTest.php b/test/phpunit/Cli/ExecuteCommandTest.php new file mode 100644 index 0000000..96a51af --- /dev/null +++ b/test/phpunit/Cli/ExecuteCommandTest.php @@ -0,0 +1,192 @@ + $stream, + "in" => $in, + "out" => $out, + "err" => $err, + ]; + } + + private function readFromFiles(string $outPath, string $errPath):array { + $out = new SplFileObject($outPath, "r"); + $err = new SplFileObject($errPath, "r"); + $out->rewind(); + $err->rewind(); + return [ + "out" => $out->fread(8192), + "err" => $err->fread(8192), + ]; + } + + public function testExecuteMigratesAll():void { + $project = $this->createProjectDir(); + $sqlitePath = str_replace("\\", "/", $project . DIRECTORY_SEPARATOR . "cli-test.db"); + $this->writeConfigIni($project, $sqlitePath); + $this->createMigrations($project, 3); + + $cwdBackup = getcwd(); + chdir($project); + try { + $cmd = new ExecuteCommand(); + $streams = $this->makeStreamFiles(); + $cmd->setStream($streams["stream"]); + + $args = new ArgumentValueList(); + // No additional params; simply run + $cmd->run($args); + + list("out" => $out) = $this->readFromFiles($streams["out"], $streams["err"]); + self::assertStringContainsString("Migration 1:", $out); + self::assertStringContainsString("3 migrations were completed successfully.", $out); + + // Verify DB state + $settings = new Settings($project . DIRECTORY_SEPARATOR . "query", Settings::DRIVER_SQLITE, $sqlitePath); + $db = new Database($settings); + $result = $db->executeSql("PRAGMA table_info(test);"); + self::assertGreaterThanOrEqual(4, count($result->fetchAll())); + } + finally { + chdir($cwdBackup); + } + } + + 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); + + // Prepare base state: create table so we can skip first migration safely. + $settings = new Settings($project . DIRECTORY_SEPARATOR . "query", Settings::DRIVER_SQLITE, $sqlitePath); + $db = new Database($settings); + $db->executeSql(self::MIGRATION_CREATE); + + $cwdBackup = getcwd(); + chdir($project); + try { + $cmd = new ExecuteCommand(); + $streams = $this->makeStreamFiles(); + $cmd->setStream($streams["stream"]); + + $args = new ArgumentValueList(); + $args->set("reset"); // No number provided: should reset to latest migration number + + $cmd->run($args); + + list("out" => $out) = $this->readFromFiles($streams["out"], $streams["err"]); + // Should only execute the last migration + self::assertMatchesRegularExpression("/Migration\\s+4:/", $out); + self::assertStringContainsString("1 migrations were completed successfully.", $out); + } + finally { + chdir($cwdBackup); + } + } + + public function testExecuteWithResetWithNumber():void { + $project = $this->createProjectDir(); + $sqlitePath = str_replace("\\", "/", $project . DIRECTORY_SEPARATOR . "cli-test.db"); + $this->writeConfigIni($project, $sqlitePath); + $migrations = $this->createMigrations($project, 5); + + // Prepare base state when skipping the initial migrations. + $settings = new Settings($project . DIRECTORY_SEPARATOR . "query", Settings::DRIVER_SQLITE, $sqlitePath); + $db = new Database($settings); + $db->executeSql(self::MIGRATION_CREATE); + + $cwdBackup = getcwd(); + chdir($project); + try { + $cmd = new ExecuteCommand(); + $streams = $this->makeStreamFiles(); + $cmd->setStream($streams["stream"]); + + $args = new ArgumentValueList(); + $args->set("reset", "3"); // Should migrate from 4 and 5 only + + $cmd->run($args); + + list("out" => $out) = $this->readFromFiles($streams["out"], $streams["err"]); + self::assertStringNotContainsString("Migration 1:", $out); + self::assertStringNotContainsString("Migration 2:", $out); + self::assertStringNotContainsString("Migration 3:", $out); + self::assertStringContainsString("Migration 4:", $out); + self::assertStringContainsString("Migration 5:", $out); + self::assertStringContainsString("2 migrations were completed successfully.", $out); + } + finally { + chdir($cwdBackup); + } + } +} diff --git a/test/phpunit/Migration/MigratorTest.php b/test/phpunit/Migration/MigratorTest.php index f380d8e..92c0c1c 100644 --- a/test/phpunit/Migration/MigratorTest.php +++ b/test/phpunit/Migration/MigratorTest.php @@ -114,8 +114,12 @@ public function testCheckFileListOrderMissing(array $fileList) { $settings = $this->createSettings($path); $migrator = new Migrator($settings, $path); $actualFileList = $migrator->getMigrationFileList(); - self::expectException(MigrationSequenceOrderException::class); - $migrator->checkFileListOrder($actualFileList); + $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"); } /** @dataProvider dataMigrationFileListDuplicate */ @@ -130,6 +134,29 @@ public function testCheckFileListOrderDuplicate(array $fileList) { $migrator->checkFileListOrder($actualFileList); } + public function testCheckFileListOrderOutOfOrder():void { + $path = $this->getMigrationDirectory(); + $files = [ + str_pad(1, 4, "0", STR_PAD_LEFT) . "-" . uniqid() . ".sql", + str_pad(2, 4, "0", STR_PAD_LEFT) . "-" . uniqid() . ".sql", + str_pad(3, 4, "0", STR_PAD_LEFT) . "-" . uniqid() . ".sql", + ]; + $this->createFiles($files, $path); + + $settings = $this->createSettings($path); + $migrator = new Migrator($settings, $path); + + $absolute = array_map(function($f) use ($path) { + return implode(DIRECTORY_SEPARATOR, [$path, $f]); + }, $files); + + // Pass files deliberately out of numeric order: 1, 3, 2 + $outOfOrder = [$absolute[0], $absolute[2], $absolute[1]]; + + $this->expectException(MigrationSequenceOrderException::class); + $migrator->checkFileListOrder($outOfOrder); + } + /** @dataProvider dataMigrationFileList */ public function testCheckIntegrityGood(array $fileList) { $path = $this->getMigrationDirectory(); @@ -794,6 +821,99 @@ protected function createSettings(string $path):Settings { ); } + /** + * New tests for migrating from a specific file number and handling gaps + */ + /** @dataProvider dataMigrationFileList */ + public function testPerformMigrationFromSpecificNumber(array $fileList) { + $path = $this->getMigrationDirectory(); + $this->createMigrationFiles($fileList, $path); + $settings = $this->createSettings($path); + $migrator = new Migrator($settings, $path); + + $absoluteFileList = array_map(function($file) use ($path) { + return implode(DIRECTORY_SEPARATOR, [ $path, $file ]); + }, $fileList); + + $startNumber = $migrator->extractNumberFromFilename($absoluteFileList[0]); + $from = $startNumber - 1; + + $migrator->createMigrationTable(); + if($from >= 1) { + // Ensure base table exists when skipping the first migration + $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($absoluteFileList as $file) { + if($migrator->extractNumberFromFilename($file) >= $startNumber) { + $expected++; + } + } + + $streamOut->rewind(); + $output = $streamOut->fread(4096); + self::assertMatchesRegularExpression("/Migration\\s+{$startNumber}:/", $output); + self::assertStringContainsString("$expected migrations were completed successfully.", $output); + self::assertSame($expected, $executed); + self::assertSame($expected, $migrator->getMigrationCount()); + } + + /** @dataProvider dataMigrationFileListMissing */ + public function testPerformMigrationFromSpecificNumberWithGaps(array $fileList) { + $path = $this->getMigrationDirectory(); + $this->createMigrationFiles($fileList, $path); + $settings = $this->createSettings($path); + $migrator = new Migrator($settings, $path); + + $absoluteFileList = array_map(function($file) use ($path) { + 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); + } + protected function createFiles(array $files, string $path):void { foreach($files as $filename) { $pathName = implode(DIRECTORY_SEPARATOR, [ From 17c264f2e93293fb6d956e35b623ebd03421490e Mon Sep 17 00:00:00 2001 From: Greg Bowler Date: Sat, 4 Oct 2025 18:15:54 +0100 Subject: [PATCH 4/4] feature: reset hashing in database closes #114 --- src/Cli/ExecuteCommand.php | 90 ++++++++++++++++++++++++++------------ src/Migration/Migrator.php | 8 ++-- 2 files changed, 66 insertions(+), 32 deletions(-) diff --git a/src/Cli/ExecuteCommand.php b/src/Cli/ExecuteCommand.php index ed934b7..4fd6d5a 100644 --- a/src/Cli/ExecuteCommand.php +++ b/src/Cli/ExecuteCommand.php @@ -14,19 +14,45 @@ class ExecuteCommand extends Command { public function run(?ArgumentValueList $arguments = null):void { - $forced = $arguments->contains("force"); - $repoBasePath = getcwd(); $defaultPath = $this->getDefaultPath($repoBasePath); - $config = $this->getConfig($repoBasePath, $defaultPath); - $settings = new Settings( + $settings = $this->buildSettingsFromConfig($config, $repoBasePath); + [$migrationPath, $migrationTable] = $this->getMigrationLocation($config, $repoBasePath); + + $migrator = new Migrator($settings, $migrationPath, $migrationTable); + $migrator->setOutput( + $this->stream->getOutStream(), + $this->stream->getErrorStream() + ); + + if($this->isForced($arguments)) { + $migrator->deleteAndRecreateSchema(); + } + + $migrator->selectSchema(); + $migrator->createMigrationTable(); + $migrationCount = $migrator->getMigrationCount(); + $migrationFileList = $migrator->getMigrationFileList(); + + $runFrom = $this->calculateResetNumber($arguments, $migrationFileList, $migrator, $migrationCount); + + $this->executeMigrations($migrator, $migrationFileList, $runFrom); + } + + /** Determine whether the --force flag was provided. */ + private function isForced(?ArgumentValueList $arguments):bool { + return $arguments?->contains("force") ?? false; + } + + /** Build Settings from config for the current repository. */ + private function buildSettingsFromConfig(\Gt\Config\Config $config, string $repoBasePath): Settings { + return new Settings( implode(DIRECTORY_SEPARATOR, [ $repoBasePath, $config->get("database.query_path") ]), - $config->get("database.driver") ?? 'mysql', $config->get("database.schema"), $config->get("database.host") ?? "localhost", @@ -34,48 +60,56 @@ public function run(?ArgumentValueList $arguments = null):void { $config->get("database.username"), $config->get("database.password") ); + } + /** + * Return [migrationPath, migrationTable] derived from config. + * + * @return list + */ + private function getMigrationLocation(\Gt\Config\Config $config, string $repoBasePath): array { $migrationPath = implode(DIRECTORY_SEPARATOR, [ $repoBasePath, $config->get("database.query_path") ?? "query", $config->get("database.migration_path") ?? "_migration", ]); $migrationTable = $config->get("database.migration_table") ?? "_migration"; + return [$migrationPath, $migrationTable]; + } - $migrator = new Migrator($settings, $migrationPath, $migrationTable); - $migrator->setOutput( - $this->stream->getOutStream(), - $this->stream->getErrorStream() - ); - - if($forced) { - $migrator->deleteAndRecreateSchema(); - } - - $migrator->selectSchema(); - $migrator->createMigrationTable(); - $migrationCount = $migrator->getMigrationCount(); - $migrationFileList = $migrator->getMigrationFileList(); - -// TODO: Expected functionality is to provide a number to reset like --reset=15, OR leave it blank like --reset -// If number is provided, ignore everything before this number. -// If number is not provided, ignore everything other than the latest file in the directory. + /** + * Calculate the migration start point from --reset or current migration count. + * + * @param list $migrationFileList + */ + private function calculateResetNumber( + ?ArgumentValueList $arguments, + array $migrationFileList, + Migrator $migrator, + int $migrationCount + ): int { $resetNumber = null; - if($arguments->contains("reset")) { + if($arguments?->contains("reset")) { $resetNumber = $arguments->get("reset")->get(); if(!$resetNumber) { $lastKey = array_key_last($migrationFileList); $lastNumber = $migrator->extractNumberFromFilename($migrationFileList[$lastKey]); - // When no number provided, execute only the latest migration by - // setting the reset point to one less than the latest number. $resetNumber = max(0, $lastNumber - 1); } $resetNumber = (int)$resetNumber; } + return $resetNumber ?? $migrationCount; + } + /** + * Wrap integrity check and perform migration with error handling. + * + * @param list $migrationFileList + */ + private function executeMigrations(Migrator $migrator, array $migrationFileList, int $runFrom): void { try { - $migrator->checkIntegrity($migrationFileList, $resetNumber ?? $migrationCount); - $migrator->performMigration($migrationFileList, $resetNumber ?? $migrationCount); + $migrator->checkIntegrity($migrationFileList, $runFrom); + $migrator->performMigration($migrationFileList, $runFrom); } catch(MigrationIntegrityException $exception) { $this->writeLine( diff --git a/src/Migration/Migrator.php b/src/Migration/Migrator.php index e24b5a4..28df6ae 100644 --- a/src/Migration/Migrator.php +++ b/src/Migration/Migrator.php @@ -151,8 +151,8 @@ public function checkIntegrity( ?int $migrationStartFrom = null ):int { $fileNumber = 0; - - foreach($migrationFileList as $i => $file) { + + foreach($migrationFileList as $file) { $fileNumber = $this->extractNumberFromFilename($file); // If a start point is provided, skip files at or before that number @@ -199,8 +199,8 @@ public function performMigration( ):int { $fileNumber = 0; $numCompleted = 0; - - foreach($migrationFileList as $i => $file) { + + foreach($migrationFileList as $file) { $fileNumber = $this->extractNumberFromFilename($file); if($fileNumber <= $existingFileNumber) {