Issue #3265057 by tedbow, phenaproxima: Take out of maintenance mode when done with an update

automatic_updates.module

@@ -5,6 +5,7 @@
  * Contains hook implementations for Automatic Updates.
  */
 
+use Drupal\automatic_updates\BatchProcessor;
 use Drupal\Core\Routing\RouteMatchInterface;
 use Drupal\automatic_updates\CronUpdater;
 use Drupal\automatic_updates\UpdateRecommender;
@@ -12,6 +13,7 @@ use Drupal\automatic_updates\Validation\AdminReadinessMessages;
 use Drupal\Core\Extension\ExtensionVersion;
 use Drupal\Core\Form\FormStateInterface;
 use Drupal\Core\Url;
+use Drupal\system\Controller\DbUpdateController;
 use Drupal\update\ProjectSecurityData;
 
 /**
@@ -200,3 +202,16 @@ function automatic_updates_local_tasks_alter(array &$local_tasks) {
     }
   }
 }
+
+/**
+ * Implements hook_batch_alter().
+ *
+ * @todo Remove this in https://www.drupal.org/i/3267817.
+ */
+function automatic_updates_batch_alter(array &$batch): void {
+  foreach ($batch['sets'] as &$batch_set) {
+    if (!empty($batch_set['finished']) && $batch_set['finished'] === [DbUpdateController::class, 'batchFinished']) {
+      $batch_set['finished'] = [BatchProcessor::class, 'dbUpdateBatchFinished'];
+    }
+  }
+}

package_manager/tests/modules/package_manager_test_validation/src/EventSubscriber/TestSubscriber.php

