Issue #3249135 by phenaproxima, tedbow: Use the stage's tempstore to persist metadata about updates

automatic_updates.services.yml

@@ -11,7 +11,6 @@ services:
   automatic_updates.updater:
     class: Drupal\automatic_updates\Updater
     arguments:
-      - '@state'
       - '@package_manager.path_locator'
       - '@package_manager.beginner'
       - '@package_manager.stager'

package_manager/src/Stage.php

@@ -42,6 +42,13 @@ class Stage {
    */
   protected const TEMPSTORE_LOCK_KEY = 'lock';
 
+  /**
+   * The tempstore key under which to store arbitrary metadata for this stage.
+   *
+   * @var string
+   */
+  protected const TEMPSTORE_METADATA_KEY = 'metadata';
+
   /**
    * The path locator service.
    *
@@ -138,6 +145,44 @@ class Stage {
     return empty($this->tempStore->getMetadata(static::TEMPSTORE_LOCK_KEY));
   }
 
+  /**
+   * Returns a specific piece of metadata associated with this stage.
+   *
+   * Only the owner of the stage can access metadata, and the stage must either
+   * be claimed by its owner, or created during the current request.
+   *
+   * @param string $key
+   *   The metadata key.
+   *
+   * @return mixed
+   *   The metadata value, or NULL if it is not set.
+   */
+  protected function getMetadata(string $key) {
+    $this->checkOwnership();
+
+    $metadata = $this->tempStore->getIfOwner(static::TEMPSTORE_METADATA_KEY) ?: [];
+    return $metadata[$key] ?? NULL;
+  }
+
+  /**
+   * Stores arbitrary metadata associated with this stage.
+   *
+   * Only the owner of the stage can set metadata, and the stage must either be
+   * claimed by its owner, or created during the current request.
+   *
+   * @param string $key
+   *   The key under which to store the metadata.
+   * @param mixed $data
+   *   The metadata to store.
+   */
+  protected function setMetadata(string $key, $data): void {
+    $this->checkOwnership();
+
+    $metadata = $this->tempStore->get(static::TEMPSTORE_METADATA_KEY);
+    $metadata[$key] = $data;
+    $this->tempStore->set(static::TEMPSTORE_METADATA_KEY, $metadata);
+  }
+
   /**
    * Copies the active code base into the staging area.
    *
@@ -244,6 +289,7 @@ class Stage {
    * Marks the stage as available.
    */
   protected function markAsAvailable(): void {
+    $this->tempStore->delete(static::TEMPSTORE_METADATA_KEY);
     $this->tempStore->delete(static::TEMPSTORE_LOCK_KEY);
     $this->lock = NULL;
   }

src/BatchProcessor.php

@@ -123,7 +123,7 @@ class BatchProcessor {
    */
   public static function clean(string $stage_id, array &$context): void {
     try {
-      static::getUpdater()->claim($stage_id)->clean();
+      static::getUpdater()->claim($stage_id)->destroy();
     }
     catch (\Throwable $e) {
       static::handleException($e, $context);

src/CronUpdater.php

@@ -132,7 +132,7 @@ class CronUpdater implements ContainerInjectionInterface {
       ]);
       $this->updater->stage();
       $this->updater->apply();
-      $this->updater->clean();
+      $this->updater->destroy();
     }
     catch (\Throwable $e) {
       $this->logger->error($e->getMessage());

src/Form/UpdaterForm.php

@@ -234,7 +234,7 @@ class UpdaterForm extends FormBase {
    * Submit function to delete an existing in-progress update.
    */
   public function deleteExistingUpdate(): void {
-    $this->updater->clean(TRUE);
+    $this->updater->destroy(TRUE);
     $this->messenger()->addMessage($this->t("Staged update deleted"));
   }
 

src/Updater.php

@@ -3,7 +3,6 @@
 namespace Drupal\automatic_updates;
 
 use Drupal\automatic_updates\Exception\UpdateException;
-use Drupal\Core\State\StateInterface;
 use Drupal\package_manager\ComposerUtility;
 use Drupal\package_manager\Event\StageEvent;
 use Drupal\package_manager\Stage;
@@ -14,33 +13,6 @@ use Drupal\package_manager\StageException;
  */
 class Updater extends Stage {
 
-  /**
-   * State key under which to store the package versions targeted by the update.
-   *
-   * @var string
-   */
-  protected const PACKAGES_KEY = 'automatic_updates.packages';
-
-  /**
-   * The state service.
-   *
-   * @var \Drupal\Core\State\StateInterface
-   */
-  protected $state;
-
-  /**
-   * Constructs an Updater object.
-   *
-   * @param \Drupal\Core\State\StateInterface $state
-   *   The state service.
-   * @param mixed ...$arguments
-   *   Additional arguments to pass to the parent constructor.
-   */
-  public function __construct(StateInterface $state, ...$arguments) {
-    $this->state = $state;
-    parent::__construct(...$arguments);
-  }
-
   /**
    * Begins the update.
    *
@@ -59,7 +31,10 @@ class Updater extends Stage {
     }
 
     $composer = ComposerUtility::createForDirectory($this->pathLocator->getActiveDirectory());
-    $package_versions = $this->getPackageVersions();
+    $package_versions = [
+      'production' => [],
+      'dev' => [],
+    ];
 
     foreach ($composer->getCorePackageNames() as $package) {
       $package_versions['production'][$package] = $project_versions['drupal'];
@@ -67,7 +42,13 @@ class Updater extends Stage {
     foreach ($composer->getCoreDevPackageNames() as $package) {
       $package_versions['dev'][$package] = $project_versions['drupal'];
     }
-    $this->state->set(static::PACKAGES_KEY, $package_versions);
+
+    // Ensure that package versions are available to pre-create event
+    // subscribers. We can't use ::setMetadata() here because it requires the
+    // stage to be claimed, but that only happens during ::create().
+    $this->tempStore->set(static::TEMPSTORE_METADATA_KEY, [
+      'packages' => $package_versions,
+    ]);
     return $this->create();
   }
 
@@ -80,10 +61,7 @@ class Updater extends Stage {
    *   version constraints understood by Composer.
    */
   public function getPackageVersions(): array {
-    return $this->state->get(static::PACKAGES_KEY, [
-      'production' => [],
-      'dev' => [],
-    ]);
+    return $this->getMetadata('packages');
   }
 
   /**
@@ -110,18 +88,6 @@ class Updater extends Stage {
     }
   }
 
-  /**
-   * Cleans the current update.
-   *
-   * @param bool $force
-   *   (optional) If TRUE, the staging area will be destroyed even if it is not
-   *   owned by the current user or session. Defaults to FALSE.
-   */
-  public function clean(bool $force = FALSE): void {
-    $this->destroy($force);
-    $this->state->delete(static::PACKAGES_KEY);
-  }
-
   /**
    * {@inheritdoc}
    */

tests/modules/automatic_updates_test/src/TestController.php

@@ -34,7 +34,7 @@ class TestController extends ControllerBase {
       $updater->begin(['drupal' => $to_version]);
       $updater->stage();
       $updater->apply();
-      $updater->clean();
+      $updater->destroy();
 
       $project = (new UpdateRecommender())->getProjectInfo();
       $content = $project['existing_version'];

tests/src/Functional/FileSystemOperationsTest.php

@@ -70,7 +70,6 @@ class FileSystemOperationsTest extends AutomaticUpdatesFunctionalTestBase {
     );
 
     $this->updater = new Updater(
-      $this->container->get('state'),
       $locator->reveal(),
       $this->container->get('package_manager.beginner'),
       $this->container->get('package_manager.stager'),
@@ -131,7 +130,7 @@ class FileSystemOperationsTest extends AutomaticUpdatesFunctionalTestBase {
     $this->assertTrue(chmod("$this->stageDir/sites/default", 0400));
     $this->assertNotIsWritable("$this->stageDir/sites/default/staged.txt");
     // If the site directory is not writable, this will throw an exception.
-    $this->updater->clean();
+    $this->updater->destroy();
     $this->assertDirectoryDoesNotExist($this->stageDir);
   }
 

tests/src/Kernel/CronUpdaterTest.php

@@ -108,7 +108,7 @@ class CronUpdaterTest extends AutomaticUpdatesKernelTestBase {
     $updater->begin(['drupal' => '9.8.1'])->shouldBeCalledTimes($will_update);
     $updater->stage()->shouldBeCalledTimes($will_update);
     $updater->apply()->shouldBeCalledTimes($will_update);
-    $updater->clean()->shouldBeCalledTimes($will_update);
+    $updater->destroy()->shouldBeCalledTimes($will_update);
     $this->container->set('automatic_updates.updater', $updater->reveal());
 
     $this->container->get('cron')->run();