Commit 2d52693c authored by Adam G-H's avatar Adam G-H
Browse files

Issue #3248909 by phenaproxima, tedbow: Do not allow destroying a stage during Apply

parent e841a73c

automatic_updates.services.yml

+2 −0
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@ services: 
      - '@file_system'

      - '@event_dispatcher'

      - '@tempstore.shared'

      - '@datetime.time'

  automatic_updates.cron_updater:

    class: Drupal\automatic_updates\CronUpdater

    arguments:
@@ -33,6 +34,7 @@ services: 
      - '@file_system'

      - '@event_dispatcher'

      - '@tempstore.shared'

      - '@datetime.time'

  automatic_updates.staged_projects_validator:

    class: Drupal\automatic_updates\Validator\StagedProjectsValidator

    arguments:

package_manager/src/Stage.php

+49 −10
Original line number Diff line number Diff line
@@ -2,6 +2,7 @@ 


namespace Drupal\package_manager;



use Drupal\Component\Datetime\TimeInterface;

use Drupal\Component\FileSystem\FileSystem;

use Drupal\Component\Utility\Crypt;

use Drupal\Core\File\Exception\FileException;
@@ -55,6 +56,16 @@ class Stage { 
   */

  protected const TEMPSTORE_METADATA_KEY = 'metadata';



  /**

   * The tempstore key under which to store the time that ::apply() was called.

   *

   * @var string

   *

   * @see ::apply()

   * @see ::destroy()

   */

  private const TEMPSTORE_APPLY_TIME_KEY = 'apply_time';



  /**

   * The path locator service.

   *
@@ -104,6 +115,13 @@ class Stage { 
   */

  protected $tempStore;



  /**

   * The time service.

   *

   * @var \Drupal\Component\Datetime\TimeInterface

   */

  protected $time;



  /**

   * The lock info for the stage.

   *
@@ -130,14 +148,17 @@ class Stage { 
   *   The event dispatcher service.

   * @param \Drupal\Core\TempStore\SharedTempStoreFactory $shared_tempstore

   *   The shared tempstore factory.

   * @param \Drupal\Component\Datetime\TimeInterface $time

   *   The time service.

   */

  public function __construct(PathLocator $path_locator, BeginnerInterface $beginner, StagerInterface $stager, CommitterInterface $committer, FileSystemInterface $file_system, EventDispatcherInterface $event_dispatcher, SharedTempStoreFactory $shared_tempstore) {

  public function __construct(PathLocator $path_locator, BeginnerInterface $beginner, StagerInterface $stager, CommitterInterface $committer, FileSystemInterface $file_system, EventDispatcherInterface $event_dispatcher, SharedTempStoreFactory $shared_tempstore, TimeInterface $time) {

    $this->pathLocator = $path_locator;

    $this->beginner = $beginner;

    $this->stager = $stager;

    $this->committer = $committer;

    $this->fileSystem = $file_system;

    $this->eventDispatcher = $event_dispatcher;

    $this->time = $time;

    $this->tempStore = $shared_tempstore->get('package_manager_stage');

  }
@@ -224,7 +245,9 @@ class Stage { 
    $stage_dir = $this->getStageDirectory();



    $event = new PreCreateEvent($this);

    $this->dispatch($event);

    // If an error occurs and we won't be able to create the stage, mark it as

    // available.

    $this->dispatch($event, [$this, 'markAsAvailable']);



    $this->beginner->begin($active_dir, $stage_dir, $event->getExcludedPaths());

    $this->dispatch(new PostCreateEvent($this));
@@ -277,9 +300,16 @@ class Stage { 
    $stage_dir = $this->getStageDirectory();



    $event = new PreApplyEvent($this);

    $this->dispatch($event);

    $this->tempStore->set(self::TEMPSTORE_APPLY_TIME_KEY, $this->time->getRequestTime());

    // If an error occurs while dispatching the event, ensure that ::destroy()

    // doesn't think we're in the middle of applying the staged changes to the

    // active directory.

    $this->dispatch($event, function (): void {

      $this->tempStore->delete(self::TEMPSTORE_APPLY_TIME_KEY);

    });



    $this->committer->commit($stage_dir, $active_dir, $event->getExcludedPaths());

    $this->tempStore->delete(self::TEMPSTORE_APPLY_TIME_KEY);

    $this->dispatch(new PostApplyEvent($this));

  }
@@ -290,14 +320,22 @@ class Stage { 
   *   (optional) If TRUE, the staging area will be destroyed even if it is not

   *   owned by the current user or session. Defaults to FALSE.

   *

   * @todo Do not allow the stage to be destroyed while it's being applied to

   *   the active directory in https://www.drupal.org/i/3248909.

   * @throws \Drupal\package_manager\Exception\StageException

   *   If the staged changes are being applied to the active directory.

   */

  public function destroy(bool $force = FALSE): void {

    if (!$force) {

      $this->checkOwnership();

    }



    // If we started applying staged changes to the active directory less than

    // an hour ago, prevent the stage from being destroyed.

    // @see :apply()

    $apply_time = $this->tempStore->get(self::TEMPSTORE_APPLY_TIME_KEY);

    if (isset($apply_time) && $this->time->getRequestTime() - $apply_time < 3600) {

      throw new StageException('Cannot destroy the staging area while it is being applied to the active directory.');

    }



    $this->dispatch(new PreDestroyEvent($this));

    // Delete all directories in parent staging directory.

    $parent_stage_dir = static::getStagingRoot();
@@ -332,13 +370,16 @@ class Stage { 
   *

   * @param \Drupal\package_manager\Event\StageEvent $event

   *   The event object.

   * @param callable $on_error

   *   (optional) A callback function to call if an error occurs, before any

   *   exceptions are thrown.

   *

   * @throws \Drupal\package_manager\Exception\StageValidationException

   *   If the event collects any validation errors.

   * @throws \Drupal\package_manager\Exception\StageException

   *   If any other sort of error occurs.

   */

  protected function dispatch(StageEvent $event): void {

  protected function dispatch(StageEvent $event, callable $on_error = NULL): void {

    try {

      $this->eventDispatcher->dispatch($event);
@@ -354,10 +395,8 @@ class Stage { 
    }



    if (isset($error)) {

      // If we won't be able to create the staging area, mark it as available.

      // @see ::create()

      if ($event instanceof PreCreateEvent) {

        $this->markAsAvailable();

      if ($on_error) {

        $on_error();

      }

      throw $error;

    }

package_manager/tests/modules/package_manager_test_api/src/ApiController.php

+1 −0
Original line number Diff line number Diff line
@@ -42,6 +42,7 @@ class ApiController extends ControllerBase { 
      $container->get('file_system'),

      $container->get('event_dispatcher'),

      $container->get('tempstore.shared'),

      $container->get('datetime.time')

    );

    return new static($stage);

  }

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

+16 −0
Original line number Diff line number Diff line
@@ -44,6 +44,19 @@ class TestSubscriber implements EventSubscriberInterface { 
    $this->state = $state;

  }



  /**

   * Sets whether a specific event will call exit().

   *

   * This is useful for simulating an unrecoverable (fatal) error when handling

   * the given event.

   *

   * @param string $event

   *   The event class.

   */

  public static function setExit(string $event): void {

    \Drupal::state()->set(static::STATE_KEY . ".$event", 'exit');

  }



  /**

   * Sets validation results for a specific event.

   *
@@ -102,6 +115,9 @@ class TestSubscriber implements EventSubscriberInterface { 
    if ($results instanceof \Throwable) {

      throw $results;

    }

    elseif ($results === 'exit') {

      exit();

    }

    /** @var \Drupal\package_manager\ValidationResult $result */

    foreach ($results as $result) {

      if ($result->getSeverity() === SystemManager::REQUIREMENT_ERROR) {

package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php

+4 −3
Original line number Diff line number Diff line
@@ -78,7 +78,8 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase { 
      $this->container->get('package_manager.committer'),

      $this->container->get('file_system'),

      $this->container->get('event_dispatcher'),

      $this->container->get('tempstore.shared')

      $this->container->get('tempstore.shared'),

      $this->container->get('datetime.time')

    );

  }
@@ -297,9 +298,9 @@ class TestStage extends Stage { 
  /**

   * {@inheritdoc}

   */

  protected function dispatch(StageEvent $event): void {

  protected function dispatch(StageEvent $event, callable $on_error = NULL): void {

    try {

      parent::dispatch($event);

      parent::dispatch($event, $on_error);

    }

    catch (StageException $e) {

      // Attach the event object to the exception so that test code can verify