From 62b88cd0a3488cd70c7539c1e2c99c062afcc114 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sat, 28 Sep 2024 23:40:38 -0400 Subject: [PATCH 1/4] Fix up returns and doctypes --- src/Shim/SeedAdapter.php | 95 +++++++++------------------------------- 1 file changed, 20 insertions(+), 75 deletions(-) diff --git a/src/Shim/SeedAdapter.php b/src/Shim/SeedAdapter.php index e404bc39..c33840e8 100644 --- a/src/Shim/SeedAdapter.php +++ b/src/Shim/SeedAdapter.php @@ -38,7 +38,7 @@ class SeedAdapter implements SeedInterface { /** - * A ConsoleIo instance + * A migrations adapter instance * * @var \Migrations\Db\Adapter\AdapterInterface|null */ @@ -98,10 +98,7 @@ public function getDependencies(): array } /** - * Sets the database adapter. - * - * @param \Migrations\Db\Adapter\AdapterInterface $adapter Database Adapter - * @return $this + * {@inheritDoc} */ public function setAdapter(AdapterInterface $adapter) { @@ -113,9 +110,7 @@ public function setAdapter(AdapterInterface $adapter) } /** - * Gets the database adapter. - * - * @return \Migrations\Db\Adapter\AdapterInterface + * {@inheritDoc} */ public function getAdapter(): AdapterInterface { @@ -146,9 +141,7 @@ public function getIo(): ?ConsoleIo } /** - * Gets the config. - * - * @return ?\Migrations\Config\ConfigInterface + * {@inheritDoc} */ public function getConfig(): ?ConfigInterface { @@ -156,10 +149,7 @@ public function getConfig(): ?ConfigInterface } /** - * Sets the config. - * - * @param \Migrations\Config\ConfigInterface $config Configuration Object - * @return $this + * {@inheritDoc} */ public function setConfig(ConfigInterface $config) { @@ -181,9 +171,7 @@ public function setConfig(ConfigInterface $config) } /** - * Gets the name. - * - * @return string + * {@inheritDoc} */ public function getName(): string { @@ -191,11 +179,7 @@ public function getName(): string } /** - * Executes a SQL statement and returns the number of affected rows. - * - * @param string $sql SQL - * @param array $params parameters to use for prepared query - * @return int + * {@inheritDoc} */ public function execute(string $sql, array $params = []): int { @@ -203,75 +187,47 @@ public function execute(string $sql, array $params = []): int } /** - * Executes a SQL statement. - * - * The return type depends on the underlying adapter being used. To improve - * IDE auto-completion possibility, you can overwrite the query method - * phpDoc in your (typically custom abstract parent) seed class, where - * you can set the return type by the adapter in your current use. - * - * @param string $sql SQL - * @param array $params parameters to use for prepared query - * @return mixed + * {@inheritDoc} */ public function query(string $sql, array $params = []): mixed { - return $this->query($sql, $params); + return $this->seed->query($sql, $params); } /** - * Executes a query and returns only one row as an array. - * - * @param string $sql SQL - * @return array|false + * {@inheritDoc} */ public function fetchRow(string $sql): array|false { - return $this->fetchRow($sql); + return $this->seed->fetchRow($sql); } /** - * Executes a query and returns an array of rows. - * - * @param string $sql SQL - * @return array + * {@inheritDoc} */ public function fetchAll(string $sql): array { - return $this->fetchAll($sql); + return $this->seed->fetchAll($sql); } /** - * Insert data into a table. - * - * @param string $tableName Table name - * @param array $data Data - * @return void + * {@inheritDoc} */ public function insert(string $tableName, array $data): void { - $this->insert($tableName, $data); + $this->seed->insert($tableName, $data); } /** - * Checks to see if a table exists. - * - * @param string $tableName Table name - * @return bool + * {@inheritDoc} */ public function hasTable(string $tableName): bool { - return $this->hasTable($tableName); + return $this->seed->hasTable($tableName); } /** - * Returns an instance of the \Table class. - * - * You can use this class to create and manipulate tables. - * - * @param string $tableName Table name - * @param array $options Options - * @return \Migrations\Db\Table + * {@inheritDoc} */ public function table(string $tableName, array $options): Table { @@ -279,13 +235,7 @@ public function table(string $tableName, array $options): Table } /** - * Checks to see if the seed should be executed. - * - * Returns true by default. - * - * You can use this to prevent a seed from executing. - * - * @return bool + * {@inheritDoc} */ public function shouldExecute(): bool { @@ -293,12 +243,7 @@ public function shouldExecute(): bool } /** - * Gives the ability to a seeder to call another seeder. - * This is particularly useful if you need to run the seeders of your applications in a specific sequences, - * for instance to respect foreign key constraints. - * - * @param string $seeder Name of the seeder to call from the current seed - * @return void + * {@inheritDoc} */ public function call(string $seeder): void { From 131c717c307ed2df4f42d8bc3718cd955d05a18e Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sat, 28 Sep 2024 23:40:49 -0400 Subject: [PATCH 2/4] Move phinx compatibility shim logic into a migration wrapper Start adapting the internals for Migrations to not rely on phinx. Like seeds, there will be a wrapping adapter to shim the behavior up. --- src/Db/Adapter/AdapterInterface.php | 10 +- src/Db/Adapter/AdapterWrapper.php | 2 +- src/Db/Adapter/PdoAdapter.php | 4 +- src/Db/Adapter/SqlserverAdapter.php | 4 +- src/Migration/Environment.php | 17 +- src/Migration/Manager.php | 43 +- src/MigrationInterface.php | 24 +- src/Shim/MigrationAdapter.php | 430 +++++++++++++++++++ tests/TestCase/Migration/EnvironmentTest.php | 5 +- tests/TestCase/Migration/ManagerTest.php | 14 +- 10 files changed, 470 insertions(+), 83 deletions(-) create mode 100644 src/Shim/MigrationAdapter.php diff --git a/src/Db/Adapter/AdapterInterface.php b/src/Db/Adapter/AdapterInterface.php index 96d4f538..fcc67cc7 100644 --- a/src/Db/Adapter/AdapterInterface.php +++ b/src/Db/Adapter/AdapterInterface.php @@ -18,7 +18,7 @@ use Migrations\Db\Literal; use Migrations\Db\Table\Column; use Migrations\Db\Table\Table; -use Phinx\Migration\MigrationInterface; +use Migrations\MigrationInterface; /** * Adapter Interface. @@ -138,7 +138,7 @@ public function getColumnForType(string $columnName, string $type, array $option /** * Records a migration being run. * - * @param \Phinx\Migration\MigrationInterface $migration Migration + * @param \Migrations\MigrationInterface $migration Migration * @param string $direction Direction * @param string $startTime Start Time * @param string $endTime End Time @@ -149,7 +149,7 @@ public function migrated(MigrationInterface $migration, string $direction, strin /** * Toggle a migration breakpoint. * - * @param \Phinx\Migration\MigrationInterface $migration Migration + * @param \Migrations\MigrationInterface $migration Migration * @return $this */ public function toggleBreakpoint(MigrationInterface $migration); @@ -164,7 +164,7 @@ public function resetAllBreakpoints(): int; /** * Set a migration breakpoint. * - * @param \Phinx\Migration\MigrationInterface $migration The migration target for the breakpoint set + * @param \Migrations\MigrationInterface $migration The migration target for the breakpoint set * @return $this */ public function setBreakpoint(MigrationInterface $migration); @@ -172,7 +172,7 @@ public function setBreakpoint(MigrationInterface $migration); /** * Unset a migration breakpoint. * - * @param \Phinx\Migration\MigrationInterface $migration The migration target for the breakpoint unset + * @param \Migrations\MigrationInterface $migration The migration target for the breakpoint unset * @return $this */ public function unsetBreakpoint(MigrationInterface $migration); diff --git a/src/Db/Adapter/AdapterWrapper.php b/src/Db/Adapter/AdapterWrapper.php index 42e4f83e..7fe5ba63 100644 --- a/src/Db/Adapter/AdapterWrapper.php +++ b/src/Db/Adapter/AdapterWrapper.php @@ -18,7 +18,7 @@ use Migrations\Db\Literal; use Migrations\Db\Table\Column; use Migrations\Db\Table\Table; -use Phinx\Migration\MigrationInterface; +use Migrations\MigrationInterface; /** * Adapter Wrapper. diff --git a/src/Db/Adapter/PdoAdapter.php b/src/Db/Adapter/PdoAdapter.php index d1a462a5..1c30f755 100644 --- a/src/Db/Adapter/PdoAdapter.php +++ b/src/Db/Adapter/PdoAdapter.php @@ -36,10 +36,10 @@ use Migrations\Db\Table\ForeignKey; use Migrations\Db\Table\Index; use Migrations\Db\Table\Table; +use Migrations\MigrationInterface; use PDO; use PDOException; use Phinx\Config\Config; -use Phinx\Migration\MigrationInterface; use Phinx\Util\Literal as PhinxLiteral; use ReflectionMethod; use RuntimeException; @@ -570,7 +570,7 @@ public function unsetBreakpoint(MigrationInterface $migration): AdapterInterface /** * Mark a migration breakpoint. * - * @param \Phinx\Migration\MigrationInterface $migration The migration target for the breakpoint + * @param \Migrations\MigrationInterface $migration The migration target for the breakpoint * @param bool $state The required state of the breakpoint * @return \Migrations\Db\Adapter\AdapterInterface */ diff --git a/src/Db/Adapter/SqlserverAdapter.php b/src/Db/Adapter/SqlserverAdapter.php index f3667c3e..99f0ec4f 100644 --- a/src/Db/Adapter/SqlserverAdapter.php +++ b/src/Db/Adapter/SqlserverAdapter.php @@ -16,7 +16,7 @@ use Migrations\Db\Table\ForeignKey; use Migrations\Db\Table\Index; use Migrations\Db\Table\Table; -use Phinx\Migration\MigrationInterface; +use Migrations\MigrationInterface; /** * Migrations SqlServer Adapter. @@ -1238,7 +1238,7 @@ public function getColumnTypes(): array /** * Records a migration being run. * - * @param \Phinx\Migration\MigrationInterface $migration Migration + * @param \Migrations\MigrationInterface $migration Migration * @param string $direction Direction * @param string $startTime Start Time * @param string $endTime End Time diff --git a/src/Migration/Environment.php b/src/Migration/Environment.php index 25bc8a5f..6077cba9 100644 --- a/src/Migration/Environment.php +++ b/src/Migration/Environment.php @@ -14,7 +14,7 @@ use Migrations\Db\Adapter\AdapterInterface; use Migrations\Db\Adapter\PhinxAdapter; use Migrations\SeedInterface; -use Phinx\Migration\MigrationInterface; +use Migrations\MigrationInterface; use RuntimeException; class Environment @@ -62,7 +62,7 @@ public function __construct(string $name, array $options) /** * Executes the specified migration on this environment. * - * @param \Phinx\Migration\MigrationInterface $migration Migration + * @param \Migrations\MigrationInterface $migration Migration * @param string $direction Direction * @param bool $fake flag that if true, we just record running the migration, but not actually do the migration * @return void @@ -73,11 +73,11 @@ public function executeMigration(MigrationInterface $migration, string $directio $migration->setMigratingUp($direction === MigrationInterface::UP); $startTime = time(); + // Use an adapter shim to bridge between the new migrations // engine and the Phinx compatible interface $adapter = $this->getAdapter(); - $phinxShim = new PhinxAdapter($adapter); - $migration->setAdapter($phinxShim); + $migration->setAdapter($adapter); $migration->preFlightCheck(); @@ -91,6 +91,10 @@ public function executeMigration(MigrationInterface $migration, string $directio } if (!$fake) { + // TODO this is tricky as the adapter would need to implement + // this method, but then it always exists. One option is to copy this + // is to special case the adapter and move this logic there? + // Run the migration if (method_exists($migration, MigrationInterface::CHANGE)) { if ($direction === MigrationInterface::DOWN) { @@ -102,13 +106,12 @@ public function executeMigration(MigrationInterface $migration, string $directio ->getWrapper('record', $adapter); // Wrap the adapter with a phinx shim to maintain contain - $phinxAdapter = new PhinxAdapter($recordAdapter); - $migration->setAdapter($phinxAdapter); + $migration->setAdapter($adapter); $migration->{MigrationInterface::CHANGE}(); $recordAdapter->executeInvertedCommands(); - $migration->setAdapter(new PhinxAdapter($this->getAdapter())); + $migration->setAdapter($this->getAdapter()); } else { $migration->{MigrationInterface::CHANGE}(); } diff --git a/src/Migration/Manager.php b/src/Migration/Manager.php index a50e2a93..a9ed2c96 100644 --- a/src/Migration/Manager.php +++ b/src/Migration/Manager.php @@ -14,16 +14,15 @@ use Exception; use InvalidArgumentException; use Migrations\Config\ConfigInterface; +use Migrations\MigrationInterface; use Migrations\SeedInterface; -use Migrations\Shim\OutputAdapter; +use Migrations\Shim\MigrationAdapter; use Migrations\Shim\SeedAdapter; -use Phinx\Migration\AbstractMigration; -use Phinx\Migration\MigrationInterface; +use Phinx\Migration\MigrationInterface as PhinxMigrationInterface; use Phinx\Seed\SeedInterface as PhinxSeedInterface; use Phinx\Util\Util; use Psr\Container\ContainerInterface; use RuntimeException; -use Symfony\Component\Console\Input\ArrayInput; class Manager { @@ -771,7 +770,7 @@ public function setContainer(ContainerInterface $container) /** * Sets the database migrations. * - * @param \Phinx\Migration\AbstractMigration[] $migrations Migrations + * @param \Migrations\MigrationInterface[] $migrations Migrations * @return $this */ public function setMigrations(array $migrations) @@ -786,7 +785,7 @@ public function setMigrations(array $migrations) * order * * @throws \InvalidArgumentException - * @return \Phinx\Migration\MigrationInterface[] + * @return \Migrations\MigrationInterface[] */ public function getMigrations(): array { @@ -806,7 +805,7 @@ function ($phpFile) { // filter the files to only get the ones that match our naming scheme $fileNames = []; - /** @var \Phinx\Migration\AbstractMigration[] $versions */ + /** @var \Migration\MigrationInterface[] $versions */ $versions = []; $io = $this->getIo(); @@ -850,29 +849,15 @@ function ($phpFile) { } $io->verbose("Constructing $class."); - - $config = $this->getConfig(); - // TODO Subset config and pass forward. - // Move this to the Migration/phinx shim - $input = new ArrayInput([ - '--plugin' => $config['plugin'] ?? null, - '--source' => $config['source'] ?? null, - '--connection' => $config->getConnection(), - ]); - // TODO move this to the migration/phinx shim - $output = new OutputAdapter($io); - - // TODO constructor should take $io and $config - // instantiate it - $migration = new $class('default', $version, $input, $output); - - if (!($migration instanceof AbstractMigration)) { - throw new InvalidArgumentException(sprintf( - 'The class "%s" in file "%s" must extend \Phinx\Migration\AbstractMigration', - $class, - $filePath - )); + /** @var \Migrations\MigrationInterface $migration */ + if (is_subclass_of($class, PhinxMigrationInterface::class)) { + $migration = new MigrationAdapter($class, $version); + } else { + $migration = new $class($version); } + $config = $this->getConfig(); + $migration->setConfig($config); + $migration->setIo($io); $versions[$version] = $migration; } else { diff --git a/src/MigrationInterface.php b/src/MigrationInterface.php index 2a8e505d..289a1065 100644 --- a/src/MigrationInterface.php +++ b/src/MigrationInterface.php @@ -80,9 +80,9 @@ public function getIo(): ?ConsoleIo; /** * Gets the config. * - * @return \Migrations\Config\ConfigInterface + * @return \Migrations\Config\ConfigInterface|null */ - public function getConfig(): ConfigInterface; + public function getConfig(): ?ConfigInterface; /** * Sets the config. @@ -92,26 +92,6 @@ public function getConfig(): ConfigInterface; */ public function setConfig(ConfigInterface $config); - /** - * Gets the input object to be used in migration object - * - * A new InputInterface will be generated each time `getOutput` is called. - * - * @return \Symfony\Component\Console\Input\InputInterface|null - * @deprecated 4.5.0 Use getIo() instead. - */ - public function getInput(): ?InputInterface; - - /** - * Gets the output object to be used in migration object - * - * A new OutputInterface will be generated each time `getOutput` is called. - * - * @return \Symfony\Component\Console\Output\OutputInterface|null - * @deprecated 4.5.0 Use getIo() instead. - */ - public function getOutput(): ?OutputInterface; - /** * Gets the name. * diff --git a/src/Shim/MigrationAdapter.php b/src/Shim/MigrationAdapter.php new file mode 100644 index 00000000..fbd31b1b --- /dev/null +++ b/src/Shim/MigrationAdapter.php @@ -0,0 +1,430 @@ +migration = new $migrationClass('default', $version); + } else { + if (!is_subclass_of($migrationClass, PhinxMigrationInterface::class)) { + throw new RuntimeException( + 'The provided $migrationClass must be a ' . + 'subclass of Phinx\Migration\MigrationInterface' + ); + } + $this->migration = $migrationClass; + } + } + + /** + * Because we're a compatibility shim, we implement this hook + * so that it can be conditionally called when it is implemented. + * + * @return void + */ + public function init(): void + { + if (method_exists($this->migration, MigrationInterface::INIT)) { + $this->migration->{MigrationInterface::INIT}(); + } + } + + /** + * {@inheritDoc} + */ + public function setAdapter(AdapterInterface $adapter) + { + $phinxAdapter = new PhinxAdapter($adapter); + $this->migration->setAdapter($phinxAdapter); + $this->adapter = $adapter; + + return $this; + } + + /** + * {@inheritDoc} + */ + public function getAdapter(): AdapterInterface + { + if (!$this->adapter) { + throw new RuntimeException('Cannot call getAdapter() until after setAdapter().'); + } + + return $this->adapter; + } + + /** + * {@inheritDoc} + */ + public function setIo(ConsoleIo $io) + { + $this->io = $io; + $this->migration->setOutput(new OutputAdapter($io)); + + return $this; + } + + /** + * {@inheritDoc} + */ + public function getIo(): ?ConsoleIo + { + return $this->io; + } + + /** + * {@inheritDoc} + */ + public function getConfig(): ?ConfigInterface + { + return $this->config; + } + + /** + * {@inheritDoc} + */ + public function setConfig(ConfigInterface $config) + { + $input = new ArrayInput([ + '--plugin' => $config['plugin'] ?? null, + '--source' => $config['source'] ?? null, + '--connection' => $config->getConnection(), + ]); + + $this->migration->setInput($input); + $this->config = $config; + + return $this; + } + + /** + * Gets the name. + * + * @return string + */ + public function getName(): string + { + return $this->migration->getName(); + } + + /** + * Sets the migration version number. + * + * @param int $version Version + * @return $this + */ + public function setVersion(int $version) + { + $this->migration->setVersion($version); + + return $this; + } + + /** + * Gets the migration version number. + * + * @return int + */ + public function getVersion(): int + { + return $this->migration->getVersion(); + } + + /** + * Sets whether this migration is being applied or reverted + * + * @param bool $isMigratingUp True if the migration is being applied + * @return $this + */ + public function setMigratingUp(bool $isMigratingUp) + { + $this->migration->setMigratingUp($isMigratingUp); + + return $this; + } + + /** + * Gets whether this migration is being applied or reverted. + * True means that the migration is being applied. + * + * @return bool + */ + public function isMigratingUp(): bool + { + return $this->migration->isMigratingUp(); + } + + /** + * Executes a SQL statement and returns the number of affected rows. + * + * @param string $sql SQL + * @param array $params parameters to use for prepared query + * @return int + */ + public function execute(string $sql, array $params = []): int + { + return $this->migration->execute($sql, $params); + } + + /** + * Executes a SQL statement. + * + * The return type depends on the underlying adapter being used. To improve + * IDE auto-completion possibility, you can overwrite the query method + * phpDoc in your (typically custom abstract parent) migration class, where + * you can set the return type by the adapter in your current use. + * + * @param string $sql SQL + * @param array $params parameters to use for prepared query + * @return mixed + */ + public function query(string $sql, array $params = []): mixed + { + return $this->migration->query($sql, $params); + } + + /** + * Returns a new Query object that can be used to build complex SELECT, UPDATE, INSERT or DELETE + * queries and execute them against the current database. + * + * Queries executed through the query builder are always sent to the database, regardless of the + * the dry-run settings. + * + * @see https://api.cakephp.org/3.6/class-Cake.Database.Query.html + * @param string $type Query + * @return \Cake\Database\Query + */ + public function getQueryBuilder(string $type): Query + { + return $this->migration->getQueryBuilder($type); + } + + /** + * Returns a new SelectQuery object that can be used to build complex + * SELECT queries and execute them against the current database. + * + * Queries executed through the query builder are always sent to the database, regardless of the + * the dry-run settings. + * + * @return \Cake\Database\Query\SelectQuery + */ + public function getSelectBuilder(): SelectQuery + { + return $this->migration->getSelectBuilder(); + } + + /** + * Returns a new InsertQuery object that can be used to build complex + * INSERT queries and execute them against the current database. + * + * Queries executed through the query builder are always sent to the database, regardless of the + * the dry-run settings. + * + * @return \Cake\Database\Query\InsertQuery + */ + public function getInsertBuilder(): InsertQuery + { + return $this->migration->getInsertBuilder(); + } + + /** + * Returns a new UpdateQuery object that can be used to build complex + * UPDATE queries and execute them against the current database. + * + * Queries executed through the query builder are always sent to the database, regardless of the + * the dry-run settings. + * + * @return \Cake\Database\Query\UpdateQuery + */ + public function getUpdateBuilder(): UpdateQuery + { + return $this->migration->getUpdateBuilder(); + } + + /** + * Returns a new DeleteQuery object that can be used to build complex + * DELETE queries and execute them against the current database. + * + * Queries executed through the query builder are always sent to the database, regardless of the + * the dry-run settings. + * + * @return \Cake\Database\Query\DeleteQuery + */ + public function getDeleteBuilder(): DeleteQuery + { + return $this->migration->getDeleteBuilder(); + } + + /** + * Executes a query and returns only one row as an array. + * + * @param string $sql SQL + * @return array|false + */ + public function fetchRow(string $sql): array|false + { + return $this->migration->fetchRow($sql); + } + + /** + * Executes a query and returns an array of rows. + * + * @param string $sql SQL + * @return array + */ + public function fetchAll(string $sql): array + { + return $this->migration->fetchAll($sql); + } + + /** + * Create a new database. + * + * @param string $name Database Name + * @param array $options Options + * @return void + */ + public function createDatabase(string $name, array $options): void + { + $this->migration->createDatabase($name, $options); + } + + /** + * Drop a database. + * + * @param string $name Database Name + * @return void + */ + public function dropDatabase(string $name): void + { + $this->migration->dropDatabase($name); + } + + /** + * {@inheritDoc} + */ + public function createSchema(string $name): void + { + $this->migration->createSchema($name); + } + + /** + * {@inheritDoc} + */ + public function dropSchema(string $name): void + { + $this->migration->dropSchema($name); + } + + /** + * {@inheritDoc} + */ + public function hasTable(string $tableName): bool + { + return $this->migration->hasTable($tableName); + } + + /** + * {@inheritDoc} + */ + public function table(string $tableName, array $options): Table + { + throw new RuntimeException('MigrationAdapter::table is not implemented'); + } + + /** + * {@inheritDoc} + */ + public function preFlightCheck(): void + { + $this->migration->preFlightCheck(); + } + + /** + * {@inheritDoc} + */ + public function postFlightCheck(): void + { + $this->migration->postFlightCheck(); + } + + /** + * {@inheritDoc} + */ + public function shouldExecute(): bool + { + return $this->migration->shouldExecute(); + } +} diff --git a/tests/TestCase/Migration/EnvironmentTest.php b/tests/TestCase/Migration/EnvironmentTest.php index 0778242e..72fceba0 100644 --- a/tests/TestCase/Migration/EnvironmentTest.php +++ b/tests/TestCase/Migration/EnvironmentTest.php @@ -8,6 +8,7 @@ use Migrations\Db\Adapter\AdapterWrapper; use Migrations\Db\Adapter\PdoAdapter; use Migrations\Migration\Environment; +use Migrations\Shim\MigrationAdapter; use Migrations\Shim\SeedAdapter; use Phinx\Migration\AbstractMigration; use Phinx\Migration\MigrationInterface; @@ -300,8 +301,8 @@ public function up(): void $this->upExecuted = true; } }; - - $this->environment->executeMigration($upMigration, MigrationInterface::UP); + $migrationWrapper = new MigrationAdapter($upMigration, $upMigration->getVersion()); + $this->environment->executeMigration($migrationWrapper, MigrationInterface::UP); $this->assertTrue($upMigration->initExecuted); $this->assertTrue($upMigration->upExecuted); } diff --git a/tests/TestCase/Migration/ManagerTest.php b/tests/TestCase/Migration/ManagerTest.php index b739ace8..ad654b15 100644 --- a/tests/TestCase/Migration/ManagerTest.php +++ b/tests/TestCase/Migration/ManagerTest.php @@ -2205,18 +2205,6 @@ public function testSeedWillNotBeExecuted(): void $this->assertStringContainsString('skipped', $output); } - public function testGettingInputObject(): void - { - $migrations = $this->manager->getMigrations(); - $seeds = $this->manager->getSeeds(); - foreach ($migrations as $migration) { - $this->assertInstanceOf(InputInterface::class, $migration->getInput()); - } - foreach ($seeds as $seed) { - $this->assertInstanceOf(InputInterface::class, $migration->getInput()); - } - } - public function testGettingIo(): void { $migrations = $this->manager->getMigrations(); @@ -2225,7 +2213,7 @@ public function testGettingIo(): void $this->assertInstanceOf(ConsoleIo::class, $io); foreach ($migrations as $migration) { - $this->assertInstanceOf(OutputAdapter::class, $migration->getOutput()); + $this->assertInstanceOf(ConsoleIo::class, $migration->getIo()); } foreach ($seeds as $seed) { $this->assertInstanceOf(ConsoleIo::class, $seed->getIo()); From 0629a5e3d34fe808a724456a9eb10e36070fe9dc Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sun, 29 Sep 2024 00:17:19 -0400 Subject: [PATCH 3/4] Get more tests passing --- src/Migration/Environment.php | 49 ++++++++++---------- src/Migration/Manager.php | 12 ++++- src/Shim/MigrationAdapter.php | 35 ++++++++++++++ tests/TestCase/Migration/EnvironmentTest.php | 18 ++++--- 4 files changed, 82 insertions(+), 32 deletions(-) diff --git a/src/Migration/Environment.php b/src/Migration/Environment.php index 6077cba9..0c1ff6a7 100644 --- a/src/Migration/Environment.php +++ b/src/Migration/Environment.php @@ -15,6 +15,7 @@ use Migrations\Db\Adapter\PhinxAdapter; use Migrations\SeedInterface; use Migrations\MigrationInterface; +use Migrations\Shim\MigrationAdapter; use RuntimeException; class Environment @@ -91,32 +92,32 @@ public function executeMigration(MigrationInterface $migration, string $directio } if (!$fake) { - // TODO this is tricky as the adapter would need to implement - // this method, but then it always exists. One option is to copy this - // is to special case the adapter and move this logic there? - - // Run the migration - if (method_exists($migration, MigrationInterface::CHANGE)) { - if ($direction === MigrationInterface::DOWN) { - // Create an instance of the RecordingAdapter so we can record all - // of the migration commands for reverse playback - - /** @var \Migrations\Db\Adapter\RecordingAdapter $recordAdapter */ - $recordAdapter = AdapterFactory::instance() - ->getWrapper('record', $adapter); - - // Wrap the adapter with a phinx shim to maintain contain - $migration->setAdapter($adapter); - - $migration->{MigrationInterface::CHANGE}(); - $recordAdapter->executeInvertedCommands(); - - $migration->setAdapter($this->getAdapter()); + if ($migration instanceof MigrationAdapter) { + $migration->applyDirection($direction); + } else { + // Run the migration + if (method_exists($migration, MigrationInterface::CHANGE)) { + if ($direction === MigrationInterface::DOWN) { + // Create an instance of the RecordingAdapter so we can record all + // of the migration commands for reverse playback + + /** @var \Migrations\Db\Adapter\RecordingAdapter $recordAdapter */ + $recordAdapter = AdapterFactory::instance() + ->getWrapper('record', $adapter); + + // Wrap the adapter with a phinx shim to maintain contain + $migration->setAdapter($adapter); + + $migration->{MigrationInterface::CHANGE}(); + $recordAdapter->executeInvertedCommands(); + + $migration->setAdapter($this->getAdapter()); + } else { + $migration->{MigrationInterface::CHANGE}(); + } } else { - $migration->{MigrationInterface::CHANGE}(); + $migration->{$direction}(); } - } else { - $migration->{$direction}(); } } diff --git a/src/Migration/Manager.php b/src/Migration/Manager.php index a9ed2c96..b18591df 100644 --- a/src/Migration/Manager.php +++ b/src/Migration/Manager.php @@ -257,11 +257,19 @@ public function markMigrated(int $version, string $path): bool /** @var class-string<\Phinx\Migration\MigrationInterface> $className */ $className = $this->getMigrationClassName($migrationFile); require_once $migrationFile; - $Migration = new $className('default', $version); + + /** @var \Migrations\MigrationInterface $migration */ + if (is_subclass_of($className, PhinxMigrationInterface::class)) { + $migration = new MigrationAdapter($className, $version); + } else { + $migration = new $className($version); + } + $config = $this->getConfig(); + $migration->setConfig($config); $time = date('Y-m-d H:i:s', time()); - $adapter->migrated($Migration, 'up', $time, $time); + $adapter->migrated($migration, 'up', $time, $time); return true; } diff --git a/src/Shim/MigrationAdapter.php b/src/Shim/MigrationAdapter.php index fbd31b1b..4452efae 100644 --- a/src/Shim/MigrationAdapter.php +++ b/src/Shim/MigrationAdapter.php @@ -19,6 +19,7 @@ use Migrations\Db\Adapter\PhinxAdapter; use Migrations\Db\Table; use Migrations\MigrationInterface; +use Phinx\Db\Adapter\AdapterFactory as PhinxAdapterFactory; use Phinx\Migration\MigrationInterface as PhinxMigrationInterface; use RuntimeException; use Symfony\Component\Console\Input\ArrayInput; @@ -101,6 +102,40 @@ public function init(): void } } + /** + * Compatibility shim for executing change/up/down + */ + public function applyDirection(string $direction): void + { + $adapter = $this->getAdapter(); + + // Run the migration + if (method_exists($this->migration, MigrationInterface::CHANGE)) { + if ($direction === MigrationInterface::DOWN) { + // Create an instance of the RecordingAdapter so we can record all + // of the migration commands for reverse playback + $adapter = $this->migration->getAdapter(); + assert($adapter !== null, 'Adapter must be set in migration'); + + /** @var \Phinx\Db\Adapter\ProxyAdapter $proxyAdapter */ + $proxyAdapter = PhinxAdapterFactory::instance() + ->getWrapper('proxy', $adapter); + + // Wrap the adapter with a phinx shim to maintain contain + $this->migration->setAdapter($proxyAdapter); + + $this->migration->{MigrationInterface::CHANGE}(); + $proxyAdapter->executeInvertedCommands(); + + $this->migration->setAdapter($adapter); + } else { + $this->migration->{MigrationInterface::CHANGE}(); + } + } else { + $this->migration->{$direction}(); + } + } + /** * {@inheritDoc} */ diff --git a/tests/TestCase/Migration/EnvironmentTest.php b/tests/TestCase/Migration/EnvironmentTest.php index 72fceba0..bb6a5c4e 100644 --- a/tests/TestCase/Migration/EnvironmentTest.php +++ b/tests/TestCase/Migration/EnvironmentTest.php @@ -131,7 +131,8 @@ public function up(): void } }; - $this->environment->executeMigration($upMigration, MigrationInterface::UP); + $migrationWrapper = new MigrationAdapter($upMigration, $upMigration->getVersion()); + $this->environment->executeMigration($migrationWrapper, MigrationInterface::UP); $this->assertTrue($upMigration->executed); } @@ -156,7 +157,8 @@ public function down(): void } }; - $this->environment->executeMigration($downMigration, MigrationInterface::DOWN); + $migrationWrapper = new MigrationAdapter($downMigration, $downMigration->getVersion()); + $this->environment->executeMigration($migrationWrapper, MigrationInterface::DOWN); $this->assertTrue($downMigration->executed); } @@ -187,7 +189,8 @@ public function up(): void } }; - $this->environment->executeMigration($migration, MigrationInterface::UP); + $migrationWrapper = new MigrationAdapter($migration, $migration->getVersion()); + $this->environment->executeMigration($migrationWrapper, MigrationInterface::UP); $this->assertTrue($migration->executed); } @@ -212,7 +215,8 @@ public function change(): void } }; - $this->environment->executeMigration($migration, MigrationInterface::UP); + $migrationWrapper = new MigrationAdapter($migration, $migration->getVersion()); + $this->environment->executeMigration($migrationWrapper, MigrationInterface::UP); $this->assertTrue($migration->executed); } @@ -237,7 +241,8 @@ public function change(): void } }; - $this->environment->executeMigration($migration, MigrationInterface::DOWN); + $migrationWrapper = new MigrationAdapter($migration, $migration->getVersion()); + $this->environment->executeMigration($migrationWrapper, MigrationInterface::DOWN); $this->assertTrue($migration->executed); } @@ -262,7 +267,8 @@ public function change(): void } }; - $this->environment->executeMigration($migration, MigrationInterface::UP, true); + $migrationWrapper = new MigrationAdapter($migration, $migration->getVersion()); + $this->environment->executeMigration($migrationWrapper, MigrationInterface::UP, true); $this->assertFalse($migration->executed); } From 46a344366838fa57d95162eb52eae1e5b66108f9 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Mon, 30 Sep 2024 09:04:17 -0400 Subject: [PATCH 4/4] Fix phpcs and psalm --- src/Db/Adapter/PhinxAdapter.php | 23 ++++++++++++++--------- src/Migration/Environment.php | 3 +-- src/Migration/Manager.php | 14 +++++++------- src/MigrationInterface.php | 2 -- src/Shim/MigrationAdapter.php | 1 - tests/TestCase/Migration/ManagerTest.php | 2 -- 6 files changed, 22 insertions(+), 23 deletions(-) diff --git a/src/Db/Adapter/PhinxAdapter.php b/src/Db/Adapter/PhinxAdapter.php index f3cf451d..492ca369 100644 --- a/src/Db/Adapter/PhinxAdapter.php +++ b/src/Db/Adapter/PhinxAdapter.php @@ -33,6 +33,7 @@ use Migrations\Db\Table\ForeignKey; use Migrations\Db\Table\Index; use Migrations\Db\Table\Table; +use Migrations\Shim\MigrationAdapter; use Phinx\Db\Action\Action as PhinxAction; use Phinx\Db\Action\AddColumn as PhinxAddColumn; use Phinx\Db\Action\AddForeignKey as PhinxAddForeignKey; @@ -52,7 +53,7 @@ use Phinx\Db\Table\ForeignKey as PhinxForeignKey; use Phinx\Db\Table\Index as PhinxIndex; use Phinx\Db\Table\Table as PhinxTable; -use Phinx\Migration\MigrationInterface; +use Phinx\Migration\MigrationInterface as PhinxMigrationInterface; use Phinx\Util\Literal as PhinxLiteral; use RuntimeException; use Symfony\Component\Console\Input\InputInterface; @@ -491,9 +492,10 @@ public function getVersionLog(): array /** * @inheritDoc */ - public function migrated(MigrationInterface $migration, string $direction, string $startTime, string $endTime): PhinxAdapterInterface + public function migrated(PhinxMigrationInterface $migration, string $direction, string $startTime, string $endTime): PhinxAdapterInterface { - $this->adapter->migrated($migration, $direction, $startTime, $endTime); + $wrapped = new MigrationAdapter($migration, $migration->getVersion()); + $this->adapter->migrated($wrapped, $direction, $startTime, $endTime); return $this; } @@ -501,9 +503,10 @@ public function migrated(MigrationInterface $migration, string $direction, strin /** * @inheritDoc */ - public function toggleBreakpoint(MigrationInterface $migration): PhinxAdapterInterface + public function toggleBreakpoint(PhinxMigrationInterface $migration): PhinxAdapterInterface { - $this->adapter->toggleBreakpoint($migration); + $wrapped = new MigrationAdapter($migration, $migration->getVersion()); + $this->adapter->toggleBreakpoint($wrapped); return $this; } @@ -519,9 +522,10 @@ public function resetAllBreakpoints(): int /** * @inheritDoc */ - public function setBreakpoint(MigrationInterface $migration): PhinxAdapterInterface + public function setBreakpoint(PhinxMigrationInterface $migration): PhinxAdapterInterface { - $this->adapter->setBreakpoint($migration); + $wrapped = new MigrationAdapter($migration, $migration->getVersion()); + $this->adapter->setBreakpoint($wrapped); return $this; } @@ -529,9 +533,10 @@ public function setBreakpoint(MigrationInterface $migration): PhinxAdapterInterf /** * @inheritDoc */ - public function unsetBreakpoint(MigrationInterface $migration): PhinxAdapterInterface + public function unsetBreakpoint(PhinxMigrationInterface $migration): PhinxAdapterInterface { - $this->adapter->unsetBreakpoint($migration); + $wrapped = new MigrationAdapter($migration, $migration->getVersion()); + $this->adapter->unsetBreakpoint($wrapped); return $this; } diff --git a/src/Migration/Environment.php b/src/Migration/Environment.php index 0c1ff6a7..28d72572 100644 --- a/src/Migration/Environment.php +++ b/src/Migration/Environment.php @@ -12,9 +12,8 @@ use Cake\Datasource\ConnectionManager; use Migrations\Db\Adapter\AdapterFactory; use Migrations\Db\Adapter\AdapterInterface; -use Migrations\Db\Adapter\PhinxAdapter; -use Migrations\SeedInterface; use Migrations\MigrationInterface; +use Migrations\SeedInterface; use Migrations\Shim\MigrationAdapter; use RuntimeException; diff --git a/src/Migration/Manager.php b/src/Migration/Manager.php index b18591df..59872621 100644 --- a/src/Migration/Manager.php +++ b/src/Migration/Manager.php @@ -46,7 +46,7 @@ class Manager protected ?Environment $environment; /** - * @var \Phinx\Migration\MigrationInterface[]|null + * @var \Migrations\MigrationInterface[]|null */ protected ?array $migrations = null; @@ -254,16 +254,16 @@ public function markMigrated(int $version, string $path): bool } $migrationFile = $migrationFile[0]; - /** @var class-string<\Phinx\Migration\MigrationInterface> $className */ + /** @var class-string<\Phinx\Migration\MigrationInterface|\Migrations\MigrationInterface> $className */ $className = $this->getMigrationClassName($migrationFile); require_once $migrationFile; - /** @var \Migrations\MigrationInterface $migration */ if (is_subclass_of($className, PhinxMigrationInterface::class)) { $migration = new MigrationAdapter($className, $version); } else { $migration = new $className($version); } + /** @var \Migrations\MigrationInterface $migration */ $config = $this->getConfig(); $migration->setConfig($config); @@ -448,7 +448,7 @@ public function migrate(?int $version = null, bool $fake = false): void /** * Execute a migration against the specified environment. * - * @param \Phinx\Migration\MigrationInterface $migration Migration + * @param \Migrations\MigrationInterface $migration Migration * @param string $direction Direction * @param bool $fake flag that if true, we just record running the migration, but not actually do the migration * @return void @@ -512,7 +512,7 @@ public function executeSeed(SeedInterface $seed): void /** * Print Migration Status * - * @param \Phinx\Migration\MigrationInterface $migration Migration + * @param \Migrations\MigrationInterface $migration Migration * @param string $status Status of the migration * @param string|null $duration Duration the migration took the be executed * @return void @@ -813,7 +813,7 @@ function ($phpFile) { // filter the files to only get the ones that match our naming scheme $fileNames = []; - /** @var \Migration\MigrationInterface[] $versions */ + /** @var \Migrations\MigrationInterface[] $versions */ $versions = []; $io = $this->getIo(); @@ -857,12 +857,12 @@ function ($phpFile) { } $io->verbose("Constructing $class."); - /** @var \Migrations\MigrationInterface $migration */ if (is_subclass_of($class, PhinxMigrationInterface::class)) { $migration = new MigrationAdapter($class, $version); } else { $migration = new $class($version); } + /** @var \Migrations\MigrationInterface $migration */ $config = $this->getConfig(); $migration->setConfig($config); $migration->setIo($io); diff --git a/src/MigrationInterface.php b/src/MigrationInterface.php index 289a1065..7b6153f9 100644 --- a/src/MigrationInterface.php +++ b/src/MigrationInterface.php @@ -17,8 +17,6 @@ use Migrations\Config\ConfigInterface; use Migrations\Db\Adapter\AdapterInterface; use Migrations\Db\Table; -use Symfony\Component\Console\Input\InputInterface; -use Symfony\Component\Console\Output\OutputInterface; /** * Migration interface. diff --git a/src/Shim/MigrationAdapter.php b/src/Shim/MigrationAdapter.php index 4452efae..6cc13236 100644 --- a/src/Shim/MigrationAdapter.php +++ b/src/Shim/MigrationAdapter.php @@ -64,7 +64,6 @@ class MigrationAdapter implements MigrationInterface * * @param string|object $migrationClass The phinx migration to adapt * @param int $version The migration version - * @param \Cake\Console\ConsoleIo $io The io */ public function __construct( string|object $migrationClass, diff --git a/tests/TestCase/Migration/ManagerTest.php b/tests/TestCase/Migration/ManagerTest.php index ad654b15..58393ef7 100644 --- a/tests/TestCase/Migration/ManagerTest.php +++ b/tests/TestCase/Migration/ManagerTest.php @@ -13,12 +13,10 @@ use Migrations\Db\Adapter\AdapterInterface; use Migrations\Migration\Environment; use Migrations\Migration\Manager; -use Migrations\Shim\OutputAdapter; use Phinx\Console\Command\AbstractCommand; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use RuntimeException; -use Symfony\Component\Console\Input\InputInterface; class ManagerTest extends TestCase {