diff --git a/Command/MassMigrateCommand.php b/Command/MassMigrateCommand.php index e7d839c3..ce171bd6 100644 --- a/Command/MassMigrateCommand.php +++ b/Command/MassMigrateCommand.php @@ -116,7 +116,7 @@ protected function executeAsParent($input, $output, $toExecute, $start) $concurrency = $input->getOption('concurrency'); $this->writeln("Executing migrations using " . count($paths) . " processes with a concurrency of $concurrency"); - // allow forcing handling of sigchild. Useful on eg. Debian and Ubuntu + // Allow forcing handling of sigchild. Useful on eg. Debian and Ubuntu if ($input->getOption('force-sigchild-enabled')) { Process::forceSigchildEnabled(true); } @@ -231,9 +231,27 @@ protected function executeAsChild($input, $output, $toExecute, $force, $migratio } $builderArgs = parent::createChildProcessArgs($input); + } else { + $forcedRefs = array(); + if ($input->getOption('set-reference')) { + foreach ($input->getOption('set-reference') as $refSpec) { + $ref = explode(':', $refSpec, 2); + if (count($ref) < 2 || $ref[0] === '') { + throw new \InvalidArgumentException("Invalid reference specification: '$refSpec'"); + } + $forcedRefs[$ref[0]] = $ref[1]; + } + } + $migrationContext = array( + 'useTransactions' => !$input->getOption('no-transactions'), + 'defaultLanguageCode' => $input->getOption('default-language'), + 'adminUserLogin' => $input->getOption('admin-login'), + 'forceExecution' => $force, + 'forcedReferences' => $forcedRefs + ); } - // allow forcing handling of sigchild. Useful on eg. Debian and Ubuntu + // Allow forcing handling of sigchild. Useful on eg. Debian and Ubuntu if ($input->getOption('force-sigchild-enabled')) { Process::forceSigchildEnabled(true); } @@ -284,9 +302,11 @@ protected function executeAsChild($input, $output, $toExecute, $force, $migratio } else { try { - $this->executeMigrationInProcess($migrationDefinition, $force, $migrationService, $input); + $this->executeMigrationInProcess($migrationDefinition, $migrationService, $migrationContext); $executed++; + // in case the 1st mig changes values to the refs, we avoid injecting them in the 2nd mig and later + $migrationContext['forcedReferences'] = array(); } catch (\Exception $e) { $failed++; diff --git a/Command/MigrateCommand.php b/Command/MigrateCommand.php index a710c32d..963031bc 100644 --- a/Command/MigrateCommand.php +++ b/Command/MigrateCommand.php @@ -136,6 +136,24 @@ protected function execute(InputInterface $input, OutputInterface $output) $builder->setPrefix($prefix); } $builderArgs = $this->createChildProcessArgs($input); + } else { + $forcedRefs = array(); + if ($input->getOption('set-reference')) { + foreach ($input->getOption('set-reference') as $refSpec) { + $ref = explode(':', $refSpec, 2); + if (count($ref) < 2 || $ref[0] === '') { + throw new \InvalidArgumentException("Invalid reference specification: '$refSpec'"); + } + $forcedRefs[$ref[0]] = $ref[1]; + } + } + $migrationContext = array( + 'useTransaction' => !$input->getOption('no-transactions'), + 'defaultLanguageCode' => $input->getOption('default-language'), + 'adminUserLogin' => $input->getOption('admin-login'), + 'forceExecution' => $force, + 'forcedReferences' => $forcedRefs + ); } // For cli scripts, this means: do not die if anyone yanks out our stdout. @@ -143,22 +161,11 @@ protected function execute(InputInterface $input, OutputInterface $output) ignore_user_abort(true); } - // allow forcing handling of sigchild. Useful on eg. Debian and Ubuntu + // Allow forcing handling of sigchild. Useful on eg. Debian and Ubuntu if ($input->getOption('force-sigchild-enabled')) { Process::forceSigchildEnabled(true); } - if ($input->getOption('set-reference') && !$input->getOption('separate-process')) { - $refResolver = $this->getContainer()->get('ez_migration_bundle.reference_resolver.customreference'); - foreach ($input->getOption('set-reference') as $refSpec) { - $ref = explode(':', $refSpec, 2); - if (count($ref) < 2 || $ref[0] === '') { - throw new \InvalidArgumentException("Invalid reference specification: '$refSpec'"); - } - $refResolver->addReference($ref[0], $ref[1], true); - } - } - $aborted = false; $executed = 0; $failed = 0; @@ -212,9 +219,12 @@ protected function execute(InputInterface $input, OutputInterface $output) } else { try { - $this->executeMigrationInProcess($migrationDefinition, $force, $migrationService, $input); + $this->executeMigrationInProcess($migrationDefinition, $migrationService, $migrationContext); $executed++; + + // in case the 1st mig changes values to the refs, we avoid injecting them in the 2nd mig and later + $migrationContext['forcedReferences'] = array(); } catch (\Exception $e) { $failed++; @@ -260,18 +270,14 @@ protected function execute(InputInterface $input, OutputInterface $output) /** * @param MigrationDefinition $migrationDefinition - * @param bool $force * @param MigrationService $migrationService - * @param InputInterface $input + * @param array $migrationContext */ - protected function executeMigrationInProcess($migrationDefinition, $force, $migrationService, $input) + protected function executeMigrationInProcess($migrationDefinition, $migrationService, $migrationContext) { $migrationService->executeMigration( $migrationDefinition, - !$input->getOption('no-transactions'), - $input->getOption('default-language'), - $input->getOption('admin-login'), - $force + $migrationContext ); } diff --git a/Command/ResumeCommand.php b/Command/ResumeCommand.php index 8e889fad..56c75183 100644 --- a/Command/ResumeCommand.php +++ b/Command/ResumeCommand.php @@ -12,7 +12,7 @@ * Command to resume suspended migrations. * * @todo add support for resuming a set based on path - * @todo add support for the separate-process cli switch + * @todo add support for the separate-process cli switch, as well as clear-cache, default-language, force-sigchild-enabled, survive-disconnected-tty */ class ResumeCommand extends AbstractCommand { @@ -97,7 +97,6 @@ protected function execute(InputInterface $input, OutputInterface $output) $forcedRefs = array(); if ($input->getOption('set-reference') /*&& !$input->getOption('separate-process')*/) { - //$refResolver = $this->getContainer()->get('ez_migration_bundle.reference_resolver.customreference'); foreach ($input->getOption('set-reference') as $refSpec) { $ref = explode(':', $refSpec, 2); if (count($ref) < 2 || $ref[0] === '') { @@ -110,11 +109,16 @@ protected function execute(InputInterface $input, OutputInterface $output) $executed = 0; $failed = 0; + $migrationContext = array( + 'useTransaction' => !$input->getOption('no-transactions'), + 'forcedReferences' => $forcedRefs, + ); + foreach ($suspendedMigrations as $suspendedMigration) { $output->writeln("Resuming {$suspendedMigration->name}"); try { - $migrationService->resumeMigration($suspendedMigration, !$input->getOption('no-transactions'), $forcedRefs); + $migrationService->resumeMigration($suspendedMigration, $migrationContext); $executed++; } catch (\Exception $e) { @@ -126,6 +130,10 @@ protected function execute(InputInterface $input, OutputInterface $output) $this->errOutput->writeln("\nMigration aborted! Reason: " . $e->getMessage() . ""); return 1; } + + // in case we are resuming many migrations, and the 1st one changes values to the injected refs, we do not + // inject them any more from the 2nd onwards + $migrationContext['forcedReferences'] = array(); } $time = microtime(true) - $start; diff --git a/Core/Executor/MigrationDefinitionExecutor.php b/Core/Executor/MigrationDefinitionExecutor.php index d523f707..8c9aa59b 100644 --- a/Core/Executor/MigrationDefinitionExecutor.php +++ b/Core/Executor/MigrationDefinitionExecutor.php @@ -20,7 +20,7 @@ class MigrationDefinitionExecutor extends AbstractExecutor use ReferenceSetterTrait; protected $supportedStepTypes = array('migration_definition'); - protected $supportedActions = array('generate', 'save'); + protected $supportedActions = array('generate', 'save', 'include'); /** @var \Kaliop\eZMigrationBundle\Core\MigrationService $migrationService */ protected $migrationService; @@ -52,9 +52,45 @@ public function execute(MigrationStep $step) $this->skipStepIfNeeded($step); + if ($action === 'include') { + // we can not use a keyword as method name + $action = 'run'; + } + return $this->$action($step->dsl, $step->context); } + public function run($dsl, $context) + { + if (!isset($dsl['file'])) { + throw new InvalidStepDefinitionException("Invalid step definition: miss 'file'"); + } + $fileName = $this->resolveReference($dsl['file']); + + // default format: path is relative to the current mig dir + $realFilePath = dirname($context['path']) . $fileName; + // but we support as well absolute paths + if (!is_file($realFilePath) && is_file($fileName)) { + $realFilePath = $fileName; + } + + $migrationDefinitions = $this->migrationService->getMigrationsDefinitions(array($realFilePath)); + if (!count($migrationDefinitions)) { + throw new InvalidStepDefinitionException("Invalid step definition: '$fileName' is not a valid migration definition"); + } + + // avoid overwriting the 'current migration definition file path' for the included definition + unset($context['path']); + + /// @todo can we could have the included migration's steps be printed as 1.1, 1.2 etc... + /// @todo we could return the result of the included migration's last step + foreach($migrationDefinitions as $migrationDefinition) { + $this->migrationService->executeMigration($migrationDefinition, $context); + } + + return true; + } + /** * @todo allow to save to disk * @param array $dsl diff --git a/Core/MigrationService.php b/Core/MigrationService.php index c8dccac1..4d464b9e 100644 --- a/Core/MigrationService.php +++ b/Core/MigrationService.php @@ -290,19 +290,23 @@ public function parseMigrationDefinition(MigrationDefinition $migrationDefinitio } /** + * Note: previous API is kept for BC (subclasses reimplementing this method). * @param MigrationDefinition $migrationDefinition - * @param bool $useTransaction when set to false, no repo transaction will be used to wrap the migration - * @param string $defaultLanguageCode - * @param string|int|false|null $adminLogin when false, current user is used; when null, hardcoded admin account - * @param bool $force when true, execute a migration if it was already in status DONE or SKIPPED (would throw by default) - * @param bool|null forceSigchildEnabled + * @param array $migrationContext Supported array keys are: adminUserLogin, defaultLanguageCode, + * forcedReferences, forceExecution, forceSigchildEnabled, userContentType, userGroupContentType, + * useTransaction. + * Bool usage is deprecated. It was: when set to false, no repo transaction will be used to wrap the migration + * @param string $defaultLanguageCode Deprecated - use $migrationContext['defaultLanguageCode'] + * @param string|int|false|null $adminLogin Deprecated - use $migrationContext['adminLogin']; when false, current user is used; when null, hardcoded admin account + * @param bool $force Deprecated - use $migrationContext['forceExecution']; when true, execute a migration if it was already in status DONE or SKIPPED (would throw by default) + * @param bool|null $forceSigchildEnabled Deprecated * @throws \Exception * + * @todo add support for setting in $migrationContext a userContentType, userGroupContentType ? * @todo treating a null and false $adminLogin values differently is prone to hard-to-track errors. * Shall we use instead -1 to indicate the desire to not-login-as-admin-user-at-all ? - * @todo refactor. There are too many parameters here to add more. Move to a single parameter: array of options or value-object */ - public function executeMigration(MigrationDefinition $migrationDefinition, $useTransaction = true, + public function executeMigration(MigrationDefinition $migrationDefinition, $migrationContext = true, $defaultLanguageCode = null, $adminLogin = null, $force = false, $forceSigchildEnabled = null) { if ($migrationDefinition->status == MigrationDefinition::STATUS_TO_PARSE) { @@ -313,27 +317,57 @@ public function executeMigration(MigrationDefinition $migrationDefinition, $useT throw new MigrationBundleException("Can not execute " . $this->getEntityName($migrationDefinition). " '{$migrationDefinition->name}': {$migrationDefinition->parsingError}"); } - /// @todo add support for setting in $migrationContext a userContentType, userGroupContentType ? - $migrationContext = $this->migrationContextFromParameters($defaultLanguageCode, $adminLogin, $forceSigchildEnabled); + // BC: handling of legacy method call signature + if (!is_array($migrationContext)) { + $useTransaction = $migrationContext; + $migrationContext = $this->migrationContextFromParameters($defaultLanguageCode, $adminLogin, $forceSigchildEnabled); + $migrationContext['useTransaction'] = $useTransaction; + $migrationContext['forceExecution'] = $force; + } else { + if ($defaultLanguageCode !== null || $adminLogin !== null || $force !== false || $forceSigchildEnabled !== null) { + throw new MigrationBundleException("Invalid call to executeMigration: argument types mismatch"); + } + } + if ($this->output) { + $migrationContext['output'] = $this->output; + } + $forceExecution = array_key_exists('forceExecution', $migrationContext) ? $migrationContext['forceExecution'] : false; // set migration as begun - has to be in own db transaction - $migration = $this->storageHandler->startMigration($migrationDefinition, $force); + $migration = $this->storageHandler->startMigration($migrationDefinition, $forceExecution); - $this->executeMigrationInner($migration, $migrationDefinition, $migrationContext, 0, $useTransaction, $adminLogin); + $this->executeMigrationInner($migration, $migrationDefinition, $migrationContext); } /** + * Note: previous API is kept for BC (subclasses reimplementing this method). * @param Migration $migration * @param MigrationDefinition $migrationDefinition * @param array $migrationContext * @param int $stepOffset - * @param bool $useTransaction when set to false, no repo transaction will be used to wrap the migration - * @param string|int|false|null $adminLogin used only for committing db transaction if needed. If false or null, hardcoded admin is used + * @param bool $useTransaction Deprecated - replaced by $migrationContext['useTransaction']. When set to false, no repo transaction will be used to wrap the migration + * @param string|int|false|null $adminLogin Deprecated - $migrationContext['adminLogin']. Used only for committing db transaction if needed. If false or null, hardcoded admin is used * @throws \Exception */ protected function executeMigrationInner(Migration $migration, MigrationDefinition $migrationDefinition, $migrationContext, $stepOffset = 0, $useTransaction = true, $adminLogin = null) { + // BC: handling of legacy method call signature + $useTransaction = array_key_exists('useTransaction', $migrationContext) ? $migrationContext['useTransaction'] : $useTransaction; + $adminLogin = array_key_exists('adminLogin', $migrationContext) ? $migrationContext['adminLogin'] : $adminLogin; + + /// @todo cane we make this validation smarter / move it somewhere else? + if (array_key_exists('path', $migrationContext) || array_key_exists('contentTypeIdentifier', $migrationContext) || + array_key_exists('fieldIdentifier', $migrationContext)) { + throw new MigrationBundleException("Invalid call to executeMigrationInner: forbidden elements in migrationContext"); + } + + if (isset($migrationContext['forcedReferences'])) { + foreach ($migrationContext['forcedReferences'] as $name => $value) { + $this->referenceResolver->addReference($name, $value, true); + } + } + if ($useTransaction) { $this->repository->beginTransaction(); } @@ -466,20 +500,31 @@ protected function executeMigrationInner(Migration $migration, MigrationDefiniti true ); - throw $exception ? $exception : new MigrationStepExecutionException($errorMessage, $i, $e); + throw ($exception ? $exception : new MigrationStepExecutionException($errorMessage, $i, $e)); } } /** + * Note: previous API is kept for BC (subclasses reimplementing this method). * @param Migration $migration - * @param bool $useTransaction - * @param array $forcedReferences + * @param array $migrationContext see executeMigration + * @param array $forcedReferences Deprecated - use $migrationContext['forcedReferences'] * @throws \Exception - * - * @todo add support for adminLogin ? */ - public function resumeMigration(Migration $migration, $useTransaction = true, array $forcedReferences = array()) + public function resumeMigration(Migration $migration, $migrationContext = true, array $forcedReferences = array()) { + // BC: handling of legacy method call signature + if (!is_array($migrationContext)) { + $migrationContext = array( + 'useTransaction' => $migrationContext, + 'forcedReferences' => $forcedReferences, + ); + } else { + if (!is_array($forcedReferences) || count($forcedReferences)) { + throw new MigrationBundleException("Invalid call to resumeMigration: argument types mismatch"); + } + } + if ($migration->status != Migration::STATUS_SUSPENDED) { throw new MigrationBundleException("Can not resume ".$this->getEntityName($migration)." '{$migration->name}': it is not in suspended status"); } @@ -499,13 +544,6 @@ public function resumeMigration(Migration $migration, $useTransaction = true, ar // restore context $this->contextHandler->restoreCurrentContext($migration->name); - - if ($forcedReferences) { - foreach ($forcedReferences as $name => $value) { - $this->referenceResolver->addReference($name, $value, true); - } - } - if (!isset($this->migrationContext[$migration->name])) { throw new MigrationBundleException("Can not resume ".$this->getEntityName($migration)." '{$migration->name}': the stored context is missing"); } @@ -522,15 +560,16 @@ public function resumeMigration(Migration $migration, $useTransaction = true, ar // and go // note: we store the current step counting starting at 1, but use offset starting at 0, hence the -1 here - $this->executeMigrationInner($migration, $migrationDefinition, $restoredContext['context'], - $restoredContext['step'] - 1, $useTransaction); + $this->executeMigrationInner($migration, $migrationDefinition, array_merge($restoredContext['context'], $migrationContext), + $restoredContext['step'] - 1); } /** * @param string $defaultLanguageCode * @param string|int|false $adminLogin - * @param bool|null $forceSigchildEnabled + * @param bool|null $forceSigchildEnabled Doubly Deprecated! * @return array + * @deprecated kept for BC */ protected function migrationContextFromParameters($defaultLanguageCode = null, $adminLogin = null, $forceSigchildEnabled = null) { @@ -543,14 +582,10 @@ protected function migrationContextFromParameters($defaultLanguageCode = null, $ if ($adminLogin !== null) { $properties['adminUserLogin'] = $adminLogin; } - if ($forceSigchildEnabled !== null) - { - $properties['forceSigchildEnabled'] = $forceSigchildEnabled; - } - - if ($this->output) { - $properties['output'] = $this->output; - } + //if ($forceSigchildEnabled !== null) + //{ + // $properties['forceSigchildEnabled'] = $forceSigchildEnabled; + //} return $properties; } @@ -649,6 +684,7 @@ public function getCurrentContext($migrationName) } /** + * This gets called when we call $this->contextHandler->restoreCurrentContext(). * @param string $migrationName * @param array $context */ diff --git a/Resources/doc/DSL/MigrationDefinitions.yml b/Resources/doc/DSL/MigrationDefinitions.yml index eb6436d2..ae7a38af 100644 --- a/Resources/doc/DSL/MigrationDefinitions.yml +++ b/Resources/doc/DSL/MigrationDefinitions.yml @@ -42,3 +42,11 @@ if: # Optional. If set, the migration step will be skipped unless the condition is matched "reference:_ref_name": # name of a reference to be used for the test _operator_: value # allowed operators: eq, gt, gte, lt, lte, ne, count, length, regexp, satisfies + +- + type: migration_definition + mode: include + file: string # path to the migration definition file to be included. Mandatory. References will be resolved + if: # Optional. If set, the migration step will be skipped unless the condition is matched + "reference:_ref_name": # name of a reference to be used for the test + _operator_: value # allowed operators: eq, gt, gte, lt, lte, ne, count, length, regexp, satisfies diff --git a/Tests/dsl/good/UnitTestOK042_includes.yml b/Tests/dsl/good/UnitTestOK042_includes.yml new file mode 100644 index 00000000..dbf42ebb --- /dev/null +++ b/Tests/dsl/good/UnitTestOK042_includes.yml @@ -0,0 +1,35 @@ +- + type: reference + mode: set + identifier: kmb_test_ref_42 + value: 1 + +- + type: file + mode: save + file: /tmp/UnitTestOK042_included.yml + overwrite: true + body: | + - + type: reference + mode: set + identifier: kmb_test_ref_42 + value: "eval: 1 + resolve('reference:kmb_test_ref_42')" + overwrite: true + +- + type: migration_definition + mode: include + file: /tmp/UnitTestOK042_included.yml + +- + type: assert + target: reference + identifier: reference:kmb_test_ref_42 + test: + Equals: 2 + +- + type: file + mode: delete + file: /tmp/UnitTestOK042_included.yml diff --git a/WHATSNEW.md b/WHATSNEW.md index 3ed80de3..18b2a254 100644 --- a/WHATSNEW.md +++ b/WHATSNEW.md @@ -1,3 +1,32 @@ +Version 6.3.0 (unreleased) +========================== + +* New: migration step `migration_definition/include`. This allows one migration to basically include another, the same + way it is possible to do that in php. + + It is useful for scenarios such as fe. creating a library of reusable migrations, which can be run multiple times with + different target contents every time. This is often achieved by copy-pasting the same migration logic many times. + As an alternative it is now possible to create a "library" migration, driven by references, and store it only once, + in a separate folder, then create many "specific execution" migrations which set up values for the required references + and include the library migration's definition. + + Please note that migrations which rely on external resources, such as in this case would be the included migration, go + against the principle of migrations being immutable for ease of replay and analysis. + + Ex: + + - + type: migration_definition + mode: include + file: a_path + +* BC change (for developers extending the bundle): method `MigrateCommand::executeMigrationInProcess` changed its signature + +* BC change (for developers extending the bundle): `Migrationservice` methods `executeMigration`, `executeMigrationInner` + and `resumeMigration` should now be called using a different signature. They do still work with the previous signature, + but that usage is considered deprecated + + Version 6.2.1 =============