From 58026a4c2c1609cd91a75ef094e62b3bec737d13 Mon Sep 17 00:00:00 2001
From: phenaproxima <phenaproxima@205645.no-reply.drupal.org>
Date: Wed, 16 Feb 2022 17:31:52 +0000
Subject: [PATCH] Issue #3248928 by phenaproxima: If an update has already
started, redirect the user who started the update to the confirmation page
---
src/BatchProcessor.php | 18 ++++--
src/Form/UpdaterForm.php | 75 +++++++++++++++---------
tests/src/Functional/UpdaterFormTest.php | 37 +++++++-----
3 files changed, 84 insertions(+), 46 deletions(-)
diff --git a/src/BatchProcessor.php b/src/BatchProcessor.php
index 4304aeabc4..10d8462bd2 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 eb93cfd881..d6e2daa4f0 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 bf5c7f44b6..8e762d7a53 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');
}
--
GitLab