diff --git a/automatic_updates.module b/automatic_updates.module index 5b634be703adca5ca481ab64ee7075b79aef04b0..b683f5621865b6493eaeb46c05d6385b4abb9562 100644 --- a/automatic_updates.module +++ b/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']; + } + } +} diff --git a/package_manager/tests/modules/package_manager_test_validation/src/EventSubscriber/TestSubscriber.php b/package_manager/tests/modules/package_manager_test_validation/src/EventSubscriber/TestSubscriber.php index 0aeb5ede68ef6d92059aa8ef6e83552138521302..f88eb4f9a4de11490593ed85678921c7b06f9cf0 100644 --- a/package_manager/tests/modules/package_manager_test_validation/src/EventSubscriber/TestSubscriber.php +++ b/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; } diff --git a/src/BatchProcessor.php b/src/BatchProcessor.php index 4b73583cd1d7db6bfd87729606341608690f7c20..6584ff982c2ce959b7495f1822c15d52c5ff3318 100644 --- a/src/BatchProcessor.php +++ b/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); + } + } + } diff --git a/src/Controller/UpdateController.php b/src/Controller/UpdateController.php index 94754faa6f461e47cc9c053104ec209675350417..54737ec0bbd86c80ca091b0079121099910afdea 100644 --- a/src/Controller/UpdateController.php +++ b/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()); diff --git a/src/Form/UpdateReady.php b/src/Form/UpdateReady.php index ae2759d56377ef61780adc572e88069b247b6ac5..5d872e729f598bab3b21eb047c4044f3f1127054 100644 --- a/src/Form/UpdateReady.php +++ b/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()) diff --git a/tests/src/Functional/UpdaterFormTest.php b/tests/src/Functional/UpdaterFormTest.php index 9098ad7b45f2056d401c349acdafe5b59bf0ade6..6489ce3d8540159f1ae97eb8944a026f701cb9e5 100644 --- a/tests/src/Functional/UpdaterFormTest.php +++ b/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); } /**