From 06a3f719a414deee84728b5941b9ce71bb4b53c5 Mon Sep 17 00:00:00 2001 From: Christoph Burschka Date: Sun, 5 Feb 2017 20:28:39 +0100 Subject: [PATCH] [update:execute] Errors in update code (#3108, #3137) (#3138) * [update:execute] Fix a missing variable argument (#3108) * [update:execute] Fix updating logic (#3137) Only apply each pending update once. * Cancel update command on bad selection. * Abort update on failure, only run post-update on completion. Post-update functions may rely on any API, which requires all modules to have up-to-date schemas. * Remove update-n code from post-updates. Post-update functions have names instead of numbers; they can't be selected like this. * Make module argument optional, default to 'all'. --- src/Command/Update/ExecuteCommand.php | 146 +++++++++++++------------- 1 file changed, 75 insertions(+), 71 deletions(-) diff --git a/src/Command/Update/ExecuteCommand.php b/src/Command/Update/ExecuteCommand.php index cd36e3560..26f9a206e 100644 --- a/src/Command/Update/ExecuteCommand.php +++ b/src/Command/Update/ExecuteCommand.php @@ -46,8 +46,8 @@ class ExecuteCommand extends Command /** - * @var Manager -*/ + * @var Manager + */ protected $extensionManager; /** @@ -102,7 +102,7 @@ protected function configure() ->setDescription($this->trans('commands.update.execute.description')) ->addArgument( 'module', - InputArgument::REQUIRED, + InputArgument::OPTIONAL, $this->trans('commands.common.options.module') ) ->addArgument( @@ -118,7 +118,7 @@ protected function configure() protected function execute(InputInterface $input, OutputInterface $output) { $io = new DrupalStyle($input, $output); - $this->module = $input->getArgument('module'); + $this->module = $input->getArgument('module') ?: 'all'; $this->update_n = $input->getArgument('update-n'); $this->site->loadLegacyFile('/core/includes/install.inc'); @@ -127,7 +127,10 @@ protected function execute(InputInterface $input, OutputInterface $output) drupal_load_updates(); update_fix_compatibility(); $updates = update_get_update_list(); - $this->checkUpdates($io); + if (!$this->checkUpdates($io, $updates)) { + return 1; + } + $maintenance_mode = $this->state->get('system.maintenance_mode', false); if (!$maintenance_mode) { @@ -135,8 +138,18 @@ protected function execute(InputInterface $input, OutputInterface $output) $this->state->set('system.maintenance_mode', true); } - $this->runUpdates($io, $updates); - $this->runPostUpdates($io); + try { + $complete = $this->runUpdates($io, $updates); + + // Post Updates are only safe to run after all schemas have been updated. + if ($complete) { + $this->runPostUpdates($io); + } + } catch (\Exception $e) { + watchdog_exception('update', $e); + $io->error($e->getMessage()); + return 1; + } if (!$maintenance_mode) { $this->state->set('system.maintenance_mode', false); @@ -149,8 +162,11 @@ protected function execute(InputInterface $input, OutputInterface $output) /** * @param \Drupal\Console\Core\Style\DrupalStyle $io + * @param array $updates + * + * @return bool true if the selected module/update number exists. */ - private function checkUpdates(DrupalStyle $io) + private function checkUpdates(DrupalStyle $io, array $updates) { if ($this->module != 'all') { if (!isset($updates[$this->module])) { @@ -160,30 +176,39 @@ private function checkUpdates(DrupalStyle $io) $this->module ) ); - return; + return false; } else { - // filter to execute only a specific module updates - $updates = [$this->module => $updates[$this->module]]; - if ($this->update_n && !isset($updates[$this->module]['pending'][$this->update_n])) { - $io->info( + $io->error( sprintf( $this->trans('commands.update.execute.messages.module-update-function-not-found'), $this->module, $this->update_n ) ); + return false; } } } + return true; } /** * @param \Drupal\Console\Core\Style\DrupalStyle $io - * @param $updates + * @param array $updates + * + * @return bool True if all available updates have been run. */ - private function runUpdates(DrupalStyle $io, $updates) + private function runUpdates(DrupalStyle $io, array $updates) { + if ($this->module != 'all') { + $complete = count($updates) == 1; + $updates = [$this->module => $updates[$this->module]]; + } + else { + $complete = true; + } + foreach ($updates as $module_name => $module_updates) { $extension = $this->extensionManager->getModule($module_name); if (!$extension) { @@ -194,37 +219,31 @@ private function runUpdates(DrupalStyle $io, $updates) ->loadLegacyFile($extension->getPath() . '/'. $module_name . '.install', false); } - foreach ($module_updates['pending'] as $update_number => $update) { - if ($this->module != 'all' && $this->update_n !== null && $this->update_n != $update_number) { - continue; - } + if ($this->update_n > $module_updates['start']) { + $io->info( + $this->trans('commands.update.execute.messages.executing-required-previous-updates') + ); + } - if ($this->update_n > $module_updates['start']) { - $io->info( - $this->trans('commands.update.execute.messages.executing-required-previous-updates') - ); + foreach ($module_updates['pending'] as $update_number => $update) { + if ($this->module != 'all' && $this->update_n !== null && $this->update_n < $update_number) { + return false; } - for ($update_index=$module_updates['start']; $update_index<=$update_number; $update_index++) { - $io->info( - sprintf( - $this->trans('commands.update.execute.messages.executing-update'), - $update_index, - $module_name - ) - ); - - try { - $this->moduleHandler->invoke($module_name, 'update_' . $update_index); - } catch (\Exception $e) { - watchdog_exception('update', $e); - $io->error($e->getMessage()); - } + $io->info( + sprintf( + $this->trans('commands.update.execute.messages.executing-update'), + $update_number, + $module_name + ) + ); - drupal_set_installed_schema_version($module_name, $update_index); - } + $this->moduleHandler->invoke($module_name, 'update_' . $update_number); + drupal_set_installed_schema_version($module_name, $update_number); } } + + return $complete; } /** @@ -234,38 +253,23 @@ private function runPostUpdates(DrupalStyle $io) { $postUpdates = $this->postUpdateRegistry->getPendingUpdateInformation(); foreach ($postUpdates as $module_name => $module_updates) { - foreach ($module_updates['pending'] as $update_number => $update) { - if ($this->module != 'all' && $this->update_n !== null && $this->update_n != $update_number) { - continue; - } - - if ($this->update_n > $module_updates['start']) { - $io->info( - $this->trans('commands.update.execute.messages.executing-required-previous-updates') - ); - } - for ($update_index=$module_updates['start']; $update_index<=$update_number; $update_index++) { - $io->info( - sprintf( - $this->trans('commands.update.execute.messages.executing-update'), - $update_index, - $module_name - ) - ); + foreach ($module_updates['pending'] as $update_name => $update) { + $io->info( + sprintf( + $this->trans('commands.update.execute.messages.executing-update'), + $update_name, + $module_name + ) + ); - try { - $function = sprintf( - '%s_post_update_%s', - $module_name, - $update_index - ); - drupal_flush_all_caches(); - update_invoke_post_update($function); - } catch (\Exception $e) { - watchdog_exception('update', $e); - $io->error($e->getMessage()); - } - } + $function = sprintf( + '%s_post_update_%s', + $module_name, + $update_name + ); + drupal_flush_all_caches(); + $context = []; + update_invoke_post_update($function, $context); } } }