@@ -112,6 +112,9 @@ class TestSubscriber implements EventSubscriberInterface {
   public function handleEvent(StageEvent $event): void {
     $results = $this->state->get(static::STATE_KEY . '.' . get_class($event), []);
 
+    // Record that value of maintenance mode for each event.
+    $this->state->set(get_class($event) . '.' . 'system.maintenance_mode', $this->state->get('system.maintenance_mode'));
+
     if ($results instanceof \Throwable) {
       throw $results;
     }

src/BatchProcessor.php

@@ -4,6 +4,7 @@ namespace Drupal\automatic_updates;
 
 use Drupal\Core\Url;
 use Drupal\package_manager\Exception\StageValidationException;
+use Drupal\system\Controller\DbUpdateController;
 use Symfony\Component\HttpFoundation\RedirectResponse;
 
 /**
@@ -18,6 +19,13 @@ class BatchProcessor {
    */
   public const STAGE_ID_SESSION_KEY = '_automatic_updates_stage_id';
 
+  /**
+   * The session key which indicates if the update is done in maintenance mode.
+   *
+   * @var string
+   */
+  public const MAINTENANCE_MODE_SESSION_KEY = '_automatic_updates_maintenance_mode';
+
   /**
    * Gets the updater service.
    *
@@ -200,4 +208,39 @@ class BatchProcessor {
     }
   }
 
+  /**
+   * Reset maintenance mode after update.php.
+   *
+   * This wraps \Drupal\system\Controller\DbUpdateController::batchFinished()
+   * because that function would leave the site in maintenance mode if we
+   * redirected the user to update.php already in maintenance mode. We need to
+   * take the site out of maintenance mode, if it was not enabled before they
+   * submitted our confirmation form.
+   *
+   * @param bool $success
+   *   Whether the batch API tasks were all completed successfully.
+   * @param array $results
+   *   An array of all the results.
+   * @param array $operations
+   *   A list of the operations that had not been completed by the batch API.
+   *
+   * @todo Remove the need for this workaround in
+   *    https://www.drupal.org/i/3267817.
+   *
+   * @see \Drupal\update\Form\UpdateReady::submitForm()
+   * @see automatic_updates_batch_alter()
+   */
+  public static function dbUpdateBatchFinished(bool $success, array $results, array $operations) {
+    DbUpdateController::batchFinished($success, $results, $operations);
+    // Now that the update is done, we can put the site back online if it was
+    // previously not in maintenance mode.
+    // \Drupal\system\Controller\DbUpdateController::batchFinished() will not
+    // unset maintenance mode if the site was in maintenance mode when the user
+    // was redirected to update.php by
+    // \Drupal\automatic_updates\Controller\UpdateController::onFinish().
+    if (!\Drupal::request()->getSession()->remove(static::MAINTENANCE_MODE_SESSION_KEY)) {
+      \Drupal::state()->set('system.maintenance_mode', FALSE);
+    }
+  }
+
 }

src/Controller/UpdateController.php

@@ -2,11 +2,14 @@
 
 namespace Drupal\automatic_updates\Controller;
 
+use Drupal\automatic_updates\BatchProcessor;
 use Drupal\Core\Controller\ControllerBase;
+use Drupal\Core\State\StateInterface;
 use Drupal\Core\Url;
 use Drupal\package_manager\Validator\PendingUpdatesValidator;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\HttpFoundation\RedirectResponse;
+use Symfony\Component\HttpFoundation\Request;
 
 /**
  * Defines a controller to handle various stages of an automatic update.
@@ -28,9 +31,12 @@ class UpdateController extends ControllerBase {
    *
    * @param \Drupal\package_manager\Validator\PendingUpdatesValidator $pending_updates_validator
    *   The pending updates validator.
+   * @param \Drupal\Core\State\StateInterface $state
+   *   The state service.
    */
-  public function __construct(PendingUpdatesValidator $pending_updates_validator) {
+  public function __construct(PendingUpdatesValidator $pending_updates_validator, StateInterface $state) {
     $this->pendingUpdatesValidator = $pending_updates_validator;
+    $this->stateService = $state;
   }
 
   /**
@@ -38,7 +44,8 @@ class UpdateController extends ControllerBase {
    */
   public static function create(ContainerInterface $container) {
     return new static(
-      $container->get('package_manager.validator.pending_updates')
+      $container->get('package_manager.validator.pending_updates'),
+      $container->get('state')
     );
   }
 
@@ -49,10 +56,13 @@ class UpdateController extends ControllerBase {
    * update.php to run those. Otherwise, they are redirected to the status
    * report.
    *
+   * @param \Symfony\Component\HttpFoundation\Request $request
+   *   The current request object.
+   *
    * @return \Symfony\Component\HttpFoundation\RedirectResponse
    *   A redirect to the appropriate destination.
    */
-  public function onFinish(): RedirectResponse {
+  public function onFinish(Request $request): RedirectResponse {
     if ($this->pendingUpdatesValidator->updatesExist()) {
       $message = $this->t('Please apply database updates to complete the update process.');
       $url = Url::fromRoute('system.db_update');
@@ -60,6 +70,11 @@ class UpdateController extends ControllerBase {
     else {
       $message = $this->t('Update complete!');
       $url = Url::fromRoute('update.status');
+      // Now that the update is done, we can put the site back online if it was
+      // previously not in maintenance mode.
+      if (!$request->getSession()->remove(BatchProcessor::MAINTENANCE_MODE_SESSION_KEY)) {
+        $this->state()->set('system.maintenance_mode', FALSE);
+      }
     }
     $this->messenger()->addStatus($message);
     return new RedirectResponse($url->setAbsolute()->toString());

src/Form/UpdateReady.php

@@ -196,12 +196,13 @@ class UpdateReady extends FormBase {
    * {@inheritdoc}
    */
   public function submitForm(array &$form, FormStateInterface $form_state) {
-    $session = $this->getRequest()->getSession();
     // Store maintenance_mode setting so we can restore it when done.
-    $session->set('maintenance_mode', $this->state->get('system.maintenance_mode'));
+    $this->getRequest()
+      ->getSession()
+      ->set(BatchProcessor::MAINTENANCE_MODE_SESSION_KEY, $this->state->get('system.maintenance_mode'));
+
     if ($form_state->getValue('maintenance_mode')) {
       $this->state->set('system.maintenance_mode', TRUE);
-      // @todo unset after updater. After db update?
     }
     $stage_id = $form_state->getValue('stage_id');
     $batch = (new BatchBuilder())

tests/src/Functional/UpdaterFormTest.php

@@ -344,13 +344,35 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase {
     $assert_session->pageTextContains($cancelled_message);
   }
 
+  /**
+   * Data provider for testStagedDatabaseUpdates().
+   *
+   * @return bool[][]
+   *   The test cases.
+   */
+  public function providerStagedDatabaseUpdates() {
+    return [
+      'maintenance mode on' => [TRUE],
+      'maintenance mode off' => [FALSE],
+    ];
+  }
+
   /**
    * Tests the update form when staged modules have database updates.
+   *
+   * @param bool $maintenance_mode_on
+   *   Whether the site should be in maintenance mode at the beginning of the
+   *   update process.
+   *
+   * @dataProvider providerStagedDatabaseUpdates
    */
-  public function testStagedDatabaseUpdates(): void {
+  public function testStagedDatabaseUpdates(bool $maintenance_mode_on): void {
     $this->setCoreVersion('9.8.0');
     $this->checkForUpdates();
 
+    $state = $this->container->get('state');
+    $state->set('system.maintenance_mode', $maintenance_mode_on);
+
     // Flag a warning, which will not block the update but should be displayed
     // on the updater form.
     $this->createTestValidationResults();
@@ -371,7 +393,7 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase {
     // Simulate a staged database update in the automatic_updates_test module.
     // We must do this after the update has started, because the pending updates
     // validator will prevent an update from starting.
-    $this->container->get('state')->set('automatic_updates_test.new_update', TRUE);
+    $state->set('automatic_updates_test.new_update', TRUE);
     // The warning from the updater form should be not be repeated, but we
     // should see a warning about pending database updates, and once the staged
     // changes have been applied, we should be redirected to update.php, where
@@ -380,12 +402,55 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase {
     $possible_update_message = 'Possible database updates were detected in the following modules; you may be redirected to the database update page in order to complete the update process.';
     $assert_session->pageTextContains($possible_update_message);
     $assert_session->pageTextContains('System');
+    $assert_session->checkboxChecked('maintenance_mode');
     $page->pressButton('Continue');
     $this->checkForMetaRefresh();
+    // Confirm that the site was in maintenance before the update was applied.
+    // @see \Drupal\package_manager_test_validation\EventSubscriber\TestSubscriber::handleEvent()
+    $this->assertTrue($state->get(PreApplyEvent::class . '.system.maintenance_mode'));
+    // Confirm the site remains in maintenance more when redirected to
+    // update.php.
+    $this->assertTrue($state->get('system.maintenance_mode'));
     $assert_session->addressEquals('/update.php');
     $assert_session->pageTextNotContains(reset($messages));
     $assert_session->pageTextNotContains($possible_update_message);
     $assert_session->pageTextContainsOnce('Please apply database updates to complete the update process.');
+    $this->assertTrue($state->get('system.maintenance_mode'));
+    $page->clickLink('Continue');
+    // @see automatic_updates_update_9001()
+    $assert_session->pageTextContains('Dynamic automatic_updates_update_9001');
+    $page->clickLink('Apply pending updates');
+    $this->checkForMetaRefresh();
+    $assert_session->pageTextContains('Updates were attempted.');
+    // Confirm the site was returned to the original maintenance module state.
+    $this->assertSame($state->get('system.maintenance_mode'), $maintenance_mode_on);
+  }
+
+  /**
+   * Data provider for testSuccessfulUpdate().
+   *
+   * @return string[][]
+   *   Test case parameters.
+   */
+  public function providerSuccessfulUpdate(): array {
+    return [
+      'Modules page, maintenance mode on' => [
+        '/admin/modules/automatic-update',
+        TRUE,
+      ],
+      'Modules page, maintenance mode off' => [
+        '/admin/modules/automatic-update',
+        FALSE,
+      ],
+      'Reports page, maintenance mode on' => [
+        '/admin/reports/updates/automatic-update',
+        TRUE,
+      ],
+      'Reports page, maintenance mode off' => [
+        '/admin/reports/updates/automatic-update',
+        FALSE,
+      ],
+    ];
   }
 
   /**
@@ -393,12 +458,16 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase {
    *
    * @param string $update_form_url
    *   The URL of the update form to visit.
+   * @param bool $maintenance_mode_on
+   *   Whether maintenance should be on at the beginning of the update.
    *
-   * @dataProvider providerUpdateFormReferringUrl
+   * @dataProvider providerSuccessfulUpdate
    */
-  public function testSuccessfulUpdate(string $update_form_url): void {
+  public function testSuccessfulUpdate(string $update_form_url, bool $maintenance_mode_on): void {
     $this->setCoreVersion('9.8.0');
     $this->checkForUpdates();
+    $state = $this->container->get('state');
+    $state->set('system.maintenance_mode', $maintenance_mode_on);
 
     $page = $this->getSession()->getPage();
     $this->drupalGet($update_form_url);
@@ -407,17 +476,18 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase {
     $this->checkForMetaRefresh();
     $this->assertUpdateStagedTimes(1);
     $this->assertUpdateReady('9.8.1');
-    $this->assertNotTrue($this->container->get('state')->get('system.maintenance_mode'));
+    // Confirm that the site was put into maintenance mode if needed.
+    $this->assertSame($state->get('system.maintenance_mode'), $maintenance_mode_on);
     $page->pressButton('Continue');
     $this->checkForMetaRefresh();
     $assert_session = $this->assertSession();
     $assert_session->addressEquals('/admin/reports/updates');
-    // Assert that the site was put into maintenance mode.
-    // @todo Add test coverage to ensure that site is taken back out of
-    //   maintenance if it was not originally in maintenance mode when the
-    //   update started in https://www.drupal.org/i/3265057.
-    $this->assertTrue($this->container->get('state')->get('system.maintenance_mode'));
+    // Confirm that the site was in maintenance before the update was applied.
+    // @see \Drupal\package_manager_test_validation\EventSubscriber\TestSubscriber::handleEvent()
+    $this->assertTrue($state->get(PreApplyEvent::class . '.system.maintenance_mode'));
     $assert_session->pageTextContainsOnce('Update complete!');
+    // Confirm the site was returned to the original maintenance mode state.
+    $this->assertSame($state->get('system.maintenance_mode'), $maintenance_mode_on);
   }
 
   /**