From 57d19c8879d02b5c3fb11065db33ee65b85e4613 Mon Sep 17 00:00:00 2001 From: gggeek Date: Wed, 16 Jan 2019 16:27:27 +0000 Subject: [PATCH] fix: propagate -a flag to subprocesses --- API/StorageHandlerInterface.php | 5 +- Command/GenerateCommand.php | 31 +++++-- Command/MassMigrateCommand.php | 94 ++++++++++++---------- Command/MigrateCommand.php | 81 ++++++++++++------- Core/StorageHandler/Database/Migration.php | 31 ++++--- Resources/doc/DSL/ContentTypes.yml | 1 + WHATSNEW.md | 15 ++++ 7 files changed, 167 insertions(+), 91 deletions(-) diff --git a/API/StorageHandlerInterface.php b/API/StorageHandlerInterface.php index 1ac9155b..06c6d361 100644 --- a/API/StorageHandlerInterface.php +++ b/API/StorageHandlerInterface.php @@ -45,10 +45,11 @@ public function addMigration(MigrationDefinition $migrationDefinition); * Starts a migration (creates and stores it, in STARTED status) * * @param MigrationDefinition $migrationDefinition + * @param bool $force * @return Migration - * @throws \Exception If the migration was already executed, skipped or executing + * @throws \Exception If the migration was already executing. Also if it as already (executed|skipped) unless $force is true */ - public function startMigration(MigrationDefinition $migrationDefinition); + public function startMigration(MigrationDefinition $migrationDefinition, $force = false); /** * Ends a migration (updates it) diff --git a/Command/GenerateCommand.php b/Command/GenerateCommand.php index 32fb3ded..c70a615e 100644 --- a/Command/GenerateCommand.php +++ b/Command/GenerateCommand.php @@ -33,9 +33,10 @@ protected function configure() ->addOption('match-type', null, InputOption::VALUE_REQUIRED, 'The type of identifier used to find the entity to generate the migration for', null) ->addOption('match-value', null, InputOption::VALUE_REQUIRED, 'The identifier value used to find the entity to generate the migration for. Can have many values separated by commas', null) ->addOption('match-except', null, InputOption::VALUE_NONE, 'Used to match all entities except the ones satisfying the match-value condition', null) - ->addOption('lang', null, InputOption::VALUE_REQUIRED, 'The language of the migration (eng-GB, ger-DE, ...)', 'eng-GB') + ->addOption('lang', 'l', InputOption::VALUE_REQUIRED, 'The language of the migration (eng-GB, ger-DE, ...). If null, the default language of the current siteaccess is used') ->addOption('dbserver', null, InputOption::VALUE_REQUIRED, 'The type of the database server the sql migration is for, when type=db (mysql, postgresql, ...)', 'mysql') ->addOption('role', null, InputOption::VALUE_REQUIRED, 'Deprecated: The role identifier (or id) that you would like to update, for type=role', null) + ->addOption('admin-login', 'a', InputOption::VALUE_REQUIRED, "Login of admin account used whenever elevated privileges are needed (user id 14 used by default)") ->addArgument('bundle', InputArgument::REQUIRED, 'The bundle to generate the migration definition file in. eg.: AcmeMigrationBundle') ->addArgument('name', InputArgument::OPTIONAL, 'The migration name (will be prefixed with current date)', null) ->setHelp(<< $matchValue, 'matchExcept' => $matchExcept, 'mode' => $mode, - 'lang' => $input->getOption('lang') + 'lang' => $input->getOption('lang'), + 'adminLogin' => $input->getOption('admin-login') ); $date = date('YmdHis'); @@ -207,6 +209,7 @@ public function execute(InputInterface $input, OutputInterface $output) /** * Generates a migration definition file. + * @todo allow non-filesystem storage * * @param string $path filename to file to generate (full path) * @param string $fileType The type of migration file to generate @@ -241,10 +244,7 @@ protected function generateMigrationFile($path, $fileType, $migrationType, array } $executor = $this->getMigrationService()->getExecutor($migrationType); - $context = array(); - if (isset($parameters['lang']) && $parameters['lang'] != '') { - $context['defaultLanguageCode'] = $parameters['lang']; - } + $context = $this->migrationContextFromParameters($parameters); $matchCondition = array($parameters['matchType'] => $parameters['matchValue']); if ($parameters['matchExcept']) { @@ -312,4 +312,23 @@ protected function getGeneratingExecutors() } return $executors; } + + /** + * @see MigrationService::migrationContextFromParameters + * @param array $parameters + * @return array + */ + protected function migrationContextFromParameters(array $parameters) + { + $context = array(); + + if (isset($parameters['lang']) && $parameters['lang'] != '') { + $context['defaultLanguageCode'] = $parameters['lang']; + } + if (isset($parameters['adminLogin']) && $parameters['adminLogin'] != '') { + $context['adminUserLogin'] = $parameters['adminLogin']; + } + + return $context; + } } diff --git a/Command/MassMigrateCommand.php b/Command/MassMigrateCommand.php index 091e9d58..ebcfa191 100644 --- a/Command/MassMigrateCommand.php +++ b/Command/MassMigrateCommand.php @@ -89,41 +89,17 @@ protected function execute(InputInterface $input, OutputInterface $output) } $concurrency = $input->getOption('concurrency'); - $this->writeln("Executing migrations using ".count($paths)." processes with a concurrency of $concurrency"); + $this->writeln("Executing migrations using " . count($paths) . " processes with a concurrency of $concurrency"); - $kernel = $this->getContainer()->get('kernel'); $builder = new ProcessBuilder(); $executableFinder = new PhpExecutableFinder(); if (false !== ($php = $executableFinder->find())) { $builder->setPrefix($php); } + // mandatory args and options - $builderArgs = array( - $_SERVER['argv'][0], // sf console - self::COMMAND_NAME, // name of sf command. Can we get it from the Application instead of hardcoding? - '--env=' . $kernel-> getEnvironment(), // sf env - '--child' - ); - // 'optional' options - // note: options 'clear-cache' we never propagate - if (!$kernel->isDebug()) { - $builderArgs[] = '--no-debug'; - } - if ($input->getOption('default-language')) { - $builderArgs[]='--default-language='.$input->getOption('default-language'); - } - if ($input->getOption('no-transactions')) { - $builderArgs[]='--no-transactions'; - } - if ($input->getOption('siteaccess')) { - $builderArgs[]='--siteaccess='.$input->getOption('siteaccess'); - } - if ($input->getOption('ignore-failures')) { - $builderArgs[]='--ignore-failures'; - } - if ($input->getOption('separate-process')) { - $builderArgs[]='--separate-process'; - } + $builderArgs = $this->createChildProcessArgs($input); + $processes = array(); /** @var MigrationDefinition $migrationDefinition */ foreach($paths as $path => $count) { @@ -181,21 +157,8 @@ protected function execute(InputInterface $input, OutputInterface $output) if (false !== $php = $executableFinder->find()) { $builder->setPrefix($php); } - // mandatory args and options - $builderArgs = array( - $_SERVER['argv'][0], // sf console - MigrateCommand::COMMAND_NAME, // name of sf command - '--env=' . $this->getContainer()->get('kernel')->getEnvironment(), // sf env - '--child' - ); - // 'optional' options - // note: options 'clear-cache', 'ignore-failures' and 'no-transactions' we never propagate - if ($input->getOption('default-language')) { - $builderArgs[] = '--default-language=' . $input->getOption('default-language'); - } - if ($input->getOption('no-transactions')) { - $builderArgs[] = '--no-transactions'; - } + + $builderArgs = parent::createChildProcessArgs($input); } $failed = 0; @@ -433,4 +396,49 @@ protected function groupMigrationsByPath($toExecute) return $paths; } + /** + * Returns the command-line arguments needed to execute a migration in a separate subprocess (omitting 'path') + * @param InputInterface $input + * @return array + */ + protected function createChildProcessArgs(InputInterface $input) + { + $kernel = $this->getContainer()->get('kernel'); + + // mandatory args and options + $builderArgs = array( + $_SERVER['argv'][0], // sf console + self::COMMAND_NAME, // name of sf command. Can we get it from the Application instead of hardcoding? + '--env=' . $kernel-> getEnvironment(), // sf env + '--child' + ); + // sf/ez env options + if (!$kernel->isDebug()) { + $builderArgs[] = '--no-debug'; + } + if ($input->getOption('siteaccess')) { + $builderArgs[] = '--siteaccess=' . $input->getOption('siteaccess'); + } + // 'optional' options + // note: options 'clear-cache', 'no-interaction', 'path' we never propagate + if ($input->getOption('admin-login')) { + $builderArgs[] = '--admin-login=' . $input->getOption('admin-login'); + } + if ($input->getOption('default-language')) { + $builderArgs[] = '--default-language=' . $input->getOption('default-language'); + } + if ($input->getOption('force')) { + $builderArgs[] = '--force'; + } + if ($input->getOption('ignore-failures')) { + $builderArgs[] = '--ignore-failures'; + } + if ($input->getOption('no-transactions')) { + $builderArgs[] = '--no-transactions'; + } + if ($input->getOption('separate-process')) { + $builderArgs[] = '--separate-process'; + } + + } } diff --git a/Command/MigrateCommand.php b/Command/MigrateCommand.php index 09cadd23..c91a0208 100644 --- a/Command/MigrateCommand.php +++ b/Command/MigrateCommand.php @@ -41,14 +41,15 @@ protected function configure() ->setName(self::COMMAND_NAME) ->setAliases(array('kaliop:migration:update')) ->setDescription('Execute available migration definitions.') - ->addOption('path', null, InputOption::VALUE_OPTIONAL | InputOption::VALUE_IS_ARRAY, "The directory or file to load the migration definitions from") // nb: when adding options, remember to forward them to sub-commands executed in 'separate-process' mode - ->addOption('default-language', 'l', InputOption::VALUE_REQUIRED, "Default language code that will be used if no language is provided in migration steps") ->addOption('admin-login', 'a', InputOption::VALUE_REQUIRED, "Login of admin account used whenever elevated privileges are needed (user id 14 used by default)") - ->addOption('ignore-failures', 'i', InputOption::VALUE_NONE, "Keep executing migrations even if one fails") ->addOption('clear-cache', 'c', InputOption::VALUE_NONE, "Clear the cache after the command finishes") + ->addOption('default-language', 'l', InputOption::VALUE_REQUIRED, "Default language code that will be used if no language is provided in migration steps") + ->addOption('force', 'f', InputOption::VALUE_NONE, "Force (re)execution of migrations already DONE, SKIPPED or FAILED. Use with great care!") + ->addOption('ignore-failures', 'i', InputOption::VALUE_NONE, "Keep executing migrations even if one fails") ->addOption('no-interaction', 'n', InputOption::VALUE_NONE, "Do not ask any interactive question") ->addOption('no-transactions', 'u', InputOption::VALUE_NONE, "Do not use a repository transaction to wrap each migration. Unsafe, but needed for legacy slot handlers") + ->addOption('path', null, InputOption::VALUE_OPTIONAL | InputOption::VALUE_IS_ARRAY, "The directory or file to load the migration definitions from") ->addOption('separate-process', 'p', InputOption::VALUE_NONE, "Use a separate php process to run each migration. Safe if your migration leak memory. A tad slower") ->addOption('child', null, InputOption::VALUE_NONE, "*DO NOT USE* Internal option for when forking separate processes") ->setHelp(<<find()) { $builder->setPrefix($php); } - // mandatory args and options - $builderArgs = array( - $_SERVER['argv'][0], // sf console - self::COMMAND_NAME, // name of sf command. Can we get it from the Application instead of hardcoding? - '--env=' . $kernel->getEnvironment(), // sf env - '--child' - ); - // 'optional' options - // note: options 'clear-cache', 'ignore-failures' and 'no-transactions' we never propagate - if (!$kernel->isDebug()) { - $builderArgs[] = '--no-debug'; - } - if ($input->getOption('default-language')) { - $builderArgs[] = '--default-language=' . $input->getOption('default-language'); - } - if ($input->getOption('no-transactions')) { - $builderArgs[] = '--no-transactions'; - } - if ($input->getOption('siteaccess')) { - $builderArgs[]='--siteaccess='.$input->getOption('siteaccess'); - } + $builderArgs = $this->createChildProcessArgs($input); } $executed = 0; @@ -247,19 +228,25 @@ function($type, $buffer) { /** * @param string[] $paths * @param MigrationService $migrationService + * @param bool $force when true, look not only for TODO migrations, but also DONE, SKIPPED, FAILED ones (we still omit STARTED and SUSPENDED ones) * @return MigrationDefinition[] * * @todo this does not scale well with many definitions or migrations */ - protected function buildMigrationsList($paths, $migrationService) + protected function buildMigrationsList($paths, $migrationService, $force = false) { $migrationDefinitions = $migrationService->getMigrationsDefinitions($paths); $migrations = $migrationService->getMigrations(); + $allowedStatuses = array(Migration::STATUS_TODO); + if ($force) { + $allowedStatuses = array_merge($allowedStatuses, array(Migration::STATUS_DONE, Migration::STATUS_FAILED, Migration::STATUS_SKIPPED)); + } + // filter away all migrations except 'to do' ones $toExecute = array(); foreach ($migrationDefinitions as $name => $migrationDefinition) { - if (!isset($migrations[$name]) || (($migration = $migrations[$name]) && $migration->status == Migration::STATUS_TODO)) { + if (!isset($migrations[$name]) || (($migration = $migrations[$name]) && in_array($migration->status, $allowedStatuses))) { $toExecute[$name] = $migrationService->parseMigrationDefinition($migrationDefinition); } } @@ -268,7 +255,7 @@ protected function buildMigrationsList($paths, $migrationService) // found by the loader if (empty($paths)) { foreach ($migrations as $migration) { - if ($migration->status == Migration::STATUS_TODO && !isset($toExecute[$migration->name])) { + if (in_array($migration->status, $allowedStatuses) && !isset($toExecute[$migration->name])) { $migrationDefinitions = $migrationService->getMigrationsDefinitions(array($migration->path)); if (count($migrationDefinitions)) { $migrationDefinition = reset($migrationDefinitions); @@ -361,4 +348,44 @@ protected function setVerbosity($verbosity) $this->verbosity = $verbosity; } + /** + * Returns the command-line arguments needed to execute a migration in a separate subprocess (omitting 'path') + * @param InputInterface $input + * @return array + */ + protected function createChildProcessArgs(InputInterface $input) + { + $kernel = $this->getContainer()->get('kernel'); + + // mandatory args and options + $builderArgs = array( + $_SERVER['argv'][0], // sf console + self::COMMAND_NAME, // name of sf command. Can we get it from the Application instead of hardcoding? + '--env=' . $kernel->getEnvironment(), // sf env + '--child' + ); + // sf/ez env options + if (!$kernel->isDebug()) { + $builderArgs[] = '--no-debug'; + } + if ($input->getOption('siteaccess')) { + $builderArgs[]='--siteaccess='.$input->getOption('siteaccess'); + } + // 'optional' options + // note: options 'clear-cache', 'ignore-failures', 'no-interaction', 'path' and 'separate-process' we never propagate + if ($input->getOption('admin-login')) { + $builderArgs[] = '--admin-login=' . $input->getOption('admin-login'); + } + if ($input->getOption('default-language')) { + $builderArgs[] = '--default-language=' . $input->getOption('default-language'); + } + if ($input->getOption('force')) { + $builderArgs[] = '--force'; + } + if ($input->getOption('no-transactions')) { + $builderArgs[] = '--no-transactions'; + } + + return $builderArgs; + } } diff --git a/Core/StorageHandler/Database/Migration.php b/Core/StorageHandler/Database/Migration.php index feebefd7..6eecf174 100644 --- a/Core/StorageHandler/Database/Migration.php +++ b/Core/StorageHandler/Database/Migration.php @@ -149,13 +149,14 @@ public function addMigration(MigrationDefinition $migrationDefinition) * Starts a migration, given its definition: stores its status in the db, returns the Migration object * * @param MigrationDefinition $migrationDefinition + * @param bool $force when true, starts migrations even if they already exist in DONE, SKIPPED status * @return APIMigration * @throws \Exception if migration was already executing or already done * @todo add a parameter to allow re-execution of already-done migrations */ - public function startMigration(MigrationDefinition $migrationDefinition) + public function startMigration(MigrationDefinition $migrationDefinition, $force = false) { - return $this->createMigration($migrationDefinition, APIMigration::STATUS_STARTED, 'started'); + return $this->createMigration($migrationDefinition, APIMigration::STATUS_STARTED, 'started', $force); } /** @@ -253,10 +254,11 @@ public function skipMigration(MigrationDefinition $migrationDefinition) * @param MigrationDefinition $migrationDefinition * @param int $status * @param string $action + * @param bool $force * @return APIMigration * @throws \Exception */ - protected function createMigration(MigrationDefinition $migrationDefinition, $status, $action) + protected function createMigration(MigrationDefinition $migrationDefinition, $status, $action, $force = false) { $this->createTableIfNeeded(); @@ -280,21 +282,24 @@ protected function createMigration(MigrationDefinition $migrationDefinition, $st if (is_array($existingMigrationData)) { // migration exists - // fail if it was already executing or already done + // fail if it was already executing if ($existingMigrationData['status'] == APIMigration::STATUS_STARTED) { // commit to release the lock $conn->commit(); throw new \Exception("Migration '{$migrationDefinition->name}' can not be $action as it is already executing"); } - if ($existingMigrationData['status'] == APIMigration::STATUS_DONE) { - // commit to release the lock - $conn->commit(); - throw new \Exception("Migration '{$migrationDefinition->name}' can not be $action as it was already executed"); - } - if ($existingMigrationData['status'] == APIMigration::STATUS_SKIPPED) { - // commit to release the lock - $conn->commit(); - throw new \Exception("Migration '{$migrationDefinition->name}' can not be $action as it was already skipped"); + // fail if it was already already done, unless in 'force' mode + if (!$force) { + if ($existingMigrationData['status'] == APIMigration::STATUS_DONE) { + // commit to release the lock + $conn->commit(); + throw new \Exception("Migration '{$migrationDefinition->name}' can not be $action as it was already executed"); + } + if ($existingMigrationData['status'] == APIMigration::STATUS_SKIPPED) { + // commit to release the lock + $conn->commit(); + throw new \Exception("Migration '{$migrationDefinition->name}' can not be $action as it was already skipped"); + } } // do not set migration start date if we are skipping it diff --git a/Resources/doc/DSL/ContentTypes.yml b/Resources/doc/DSL/ContentTypes.yml index 2bb402c7..71d2d285 100644 --- a/Resources/doc/DSL/ContentTypes.yml +++ b/Resources/doc/DSL/ContentTypes.yml @@ -155,6 +155,7 @@ _condition_: value # where _condition_ can be any of ones specified above, including 'and' and 'or' content_type_group: x # Id or identifier of a new group we want to put the new content type into (this will not remove it from the current group) remove_content_type_group: x # Id or identifier of a group we want to remove this content type from + default_always_available: true|false # Optional description: xyz # Optional, will be updated if set. Either a string, or an array in the format { "eng-GB": "This", "fre-FR": "ceci", ... } is_container: true|false # Optional, will be updated if set lang: xxx-YY # Optional, will fallback to default language if not provided (--default-language option or 'eng-GB' by default) diff --git a/WHATSNEW.md b/WHATSNEW.md index d9281b62..bb241334 100644 --- a/WHATSNEW.md +++ b/WHATSNEW.md @@ -1,3 +1,18 @@ +Version 5.8.0 (unreleased) +========================== + +* New: the `content_type/update` migration step now accepts `the default_always_available` element (issue #189) + +* New: the `kaliop:migration:generate` command now accepts a `-a` flag to allow specifying custom admin users (issue #187) + +* Fix: usage of the `-a` flag when running `kaliop:migration:mass_migrate` and when running `kaliop:migration:migrate -p` + was not propagated to subprocesses + +* BC changes: + + - the `kaliop:migration:generate` command now uses as default language for the generated migrations the default one + of the current siteaccess, instead of 'eng-GB' + Version 5.7.3 =============