From 028287adfe08473af3c95004ff9e2b0f94a71ed7 Mon Sep 17 00:00:00 2001
From: phenaproxima <phenaproxima@205645.no-reply.drupal.org>
Date: Mon, 29 Nov 2021 21:47:15 +0000
Subject: [PATCH] Issue #3249135 by phenaproxima, tedbow: Use the stage's
tempstore to persist metadata about updates
---
automatic_updates.services.yml | 1 -
package_manager/src/Stage.php | 46 +++++++++++++++
src/BatchProcessor.php | 2 +-
src/CronUpdater.php | 2 +-
src/Form/UpdaterForm.php | 2 +-
src/Updater.php | 58 ++++---------------
.../src/TestController.php | 2 +-
.../Functional/FileSystemOperationsTest.php | 3 +-
tests/src/Kernel/CronUpdaterTest.php | 2 +-
9 files changed, 64 insertions(+), 54 deletions(-)
diff --git a/automatic_updates.services.yml b/automatic_updates.services.yml
index 07e2a63068..c61b581d97 100644
--- a/automatic_updates.services.yml
+++ b/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'
diff --git a/package_manager/src/Stage.php b/package_manager/src/Stage.php
index af15bae087..eb5da9d212 100644
--- a/package_manager/src/Stage.php
+++ b/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;
}
diff --git a/src/BatchProcessor.php b/src/BatchProcessor.php
index 0b68b0a6b1..fbf12af472 100644
--- a/src/BatchProcessor.php
+++ b/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);
diff --git a/src/CronUpdater.php b/src/CronUpdater.php
index ff111659af..f989c58556 100644
--- a/src/CronUpdater.php
+++ b/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());
diff --git a/src/Form/UpdaterForm.php b/src/Form/UpdaterForm.php
index 83131b4e12..42eba8fcf0 100644
--- a/src/Form/UpdaterForm.php
+++ b/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"));
}
diff --git a/src/Updater.php b/src/Updater.php
index 4a18cde593..540c994048 100644
--- a/src/Updater.php
+++ b/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}
*/
diff --git a/tests/modules/automatic_updates_test/src/TestController.php b/tests/modules/automatic_updates_test/src/TestController.php
index d96e56a01f..abb448bcd1 100644
--- a/tests/modules/automatic_updates_test/src/TestController.php
+++ b/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'];
diff --git a/tests/src/Functional/FileSystemOperationsTest.php b/tests/src/Functional/FileSystemOperationsTest.php
index df82931299..315d59f769 100644
--- a/tests/src/Functional/FileSystemOperationsTest.php
+++ b/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);
}
diff --git a/tests/src/Kernel/CronUpdaterTest.php b/tests/src/Kernel/CronUpdaterTest.php
index e31a4f2c40..78dc4d7d13 100644
--- a/tests/src/Kernel/CronUpdaterTest.php
+++ b/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();
--
GitLab