diff --git a/src/BatchProcessor.php b/src/BatchProcessor.php index 4304aeabc494b6910771bcd3ce009591bd56e73f..10d8462bd2ef0a6721477fa22ec9be45740e60a4 100644 --- a/src/BatchProcessor.php +++ b/src/BatchProcessor.php @@ -11,6 +11,13 @@ use Symfony\Component\HttpFoundation\RedirectResponse; */ class BatchProcessor { + /** + * The session key under which the stage ID is stored. + * + * @var string + */ + public const STAGE_ID_SESSION_KEY = '_automatic_updates_stage_id'; + /** * Gets the updater service. * @@ -66,8 +73,8 @@ class BatchProcessor { */ public static function begin(array $project_versions, array &$context): void { try { - $stage_unique = static::getUpdater()->begin($project_versions); - $context['results']['stage_id'] = $stage_unique; + $stage_id = static::getUpdater()->begin($project_versions); + \Drupal::service('session')->set(static::STAGE_ID_SESSION_KEY, $stage_id); } catch (\Throwable $e) { static::handleException($e, $context); @@ -84,7 +91,7 @@ class BatchProcessor { */ public static function stage(array &$context): void { try { - $stage_id = $context['results']['stage_id']; + $stage_id = \Drupal::service('session')->get(static::STAGE_ID_SESSION_KEY); static::getUpdater()->claim($stage_id)->stage(); } catch (\Throwable $e) { @@ -142,8 +149,9 @@ class BatchProcessor { */ public static function finishStage(bool $success, array $results, array $operations): ?RedirectResponse { if ($success) { + $stage_id = \Drupal::service('session')->get(static::STAGE_ID_SESSION_KEY); $url = Url::fromRoute('automatic_updates.confirmation_page', [ - 'stage_id' => $results['stage_id'], + 'stage_id' => $stage_id, ]); return new RedirectResponse($url->setAbsolute()->toString()); } @@ -162,6 +170,8 @@ class BatchProcessor { * A list of the operations that had not been completed by the batch API. */ public static function finishCommit(bool $success, array $results, array $operations): ?RedirectResponse { + \Drupal::service('session')->remove(static::STAGE_ID_SESSION_KEY); + if ($success) { $url = Url::fromRoute('automatic_updates.finish') ->setAbsolute() diff --git a/src/Form/UpdaterForm.php b/src/Form/UpdaterForm.php index eb93cfd8814bd68c949aae895427bed2bacce122..d6e2daa4f069d98e4415eef83d315612eb4268a0 100644 --- a/src/Form/UpdaterForm.php +++ b/src/Form/UpdaterForm.php @@ -14,10 +14,12 @@ use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Link; use Drupal\Core\State\StateInterface; use Drupal\Core\Url; +use Drupal\package_manager\Exception\StageOwnershipException; use Drupal\system\SystemManager; use Drupal\update\UpdateManagerInterface; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\EventDispatcher\EventDispatcherInterface; +use Symfony\Component\HttpFoundation\Session\SessionInterface; /** * Defines a form to update Drupal core. @@ -57,6 +59,13 @@ class UpdaterForm extends FormBase { */ protected $eventDispatcher; + /** + * The current session. + * + * @var \Symfony\Component\HttpFoundation\Session\SessionInterface + */ + protected $session; + /** * Constructs a new UpdaterForm object. * @@ -68,12 +77,15 @@ class UpdaterForm extends FormBase { * The readiness validation manager service. * @param \Symfony\Component\EventDispatcher\EventDispatcherInterface $event_dispatcher * The event dispatcher service. + * @param \Symfony\Component\HttpFoundation\Session\SessionInterface $session + * The current session. */ - public function __construct(StateInterface $state, Updater $updater, ReadinessValidationManager $readiness_validation_manager, EventDispatcherInterface $event_dispatcher) { + public function __construct(StateInterface $state, Updater $updater, ReadinessValidationManager $readiness_validation_manager, EventDispatcherInterface $event_dispatcher, SessionInterface $session) { $this->updater = $updater; $this->state = $state; $this->readinessValidationManager = $readiness_validation_manager; $this->eventDispatcher = $event_dispatcher; + $this->session = $session; } /** @@ -91,7 +103,8 @@ class UpdaterForm extends FormBase { $container->get('state'), $container->get('automatic_updates.updater'), $container->get('automatic_updates.readiness_validation_manager'), - $container->get('event_dispatcher') + $container->get('event_dispatcher'), + $container->get('session') ); } @@ -101,6 +114,30 @@ class UpdaterForm extends FormBase { public function buildForm(array $form, FormStateInterface $form_state) { $this->messenger()->addWarning($this->t('This is an experimental Automatic Updater using Composer. Use at your own risk.')); + if ($this->updater->isAvailable()) { + $stage_exists = FALSE; + } + else { + $stage_exists = TRUE; + + // If there's a stage ID stored in the session, try to claim the stage + // with it. If we succeed, then an update is already in progress, and the + // current session started it, so redirect them to the confirmation form. + $stage_id = $this->session->get(BatchProcessor::STAGE_ID_SESSION_KEY); + if ($stage_id) { + try { + $this->updater->claim($stage_id); + return $this->redirect('automatic_updates.confirmation_page', [ + 'stage_id' => $stage_id, + ]); + } + catch (StageOwnershipException $e) { + // We already know a stage exists, even if it's not ours, so we don't + // have to do anything else here. + } + } + } + $form['last_check'] = [ '#theme' => 'update_last_check', '#last' => $this->state->get('update.last_check', 0), @@ -210,45 +247,29 @@ class UpdaterForm extends FormBase { } $this->displayResults($results, $this->messenger()); - // If there were no errors, allow the user to proceed with the update. - if ($this->getOverallSeverity($results) !== SystemManager::REQUIREMENT_ERROR) { - $form['actions'] = $this->actions($form_state); - } - return $form; - } - - /** - * Builds the form actions. - * - * @param \Drupal\Core\Form\FormStateInterface $form_state - * The form state. - * - * @return mixed[][] - * The form's actions elements. - */ - protected function actions(FormStateInterface $form_state): array { - $actions = ['#type' => 'actions']; - - if (!$this->updater->isAvailable()) { - // If the form has been submitted do not display this error message + if ($stage_exists) { + // If the form has been submitted, do not display this error message // because ::deleteExistingUpdate() may run on submit. The message will // still be displayed on form build if needed. if (!$form_state->getUserInput()) { $this->messenger()->addError($this->t('Cannot begin an update because another Composer operation is currently in progress.')); } - $actions['delete'] = [ + $form['actions']['delete'] = [ '#type' => 'submit', '#value' => $this->t('Delete existing update'), '#submit' => ['::deleteExistingUpdate'], ]; } - else { - $actions['submit'] = [ + // If there were no errors, allow the user to proceed with the update. + elseif ($this->getOverallSeverity($results) !== SystemManager::REQUIREMENT_ERROR) { + $form['actions']['submit'] = [ '#type' => 'submit', '#value' => $this->t('Update'), ]; } - return $actions; + $form['actions']['#type'] = 'actions'; + + return $form; } /** diff --git a/tests/src/Functional/UpdaterFormTest.php b/tests/src/Functional/UpdaterFormTest.php index bf5c7f44b6072a33ebe315cc9910412da66ce060..8e762d7a5323d25ff2bc820b19211208f7d0dfa6 100644 --- a/tests/src/Functional/UpdaterFormTest.php +++ b/tests/src/Functional/UpdaterFormTest.php @@ -237,11 +237,10 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase { /** * Tests deleting an existing update. - * - * @todo Add test coverage for differences between stage owner and other users - * in https://www.drupal.org/i/3248928. */ public function testDeleteExistingUpdate() { + $conflict_message = 'Cannot begin an update because another Composer operation is currently in progress.'; + $assert_session = $this->assertSession(); $page = $this->getSession()->getPage(); $this->setCoreVersion('9.8.0'); @@ -256,16 +255,16 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase { $this->assertUpdateReady(); $assert_session->buttonExists('Continue'); - // Return to the start page. + // If we try to return to the start page, we should be redirected back to + // the confirmation page. $this->drupalGet('/admin/modules/automatic-update'); - $assert_session->pageTextContainsOnce('Cannot begin an update because another Composer operation is currently in progress.'); - $assert_session->buttonNotExists('Update'); + $this->assertUpdateReady(); // Delete the existing update. - $page->pressButton('Delete existing update'); - $assert_session->pageTextContains('Staged update deleted'); - $assert_session->pageTextNotContains('Cannot begin an update because another Composer operation is currently in progress.'); - + $page->pressButton('Cancel update'); + $assert_session->addressEquals('/admin/reports/updates/automatic-update'); + $assert_session->pageTextContains('The update was successfully cancelled.'); + $assert_session->pageTextNotContains($conflict_message); // Ensure we can start another update after deleting the existing one. $page->pressButton('Update'); $this->checkForMetaRefresh(); @@ -274,11 +273,19 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase { $this->assertUpdateReady(); $this->assertUpdateStagedTimes(2); $assert_session->buttonExists('Continue'); - // Cancel the update, then ensure that we are bounced back to the start - // page, and that it will allow us to begin the update anew. - $page->pressButton('Cancel update'); - $assert_session->addressEquals('/admin/reports/updates/automatic-update'); - $assert_session->pageTextContains('The update was successfully cancelled.'); + + // Log in as another administrative user and ensure that we cannot begin an + // update because the previous session already started one. + $account = $this->createUser([], NULL, TRUE); + $this->drupalLogin($account); + $this->drupalGet('/admin/reports/updates/automatic-update'); + $assert_session->pageTextContains($conflict_message); + $assert_session->buttonNotExists('Update'); + // We should be able to delete the previous update, and then we should be + // able to start a new one. + $page->pressButton('Delete existing update'); + $assert_session->pageTextContains('Staged update deleted'); + $assert_session->pageTextNotContains($conflict_message); $assert_session->buttonExists('Update'); }