kaliop-uk /
ezmigrationbundle
Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.
Common duplication problems, and corresponding solutions are:
Complex classes like MigrateCommand often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use MigrateCommand, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 21 | class MigrateCommand extends AbstractCommand |
||
| 22 | { |
||
| 23 | // in between QUIET and NORMAL |
||
| 24 | const VERBOSITY_CHILD = 0.5; |
||
| 25 | /** @var OutputInterface $output */ |
||
| 26 | protected $output; |
||
| 27 | protected $verbosity = OutputInterface::VERBOSITY_NORMAL; |
||
| 28 | |||
| 29 | const COMMAND_NAME = 'kaliop:migration:migrate'; |
||
| 30 | |||
| 31 | /** |
||
| 32 | * Set up the command. |
||
| 33 | * |
||
| 34 | * Define the name, options and help text. |
||
| 35 | */ |
||
| 36 | 76 | protected function configure() |
|
| 37 | { |
||
| 38 | 76 | parent::configure(); |
|
| 39 | |||
| 40 | $this |
||
| 41 | 76 | ->setName(self::COMMAND_NAME) |
|
| 42 | 76 | ->setAliases(array('kaliop:migration:update')) |
|
| 43 | 76 | ->setDescription('Execute available migration definitions.') |
|
| 44 | // nb: when adding options, remember to forward them to sub-commands executed in 'separate-process' mode |
||
| 45 | 76 | ->addOption('admin-login', 'a', InputOption::VALUE_REQUIRED, "Login of admin account used whenever elevated privileges are needed (user id 14 used by default)") |
|
| 46 | 76 | ->addOption('clear-cache', 'c', InputOption::VALUE_NONE, "Clear the cache after the command finishes") |
|
| 47 | 76 | ->addOption('default-language', 'l', InputOption::VALUE_REQUIRED, "Default language code that will be used if no language is provided in migration steps") |
|
| 48 | 76 | ->addOption('force', 'f', InputOption::VALUE_NONE, "Force (re)execution of migrations already DONE, SKIPPED or FAILED. Use with great care!") |
|
| 49 | 76 | ->addOption('ignore-failures', 'i', InputOption::VALUE_NONE, "Keep executing migrations even if one fails") |
|
| 50 | 76 | ->addOption('no-interaction', 'n', InputOption::VALUE_NONE, "Do not ask any interactive question") |
|
| 51 | 76 | ->addOption('no-transactions', 'u', InputOption::VALUE_NONE, "Do not use a repository transaction to wrap each migration. Unsafe, but needed for legacy slot handlers") |
|
| 52 | 76 | ->addOption('path', null, InputOption::VALUE_OPTIONAL | InputOption::VALUE_IS_ARRAY, "The directory or file to load the migration definitions from") |
|
| 53 | 76 | ->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") |
|
| 54 | 76 | ->addOption('child', null, InputOption::VALUE_NONE, "*DO NOT USE* Internal option for when forking separate processes") |
|
| 55 | 76 | ->setHelp(<<<EOT |
|
| 56 | 76 | The <info>kaliop:migration:migrate</info> command loads and executes migrations: |
|
| 57 | |||
| 58 | <info>./ezpublish/console kaliop:migration:migrate</info> |
||
| 59 | |||
| 60 | You can optionally specify the path to migration definitions with <info>--path</info>: |
||
| 61 | |||
| 62 | <info>./ezpublish/console kaliop:migrations:migrate --path=/path/to/bundle/version_directory --path=/path/to/bundle/version_directory/single_migration_file</info> |
||
| 63 | EOT |
||
| 64 | ); |
||
| 65 | 76 | } |
|
| 66 | |||
| 67 | /** |
||
| 68 | * Execute the command. |
||
| 69 | * |
||
| 70 | * @param InputInterface $input |
||
| 71 | * @param OutputInterface $output |
||
| 72 | * @return null|int null or 0 if everything went fine, or an error code |
||
| 73 | */ |
||
| 74 | protected function execute(InputInterface $input, OutputInterface $output) |
||
| 75 | { |
||
| 76 | $start = microtime(true); |
||
| 77 | |||
| 78 | $this->setOutput($output); |
||
| 79 | $this->setVerbosity($output->getVerbosity()); |
||
| 80 | |||
| 81 | if ($input->getOption('child')) { |
||
| 82 | $this->setVerbosity(self::VERBOSITY_CHILD); |
||
| 83 | } |
||
| 84 | |||
| 85 | $this->getContainer()->get('ez_migration_bundle.step_executed_listener.tracing')->setOutput($output); |
||
| 86 | |||
| 87 | $migrationService = $this->getMigrationService(); |
||
| 88 | |||
| 89 | $force = $input->getOption('force'); |
||
| 90 | |||
| 91 | $toExecute = $this->buildMigrationsList($input->getOption('path'), $migrationService, $force); |
||
| 92 | |||
| 93 | if (!count($toExecute)) { |
||
| 94 | $output->writeln('<info>No migrations to execute</info>'); |
||
| 95 | return 0; |
||
| 96 | } |
||
| 97 | |||
| 98 | $this->printMigrationsList($toExecute, $input, $output); |
||
| 99 | |||
| 100 | // ask user for confirmation to make changes |
||
| 101 | if (!$this->askForConfirmation($input, $output)) { |
||
| 102 | return 0; |
||
| 103 | } |
||
| 104 | |||
| 105 | View Code Duplication | if ($input->getOption('separate-process')) { |
|
|
|
|||
| 106 | $builder = new ProcessBuilder(); |
||
| 107 | $executableFinder = new PhpExecutableFinder(); |
||
| 108 | if (false !== $php = $executableFinder->find()) { |
||
| 109 | $builder->setPrefix($php); |
||
| 110 | } |
||
| 111 | $builderArgs = $this->createChildProcessArgs($input); |
||
| 112 | } |
||
| 113 | |||
| 114 | $executed = 0; |
||
| 115 | $failed = 0; |
||
| 116 | $skipped = 0; |
||
| 117 | |||
| 118 | /** @var MigrationDefinition $migrationDefinition */ |
||
| 119 | foreach ($toExecute as $name => $migrationDefinition) { |
||
| 120 | |||
| 121 | // let's skip migrations that we know are invalid - user was warned and he decided to proceed anyway |
||
| 122 | if ($migrationDefinition->status == MigrationDefinition::STATUS_INVALID) { |
||
| 123 | $output->writeln("<comment>Skipping $name</comment>\n"); |
||
| 124 | $skipped++; |
||
| 125 | continue; |
||
| 126 | } |
||
| 127 | |||
| 128 | $this->writeln("<info>Processing $name</info>"); |
||
| 129 | |||
| 130 | if ($input->getOption('separate-process')) { |
||
| 131 | |||
| 132 | try { |
||
| 133 | $this->executeMigrationInSeparateProcess($migrationDefinition, $migrationService, $builder, $builderArgs); |
||
| 134 | |||
| 135 | $executed++; |
||
| 136 | } catch (\Exception $e) { |
||
| 137 | if ($input->getOption('ignore-failures')) { |
||
| 138 | $output->writeln("\n<error>Migration failed! Reason: " . $e->getMessage() . "</error>\n"); |
||
| 139 | $failed++; |
||
| 140 | continue; |
||
| 141 | } |
||
| 142 | if ($e instanceof AfterMigrationExecutionException) { |
||
| 143 | $output->writeln("\n<error>Failure after migration end! Reason: " . $e->getMessage() . "</error>"); |
||
| 144 | } else { |
||
| 145 | $output->writeln("\n<error>Migration aborted! Reason: " . $e->getMessage() . "</error>"); |
||
| 146 | } |
||
| 147 | return 1; |
||
| 148 | } |
||
| 149 | |||
| 150 | } else { |
||
| 151 | |||
| 152 | try { |
||
| 153 | $this->executeMigrationInProcess($migrationDefinition, $force, $migrationService, $input); |
||
| 154 | |||
| 155 | $executed++; |
||
| 156 | } catch (\Exception $e) { |
||
| 157 | if ($input->getOption('ignore-failures')) { |
||
| 158 | $output->writeln("\n<error>Migration failed! Reason: " . $e->getMessage() . "</error>\n"); |
||
| 159 | $failed++; |
||
| 160 | continue; |
||
| 161 | } |
||
| 162 | $output->writeln("\n<error>Migration aborted! Reason: " . $e->getMessage() . "</error>"); |
||
| 163 | return 1; |
||
| 164 | } |
||
| 165 | |||
| 166 | } |
||
| 167 | } |
||
| 168 | |||
| 169 | View Code Duplication | if ($input->getOption('clear-cache')) { |
|
| 170 | $command = $this->getApplication()->find('cache:clear'); |
||
| 171 | $inputArray = new ArrayInput(array('command' => 'cache:clear')); |
||
| 172 | $command->run($inputArray, $output); |
||
| 173 | } |
||
| 174 | |||
| 175 | $time = microtime(true) - $start; |
||
| 176 | $this->writeln("Executed $executed migrations, failed $failed, skipped $skipped"); |
||
| 177 | if ($input->getOption('separate-process')) { |
||
| 178 | // in case of using subprocesses, we can not measure max memory used |
||
| 179 | $this->writeln("Time taken: ".sprintf('%.2f', $time)." secs"); |
||
| 180 | } else { |
||
| 181 | $this->writeln("Time taken: ".sprintf('%.2f', $time)." secs, memory: ".sprintf('%.2f', (memory_get_peak_usage(true) / 1000000)). ' MB'); |
||
| 182 | } |
||
| 183 | } |
||
| 184 | |||
| 185 | protected function executeMigrationInProcess($migrationDefinition, $force, $migrationService, $input) |
||
| 186 | { |
||
| 187 | $migrationService->executeMigration( |
||
| 188 | $migrationDefinition, |
||
| 189 | !$input->getOption('no-transactions'), |
||
| 190 | $input->getOption('default-language'), |
||
| 191 | $input->getOption('admin-login'), |
||
| 192 | $force |
||
| 193 | ); |
||
| 194 | } |
||
| 195 | |||
| 196 | protected function executeMigrationInSeparateProcess($migrationDefinition, $migrationService, $builder, $builderArgs, $feedback = true) |
||
| 244 | |||
| 245 | /** |
||
| 246 | * @param string[] $paths |
||
| 247 | * @param MigrationService $migrationService |
||
| 248 | * @param bool $force when true, look not only for TODO migrations, but also DONE, SKIPPED, FAILED ones (we still omit STARTED and SUSPENDED ones) |
||
| 249 | * @return MigrationDefinition[] |
||
| 250 | * |
||
| 251 | * @todo this does not scale well with many definitions or migrations |
||
| 252 | */ |
||
| 253 | protected function buildMigrationsList($paths, $migrationService, $force = false) |
||
| 291 | |||
| 292 | /** |
||
| 293 | * @param MigrationDefinition[] $toExecute |
||
| 294 | * @param InputInterface $input |
||
| 295 | * @param OutputInterface $output |
||
| 296 | * |
||
| 297 | * @todo use a more compact output when there are *many* migrations |
||
| 298 | */ |
||
| 299 | protected function printMigrationsList($toExecute, InputInterface $input, OutputInterface $output) |
||
| 325 | |||
| 326 | protected function askForConfirmation(InputInterface $input, OutputInterface $output, $nonIteractiveOutput = "=============================================\n") |
||
| 347 | |||
| 348 | /** |
||
| 349 | * Small tricks to allow us to lower verbosity between NORMAL and QUIET and have a decent writeln API, even with old SF versions |
||
| 350 | * @param $message |
||
| 351 | * @param int $verbosity |
||
| 352 | */ |
||
| 353 | protected function writeln($message, $verbosity = OutputInterface::VERBOSITY_NORMAL) |
||
| 359 | |||
| 360 | protected function setOutput(OutputInterface $output) |
||
| 364 | |||
| 365 | protected function setVerbosity($verbosity) |
||
| 369 | |||
| 370 | /** |
||
| 371 | * Returns the command-line arguments needed to execute a migration in a separate subprocess (omitting 'path') |
||
| 372 | * @param InputInterface $input |
||
| 373 | * @return array |
||
| 374 | */ |
||
| 375 | protected function createChildProcessArgs(InputInterface $input) |
||
| 410 | } |
||
| 411 |
Loading history...
Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.
You can also find more detailed suggestions in the “Code” section of your repository.