Issue #3353270 by phenaproxima, kunal.sachdev, tedbow, Wim Leers: While...

Issue #3353270 by phenaproxima, kunal.sachdev, tedbow, Wim Leers: While applying updates during cron, put the site into maintenance mode

automatic_updates.module

@@ -265,6 +265,7 @@ function automatic_updates_form_update_settings_alter(array &$form): void {
       CronUpdateStage::ALL => t('All patch releases'),
     ],
     '#default_value' => $config->get('unattended.level'),
+    '#description' => t('When background updates are applied, your site will be briefly put into maintenance mode.'),
   ];
   $form['unattended_method'] = [
     '#type' => 'radios',

automatic_updates.routing.yml

@@ -32,4 +32,6 @@ automatic_updates.cron.post_apply:
   defaults:
     _controller: 'automatic_updates.cron_update_stage:handlePostApply'
   requirements:
-    _access_system_cron: 'TRUE'
+    _custom_access: 'automatic_updates.cron_update_stage:postApplyAccess'
+  options:
+    _maintenance_access: TRUE

automatic_updates.services.yml

@@ -25,10 +25,11 @@ services:
   Drupal\automatic_updates\UpdateStage: '@automatic_updates.update_stage'
   automatic_updates.cron_update_stage:
     class: Drupal\automatic_updates\CronUpdateStage
-    calls:
-      - ['setLogger', ['@logger.channel.automatic_updates']]
     arguments:
+      $committer: '@Drupal\automatic_updates\MaintenanceModeAwareCommitter'
       $inner: '@automatic_updates.cron_update_stage.inner'
+    calls:
+      - ['setLogger', ['@logger.channel.automatic_updates']]
     decorates: 'cron'
   Drupal\automatic_updates\CronUpdateStage: '@automatic_updates.cron_update_stage'
   automatic_updates.requested_update_validator:
@@ -70,3 +71,6 @@ services:
   logger.channel.automatic_updates:
     parent: logger.channel_base
     arguments: ['automatic_updates']
+  Drupal\automatic_updates\MaintenanceModeAwareCommitter:
+    tags:
+      - { name: event_subscriber }

package_manager/src/StageBase.php

@@ -447,7 +447,7 @@ abstract class StageBase implements LoggerAwareInterface {
     // active directory.
     $event = new PreApplyEvent($this, $this->getPathsToExclude());
     $this->tempStore->set(self::TEMPSTORE_APPLY_TIME_KEY, $this->time->getRequestTime());
-    $this->dispatch($event, $this->setNotApplying());
+    $this->dispatch($event, $this->setNotApplying(...));
 
     // Create a marker file so that we can tell later on if the commit failed.
     $this->failureMarker->write($this, $this->getFailureMarkerMessage());
@@ -462,7 +462,8 @@ abstract class StageBase implements LoggerAwareInterface {
     }
     catch (InvalidArgumentException | PreconditionException $e) {
       // The commit operation has not started yet, so we can clear the failure
-      // marker.
+      // marker and release the flag that says we're applying.
+      $this->setNotApplying();
       $this->failureMarker->clear();
       $this->rethrowAsStageException($e);
     }
@@ -471,7 +472,7 @@ abstract class StageBase implements LoggerAwareInterface {
       // is in an indeterminate state. Release the flag which says we're still
       // applying, because in this situation, the site owner should probably
       // restore everything from a backup.
-      $this->setNotApplying()();
+      $this->setNotApplying();
       // Update the marker file with the information from the throwable.
       $this->failureMarker->write($this, $this->getFailureMarkerMessage(), $throwable);
       throw new ApplyFailedException($this, $this->failureMarker->getMessage(), $throwable->getCode(), $throwable);
@@ -482,15 +483,9 @@ abstract class StageBase implements LoggerAwareInterface {
 
   /**
    * Returns a closure that marks this stage as no longer being applied.
-   *
-   * @return \Closure
-   *   A closure that, when called, marks this stage as no longer in the process
-   *   of being applied to the active directory.
    */
-  private function setNotApplying(): \Closure {
-    return function (): void {
-      $this->tempStore->delete(self::TEMPSTORE_APPLY_TIME_KEY);
-    };
+  private function setNotApplying(): void {
+    $this->tempStore->delete(self::TEMPSTORE_APPLY_TIME_KEY);
   }
 
   /**
@@ -515,7 +510,7 @@ abstract class StageBase implements LoggerAwareInterface {
     // unlikely to call newly added code during the current request.
     $this->eventDispatcher = \Drupal::service('event_dispatcher');
 
-    $release_apply = $this->setNotApplying();
+    $release_apply = $this->setNotApplying(...);
     $this->dispatch(new PostApplyEvent($this), $release_apply);
     $release_apply();
   }

src/CronUpdateStage.php

@@ -5,6 +5,7 @@ declare(strict_types = 1);
 namespace Drupal\automatic_updates;
 
 use Drupal\Component\Datetime\TimeInterface;
+use Drupal\Core\Access\AccessResult;
 use Drupal\Core\Config\ConfigFactoryInterface;
 use Drupal\Core\CronInterface;
 use Drupal\Core\File\FileSystemInterface;
@@ -396,6 +397,27 @@ class CronUpdateStage extends UpdateStage implements CronInterface {
     return new Response();
   }
 
+  /**
+   * Checks access to the post-apply route.
+   *
+   * @param string $key
+   *   The cron key.
+   *
+   * @return \Drupal\Core\Access\AccessResult
+   *   The access result.
+   */
+  public function postApplyAccess(string $key): AccessResult {
+    // The normal _system_cron_access check always disallows access if
+    // maintenance mode is turned on, but we stay in maintenance mode until
+    // post-apply tasks are finished. Therefore, we only want to check that the
+    // cron key is valid, and allow access if it is, regardless of whether or
+    // not we're in maintenance mode.
+    if ($key === $this->state->get('system.cron_key')) {
+      return AccessResult::allowed()->setCacheMaxAge(0);
+    }
+    return AccessResult::forbidden()->setCacheMaxAge(0);
+  }
+
   /**
    * Gets the cron update mode.
    *

src/MaintenanceModeAwareCommitter.php

+<?php
+
+declare(strict_types = 1);
+
+namespace Drupal\automatic_updates;
+
+use Drupal\Core\State\StateInterface;
+use Drupal\package_manager\Event\PostApplyEvent;
+use PhpTuf\ComposerStager\API\Core\CommitterInterface;
+use PhpTuf\ComposerStager\API\Exception\InvalidArgumentException;
+use PhpTuf\ComposerStager\API\Exception\PreconditionException;
+use PhpTuf\ComposerStager\API\Path\Value\PathInterface;
+use PhpTuf\ComposerStager\API\Path\Value\PathListInterface;
+use PhpTuf\ComposerStager\API\Process\Service\ProcessOutputCallbackInterface;
+use PhpTuf\ComposerStager\API\Process\Service\ProcessRunnerInterface;
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+
+/**
+ * Puts the site into maintenance mode while staged changes are applied.
+ *
+ * @internal
+ *   This is an internal part of Automatic Updates and may be changed or removed
+ *   at any time without warning. External code should not interact with this
+ *   class.
+ */
+final class MaintenanceModeAwareCommitter implements CommitterInterface, EventSubscriberInterface {
+
+  /**
+   * The state key which holds the original status of maintenance mode.
+   *
+   * @var string
+   */
+  private const STATE_KEY = 'automatic_updates.maintenance_mode';
+
+  /**
+   * Constructs a MaintenanceModeAwareCommitter object.
+   *
+   * @param \PhpTuf\ComposerStager\API\Core\CommitterInterface $decorated
+   *   The decorated committer service.
+   * @param \Drupal\Core\State\StateInterface $state
+   *   The state service.
+   */
+  public function __construct(
+    private readonly CommitterInterface $decorated,
+    private readonly StateInterface $state,
+  ) {}
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function getSubscribedEvents() {
+    return [
+      PostApplyEvent::class => ['restore', PHP_INT_MAX],
+    ];
+  }
+
+  /**
+   * Restores the original maintenance mode status after the update is applied.
+   *
+   * @param \Drupal\package_manager\Event\PostApplyEvent $event
+   *   The event being handled.
+   */
+  public function restore(PostApplyEvent $event): void {
+    if ($event->stage instanceof CronUpdateStage) {
+      $this->doRestore();
+    }
+  }
+
+  /**
+   * Restores the original maintenance mode status.
+   */
+  private function doRestore(): void {
+    $this->state->set('system.maintenance_mode', $this->state->get(static::STATE_KEY));
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function commit(PathInterface $stagingDir, PathInterface $activeDir, ?PathListInterface $exclusions = NULL, ?ProcessOutputCallbackInterface $callback = NULL, ?int $timeout = ProcessRunnerInterface::DEFAULT_TIMEOUT,): void {
+    $this->state->set(static::STATE_KEY, $this->state->get('system.maintenance_mode', FALSE));
+    $this->state->set('system.maintenance_mode', TRUE);
+
+    try {
+      $this->decorated->commit($stagingDir, $activeDir, $exclusions, $callback, $timeout);
+    }
+    catch (PreconditionException | InvalidArgumentException $e) {
+      $this->doRestore();
+
+      // Re-throw the exception, wrapped by another instance of itself.
+      $message = $e->getTranslatableMessage();
+      $code = $e->getCode();
+      // PreconditionException takes the failed precondition as its first
+      // argument.
+      if ($e instanceof PreconditionException) {
+        throw new PreconditionException($e->getPrecondition(), $message, $code, $e);
+      }
+      $class = get_class($e);
+      throw new $class($message, $code, $e);
+    }
+  }
+
+}

tests/src/Kernel/AutomaticUpdatesKernelTestBase.php

@@ -99,6 +99,27 @@ abstract class AutomaticUpdatesKernelTestBase extends PackageManagerKernelTestBa
  */
 class TestCronUpdateStage extends CronUpdateStage {
 
+  /**
+   * {@inheritdoc}
+   */
+  public function apply(?int $timeout = 600): void {
+    parent::apply($timeout);
+
+    if (\Drupal::state()->get('system.maintenance_mode')) {
+      $this->logger->info('Unattended update was applied in maintenance mode.');
+    }
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function postApply(): void {
+    if (\Drupal::state()->get('system.maintenance_mode')) {
+      $this->logger->info('postApply() was called in maintenance mode.');
+    }
+    parent::postApply();
+  }
+
   /**
    * {@inheritdoc}
    */

tests/src/Kernel/CronUpdateStageTest.php

@@ -25,6 +25,10 @@ use Drupal\Tests\automatic_updates\Traits\EmailNotificationsTestTrait;
 use Drupal\Tests\package_manager\Kernel\TestStage;
 use Drupal\Tests\package_manager\Traits\PackageManagerBypassTestTrait;
 use Drupal\Tests\user\Traits\UserCreationTrait;
+use PhpTuf\ComposerStager\API\Exception\InvalidArgumentException;
+use PhpTuf\ComposerStager\API\Exception\PreconditionException;
+use PhpTuf\ComposerStager\API\Precondition\Service\PreconditionInterface;
+use PhpTuf\ComposerStager\Internal\Translation\Value\TranslatableMessage;
 use Prophecy\Argument;
 use ColinODell\PsrTestLogger\TestLogger;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
@@ -681,6 +685,73 @@ END;
     $this->assertSame('setLogger', $stage_method_calls[0][0]);
   }
 
+  /**
+   * Tests that maintenance mode is on when staged changes are applied.
+   */
+  public function testMaintenanceModeIsOnDuringApply(): void {
+    $this->getStageFixtureManipulator()->setCorePackageVersion('9.8.1');
+
+    /** @var \Drupal\Core\State\StateInterface $state */
+    $state = $this->container->get('state');
+    // Before the update begins, we should have no indication that we have ever
+    // been in maintenance mode (i.e., the value in state is NULL).
+    $this->assertNull($state->get('system.maintenance_mode'));
+    $this->container->get('cron')->run();
+    $state->resetCache();
+    // @see \Drupal\Tests\automatic_updates\Kernel\TestCronUpdateStage::apply()
+    $this->assertTrue($this->logger->hasRecord('Unattended update was applied in maintenance mode.', RfcLogLevel::INFO));
+    // @see \Drupal\Tests\automatic_updates\Kernel\TestCronUpdateStage::postApply()
+    $this->assertTrue($this->logger->hasRecord('postApply() was called in maintenance mode.', RfcLogLevel::INFO));
+    // During post-apply, maintenance mode should have been explicitly turned
+    // off (i.e., set to FALSE).
+    $this->assertFalse($state->get('system.maintenance_mode'));
+  }
+
+  /**
+   * Data provider for ::testMaintenanceModeAffectedByException().
+   *
+   * @return array[]
+   *   The test cases.
+   */
+  public function providerMaintenanceModeAffectedByException(): array {
+    return [
+      [InvalidArgumentException::class, FALSE],
+      [PreconditionException::class, FALSE],
+      [\Exception::class, TRUE],
+    ];
+  }
+
+  /**
+   * Tests that an exception during apply may keep the site in maintenance mode.
+   *
+   * @param string $exception_class
+   *   The class of the exception that should be thrown by the committer.
+   * @param bool $will_be_in_maintenance_mode
+   *   Whether or not the site will be in maintenance mode afterward.
+   *
+   * @dataProvider providerMaintenanceModeAffectedByException
+   */
+  public function testMaintenanceModeAffectedByException(string $exception_class, bool $will_be_in_maintenance_mode): void {
+    $this->getStageFixtureManipulator()->setCorePackageVersion('9.8.1');
+
+    $message = new TranslatableMessage('A fail whale upon your head!');
+    LoggingCommitter::setException(match ($exception_class) {
+      InvalidArgumentException::class =>
+        new InvalidArgumentException($message),
+      PreconditionException::class =>
+        new PreconditionException($this->createMock(PreconditionInterface::class), $message),
+      default =>
+        new $exception_class((string) $message),
+    });
+
+    /** @var \Drupal\Core\State\StateInterface $state */
+    $state = $this->container->get('state');
+    $this->assertNull($state->get('system.maintenance_mode'));
+    $this->container->get('cron')->run();
+    $this->assertFalse($this->logger->hasRecord('Unattended update was applied in maintenance mode.', RfcLogLevel::INFO));
+    $this->assertSame($will_be_in_maintenance_mode, $state->get('system.maintenance_mode'));
+  }
+
   private function assertRegularCronRun(bool $expected_cron_run) {
     $this->assertSame($expected_cron_run, $this->container->get('state')->get('common_test.cron') === 'success');
   }