From 2129503179d6e739f7d7fd40b7ce5d25c066cf00 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Sun, 7 Apr 2024 12:02:52 -0400 Subject: [PATCH 1/8] Remove completed TODOs --- src/Db/Adapter/PdoAdapter.php | 1 - src/Migration/Manager.php | 3 --- src/Migration/ManagerFactory.php | 2 -- tests/TestCase/Config/AbstractConfigTestCase.php | 1 - tests/TestCase/Migration/ManagerTest.php | 1 - 5 files changed, 8 deletions(-) diff --git a/src/Db/Adapter/PdoAdapter.php b/src/Db/Adapter/PdoAdapter.php index cc46286e..0cf25416 100644 --- a/src/Db/Adapter/PdoAdapter.php +++ b/src/Db/Adapter/PdoAdapter.php @@ -119,7 +119,6 @@ public function setOptions(array $options): AdapterInterface { parent::setOptions($options); - // TODO: Consider renaming this class to ConnectionAdapter if (isset($options['connection']) && $options['connection'] instanceof Connection) { $this->setConnection($options['connection']); } diff --git a/src/Migration/Manager.php b/src/Migration/Manager.php index 200efe85..5e216fa3 100644 --- a/src/Migration/Manager.php +++ b/src/Migration/Manager.php @@ -161,7 +161,6 @@ protected function printMissingVersion(array $version, int $maxNameLength): void */ public function migrateToDateTime(DateTime $dateTime, bool $fake = false): void { - // TODO remove the environment parameter. There is only one environment with builtin /** @var array $versions */ $versions = array_keys($this->getMigrations()); $dateString = $dateTime->format('Ymdhis'); @@ -301,7 +300,6 @@ public function getVersionsToMark(Arguments $args): array $migrations = $this->getMigrations(); $versions = array_keys($migrations); - // TODO use console arguments $versionArg = $args->getArgument('version'); $targetArg = $args->getOption('target'); $hasAllVersion = in_array($versionArg, ['all', '*'], true); @@ -1012,7 +1010,6 @@ public function getSeeds(): array } foreach ($this->seeds as $instance) { - // TODO fix this to not use input if (isset($input) && $instance instanceof AbstractSeed) { $instance->setInput($input); } diff --git a/src/Migration/ManagerFactory.php b/src/Migration/ManagerFactory.php index 2ccaa6bb..7e30f477 100644 --- a/src/Migration/ManagerFactory.php +++ b/src/Migration/ManagerFactory.php @@ -92,8 +92,6 @@ public function createConfig(): ConfigInterface $templatePath = dirname(__DIR__) . DS . 'templates' . DS; $connectionName = (string)$this->getOption('connection'); - // TODO this all needs to go away. But first Environment and Manager need to work - // with Cake's ConnectionManager. $connectionConfig = ConnectionManager::getConfig($connectionName); if (!$connectionConfig) { throw new RuntimeException("Could not find connection `{$connectionName}`"); diff --git a/tests/TestCase/Config/AbstractConfigTestCase.php b/tests/TestCase/Config/AbstractConfigTestCase.php index 5929161a..e0a0c0cf 100644 --- a/tests/TestCase/Config/AbstractConfigTestCase.php +++ b/tests/TestCase/Config/AbstractConfigTestCase.php @@ -55,7 +55,6 @@ public function getConfigArray() 'file' => '%%PHINX_CONFIG_PATH%%/tpl/testtemplate.txt', 'class' => '%%PHINX_CONFIG_PATH%%/tpl/testtemplate.php', ], - // TODO ideally we only need the connection and migration table name. 'environment' => $adapter, ]; } diff --git a/tests/TestCase/Migration/ManagerTest.php b/tests/TestCase/Migration/ManagerTest.php index 9e109235..aca9fc0c 100644 --- a/tests/TestCase/Migration/ManagerTest.php +++ b/tests/TestCase/Migration/ManagerTest.php @@ -134,7 +134,6 @@ protected function prepareEnvironment(array $paths = []): AdapterInterface $adapter = $connectionConfig['scheme'] ?? null; $adapterConfig = [ 'connection' => 'test', - // TODO all of this should go away 'adapter' => $adapter, 'user' => $connectionConfig['username'], 'pass' => $connectionConfig['password'], From 8c2ca9b9b14c17d39ce62595ad01740bae8e9771 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Wed, 10 Apr 2024 00:03:43 -0400 Subject: [PATCH 2/8] Get started on making a pluggable backend for Migrations I'm opting to create duplicate code that optimizes for code deletion instead of brevity as I hope to remove half the code soon. --- src/Migration/BuiltinBackend.php | 411 +++++++++++++++++++++++++++ src/Migration/PhinxBackend.php | 455 ++++++++++++++++++++++++++++++ src/Migrations.php | 28 +- tests/TestCase/MigrationsTest.php | 20 +- 4 files changed, 902 insertions(+), 12 deletions(-) create mode 100644 src/Migration/BuiltinBackend.php create mode 100644 src/Migration/PhinxBackend.php diff --git a/src/Migration/BuiltinBackend.php b/src/Migration/BuiltinBackend.php new file mode 100644 index 00000000..741c588e --- /dev/null +++ b/src/Migration/BuiltinBackend.php @@ -0,0 +1,411 @@ + + */ + protected array $default = []; + + /** + * Current command being run. + * Useful if some logic needs to be applied in the ConfigurationTrait depending + * on the command + * + * @var string + */ + protected string $command; + + /** + * Stub input to feed the manager class since we might not have an input ready when we get the Manager using + * the `getManager()` method + * + * @var \Symfony\Component\Console\Input\ArrayInput + */ + protected ArrayInput $stubInput; + + /** + * Constructor + * + * @param array $default Default option to be used when calling a method. + * Available options are : + * - `connection` The datasource connection to use + * - `source` The folder where migrations are in + * - `plugin` The plugin containing the migrations + */ + public function __construct(array $default = []) + { + $this->output = new NullOutput(); + $this->stubInput = new ArrayInput([]); + + if ($default) { + $this->default = $default; + } + } + + /** + * Sets the command + * + * @param string $command Command name to store. + * @return $this + */ + public function setCommand(string $command) + { + $this->command = $command; + + return $this; + } + + /** + * Sets the input object that should be used for the command class. This object + * is used to inspect the extra options that are needed for CakePHP apps. + * + * @param \Symfony\Component\Console\Input\InputInterface $input the input object + * @return void + */ + public function setInput(InputInterface $input): void + { + $this->input = $input; + } + + /** + * Gets the command + * + * @return string Command name + */ + public function getCommand(): string + { + return $this->command; + } + + /** + * Returns the status of each migrations based on the options passed + * + * @param array $options Options to pass to the command + * Available options are : + * + * - `format` Format to output the response. Can be 'json' + * - `connection` The datasource connection to use + * - `source` The folder where migrations are in + * - `plugin` The plugin containing the migrations + * + * @return array The migrations list and their statuses + */ + public function status(array $options = []): array + { + $manager = $this->getManager($options); + + return $manager->printStatus($options['format'] ?? null); + } + + /** + * Migrates available migrations + * + * @param array $options Options to pass to the command + * Available options are : + * + * - `target` The version number to migrate to. If not provided, will migrate + * everything it can + * - `connection` The datasource connection to use + * - `source` The folder where migrations are in + * - `plugin` The plugin containing the migrations + * - `date` The date to migrate to + * @return bool Success + */ + public function migrate(array $options = []): bool + { + $this->setCommand('migrate'); + $input = $this->getInput('Migrate', [], $options); + $method = 'migrate'; + $params = ['default', $input->getOption('target')]; + + if ($input->getOption('date')) { + $method = 'migrateToDateTime'; + $params[1] = new DateTime($input->getOption('date')); + } + + $this->run($method, $params, $input); + + return true; + } + + /** + * Rollbacks migrations + * + * @param array $options Options to pass to the command + * Available options are : + * + * - `target` The version number to migrate to. If not provided, will only migrate + * the last migrations registered in the phinx log + * - `connection` The datasource connection to use + * - `source` The folder where migrations are in + * - `plugin` The plugin containing the migrations + * - `date` The date to rollback to + * @return bool Success + */ + public function rollback(array $options = []): bool + { + $this->setCommand('rollback'); + $input = $this->getInput('Rollback', [], $options); + $method = 'rollback'; + $params = ['default', $input->getOption('target')]; + + if ($input->getOption('date')) { + $method = 'rollbackToDateTime'; + $params[1] = new DateTime($input->getOption('date')); + } + + $this->run($method, $params, $input); + + return true; + } + + /** + * Marks a migration as migrated + * + * @param int|string|null $version The version number of the migration to mark as migrated + * @param array $options Options to pass to the command + * Available options are : + * + * - `connection` The datasource connection to use + * - `source` The folder where migrations are in + * - `plugin` The plugin containing the migrations + * @return bool Success + */ + public function markMigrated(int|string|null $version = null, array $options = []): bool + { + $this->setCommand('mark_migrated'); + + if ( + isset($options['target']) && + isset($options['exclude']) && + isset($options['only']) + ) { + $exceptionMessage = 'You should use `exclude` OR `only` (not both) along with a `target` argument'; + throw new InvalidArgumentException($exceptionMessage); + } + + $input = $this->getInput('MarkMigrated', ['version' => $version], $options); + $this->setInput($input); + + // This will need to vary based on the config option. + $migrationPaths = $this->getConfig()->getMigrationPaths(); + $config = $this->getConfig(true); + $params = [ + array_pop($migrationPaths), + $this->getManager($config)->getVersionsToMark($input), + $this->output, + ]; + + $this->run('markVersionsAsMigrated', $params, $input); + + return true; + } + + /** + * Seed the database using a seed file + * + * @param array $options Options to pass to the command + * Available options are : + * + * - `connection` The datasource connection to use + * - `source` The folder where migrations are in + * - `plugin` The plugin containing the migrations + * - `seed` The seed file to use + * @return bool Success + */ + public function seed(array $options = []): bool + { + $this->setCommand('seed'); + $input = $this->getInput('Seed', [], $options); + + $seed = $input->getOption('seed'); + if (!$seed) { + $seed = null; + } + + $params = ['default', $seed]; + $this->run('seed', $params, $input); + + return true; + } + + /** + * Runs the method needed to execute and return + * + * @param string $method Manager method to call + * @param array $params Manager params to pass + * @param \Symfony\Component\Console\Input\InputInterface $input InputInterface needed for the + * Manager to properly run + * @return mixed The result of the CakeManager::$method() call + */ + protected function run(string $method, array $params, InputInterface $input): mixed + { + // This will need to vary based on the backend configuration + if ($this->configuration instanceof Config) { + $migrationPaths = $this->getConfig()->getMigrationPaths(); + $migrationPath = array_pop($migrationPaths); + $seedPaths = $this->getConfig()->getSeedPaths(); + $seedPath = array_pop($seedPaths); + } + + $pdo = null; + if ($this->manager instanceof Manager) { + $pdo = $this->manager->getEnvironment('default') + ->getAdapter() + ->getConnection(); + } + + $this->setInput($input); + $newConfig = $this->getConfig(true); + $manager = $this->getManager($newConfig); + $manager->setInput($input); + + // Why is this being done? Is this something we can eliminate in the new code path? + if ($pdo !== null) { + /** @var \Phinx\Db\Adapter\PdoAdapter|\Migrations\CakeAdapter $adapter */ + /** @psalm-suppress PossiblyNullReference */ + $adapter = $this->manager->getEnvironment('default')->getAdapter(); + while ($adapter instanceof WrapperInterface) { + /** @var \Phinx\Db\Adapter\PdoAdapter|\Migrations\CakeAdapter $adapter */ + $adapter = $adapter->getAdapter(); + } + $adapter->setConnection($pdo); + } + + $newMigrationPaths = $newConfig->getMigrationPaths(); + if (isset($migrationPath) && array_pop($newMigrationPaths) !== $migrationPath) { + $manager->resetMigrations(); + } + $newSeedPaths = $newConfig->getSeedPaths(); + if (isset($seedPath) && array_pop($newSeedPaths) !== $seedPath) { + $manager->resetSeeds(); + } + + /** @var callable $callable */ + $callable = [$manager, $method]; + + return call_user_func_array($callable, $params); + } + + /** + * Returns an instance of Manager + * + * @param array $options The options for manager creation + * @return \Migrations\Migration\Manager Instance of Manager + */ + public function getManager(array $options): Manager + { + $factory = new ManagerFactory([ + 'plugin' => $options['plugin'] ?? null, + 'source' => $options['source'] ?? null, + 'connection' => $options['connection'] ?? 'default', + ]); + $io = new ConsoleIo( + new StubConsoleOutput(), + new StubConsoleOutput(), + new StubConsoleInput([]), + ); + + return $factory->createManager($io); + } + + /** + * Get the input needed for each commands to be run + * + * @param string $command Command name for which we need the InputInterface + * @param array $arguments Simple key/values array representing the command arguments + * to pass to the InputInterface + * @param array $options Simple key/values array representing the command options + * to pass to the InputInterface + * @return \Symfony\Component\Console\Input\InputInterface InputInterface needed for the + * Manager to properly run + */ + public function getInput(string $command, array $arguments, array $options): InputInterface + { + // TODO this could make an array of options for the manager. + $className = 'Migrations\Command\Phinx\\' . $command; + $options = $arguments + $this->prepareOptions($options); + /** @var \Symfony\Component\Console\Command\Command $command */ + $command = new $className(); + $definition = $command->getDefinition(); + + return new ArrayInput($options, $definition); + } + + /** + * Prepares the option to pass on to the InputInterface + * + * @param array $options Simple key-values array to pass to the InputInterface + * @return array Prepared $options + */ + protected function prepareOptions(array $options = []): array + { + $options += $this->default; + if (!$options) { + return $options; + } + + foreach ($options as $name => $value) { + $options['--' . $name] = $value; + unset($options[$name]); + } + + return $options; + } +} diff --git a/src/Migration/PhinxBackend.php b/src/Migration/PhinxBackend.php new file mode 100644 index 00000000..2c41fd55 --- /dev/null +++ b/src/Migration/PhinxBackend.php @@ -0,0 +1,455 @@ + + */ + protected array $default = []; + + /** + * Current command being run. + * Useful if some logic needs to be applied in the ConfigurationTrait depending + * on the command + * + * @var string + */ + protected string $command; + + /** + * Stub input to feed the manager class since we might not have an input ready when we get the Manager using + * the `getManager()` method + * + * @var \Symfony\Component\Console\Input\ArrayInput + */ + protected ArrayInput $stubInput; + + /** + * Constructor + * + * @param array $default Default option to be used when calling a method. + * Available options are : + * - `connection` The datasource connection to use + * - `source` The folder where migrations are in + * - `plugin` The plugin containing the migrations + */ + public function __construct(array $default = []) + { + $this->output = new NullOutput(); + $this->stubInput = new ArrayInput([]); + + if ($default) { + $this->default = $default; + } + } + + /** + * Sets the command + * + * @param string $command Command name to store. + * @return $this + */ + public function setCommand(string $command) + { + $this->command = $command; + + return $this; + } + + /** + * Sets the input object that should be used for the command class. This object + * is used to inspect the extra options that are needed for CakePHP apps. + * + * @param \Symfony\Component\Console\Input\InputInterface $input the input object + * @return void + */ + public function setInput(InputInterface $input): void + { + $this->input = $input; + } + + /** + * Gets the command + * + * @return string Command name + */ + public function getCommand(): string + { + return $this->command; + } + + /** + * Returns the status of each migrations based on the options passed + * + * @param array $options Options to pass to the command + * Available options are : + * + * - `format` Format to output the response. Can be 'json' + * - `connection` The datasource connection to use + * - `source` The folder where migrations are in + * - `plugin` The plugin containing the migrations + * @return array The migrations list and their statuses + */ + public function status(array $options = []): array + { + // TODO This class could become an interface that chooses between a phinx and builtin + // implementation. Having two implementations would be easier to cleanup + // than having all the logic in one class with branching + $input = $this->getInput('Status', [], $options); + $params = ['default', $input->getOption('format')]; + + return $this->run('printStatus', $params, $input); + } + + /** + * Migrates available migrations + * + * @param array $options Options to pass to the command + * Available options are : + * + * - `target` The version number to migrate to. If not provided, will migrate + * everything it can + * - `connection` The datasource connection to use + * - `source` The folder where migrations are in + * - `plugin` The plugin containing the migrations + * - `date` The date to migrate to + * @return bool Success + */ + public function migrate(array $options = []): bool + { + $this->setCommand('migrate'); + $input = $this->getInput('Migrate', [], $options); + $method = 'migrate'; + $params = ['default', $input->getOption('target')]; + + if ($input->getOption('date')) { + $method = 'migrateToDateTime'; + $params[1] = new DateTime($input->getOption('date')); + } + + $this->run($method, $params, $input); + + return true; + } + + /** + * Rollbacks migrations + * + * @param array $options Options to pass to the command + * Available options are : + * + * - `target` The version number to migrate to. If not provided, will only migrate + * the last migrations registered in the phinx log + * - `connection` The datasource connection to use + * - `source` The folder where migrations are in + * - `plugin` The plugin containing the migrations + * - `date` The date to rollback to + * @return bool Success + */ + public function rollback(array $options = []): bool + { + $this->setCommand('rollback'); + $input = $this->getInput('Rollback', [], $options); + $method = 'rollback'; + $params = ['default', $input->getOption('target')]; + + if ($input->getOption('date')) { + $method = 'rollbackToDateTime'; + $params[1] = new DateTime($input->getOption('date')); + } + + $this->run($method, $params, $input); + + return true; + } + + /** + * Marks a migration as migrated + * + * @param int|string|null $version The version number of the migration to mark as migrated + * @param array $options Options to pass to the command + * Available options are : + * + * - `connection` The datasource connection to use + * - `source` The folder where migrations are in + * - `plugin` The plugin containing the migrations + * @return bool Success + */ + public function markMigrated(int|string|null $version = null, array $options = []): bool + { + $this->setCommand('mark_migrated'); + + if ( + isset($options['target']) && + isset($options['exclude']) && + isset($options['only']) + ) { + $exceptionMessage = 'You should use `exclude` OR `only` (not both) along with a `target` argument'; + throw new InvalidArgumentException($exceptionMessage); + } + + $input = $this->getInput('MarkMigrated', ['version' => $version], $options); + $this->setInput($input); + + // This will need to vary based on the config option. + $migrationPaths = $this->getConfig()->getMigrationPaths(); + $config = $this->getConfig(true); + $params = [ + array_pop($migrationPaths), + $this->getManager($config)->getVersionsToMark($input), + $this->output, + ]; + + $this->run('markVersionsAsMigrated', $params, $input); + + return true; + } + + /** + * Seed the database using a seed file + * + * @param array $options Options to pass to the command + * Available options are : + * + * - `connection` The datasource connection to use + * - `source` The folder where migrations are in + * - `plugin` The plugin containing the migrations + * - `seed` The seed file to use + * @return bool Success + */ + public function seed(array $options = []): bool + { + $this->setCommand('seed'); + $input = $this->getInput('Seed', [], $options); + + $seed = $input->getOption('seed'); + if (!$seed) { + $seed = null; + } + + $params = ['default', $seed]; + $this->run('seed', $params, $input); + + return true; + } + + /** + * Runs the method needed to execute and return + * + * @param string $method Manager method to call + * @param array $params Manager params to pass + * @param \Symfony\Component\Console\Input\InputInterface $input InputInterface needed for the + * Manager to properly run + * @return mixed The result of the CakeManager::$method() call + */ + protected function run(string $method, array $params, InputInterface $input): mixed + { + // This will need to vary based on the backend configuration + if ($this->configuration instanceof Config) { + $migrationPaths = $this->getConfig()->getMigrationPaths(); + $migrationPath = array_pop($migrationPaths); + $seedPaths = $this->getConfig()->getSeedPaths(); + $seedPath = array_pop($seedPaths); + } + + $pdo = null; + if ($this->manager instanceof Manager) { + $pdo = $this->manager->getEnvironment('default') + ->getAdapter() + ->getConnection(); + } + + $this->setInput($input); + $newConfig = $this->getConfig(true); + $manager = $this->getManager($newConfig); + $manager->setInput($input); + + // Why is this being done? Is this something we can eliminate in the new code path? + if ($pdo !== null) { + /** @var \Phinx\Db\Adapter\PdoAdapter|\Migrations\CakeAdapter $adapter */ + /** @psalm-suppress PossiblyNullReference */ + $adapter = $this->manager->getEnvironment('default')->getAdapter(); + while ($adapter instanceof WrapperInterface) { + /** @var \Phinx\Db\Adapter\PdoAdapter|\Migrations\CakeAdapter $adapter */ + $adapter = $adapter->getAdapter(); + } + $adapter->setConnection($pdo); + } + + $newMigrationPaths = $newConfig->getMigrationPaths(); + if (isset($migrationPath) && array_pop($newMigrationPaths) !== $migrationPath) { + $manager->resetMigrations(); + } + $newSeedPaths = $newConfig->getSeedPaths(); + if (isset($seedPath) && array_pop($newSeedPaths) !== $seedPath) { + $manager->resetSeeds(); + } + + /** @var callable $callable */ + $callable = [$manager, $method]; + + return call_user_func_array($callable, $params); + } + + /** + * Returns an instance of CakeManager + * + * @param \Phinx\Config\ConfigInterface|null $config ConfigInterface the Manager needs to run + * @return \Migrations\CakeManager Instance of CakeManager + */ + public function getManager(?ConfigInterface $config = null): CakeManager + { + if (!($this->manager instanceof CakeManager)) { + if (!($config instanceof ConfigInterface)) { + throw new RuntimeException( + 'You need to pass a ConfigInterface object for your first getManager() call' + ); + } + + $input = $this->input ?: $this->stubInput; + $this->manager = new CakeManager($config, $input, $this->output); + } elseif ($config !== null) { + $defaultEnvironment = $config->getEnvironment('default'); + try { + $environment = $this->manager->getEnvironment('default'); + $oldConfig = $environment->getOptions(); + unset($oldConfig['connection']); + if ($oldConfig === $defaultEnvironment) { + $defaultEnvironment['connection'] = $environment + ->getAdapter() + ->getConnection(); + } + } catch (InvalidArgumentException $e) { + } + $config['environments'] = ['default' => $defaultEnvironment]; + $this->manager->setEnvironments([]); + $this->manager->setConfig($config); + } + + $this->setAdapter(); + + return $this->manager; + } + + /** + * Sets the adapter the manager is going to need to operate on the DB + * This will make sure the adapter instance is a \Migrations\CakeAdapter instance + * + * @return void + */ + public function setAdapter(): void + { + if ($this->input === null) { + return; + } + + /** @var string $connectionName */ + $connectionName = $this->input()->getOption('connection') ?: 'default'; + /** @var \Cake\Database\Connection $connection */ + $connection = ConnectionManager::get($connectionName); + + /** @psalm-suppress PossiblyNullReference */ + $env = $this->manager->getEnvironment('default'); + $adapter = $env->getAdapter(); + if (!$adapter instanceof CakeAdapter) { + $env->setAdapter(new CakeAdapter($adapter, $connection)); + } + } + + /** + * Get the input needed for each commands to be run + * + * @param string $command Command name for which we need the InputInterface + * @param array $arguments Simple key/values array representing the command arguments + * to pass to the InputInterface + * @param array $options Simple key/values array representing the command options + * to pass to the InputInterface + * @return \Symfony\Component\Console\Input\InputInterface InputInterface needed for the + * Manager to properly run + */ + public function getInput(string $command, array $arguments, array $options): InputInterface + { + $className = 'Migrations\Command\Phinx\\' . $command; + $options = $arguments + $this->prepareOptions($options); + /** @var \Symfony\Component\Console\Command\Command $command */ + $command = new $className(); + $definition = $command->getDefinition(); + + return new ArrayInput($options, $definition); + } + + /** + * Prepares the option to pass on to the InputInterface + * + * @param array $options Simple key-values array to pass to the InputInterface + * @return array Prepared $options + */ + protected function prepareOptions(array $options = []): array + { + $options += $this->default; + if (!$options) { + return $options; + } + + foreach ($options as $name => $value) { + $options['--' . $name] = $value; + unset($options[$name]); + } + + return $options; + } +} diff --git a/src/Migrations.php b/src/Migrations.php index fb486fff..769961e7 100644 --- a/src/Migrations.php +++ b/src/Migrations.php @@ -13,9 +13,12 @@ */ namespace Migrations; +use Cake\Core\Configure; use Cake\Datasource\ConnectionManager; use DateTime; use InvalidArgumentException; +use Migrations\Migration\BuiltinBackend; +use Migrations\Migration\PhinxBackend; use Phinx\Config\Config; use Phinx\Config\ConfigInterface; use Phinx\Db\Adapter\WrapperInterface; @@ -129,6 +132,24 @@ public function getCommand(): string return $this->command; } + /** + * Get the Migrations interface backend based on configuration data. + * + * @return \Migrations\Migration\BuiltinBackend|\Migrations\Migration\PhinxBackend + */ + protected function getBackend(): BuiltinBackend|PhinxBackend + { + $backend = (string)(Configure::read('Migrations.backend') ?? 'phinx'); + if ($backend === 'builtin') { + return new BuiltinBackend(); + } + if ($backend === 'phinx') { + return new PhinxBackend(); + } + + throw new RuntimeException("Unknown `Migrations.backend` of `{$backend}`"); + } + /** * Returns the status of each migrations based on the options passed * @@ -143,11 +164,10 @@ public function getCommand(): string */ public function status(array $options = []): array { - $this->setCommand('status'); - $input = $this->getInput('Status', [], $options); - $params = ['default', $input->getOption('format')]; + $options = $options + $this->default; + $backend = $this->getBackend(); - return $this->run('printStatus', $params, $input); + return $backend->status($options); } /** diff --git a/tests/TestCase/MigrationsTest.php b/tests/TestCase/MigrationsTest.php index b25c2c1f..f3952169 100644 --- a/tests/TestCase/MigrationsTest.php +++ b/tests/TestCase/MigrationsTest.php @@ -123,13 +123,24 @@ public function tearDown(): void FeatureFlags::setFlagsFromConfig(Configure::read('Migrations')); } + public static function backendProvider(): array + { + return [ + ['builtin'], + ['phinx'] + ]; + } + /** * Tests the status method * + * @dataProvider backendProvider * @return void */ - public function testStatus() + public function testStatus(string $backend) { + Configure::write('Migrations.backend', $backend); + $result = $this->migrations->status(); $expected = [ [ @@ -154,13 +165,6 @@ public function testStatus() ], ]; $this->assertEquals($expected, $result); - - $adapter = $this->migrations - ->getManager() - ->getEnvironment('default') - ->getAdapter(); - - $this->assertInstanceOf(CakeAdapter::class, $adapter); } /** From af3dc185fe5afab869798b7dd8f5752793f982ef Mon Sep 17 00:00:00 2001 From: Mark Story Date: Wed, 10 Apr 2024 00:25:30 -0400 Subject: [PATCH 3/8] Get Migrations::migrate() to use new backend. --- src/Migration/BuiltinBackend.php | 18 ++++++++++-------- src/Migrations.php | 19 ++++--------------- tests/TestCase/MigrationsTest.php | 5 ++++- 3 files changed, 18 insertions(+), 24 deletions(-) diff --git a/src/Migration/BuiltinBackend.php b/src/Migration/BuiltinBackend.php index 741c588e..6f1d325b 100644 --- a/src/Migration/BuiltinBackend.php +++ b/src/Migration/BuiltinBackend.php @@ -169,17 +169,17 @@ public function status(array $options = []): array */ public function migrate(array $options = []): bool { - $this->setCommand('migrate'); - $input = $this->getInput('Migrate', [], $options); - $method = 'migrate'; - $params = ['default', $input->getOption('target')]; + $manager = $this->getManager($options); - if ($input->getOption('date')) { - $method = 'migrateToDateTime'; - $params[1] = new DateTime($input->getOption('date')); + if (!empty($options['date'])) { + $date = new DateTime($options['date']); + + $manager->migrateToDateTime($date); + + return true; } - $this->run($method, $params, $input); + $manager->migrate($options['target'] ?? null); return true; } @@ -351,6 +351,8 @@ protected function run(string $method, array $params, InputInterface $input): mi */ public function getManager(array $options): Manager { + $options += $this->default; + $factory = new ManagerFactory([ 'plugin' => $options['plugin'] ?? null, 'source' => $options['source'] ?? null, diff --git a/src/Migrations.php b/src/Migrations.php index 769961e7..4fcf8ef2 100644 --- a/src/Migrations.php +++ b/src/Migrations.php @@ -141,10 +141,10 @@ protected function getBackend(): BuiltinBackend|PhinxBackend { $backend = (string)(Configure::read('Migrations.backend') ?? 'phinx'); if ($backend === 'builtin') { - return new BuiltinBackend(); + return new BuiltinBackend($this->default); } if ($backend === 'phinx') { - return new PhinxBackend(); + return new PhinxBackend($this->default); } throw new RuntimeException("Unknown `Migrations.backend` of `{$backend}`"); @@ -164,7 +164,6 @@ protected function getBackend(): BuiltinBackend|PhinxBackend */ public function status(array $options = []): array { - $options = $options + $this->default; $backend = $this->getBackend(); return $backend->status($options); @@ -186,19 +185,9 @@ public function status(array $options = []): array */ public function migrate(array $options = []): bool { - $this->setCommand('migrate'); - $input = $this->getInput('Migrate', [], $options); - $method = 'migrate'; - $params = ['default', $input->getOption('target')]; - - if ($input->getOption('date')) { - $method = 'migrateToDateTime'; - $params[1] = new DateTime($input->getOption('date')); - } - - $this->run($method, $params, $input); + $backend = $this->getBackend(); - return true; + return $backend->migrate($options); } /** diff --git a/tests/TestCase/MigrationsTest.php b/tests/TestCase/MigrationsTest.php index f3952169..9d32f9cb 100644 --- a/tests/TestCase/MigrationsTest.php +++ b/tests/TestCase/MigrationsTest.php @@ -170,10 +170,13 @@ public function testStatus(string $backend) /** * Tests the migrations and rollbacks * + * @dataProvider backendProvider * @return void */ - public function testMigrateAndRollback() + public function testMigrateAndRollback($backend) { + Configure::write('Migrations.backend', $backend); + if ($this->Connection->getDriver() instanceof Sqlserver) { // TODO This test currently fails in CI because numbers table // has no columns in sqlserver. This table should have columns as the From 937981b3b90aa5fa0c202403d02ea614b94b832a Mon Sep 17 00:00:00 2001 From: Mark Story Date: Thu, 11 Apr 2024 00:37:41 -0400 Subject: [PATCH 4/8] Get more tests passing. --- src/Command/MarkMigratedCommand.php | 3 +- src/Migration/BuiltinBackend.php | 85 ++++---------------- src/Migration/Manager.php | 31 ++++---- tests/TestCase/MigrationsTest.php | 115 +++++++++++++++++++++++----- 4 files changed, 129 insertions(+), 105 deletions(-) diff --git a/src/Command/MarkMigratedCommand.php b/src/Command/MarkMigratedCommand.php index 716c7b32..4567b315 100644 --- a/src/Command/MarkMigratedCommand.php +++ b/src/Command/MarkMigratedCommand.php @@ -135,7 +135,8 @@ public function execute(Arguments $args, ConsoleIo $io): ?int return self::CODE_ERROR; } - $manager->markVersionsAsMigrated($path, $versions, $io); + $output = $manager->markVersionsAsMigrated($path, $versions); + array_map(fn ($line) => $io->out($line), $output); return self::CODE_SUCCESS; } diff --git a/src/Migration/BuiltinBackend.php b/src/Migration/BuiltinBackend.php index 6f1d325b..18a1d630 100644 --- a/src/Migration/BuiltinBackend.php +++ b/src/Migration/BuiltinBackend.php @@ -98,41 +98,6 @@ public function __construct(array $default = []) } } - /** - * Sets the command - * - * @param string $command Command name to store. - * @return $this - */ - public function setCommand(string $command) - { - $this->command = $command; - - return $this; - } - - /** - * Sets the input object that should be used for the command class. This object - * is used to inspect the extra options that are needed for CakePHP apps. - * - * @param \Symfony\Component\Console\Input\InputInterface $input the input object - * @return void - */ - public function setInput(InputInterface $input): void - { - $this->input = $input; - } - - /** - * Gets the command - * - * @return string Command name - */ - public function getCommand(): string - { - return $this->command; - } - /** * Returns the status of each migrations based on the options passed * @@ -200,17 +165,17 @@ public function migrate(array $options = []): bool */ public function rollback(array $options = []): bool { - $this->setCommand('rollback'); - $input = $this->getInput('Rollback', [], $options); - $method = 'rollback'; - $params = ['default', $input->getOption('target')]; - - if ($input->getOption('date')) { - $method = 'rollbackToDateTime'; - $params[1] = new DateTime($input->getOption('date')); + $manager = $this->getManager($options); + + if (!empty($options['date'])) { + $date = new DateTime($options['date']); + + $manager->rollbackToDateTime($date); + + return true; } - $this->run($method, $params, $input); + $manager->rollback($options['target'] ?? null); return true; } @@ -229,8 +194,6 @@ public function rollback(array $options = []): bool */ public function markMigrated(int|string|null $version = null, array $options = []): bool { - $this->setCommand('mark_migrated'); - if ( isset($options['target']) && isset($options['exclude']) && @@ -239,20 +202,11 @@ public function markMigrated(int|string|null $version = null, array $options = [ $exceptionMessage = 'You should use `exclude` OR `only` (not both) along with a `target` argument'; throw new InvalidArgumentException($exceptionMessage); } + $args = new Arguments([(string)$version], $options, ['version']); - $input = $this->getInput('MarkMigrated', ['version' => $version], $options); - $this->setInput($input); - - // This will need to vary based on the config option. - $migrationPaths = $this->getConfig()->getMigrationPaths(); - $config = $this->getConfig(true); - $params = [ - array_pop($migrationPaths), - $this->getManager($config)->getVersionsToMark($input), - $this->output, - ]; - - $this->run('markVersionsAsMigrated', $params, $input); + $manager = $this->getManager($options); + $versions = $manager->getVersionsToMark($args); + $manager->markVersionsAsMigrated($path, $versions); return true; } @@ -271,16 +225,9 @@ public function markMigrated(int|string|null $version = null, array $options = [ */ public function seed(array $options = []): bool { - $this->setCommand('seed'); - $input = $this->getInput('Seed', [], $options); - - $seed = $input->getOption('seed'); - if (!$seed) { - $seed = null; - } - - $params = ['default', $seed]; - $this->run('seed', $params, $input); + $seed = $options['seed'] ?? null; + $manager = $this->getManager($options); + $manager->seed($seed); return true; } diff --git a/src/Migration/Manager.php b/src/Migration/Manager.php index 5e216fa3..07523b2e 100644 --- a/src/Migration/Manager.php +++ b/src/Migration/Manager.php @@ -335,45 +335,44 @@ public function getVersionsToMark(Arguments $args): array * @param string $path Path where to look for migrations * @param array $versions Versions which should be marked * @param \Cake\Console\ConsoleIo $io ConsoleIo to write output too - * @return void + * @return list Output from the operation */ - public function markVersionsAsMigrated(string $path, array $versions, ConsoleIo $io): void + public function markVersionsAsMigrated(string $path, array $versions): array { $adapter = $this->getEnvironment()->getAdapter(); + $out = []; if (!$versions) { - $io->out('No migrations were found. Nothing to mark as migrated.'); + $out[] = 'No migrations were found. Nothing to mark as migrated.'; - return; + return $out; } $adapter->beginTransaction(); foreach ($versions as $version) { if ($this->isMigrated($version)) { - $io->out(sprintf('Skipping migration `%s` (already migrated).', $version)); + $out[] = sprintf('Skipping migration `%s` (already migrated).', $version); continue; } try { $this->markMigrated($version, $path); - $io->out( - sprintf('Migration `%s` successfully marked migrated !', $version) - ); + $out[] = sprintf('Migration `%s` successfully marked migrated !', $version); } catch (Exception $e) { $adapter->rollbackTransaction(); - $io->out( - sprintf( - 'An error occurred while marking migration `%s` as migrated : %s', - $version, - $e->getMessage() - ) + $out[] = sprintf( + 'An error occurred while marking migration `%s` as migrated : %s', + $version, + $e->getMessage() ); - $io->out('All marked migrations during this process were unmarked.'); + $out[] = 'All marked migrations during this process were unmarked.'; - return; + return $out; } } $adapter->commitTransaction(); + + return $out; } /** diff --git a/tests/TestCase/MigrationsTest.php b/tests/TestCase/MigrationsTest.php index 9d32f9cb..d08b8bb9 100644 --- a/tests/TestCase/MigrationsTest.php +++ b/tests/TestCase/MigrationsTest.php @@ -259,10 +259,13 @@ public function testMigrateAndRollback($backend) /** * Tests the collation table behavior when using MySQL * + * @dataProvider backendProvider * @return void */ - public function testCreateWithEncoding() + public function testCreateWithEncoding($backend) { + Configure::write('Migrations.backend', $backend); + $this->skipIf(env('DB') !== 'mysql', 'Requires MySQL'); $migrate = $this->migrations->migrate(); @@ -285,10 +288,13 @@ public function testCreateWithEncoding() * Tests calling Migrations::markMigrated without params marks everything * as migrated * + * @dataProvider backendProvider * @return void */ - public function testMarkMigratedAll() + public function testMarkMigratedAll($backend) { + Configure::write('Migrations.backend', $backend); + $markMigrated = $this->migrations->markMigrated(); $this->assertTrue($markMigrated); $status = $this->migrations->status(); @@ -322,10 +328,13 @@ public function testMarkMigratedAll() * string 'all' marks everything * as migrated * + * @dataProvider backendProvider * @return void */ - public function testMarkMigratedAllAsVersion() + public function testMarkMigratedAllAsVersion($backend) { + Configure::write('Migrations.backend', $backend); + $markMigrated = $this->migrations->markMigrated('all'); $this->assertTrue($markMigrated); $status = $this->migrations->status(); @@ -358,10 +367,13 @@ public function testMarkMigratedAllAsVersion() * Tests calling Migrations::markMigrated with the target option will mark * only up to that one * + * @dataProvider backendProvider * @return void */ - public function testMarkMigratedTarget() + public function testMarkMigratedTarget($backend) { + Configure::write('Migrations.backend', $backend); + $markMigrated = $this->migrations->markMigrated(null, ['target' => '20150704160200']); $this->assertTrue($markMigrated); $status = $this->migrations->status(); @@ -400,10 +412,13 @@ public function testMarkMigratedTarget() * Tests calling Migrations::markMigrated with the target option set to a * non-existent target will throw an exception * + * @dataProvider backendProvider * @return void */ - public function testMarkMigratedTargetError() + public function testMarkMigratedTargetError($backend) { + Configure::write('Migrations.backend', $backend); + $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('Migration `20150704160610` was not found !'); $this->migrations->markMigrated(null, ['target' => '20150704160610']); @@ -413,10 +428,13 @@ public function testMarkMigratedTargetError() * Tests calling Migrations::markMigrated with the target option with the exclude * option will mark only up to that one, excluding it * + * @dataProvider backendProvider * @return void */ - public function testMarkMigratedTargetExclude() + public function testMarkMigratedTargetExclude($backend) { + Configure::write('Migrations.backend', $backend); + $markMigrated = $this->migrations->markMigrated(null, ['target' => '20150704160200', 'exclude' => true]); $this->assertTrue($markMigrated); $status = $this->migrations->status(); @@ -455,10 +473,13 @@ public function testMarkMigratedTargetExclude() * Tests calling Migrations::markMigrated with the target option with the only * option will mark only that specific migrations * + * @dataProvider backendProvider * @return void */ - public function testMarkMigratedTargetOnly() + public function testMarkMigratedTargetOnly($backend) { + Configure::write('Migrations.backend', $backend); + $markMigrated = $this->migrations->markMigrated(null, ['target' => '20150724233100', 'only' => true]); $this->assertTrue($markMigrated); $status = $this->migrations->status(); @@ -497,10 +518,13 @@ public function testMarkMigratedTargetOnly() * Tests calling Migrations::markMigrated with the target option, the only option * and the exclude option will throw an exception * + * @dataProvider backendProvider * @return void */ - public function testMarkMigratedTargetExcludeOnly() + public function testMarkMigratedTargetExcludeOnly($backend) { + Configure::write('Migrations.backend', $backend); + $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('You should use `exclude` OR `only` (not both) along with a `target` argument'); $this->migrations->markMigrated(null, ['target' => '20150724233100', 'only' => true, 'exclude' => true]); @@ -510,10 +534,13 @@ public function testMarkMigratedTargetExcludeOnly() * Tests calling Migrations::markMigrated with the target option with the exclude * option will mark only up to that one, excluding it * + * @dataProvider backendProvider * @return void */ - public function testMarkMigratedVersion() + public function testMarkMigratedVersion($backend) { + Configure::write('Migrations.backend', $backend); + $markMigrated = $this->migrations->markMigrated(20150704160200); $this->assertTrue($markMigrated); $status = $this->migrations->status(); @@ -552,10 +579,13 @@ public function testMarkMigratedVersion() * Tests that calling the migrations methods while passing * parameters will override the default ones * + * @dataProvider backendProvider * @return void */ - public function testOverrideOptions() + public function testOverrideOptions($backend) { + Configure::write('Migrations.backend', $backend); + $result = $this->migrations->status(); $expectedStatus = [ [ @@ -620,10 +650,13 @@ public function testOverrideOptions() * Tests that calling the migrations methods while passing the ``date`` * parameter works as expected * + * @dataProvider backendProvider * @return void */ - public function testMigrateDateOption() + public function testMigrateDateOption($backend) { + Configure::write('Migrations.backend', $backend); + // If we want to migrate to a date before the first first migration date, // we should not migrate anything $this->migrations->migrate(['date' => '20140705']); @@ -796,10 +829,13 @@ public function testMigrateDateOption() /** * Tests seeding the database * + * @dataProvider backendProvider * @return void */ - public function testSeed() + public function testSeed($backend) { + Configure::write('Migrations.backend', $backend); + $this->migrations->migrate(); $seed = $this->migrations->seed(['source' => 'Seeds']); $this->assertTrue($seed); @@ -872,10 +908,13 @@ public function testSeed() /** * Tests seeding the database with seeder * + * @dataProvider backendProvider * @return void */ - public function testSeedOneSeeder() + public function testSeedOneSeeder($backend) { + Configure::write('Migrations.backend', $backend); + $this->migrations->migrate(); $seed = $this->migrations->seed(['source' => 'AltSeeds', 'seed' => 'AnotherNumbersSeed']); @@ -921,10 +960,13 @@ public function testSeedOneSeeder() /** * Tests seeding the database with seeder * + * @dataProvider backendProvider * @return void */ - public function testSeedCallSeeder() + public function testSeedCallSeeder($backend) { + Configure::write('Migrations.backend', $backend); + $this->migrations->migrate(); $seed = $this->migrations->seed(['source' => 'CallSeeds', 'seed' => 'DatabaseSeed']); @@ -982,15 +1024,33 @@ public function testSeedCallSeeder() /** * Tests that requesting a unexistant seed throws an exception * + * @dataProvider backendProvider * @return void */ - public function testSeedWrongSeed() + public function testSeedWrongSeed($backend) { + Configure::write('Migrations.backend', $backend); + $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('The seed class "DerpSeed" does not exist'); $this->migrations->seed(['source' => 'AltSeeds', 'seed' => 'DerpSeed']); } + /** + * Tests migrating the baked snapshots with builtin backend + * + * @dataProvider snapshotMigrationsProvider + * @param string $basePath Snapshot file path + * @param string $filename Snapshot file name + * @param array $flags Feature flags + * @return void + */ + public function testMigrateSnapshotsBuiltin(string $basePath, string $filename, array $flags = []): void + { + Configure::write('Migrations.backend', 'builtin'); + $this->runMigrateSnapshots($basePath, $filename, $flags); + } + /** * Tests migrating the baked snapshots * @@ -1000,7 +1060,12 @@ public function testSeedWrongSeed() * @param array $flags Feature flags * @return void */ - public function testMigrateSnapshots(string $basePath, string $filename, array $flags = []): void + public function testMigrateSnapshotsPhinx(string $basePath, string $filename, array $flags = []): void + { + $this->runMigrateSnapshots($basePath, $filename, $flags); + } + + protected function runMigrateSnapshots(string $basePath, string $filename, array $flags): void { if ($this->Connection->getDriver() instanceof Sqlserver) { // TODO once migrations is using the inlined sqlserver adapter, this skip should @@ -1047,9 +1112,13 @@ public function testMigrateSnapshots(string $basePath, string $filename, array $ /** * Tests that migrating in case of error throws an exception + * + * @dataProvider backendProvider */ - public function testMigrateErrors() + public function testMigrateErrors($backend) { + Configure::write('Migrations.backend', $backend); + $this->expectException(Exception::class); $this->migrations->markMigrated(20150704160200); $this->migrations->migrate(); @@ -1057,9 +1126,13 @@ public function testMigrateErrors() /** * Tests that rolling back in case of error throws an exception + * + * @dataProvider backendProvider */ - public function testRollbackErrors() + public function testRollbackErrors($backend) { + Configure::write('Migrations.backend', $backend); + $this->expectException(Exception::class); $this->migrations->markMigrated('all'); $this->migrations->rollback(); @@ -1068,9 +1141,13 @@ public function testRollbackErrors() /** * Tests that marking migrated a non-existant migrations returns an error * and can return a error message + * + * @dataProvider backendProvider */ - public function testMarkMigratedErrors() + public function testMarkMigratedErrors($backend) { + Configure::write('Migrations.backend', $backend); + $this->expectException(Exception::class); $this->migrations->markMigrated(20150704000000); } From 022e9f824f2ebe263eae2e0ea3e4cf716f470b93 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Thu, 11 Apr 2024 00:40:05 -0400 Subject: [PATCH 5/8] Cleanup unused code --- src/Migration/BuiltinBackend.php | 105 +------------------------------ 1 file changed, 3 insertions(+), 102 deletions(-) diff --git a/src/Migration/BuiltinBackend.php b/src/Migration/BuiltinBackend.php index 18a1d630..041d69c4 100644 --- a/src/Migration/BuiltinBackend.php +++ b/src/Migration/BuiltinBackend.php @@ -205,6 +205,9 @@ public function markMigrated(int|string|null $version = null, array $options = [ $args = new Arguments([(string)$version], $options, ['version']); $manager = $this->getManager($options); + $config = $manager->getConfig(); + $path = $config->getMigrationPaths()[0]; + $versions = $manager->getVersionsToMark($args); $manager->markVersionsAsMigrated($path, $versions); @@ -232,64 +235,6 @@ public function seed(array $options = []): bool return true; } - /** - * Runs the method needed to execute and return - * - * @param string $method Manager method to call - * @param array $params Manager params to pass - * @param \Symfony\Component\Console\Input\InputInterface $input InputInterface needed for the - * Manager to properly run - * @return mixed The result of the CakeManager::$method() call - */ - protected function run(string $method, array $params, InputInterface $input): mixed - { - // This will need to vary based on the backend configuration - if ($this->configuration instanceof Config) { - $migrationPaths = $this->getConfig()->getMigrationPaths(); - $migrationPath = array_pop($migrationPaths); - $seedPaths = $this->getConfig()->getSeedPaths(); - $seedPath = array_pop($seedPaths); - } - - $pdo = null; - if ($this->manager instanceof Manager) { - $pdo = $this->manager->getEnvironment('default') - ->getAdapter() - ->getConnection(); - } - - $this->setInput($input); - $newConfig = $this->getConfig(true); - $manager = $this->getManager($newConfig); - $manager->setInput($input); - - // Why is this being done? Is this something we can eliminate in the new code path? - if ($pdo !== null) { - /** @var \Phinx\Db\Adapter\PdoAdapter|\Migrations\CakeAdapter $adapter */ - /** @psalm-suppress PossiblyNullReference */ - $adapter = $this->manager->getEnvironment('default')->getAdapter(); - while ($adapter instanceof WrapperInterface) { - /** @var \Phinx\Db\Adapter\PdoAdapter|\Migrations\CakeAdapter $adapter */ - $adapter = $adapter->getAdapter(); - } - $adapter->setConnection($pdo); - } - - $newMigrationPaths = $newConfig->getMigrationPaths(); - if (isset($migrationPath) && array_pop($newMigrationPaths) !== $migrationPath) { - $manager->resetMigrations(); - } - $newSeedPaths = $newConfig->getSeedPaths(); - if (isset($seedPath) && array_pop($newSeedPaths) !== $seedPath) { - $manager->resetSeeds(); - } - - /** @var callable $callable */ - $callable = [$manager, $method]; - - return call_user_func_array($callable, $params); - } - /** * Returns an instance of Manager * @@ -313,48 +258,4 @@ public function getManager(array $options): Manager return $factory->createManager($io); } - - /** - * Get the input needed for each commands to be run - * - * @param string $command Command name for which we need the InputInterface - * @param array $arguments Simple key/values array representing the command arguments - * to pass to the InputInterface - * @param array $options Simple key/values array representing the command options - * to pass to the InputInterface - * @return \Symfony\Component\Console\Input\InputInterface InputInterface needed for the - * Manager to properly run - */ - public function getInput(string $command, array $arguments, array $options): InputInterface - { - // TODO this could make an array of options for the manager. - $className = 'Migrations\Command\Phinx\\' . $command; - $options = $arguments + $this->prepareOptions($options); - /** @var \Symfony\Component\Console\Command\Command $command */ - $command = new $className(); - $definition = $command->getDefinition(); - - return new ArrayInput($options, $definition); - } - - /** - * Prepares the option to pass on to the InputInterface - * - * @param array $options Simple key-values array to pass to the InputInterface - * @return array Prepared $options - */ - protected function prepareOptions(array $options = []): array - { - $options += $this->default; - if (!$options) { - return $options; - } - - foreach ($options as $name => $value) { - $options['--' . $name] = $value; - unset($options[$name]); - } - - return $options; - } } From 02e70e1e8b8940f569830f1986175df8cd7eb6dc Mon Sep 17 00:00:00 2001 From: Mark Story Date: Thu, 11 Apr 2024 00:48:26 -0400 Subject: [PATCH 6/8] Update Migrations to use backends in more places. --- src/Migrations.php | 112 +++------------------------------------------ 1 file changed, 6 insertions(+), 106 deletions(-) diff --git a/src/Migrations.php b/src/Migrations.php index 4fcf8ef2..c0fcc5a7 100644 --- a/src/Migrations.php +++ b/src/Migrations.php @@ -206,19 +206,9 @@ public function migrate(array $options = []): bool */ public function rollback(array $options = []): bool { - $this->setCommand('rollback'); - $input = $this->getInput('Rollback', [], $options); - $method = 'rollback'; - $params = ['default', $input->getOption('target')]; - - if ($input->getOption('date')) { - $method = 'rollbackToDateTime'; - $params[1] = new DateTime($input->getOption('date')); - } - - $this->run($method, $params, $input); + $backend = $this->getBackend(); - return true; + return $backend->rollback($options); } /** @@ -235,32 +225,9 @@ public function rollback(array $options = []): bool */ public function markMigrated(int|string|null $version = null, array $options = []): bool { - $this->setCommand('mark_migrated'); - - if ( - isset($options['target']) && - isset($options['exclude']) && - isset($options['only']) - ) { - $exceptionMessage = 'You should use `exclude` OR `only` (not both) along with a `target` argument'; - throw new InvalidArgumentException($exceptionMessage); - } - - $input = $this->getInput('MarkMigrated', ['version' => $version], $options); - $this->setInput($input); - - // This will need to vary based on the config option. - $migrationPaths = $this->getConfig()->getMigrationPaths(); - $config = $this->getConfig(true); - $params = [ - array_pop($migrationPaths), - $this->getManager($config)->getVersionsToMark($input), - $this->output, - ]; - - $this->run('markVersionsAsMigrated', $params, $input); + $backend = $this->getBackend(); - return true; + return $backend->markMigrated($version, $options); } /** @@ -277,76 +244,9 @@ public function markMigrated(int|string|null $version = null, array $options = [ */ public function seed(array $options = []): bool { - $this->setCommand('seed'); - $input = $this->getInput('Seed', [], $options); - - $seed = $input->getOption('seed'); - if (!$seed) { - $seed = null; - } - - $params = ['default', $seed]; - $this->run('seed', $params, $input); - - return true; - } - - /** - * Runs the method needed to execute and return - * - * @param string $method Manager method to call - * @param array $params Manager params to pass - * @param \Symfony\Component\Console\Input\InputInterface $input InputInterface needed for the - * Manager to properly run - * @return mixed The result of the CakeManager::$method() call - */ - protected function run(string $method, array $params, InputInterface $input): mixed - { - // This will need to vary based on the backend configuration - if ($this->configuration instanceof Config) { - $migrationPaths = $this->getConfig()->getMigrationPaths(); - $migrationPath = array_pop($migrationPaths); - $seedPaths = $this->getConfig()->getSeedPaths(); - $seedPath = array_pop($seedPaths); - } - - $pdo = null; - if ($this->manager instanceof Manager) { - $pdo = $this->manager->getEnvironment('default') - ->getAdapter() - ->getConnection(); - } - - $this->setInput($input); - $newConfig = $this->getConfig(true); - $manager = $this->getManager($newConfig); - $manager->setInput($input); - - // Why is this being done? Is this something we can eliminate in the new code path? - if ($pdo !== null) { - /** @var \Phinx\Db\Adapter\PdoAdapter|\Migrations\CakeAdapter $adapter */ - /** @psalm-suppress PossiblyNullReference */ - $adapter = $this->manager->getEnvironment('default')->getAdapter(); - while ($adapter instanceof WrapperInterface) { - /** @var \Phinx\Db\Adapter\PdoAdapter|\Migrations\CakeAdapter $adapter */ - $adapter = $adapter->getAdapter(); - } - $adapter->setConnection($pdo); - } - - $newMigrationPaths = $newConfig->getMigrationPaths(); - if (isset($migrationPath) && array_pop($newMigrationPaths) !== $migrationPath) { - $manager->resetMigrations(); - } - $newSeedPaths = $newConfig->getSeedPaths(); - if (isset($seedPath) && array_pop($newSeedPaths) !== $seedPath) { - $manager->resetSeeds(); - } - - /** @var callable $callable */ - $callable = [$manager, $method]; + $backend = $this->getBackend(); - return call_user_func_array($callable, $params); + return $backend->seed($options); } /** From 43cbd5f7ffc6a823874a2d31783980baa5d17044 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Thu, 11 Apr 2024 23:32:01 -0400 Subject: [PATCH 7/8] Get remaining Migrations tests passing --- src/Migration/BuiltinBackend.php | 10 ---------- src/Migration/Manager.php | 23 ++++++++++++++++++++--- src/Migration/ManagerFactory.php | 2 ++ src/Migrations.php | 6 ------ tests/TestCase/MigrationsTest.php | 3 +-- 5 files changed, 23 insertions(+), 21 deletions(-) diff --git a/src/Migration/BuiltinBackend.php b/src/Migration/BuiltinBackend.php index 041d69c4..470621bc 100644 --- a/src/Migration/BuiltinBackend.php +++ b/src/Migration/BuiltinBackend.php @@ -13,22 +13,13 @@ */ namespace Migrations\Migration; -use Cake\Command\Command; use Cake\Console\Arguments; use Cake\Console\ConsoleIo; use Cake\Console\TestSuite\StubConsoleInput; use Cake\Console\TestSuite\StubConsoleOutput; -use Cake\Datasource\ConnectionManager; use DateTime; use InvalidArgumentException; -use Migrations\Command\StatusCommand; -use Phinx\Config\Config; -use Phinx\Config\ConfigInterface; -use Phinx\Db\Adapter\WrapperInterface; -use Migrations\Migration\Manager; -use RuntimeException; use Symfony\Component\Console\Input\ArrayInput; -use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\NullOutput; use Symfony\Component\Console\Output\OutputInterface; @@ -108,7 +99,6 @@ public function __construct(array $default = []) * - `connection` The datasource connection to use * - `source` The folder where migrations are in * - `plugin` The plugin containing the migrations - * * @return array The migrations list and their statuses */ public function status(array $options = []): array diff --git a/src/Migration/Manager.php b/src/Migration/Manager.php index 07523b2e..9d39f9ec 100644 --- a/src/Migration/Manager.php +++ b/src/Migration/Manager.php @@ -22,7 +22,9 @@ use Phinx\Util\Util; use Psr\Container\ContainerInterface; use RuntimeException; -use Symfony\Component\Console\Input\ArgvInput; +use Symfony\Component\Console\Input\ArrayInput; +use Symfony\Component\Console\Input\InputDefinition; +use Symfony\Component\Console\Input\InputOption; class Manager { @@ -848,7 +850,12 @@ function ($phpFile) { $io->verbose("Constructing $class."); - $input = new ArgvInput(); + $config = $this->getConfig(); + $input = new ArrayInput([ + '--plugin' => $config['plugin'], + '--source' => $config['source'], + '--connection' => $config->getConnection(), + ]); $output = new OutputAdapter($io); // instantiate it @@ -957,7 +964,17 @@ public function getSeeds(): array /** @var \Phinx\Seed\SeedInterface[] $seeds */ $seeds = []; - $input = new ArgvInput(); + $config = $this->getConfig(); + $optionDef = new InputDefinition([ + new InputOption('plugin', mode: InputOption::VALUE_OPTIONAL, default: ''), + new InputOption('connection', mode: InputOption::VALUE_OPTIONAL, default: ''), + new InputOption('source', mode: InputOption::VALUE_OPTIONAL, default: ''), + ]); + $input = new ArrayInput([ + '--plugin' => $config['plugin'], + '--connection' => $config->getConnection(), + '--source' => $config['source'], + ], $optionDef); $output = new OutputAdapter($this->io); foreach ($phpFiles as $filePath) { diff --git a/src/Migration/ManagerFactory.php b/src/Migration/ManagerFactory.php index 7e30f477..efeabfeb 100644 --- a/src/Migration/ManagerFactory.php +++ b/src/Migration/ManagerFactory.php @@ -118,6 +118,8 @@ public function createConfig(): ConfigInterface ], 'migration_base_class' => 'Migrations\AbstractMigration', 'environment' => $adapterConfig, + 'plugin' => $plugin, + 'source' => (string)$this->getOption('source'), // TODO do we want to support the DI container in migrations? ]; diff --git a/src/Migrations.php b/src/Migrations.php index c0fcc5a7..1ed553b9 100644 --- a/src/Migrations.php +++ b/src/Migrations.php @@ -15,14 +15,10 @@ use Cake\Core\Configure; use Cake\Datasource\ConnectionManager; -use DateTime; use InvalidArgumentException; use Migrations\Migration\BuiltinBackend; use Migrations\Migration\PhinxBackend; -use Phinx\Config\Config; use Phinx\Config\ConfigInterface; -use Phinx\Db\Adapter\WrapperInterface; -use Phinx\Migration\Manager; use RuntimeException; use Symfony\Component\Console\Input\ArrayInput; use Symfony\Component\Console\Input\InputInterface; @@ -32,8 +28,6 @@ /** * The Migrations class is responsible for handling migrations command * within an none-shell application. - * - * TODO(mark) This needs to be adapted to use the configure backend selection. */ class Migrations { diff --git a/tests/TestCase/MigrationsTest.php b/tests/TestCase/MigrationsTest.php index d08b8bb9..3fc57022 100644 --- a/tests/TestCase/MigrationsTest.php +++ b/tests/TestCase/MigrationsTest.php @@ -20,7 +20,6 @@ use Cake\TestSuite\TestCase; use Exception; use InvalidArgumentException; -use Migrations\CakeAdapter; use Migrations\Migrations; use Phinx\Config\FeatureFlags; use Phinx\Db\Adapter\WrapperInterface; @@ -127,7 +126,7 @@ public static function backendProvider(): array { return [ ['builtin'], - ['phinx'] + ['phinx'], ]; } From 31c529584cba96f4c97b4d025cd402d6e9944cbf Mon Sep 17 00:00:00 2001 From: Mark Story Date: Fri, 12 Apr 2024 23:36:51 -0400 Subject: [PATCH 8/8] Fix errors. --- psalm-baseline.xml | 5 +++++ src/Migration/Manager.php | 9 ++++----- src/Migration/ManagerFactory.php | 2 +- tests/TestCase/Command/Phinx/StatusTest.php | 8 ++++++++ 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 8fe48c1d..3e399791 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -132,6 +132,11 @@ $executedVersion + + + ConfigurationTrait + + ConfigurationTrait diff --git a/src/Migration/Manager.php b/src/Migration/Manager.php index 9d39f9ec..475620cb 100644 --- a/src/Migration/Manager.php +++ b/src/Migration/Manager.php @@ -336,7 +336,6 @@ public function getVersionsToMark(Arguments $args): array * * @param string $path Path where to look for migrations * @param array $versions Versions which should be marked - * @param \Cake\Console\ConsoleIo $io ConsoleIo to write output too * @return list Output from the operation */ public function markVersionsAsMigrated(string $path, array $versions): array @@ -852,8 +851,8 @@ function ($phpFile) { $config = $this->getConfig(); $input = new ArrayInput([ - '--plugin' => $config['plugin'], - '--source' => $config['source'], + '--plugin' => $config['plugin'] ?? null, + '--source' => $config['source'] ?? null, '--connection' => $config->getConnection(), ]); $output = new OutputAdapter($io); @@ -971,9 +970,9 @@ public function getSeeds(): array new InputOption('source', mode: InputOption::VALUE_OPTIONAL, default: ''), ]); $input = new ArrayInput([ - '--plugin' => $config['plugin'], + '--plugin' => $config['plugin'] ?? null, + '--source' => $config['source'] ?? null, '--connection' => $config->getConnection(), - '--source' => $config['source'], ], $optionDef); $output = new OutputAdapter($this->io); diff --git a/src/Migration/ManagerFactory.php b/src/Migration/ManagerFactory.php index efeabfeb..0d996e9b 100644 --- a/src/Migration/ManagerFactory.php +++ b/src/Migration/ManagerFactory.php @@ -119,7 +119,7 @@ public function createConfig(): ConfigInterface 'migration_base_class' => 'Migrations\AbstractMigration', 'environment' => $adapterConfig, 'plugin' => $plugin, - 'source' => (string)$this->getOption('source'), + 'source' => (string)$this->getOption('source'), // TODO do we want to support the DI container in migrations? ]; diff --git a/tests/TestCase/Command/Phinx/StatusTest.php b/tests/TestCase/Command/Phinx/StatusTest.php index 22e8fd3e..97f493d9 100644 --- a/tests/TestCase/Command/Phinx/StatusTest.php +++ b/tests/TestCase/Command/Phinx/StatusTest.php @@ -185,6 +185,7 @@ public function testExecuteWithInconsistency() $migrations = $this->getMigrations(); $migrations->migrate(); + $migrations = $this->getMigrations(); $migrationPaths = $migrations->getConfig()->getMigrationPaths(); $migrationPath = array_pop($migrationPaths); $origin = $migrationPath . DS . '20150724233100_update_numbers_table.php'; @@ -248,7 +249,14 @@ protected function getMigrations() 'connection' => 'test', 'source' => 'TestsMigrations', ]; + $args = [ + '--connection' => $params['connection'], + '--source' => $params['source'], + ]; + $input = new ArrayInput($args, $this->command->getDefinition()); $migrations = new Migrations($params); + $migrations->setInput($input); + $this->command->setInput($input); $adapter = $migrations ->getManager($this->command->getConfig())