diff --git a/automatic_updates.services.yml b/automatic_updates.services.yml
index d0d414912d3cc741aac57ce0a262243ba76bda7d..07e2a63068e3c97c250d9b414d7e74d833c57b92 100644
--- a/automatic_updates.services.yml
+++ b/automatic_updates.services.yml
@@ -18,6 +18,7 @@ services:
- '@package_manager.committer'
- '@package_manager.cleaner'
- '@event_dispatcher'
+ - '@tempstore.shared'
automatic_updates.update_refresh_subscriber:
class: Drupal\automatic_updates\Event\UpdateRefreshSubscriber
arguments:
diff --git a/package_manager/src/Stage.php b/package_manager/src/Stage.php
index d854e2a5d9f69b127c08d2b9f85fb32a09897f7e..e835eefd3a0e29467e132c7cb038f4a93f90e13b 100644
--- a/package_manager/src/Stage.php
+++ b/package_manager/src/Stage.php
@@ -2,6 +2,7 @@
namespace Drupal\package_manager;
+use Drupal\Core\TempStore\SharedTempStoreFactory;
use Drupal\package_manager\Event\PostApplyEvent;
use Drupal\package_manager\Event\PostCreateEvent;
use Drupal\package_manager\Event\PostDestroyEvent;
@@ -70,6 +71,21 @@ class Stage {
*/
protected $eventDispatcher;
+
+ /**
+ * The tempstore key under which to store the active status of this stage.
+ *
+ * @var string
+ */
+ protected const TEMPSTORE_ACTIVE_KEY = 'active';
+
+ /**
+ * The shared temp store.
+ *
+ * @var \Drupal\Core\TempStore\SharedTempStore
+ */
+ protected $tempStore;
+
/**
* Constructs a new Stage object.
*
@@ -85,20 +101,55 @@ class Stage {
* The cleaner service from Composer Stager.
* @param \Symfony\Contracts\EventDispatcher\EventDispatcherInterface $event_dispatcher
* The event dispatcher service.
+ * @param \Drupal\Core\TempStore\SharedTempStoreFactory $shared_tempstore
+ * The shared tempstore factory.
*/
- public function __construct(PathLocator $path_locator, BeginnerInterface $beginner, StagerInterface $stager, CommitterInterface $committer, CleanerInterface $cleaner, EventDispatcherInterface $event_dispatcher) {
+ public function __construct(PathLocator $path_locator, BeginnerInterface $beginner, StagerInterface $stager, CommitterInterface $committer, CleanerInterface $cleaner, EventDispatcherInterface $event_dispatcher, SharedTempStoreFactory $shared_tempstore) {
$this->pathLocator = $path_locator;
$this->beginner = $beginner;
$this->stager = $stager;
$this->committer = $committer;
$this->cleaner = $cleaner;
$this->eventDispatcher = $event_dispatcher;
+ $this->tempStore = $shared_tempstore->get('package_manager_stage');
+ }
+
+ /**
+ * Determines if the staging area can be created.
+ *
+ * @return bool
+ * TRUE if the staging area can be created, otherwise FALSE.
+ */
+ final public function isAvailable(): bool {
+ return empty($this->tempStore->getMetadata(static::TEMPSTORE_ACTIVE_KEY));
+ }
+
+ /**
+ * Determines if the current user or session is the owner of the staging area.
+ *
+ * @return bool
+ * TRUE if the current session or user is the owner of the staging area,
+ * otherwise FALSE.
+ */
+ final public function isOwnedByCurrentUser(): bool {
+ return !empty($this->tempStore->getIfOwner(static::TEMPSTORE_ACTIVE_KEY));
}
/**
* Copies the active code base into the staging area.
*/
public function create(): void {
+ if (!$this->isAvailable()) {
+ throw new StageException([], 'Cannot create a new stage because one already exists.');
+ }
+ // Mark the stage as unavailable as early as possible, before dispatching
+ // the pre-create event. The idea is to prevent a race condition if the
+ // event subscribers take a while to finish, and two different users attempt
+ // to create a staging area at around the same time. If an error occurs
+ // while the event is being processed, the stage is marked as available.
+ // @see ::dispatch()
+ $this->tempStore->set(static::TEMPSTORE_ACTIVE_KEY, TRUE);
+
$active_dir = $this->pathLocator->getActiveDirectory();
$stage_dir = $this->pathLocator->getStageDirectory();
@@ -119,6 +170,8 @@ class Stage {
* Defaults to FALSE.
*/
public function require(array $constraints, bool $dev = FALSE): void {
+ $this->checkOwnership();
+
$command = array_merge(['require'], $constraints);
$command[] = '--update-with-all-dependencies';
if ($dev) {
@@ -134,6 +187,8 @@ class Stage {
* Applies staged changes to the active directory.
*/
public function apply(): void {
+ $this->checkOwnership();
+
$active_dir = $this->pathLocator->getActiveDirectory();
$stage_dir = $this->pathLocator->getStageDirectory();
@@ -146,13 +201,26 @@ class Stage {
/**
* Deletes the staging area.
+ *
+ * @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.
+ *
+ * @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.
*/
- public function destroy(): void {
+ public function destroy(bool $force = FALSE): void {
+ if (!$force) {
+ $this->checkOwnership();
+ }
+
$this->dispatch(new PreDestroyEvent($this));
$stage_dir = $this->pathLocator->getStageDirectory();
if (is_dir($stage_dir)) {
$this->cleaner->clean($stage_dir);
}
+ // We're all done, so mark the stage as available.
+ $this->tempStore->delete(static::TEMPSTORE_ACTIVE_KEY);
$this->dispatch(new PostDestroyEvent($this));
}
@@ -161,13 +229,37 @@ class Stage {
*
* @param \Drupal\package_manager\Event\StageEvent $event
* The event object.
+ *
+ * @throws \Drupal\package_manager\StageException
+ * If the event collects any validation errors, or a subscriber throws a
+ * StageException directly.
+ * @throws \RuntimeException
+ * If any other sort of error occurs.
*/
protected function dispatch(StageEvent $event): void {
- $this->eventDispatcher->dispatch($event);
+ try {
+ $this->eventDispatcher->dispatch($event);
+
+ $errors = $event->getResults(SystemManager::REQUIREMENT_ERROR);
+ if ($errors) {
+ throw new StageException($errors);
+ }
+ }
+ catch (\Throwable $error) {
+ // If we are not going to be able to create the staging area, mark it as
+ // available.
+ // @see ::create()
+ if ($event instanceof PreCreateEvent) {
+ $this->tempStore->delete(static::TEMPSTORE_ACTIVE_KEY);
+ }
- $errors = $event->getResults(SystemManager::REQUIREMENT_ERROR);
- if ($errors) {
- throw new StageException($errors);
+ // Wrap the exception to preserve the backtrace, and re-throw it.
+ if ($error instanceof StageException) {
+ throw new StageException($error->getResults(), $error->getMessage(), $error->getCode(), $error);
+ }
+ else {
+ throw new \RuntimeException($error->getMessage(), $error->getCode(), $error);
+ }
}
}
@@ -193,4 +285,16 @@ class Stage {
return ComposerUtility::createForDirectory($dir);
}
+ /**
+ * Ensures that the current user or session owns the staging area.
+ *
+ * @throws \Drupal\package_manager\StageException
+ * If the current user or session does not own the staging area.
+ */
+ protected function checkOwnership(): void {
+ if (!$this->isOwnedByCurrentUser()) {
+ throw new StageException([], 'Stage is not owned by the current user or session.');
+ }
+ }
+
}
diff --git a/package_manager/tests/src/Functional/ExcludedPathsTest.php b/package_manager/tests/src/Functional/ExcludedPathsTest.php
index fd28b082abeb31ac76b886c643de488c91ffaf8e..9de0de61efab77f6331b120311e1bc280a379cc0 100644
--- a/package_manager/tests/src/Functional/ExcludedPathsTest.php
+++ b/package_manager/tests/src/Functional/ExcludedPathsTest.php
@@ -98,7 +98,8 @@ class ExcludedPathsTest extends BrowserTestBase {
$this->container->get('package_manager.stager'),
$this->container->get('package_manager.committer'),
$this->container->get('package_manager.cleaner'),
- $this->container->get('event_dispatcher')
+ $this->container->get('event_dispatcher'),
+ $this->container->get('tempstore.shared'),
);
$stage->create();
diff --git a/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php b/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php
index 846426f14fce291559e7c957a34437600f058a47..626c4e38411bf69240cf18975466bbafc1061e8d 100644
--- a/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php
+++ b/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php
@@ -59,6 +59,7 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase {
$this->container->get('package_manager.committer'),
$this->container->get('package_manager.cleaner'),
$this->container->get('event_dispatcher'),
+ $this->container->get('tempstore.shared')
);
}
@@ -93,6 +94,22 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase {
}
}
+ /**
+ * Marks all pending post-update functions as completed.
+ *
+ * Since kernel tests don't normally install modules and register their
+ * updates, this method makes sure that we are testing from a clean, fully
+ * up-to-date state.
+ */
+ protected function registerPostUpdateFunctions(): void {
+ $updates = $this->container->get('update.post_update_registry')
+ ->getPendingUpdateFunctions();
+
+ $this->container->get('keyvalue')
+ ->get('post_update')
+ ->set('existing_updates', $updates);
+ }
+
}
/**
diff --git a/package_manager/tests/src/Kernel/PendingUpdatesValidatorTest.php b/package_manager/tests/src/Kernel/PendingUpdatesValidatorTest.php
index a1a26042637c3f7730493ee2e4937e3721804172..ea90029ca28aa001c035cb076bf868158b580dca 100644
--- a/package_manager/tests/src/Kernel/PendingUpdatesValidatorTest.php
+++ b/package_manager/tests/src/Kernel/PendingUpdatesValidatorTest.php
@@ -17,22 +17,6 @@ class PendingUpdatesValidatorTest extends PackageManagerKernelTestBase {
*/
protected static $modules = ['system'];
- /**
- * Registers all of the System module's post-update functions.
- *
- * Since kernel tests don't normally install modules and register their
- * updates, this method makes sure that the validator is tested from a clean,
- * fully up-to-date state.
- */
- private function registerPostUpdateFunctions(): void {
- $updates = $this->container->get('update.post_update_registry')
- ->getPendingUpdateFunctions();
-
- $this->container->get('keyvalue')
- ->get('post_update')
- ->set('existing_updates', $updates);
- }
-
/**
* Tests that no error is raised if there are no pending updates.
*/
diff --git a/package_manager/tests/src/Kernel/StageEventsTest.php b/package_manager/tests/src/Kernel/StageEventsTest.php
index ff506bc6cc1cdf74208a6ecc1ab0e9b8b3f1e792..aa67a4b3ac214450070c966af8cc94f6bbb7d0fd 100644
--- a/package_manager/tests/src/Kernel/StageEventsTest.php
+++ b/package_manager/tests/src/Kernel/StageEventsTest.php
@@ -51,7 +51,8 @@ class StageEventsTest extends PackageManagerKernelTestBase implements EventSubsc
$this->container->get('package_manager.stager'),
$this->container->get('package_manager.committer'),
$this->container->get('package_manager.cleaner'),
- $this->container->get('event_dispatcher')
+ $this->container->get('event_dispatcher'),
+ $this->container->get('tempstore.shared')
);
}
diff --git a/package_manager/tests/src/Kernel/StageOwnershipTest.php b/package_manager/tests/src/Kernel/StageOwnershipTest.php
new file mode 100644
index 0000000000000000000000000000000000000000..b763c6ef2d6a92cb977836b91a21d23be2ce188a
--- /dev/null
+++ b/package_manager/tests/src/Kernel/StageOwnershipTest.php
@@ -0,0 +1,136 @@
+<?php
+
+namespace Drupal\Tests\package_manager\Kernel;
+
+use Drupal\package_manager\StageException;
+use Drupal\Tests\user\Traits\UserCreationTrait;
+
+/**
+ * Tests that ownership of the stage is enforced.
+ *
+ * @group package_manger
+ */
+class StageOwnershipTest extends PackageManagerKernelTestBase {
+
+ use UserCreationTrait;
+
+ /**
+ * {@inheritdoc}
+ */
+ protected static $modules = ['system', 'user'];
+
+ /**
+ * {@inheritdoc}
+ */
+ protected function setUp(): void {
+ parent::setUp();
+ $this->installSchema('system', ['sequences']);
+ $this->installEntitySchema('user');
+ $this->registerPostUpdateFunctions();
+ }
+
+ /**
+ * Tests only the owner of stage can perform operations, even if logged out.
+ */
+ public function testOwnershipEnforcedWhenLoggedOut(): void {
+ $this->assertOwnershipIsEnforced($this->createStage(), $this->createStage());
+ }
+
+ /**
+ * Tests only the owner of stage can perform operations.
+ */
+ public function testOwnershipEnforcedWhenLoggedIn(): void {
+ $user_1 = $this->createUser([], NULL, FALSE, ['uid' => 2]);
+ $this->setCurrentUser($user_1);
+
+ $will_create = $this->createStage();
+ // Rebuild the container so that the shared tempstore factory is made
+ // properly aware of the new current user ($user_2) before another stage
+ // is created.
+ $kernel = $this->container->get('kernel');
+ $this->container = $kernel->rebuildContainer();
+ $user_2 = $this->createUser();
+ $this->setCurrentUser($user_2);
+ $this->assertOwnershipIsEnforced($will_create, $this->createStage());
+ }
+
+ /**
+ * Asserts that ownership is enforced across staging areas.
+ *
+ * @param \Drupal\Tests\package_manager\Kernel\TestStage $will_create
+ * The stage that will be created, and owned by the current user or session.
+ * @param \Drupal\Tests\package_manager\Kernel\TestStage $never_create
+ * The stage that will not be created, but should still respect the
+ * ownership and status of the other stage.
+ */
+ private function assertOwnershipIsEnforced(TestStage $will_create, TestStage $never_create): void {
+ // Before the staging area is created, isOwnedByCurrentUser() should return
+ // FALSE and isAvailable() should return TRUE.
+ $this->assertFalse($will_create->isOwnedByCurrentUser());
+ $this->assertFalse($never_create->isOwnedByCurrentUser());
+ $this->assertTrue($will_create->isAvailable());
+ $this->assertTrue($never_create->isAvailable());
+
+ $will_create->create();
+ // Only the staging area that was actually created should be owned by the
+ // current user...
+ $this->assertTrue($will_create->isOwnedByCurrentUser());
+ $this->assertFalse($never_create->isOwnedByCurrentUser());
+ // ...but both staging areas should be considered unavailable (i.e., cannot
+ // be created until the existing one is destroyed first).
+ $this->assertFalse($will_create->isAvailable());
+ $this->assertFalse($never_create->isAvailable());
+
+ // We should get an error if we try to create the staging area again,
+ // regardless of who owns it.
+ foreach ([$will_create, $never_create] as $stage) {
+ try {
+ $stage->create();
+ $this->fail("Able to create a stage that already exists.");
+ }
+ catch (StageException $exception) {
+ $this->assertSame('Cannot create a new stage because one already exists.', $exception->getMessage());
+ }
+ }
+
+ // Only the stage's owner should be able to move it through its life cycle.
+ $callbacks = [
+ 'require' => [
+ ['vendor/lib:0.0.1'],
+ ],
+ 'apply' => [],
+ 'destroy' => [],
+ ];
+ foreach ($callbacks as $method => $arguments) {
+ try {
+ $never_create->$method(...$arguments);
+ $this->fail("Able to call '$method' on a stage that was never created.");
+ }
+ catch (StageException $exception) {
+ $this->assertSame('Stage is not owned by the current user or session.', $exception->getMessage());
+ }
+ // The call should succeed on the created stage.
+ $will_create->$method(...$arguments);
+ }
+ }
+
+ /**
+ * Tests a stage being destroyed by a user who doesn't own it.
+ */
+ public function testForceDestroy(): void {
+ $owned = $this->createStage();
+ $owned->create();
+
+ $not_owned = $this->createStage();
+ try {
+ $not_owned->destroy();
+ $this->fail("Able to destroy a stage that we don't own.");
+ }
+ catch (StageException $exception) {
+ $this->assertSame('Stage is not owned by the current user or session.', $exception->getMessage());
+ }
+ // We should be able to destroy the stage if we ignore ownership.
+ $not_owned->destroy(TRUE);
+ }
+
+}
diff --git a/src/Form/UpdateReady.php b/src/Form/UpdateReady.php
index 68e792b1638dffb06e2c4ef31660fd871b998b66..ece828ee94aaa9c2398af110183dbcb60627aebb 100644
--- a/src/Form/UpdateReady.php
+++ b/src/Form/UpdateReady.php
@@ -71,6 +71,11 @@ class UpdateReady extends FormBase {
* {@inheritdoc}
*/
public function buildForm(array $form, FormStateInterface $form_state) {
+ if (!$this->updater->isOwnedByCurrentUser()) {
+ $this->messenger->addError('Cannot continue the update because another Composer operation is currently in progress.');
+ return $form;
+ }
+
$form['backup'] = [
'#prefix' => '<strong>',
'#markup' => $this->t('Back up your database and site before you continue. <a href=":backup_url">Learn how</a>.', [':backup_url' => 'https://www.drupal.org/node/22281']),
diff --git a/src/Form/UpdaterForm.php b/src/Form/UpdaterForm.php
index 78b6cc44059c9ec64e9728b4f2f4237e47b75096..43ad1a01cd12454c01c55d8e4608981c2319f5c3 100644
--- a/src/Form/UpdaterForm.php
+++ b/src/Form/UpdaterForm.php
@@ -191,7 +191,7 @@ class UpdaterForm extends FormBase {
->getResults(SystemManager::REQUIREMENT_ERROR);
if (empty($errors)) {
- $form['actions'] = $this->actions();
+ $form['actions'] = $this->actions($form_state);
}
return $form;
}
@@ -199,14 +199,22 @@ class UpdaterForm extends FormBase {
/**
* Builds the form actions.
*
+ * @param \Drupal\Core\Form\FormStateInterface $form_state
+ * The form state.
+ *
* @return mixed[][]
* The form's actions elements.
*/
- protected function actions(): array {
+ protected function actions(FormStateInterface $form_state): array {
$actions = ['#type' => 'actions'];
- if ($this->updater->hasActiveUpdate()) {
- $this->messenger()->addError($this->t('Another Composer update process is currently active'));
+ if (!$this->updater->isAvailable()) {
+ // If the form has been submitted do not display this error message
+ // because ::deleteExistingUpdate() may run on submit. The message will
+ // still be displayed on form build if needed.
+ if (!$form_state->getUserInput()) {
+ $this->messenger()->addError($this->t('Cannot begin an update because another Composer operation is currently in progress.'));
+ }
$actions['delete'] = [
'#type' => 'submit',
'#value' => $this->t('Delete existing update'),
diff --git a/src/Updater.php b/src/Updater.php
index 302b3e87abcdb870db72270f56b94e0c78389d8e..4a1371423460543bb1f305e506380501a4518b8a 100644
--- a/src/Updater.php
+++ b/src/Updater.php
@@ -41,20 +41,6 @@ class Updater extends Stage {
parent::__construct(...$arguments);
}
- /**
- * Determines if there is an active update in progress.
- *
- * @return bool
- * TRUE if there is active update, otherwise FALSE.
- */
- public function hasActiveUpdate(): bool {
- $staged_dir = $this->pathLocator->getStageDirectory();
- if (is_dir($staged_dir) || $this->state->get(static::PACKAGES_KEY)) {
- return TRUE;
- }
- return FALSE;
- }
-
/**
* Begins the update.
*
diff --git a/tests/fixtures/project_staged_validation/new_project_added/active/composer.lock b/tests/fixtures/project_staged_validation/new_project_added/active/composer.lock
index a98bc9cad77679e2f11d6cab1778199d4f5fbc53..67668976a3a41b6dd7a23c72c3fd9e3e126aa0f8 100644
--- a/tests/fixtures/project_staged_validation/new_project_added/active/composer.lock
+++ b/tests/fixtures/project_staged_validation/new_project_added/active/composer.lock
@@ -1,5 +1,10 @@
{
"packages": [
+ {
+ "name": "drupal/core",
+ "version": "9.8.0",
+ "type": "drupal-core"
+ },
{
"name": "drupal/test_module",
"version": "1.3.0",
diff --git a/tests/fixtures/project_staged_validation/new_project_added/staged/composer.lock b/tests/fixtures/project_staged_validation/new_project_added/staged/composer.lock
index a5b3e1c62aea2142e215a712b998cd3d0bc360f1..ba776321961b650a90d5459d0e5441b5a3575912 100644
--- a/tests/fixtures/project_staged_validation/new_project_added/staged/composer.lock
+++ b/tests/fixtures/project_staged_validation/new_project_added/staged/composer.lock
@@ -1,5 +1,10 @@
{
"packages": [
+ {
+ "name": "drupal/core",
+ "version": "9.8.1",
+ "type": "drupal-core"
+ },
{
"name": "drupal/test_module",
"version": "1.3.0",
diff --git a/tests/fixtures/project_staged_validation/no_errors/active/composer.lock b/tests/fixtures/project_staged_validation/no_errors/active/composer.lock
index 0eeceb0c96066f250b7f55664f6258c5914f0bef..24e46dc84c259bf9a0edf3400294e65e73488615 100644
--- a/tests/fixtures/project_staged_validation/no_errors/active/composer.lock
+++ b/tests/fixtures/project_staged_validation/no_errors/active/composer.lock
@@ -1,5 +1,10 @@
{
"packages": [
+ {
+ "name": "drupal/core",
+ "version": "9.8.0",
+ "type": "drupal-core"
+ },
{
"name": "drupal/test_module",
"version": "1.3.0",
diff --git a/tests/fixtures/project_staged_validation/no_errors/staged/composer.lock b/tests/fixtures/project_staged_validation/no_errors/staged/composer.lock
index 93ef0ebbc5f7d459dad93e4361c0bdb7336ae38d..5b77d4cbbd893f431d2b27e18efaf489988feea0 100644
--- a/tests/fixtures/project_staged_validation/no_errors/staged/composer.lock
+++ b/tests/fixtures/project_staged_validation/no_errors/staged/composer.lock
@@ -1,5 +1,10 @@
{
"packages": [
+ {
+ "name": "drupal/core",
+ "version": "9.8.1",
+ "type": "drupal-core"
+ },
{
"name": "drupal/test_module",
"version": "1.3.0",
diff --git a/tests/fixtures/project_staged_validation/project_removed/active/composer.lock b/tests/fixtures/project_staged_validation/project_removed/active/composer.lock
index ef2f849c66139908f83fe4c0579e967ade4228fe..ef86aa78d8df4942a0bcf8fe67d0561b044bfe62 100644
--- a/tests/fixtures/project_staged_validation/project_removed/active/composer.lock
+++ b/tests/fixtures/project_staged_validation/project_removed/active/composer.lock
@@ -1,5 +1,10 @@
{
"packages": [
+ {
+ "name": "drupal/core",
+ "version": "9.8.0",
+ "type": "drupal-core"
+ },
{
"name": "drupal/test_theme",
"version": "1.3.0",
diff --git a/tests/fixtures/project_staged_validation/project_removed/staged/composer.lock b/tests/fixtures/project_staged_validation/project_removed/staged/composer.lock
index d8a60b12647d7d1c0d40a27dca5296b95d84886e..07773fab4cc96d6d4f41f2e0d0067e3fc51f4e59 100644
--- a/tests/fixtures/project_staged_validation/project_removed/staged/composer.lock
+++ b/tests/fixtures/project_staged_validation/project_removed/staged/composer.lock
@@ -1,5 +1,10 @@
{
"packages": [
+ {
+ "name": "drupal/core",
+ "version": "9.8.1",
+ "type": "drupal-core"
+ },
{
"name": "drupal/test_module2",
"version": "1.3.1",
diff --git a/tests/fixtures/project_staged_validation/version_changed/active/composer.lock b/tests/fixtures/project_staged_validation/version_changed/active/composer.lock
index 5da8f50209cd3a1afca10a4572651140c9751980..36980fee6ffcbaa1fc93d2c970f8a1d951dd4d2d 100644
--- a/tests/fixtures/project_staged_validation/version_changed/active/composer.lock
+++ b/tests/fixtures/project_staged_validation/version_changed/active/composer.lock
@@ -1,5 +1,10 @@
{
"packages": [
+ {
+ "name": "drupal/core",
+ "version": "9.8.0",
+ "type": "drupal-core"
+ },
{
"name": "drupal/test_module",
"version": "1.3.0",
diff --git a/tests/fixtures/project_staged_validation/version_changed/staged/composer.lock b/tests/fixtures/project_staged_validation/version_changed/staged/composer.lock
index 129f0ffdc1c914e95763bb34b9c61180d58d1754..c16841c9dd296bbf7cbeab7bd5373bd020e38890 100644
--- a/tests/fixtures/project_staged_validation/version_changed/staged/composer.lock
+++ b/tests/fixtures/project_staged_validation/version_changed/staged/composer.lock
@@ -1,5 +1,10 @@
{
"packages": [
+ {
+ "name": "drupal/core",
+ "version": "9.8.1",
+ "type": "drupal-core"
+ },
{
"name": "drupal/test_module",
"version": "1.3.1",
diff --git a/tests/src/Functional/FileSystemOperationsTest.php b/tests/src/Functional/FileSystemOperationsTest.php
index 4d325efd3a2989a9cbd1ed03e326da04a5e6d0f7..df829312998af0947f996b710bc18a2044786c5e 100644
--- a/tests/src/Functional/FileSystemOperationsTest.php
+++ b/tests/src/Functional/FileSystemOperationsTest.php
@@ -76,7 +76,8 @@ class FileSystemOperationsTest extends AutomaticUpdatesFunctionalTestBase {
$this->container->get('package_manager.stager'),
$this->container->get('package_manager.committer'),
$cleaner,
- $this->container->get('event_dispatcher')
+ $this->container->get('event_dispatcher'),
+ $this->container->get('tempstore.shared')
);
// Use the public and private files directories in the fake site fixture.
diff --git a/tests/src/Functional/UpdateLockTest.php b/tests/src/Functional/UpdateLockTest.php
new file mode 100644
index 0000000000000000000000000000000000000000..b5411aacaa4d14d3cdbc76a422ad27afd4409a06
--- /dev/null
+++ b/tests/src/Functional/UpdateLockTest.php
@@ -0,0 +1,77 @@
+<?php
+
+namespace Drupal\Tests\automatic_updates\Functional;
+
+/**
+ * Tests that only one Automatic Update operation can be performed at a time.
+ *
+ * @group automatic_updates
+ */
+class UpdateLockTest extends AutomaticUpdatesFunctionalTestBase {
+
+ /**
+ * {@inheritdoc}
+ */
+ protected $defaultTheme = 'stark';
+
+ /**
+ * {@inheritdoc}
+ */
+ protected static $modules = [
+ 'automatic_updates',
+ 'automatic_updates_test',
+ 'package_manager_bypass',
+ ];
+
+ /**
+ * {@inheritdoc}
+ */
+ protected function setUp(): void {
+ parent::setUp();
+
+ $this->setReleaseMetadata(__DIR__ . '/../../fixtures/release-history/drupal.9.8.1.xml');
+ $this->drupalLogin($this->rootUser);
+ $this->checkForUpdates();
+ }
+
+ /**
+ * Tests that only user who started an update can continue through it.
+ */
+ public function testLock() {
+ $page = $this->getSession()->getPage();
+ $assert_session = $this->assertSession();
+ $this->setCoreVersion('9.8.0');
+ $this->checkForUpdates();
+ $permissions = ['administer software updates'];
+ $user_1 = $this->createUser($permissions);
+ $user_2 = $this->createUser($permissions);
+
+ // We should be able to get partway through an update without issue.
+ $this->drupalLogin($user_1);
+ $this->drupalGet('/admin/modules/automatic-update');
+ $page->pressButton('Update');
+ $this->checkForMetaRefresh();
+ $assert_session->buttonExists('Continue');
+ $assert_session->addressEquals('/admin/automatic-update-ready');
+
+ // Another user cannot show up and try to start an update, since the other
+ // user already started one.
+ $this->drupalLogin($user_2);
+ $this->drupalGet('/admin/modules/automatic-update');
+ $assert_session->buttonNotExists('Update');
+ $assert_session->pageTextContains('Cannot begin an update because another Composer operation is currently in progress.');
+
+ // If the current user did not start the update, they should not be able to
+ // continue it, either.
+ $this->drupalGet('/admin/automatic-update-ready');
+ $assert_session->pageTextContains('Cannot continue the update because another Composer operation is currently in progress.');
+ $assert_session->buttonNotExists('Continue');
+
+ // The user who started the update should be able to continue it.
+ $this->drupalLogin($user_1);
+ $this->drupalGet('/admin/automatic-update-ready');
+ $assert_session->pageTextNotContains('Cannot continue the update because another Composer operation is currently in progress.');
+ $assert_session->buttonExists('Continue');
+ }
+
+}
diff --git a/tests/src/Functional/UpdaterFormTest.php b/tests/src/Functional/UpdaterFormTest.php
index c2cdefcde60ee111f1082b43468ad3d20a974273..f05a955fcffed8ea0ee5c1c8eee50d33c1f6d714 100644
--- a/tests/src/Functional/UpdaterFormTest.php
+++ b/tests/src/Functional/UpdaterFormTest.php
@@ -183,6 +183,8 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase {
$assert_session->pageTextNotContains(static::$warningsExplanation);
$page->pressButton('Update');
$this->checkForMetaRefresh();
+ // If a validator flags an error, but doesn't throw, the update should still
+ // be halted.
$assert_session->pageTextContainsOnce('An error has occurred.');
$page->clickLink('the error page');
$assert_session->pageTextContainsOnce((string) $expected_results[0]->getMessages()[0]);
@@ -190,11 +192,10 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase {
$assert_session->pageTextNotContains($expected_results[0]->getSummary());
// ...but we should see the exception message.
$assert_session->pageTextContainsOnce('The update exploded.');
-
- // If a validator flags an error, but doesn't throw, the update should still
- // be halted.
+ // If the error is thrown in PreCreateEvent the update stage will not have
+ // been created.
+ $assert_session->buttonNotExists('Delete existing update');
TestChecker1::setTestResult($expected_results, PreCreateEvent::class);
- $this->deleteStagedUpdate();
$page->pressButton('Update');
$this->checkForMetaRefresh();
$assert_session->pageTextContainsOnce('An error has occurred.');
@@ -207,8 +208,6 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase {
// continue.
$expected_results = $this->testResults['checker_1']['1 warning'];
TestChecker1::setTestResult($expected_results, PreCreateEvent::class);
- $session->reload();
- $this->deleteStagedUpdate();
$page->pressButton('Update');
$this->checkForMetaRefresh();
$assert_session->pageTextContains('Ready to update');
@@ -234,13 +233,41 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase {
}
/**
- * Deletes a staged, failed update.
+ * Tests deleting an existing update.
+ *
+ * @todo Add test coverage for differences between stage owner and other users
+ * in https://www.drupal.org/i/3248928.
*/
- private function deleteStagedUpdate(): void {
- $session = $this->getSession();
- $session->getPage()->pressButton('Delete existing update');
- $this->assertSession()->pageTextContains('Staged update deleted');
- $session->reload();
+ public function testDeleteExistingUpdate() {
+ $assert_session = $this->assertSession();
+ $page = $this->getSession()->getPage();
+ $this->setCoreVersion('9.8.0');
+ $this->checkForUpdates();
+
+ $this->drupalGet('/admin/modules/automatic-update');
+ $page->pressButton('Update');
+ $this->checkForMetaRefresh();
+
+ // Confirm we are on the confirmation page.
+ $assert_session->addressEquals('/admin/automatic-update-ready');
+ $assert_session->buttonExists('Continue');
+
+ // Return to the start page.
+ $this->drupalGet('/admin/modules/automatic-update');
+ $assert_session->pageTextContainsOnce('Cannot begin an update because another Composer operation is currently in progress.');
+ $assert_session->buttonNotExists('Update');
+
+ // Delete the existing update.
+ $page->pressButton('Delete existing update');
+ $assert_session->pageTextNotContains('Cannot begin an update because another Composer operation is currently in progress.');
+
+ // Ensure we can start another update after deleting the existing one.
+ $page->pressButton('Update');
+ $this->checkForMetaRefresh();
+
+ // Confirm we are on the confirmation page.
+ $assert_session->addressEquals('/admin/automatic-update-ready');
+ $assert_session->buttonExists('Continue');
}
}
diff --git a/tests/src/Kernel/ReadinessValidation/StagedProjectsValidatorTest.php b/tests/src/Kernel/ReadinessValidation/StagedProjectsValidatorTest.php
index 21d7a1d0b36cb79ef1a254357a54ad9bebe5a9bd..187c091116da0c445d7411c6cb72678658c574af 100644
--- a/tests/src/Kernel/ReadinessValidation/StagedProjectsValidatorTest.php
+++ b/tests/src/Kernel/ReadinessValidation/StagedProjectsValidatorTest.php
@@ -3,6 +3,7 @@
namespace Drupal\Tests\automatic_updates\Kernel\ReadinessValidation;
use Drupal\Core\DependencyInjection\ContainerBuilder;
+use Drupal\package_manager\Event\PreApplyEvent;
use Drupal\package_manager\PathLocator;
use Drupal\package_manager\StageException;
use Drupal\Tests\automatic_updates\Kernel\AutomaticUpdatesKernelTestBase;
@@ -20,6 +21,7 @@ class StagedProjectsValidatorTest extends AutomaticUpdatesKernelTestBase {
protected static $modules = [
'automatic_updates',
'package_manager',
+ 'package_manager_bypass',
];
/**
@@ -47,15 +49,20 @@ class StagedProjectsValidatorTest extends AutomaticUpdatesKernelTestBase {
private function validate(string $active_dir, string $stage_dir): array {
$locator = $this->prophesize(PathLocator::class);
$locator->getActiveDirectory()->willReturn($active_dir);
+ $locator->getProjectRoot()->willReturn($active_dir);
+ $locator->getVendorDirectory()->willReturn($active_dir);
$locator->getStageDirectory()->willReturn($stage_dir);
$this->container->set('package_manager.path_locator', $locator->reveal());
+ $updater = $this->container->get('automatic_updates.updater');
+ $updater->begin(['drupal' => '9.8.1']);
+
// The staged projects validator only runs before staged updates are
// applied. Since the active and stage directories may not exist, we don't
// want to invoke the other stages of the update because they may raise
// errors that are outside of the scope of what we're testing here.
try {
- $this->container->get('automatic_updates.updater')->apply();
+ $updater->apply();
return [];
}
catch (StageException $e) {
@@ -67,11 +74,29 @@ class StagedProjectsValidatorTest extends AutomaticUpdatesKernelTestBase {
* Tests that if an exception is thrown, the event will absorb it.
*/
public function testEventConsumesExceptionResults(): void {
- $results = $this->validate('/fake/active/dir', '/fake/stage/dir');
+ // Prepare a fake site in the virtual file system which contains valid
+ // Composer data.
+ $fixture = __DIR__ . '/../../../fixtures/fake-site';
+ copy("$fixture/composer.json", 'public://composer.json');
+ copy("$fixture/composer.lock", 'public://composer.lock');
+
+ $event_dispatcher = $this->container->get('event_dispatcher');
+ // Disable the disk space validator, since it doesn't work with vfsStream.
+ $disk_space_validator = $this->container->get('package_manager.validator.disk_space');
+ $event_dispatcher->removeSubscriber($disk_space_validator);
+ // Just before the staged changes are applied, delete the composer.json file
+ // to trigger an error. This uses the highest possible priority to guarantee
+ // it runs before any other subscribers.
+ $listener = function () {
+ unlink('public://composer.json');
+ };
+ $event_dispatcher->addListener(PreApplyEvent::class, $listener, PHP_INT_MAX);
+
+ $results = $this->validate('public://', '/fake/stage/dir');
$this->assertCount(1, $results);
$messages = reset($results)->getMessages();
$this->assertCount(1, $messages);
- $this->assertStringContainsString('Composer could not find the config file: /fake/active/dir/composer.json', (string) reset($messages));
+ $this->assertStringContainsString('Composer could not find the config file: public:///composer.json', (string) reset($messages));
}
/**
diff --git a/tests/src/Kernel/UpdaterTest.php b/tests/src/Kernel/UpdaterTest.php
index 93eb21c00c7666c9b0398b06adf1eeed0ebf6d76..1a42cb00d3ee953c6959c6d760b75c37f1495eca 100644
--- a/tests/src/Kernel/UpdaterTest.php
+++ b/tests/src/Kernel/UpdaterTest.php
@@ -3,6 +3,7 @@
namespace Drupal\Tests\automatic_updates\Kernel;
use Drupal\package_manager\PathLocator;
+use Drupal\Tests\user\Traits\UserCreationTrait;
use PhpTuf\ComposerStager\Domain\StagerInterface;
use Prophecy\Argument;
@@ -13,6 +14,8 @@ use Prophecy\Argument;
*/
class UpdaterTest extends AutomaticUpdatesKernelTestBase {
+ use UserCreationTrait;
+
/**
* {@inheritdoc}
*/
@@ -21,14 +24,28 @@ class UpdaterTest extends AutomaticUpdatesKernelTestBase {
'automatic_updates_test',
'package_manager',
'package_manager_bypass',
+ 'system',
+ 'user',
];
+ /**
+ * {@inheritdoc}
+ */
+ protected function setUp(): void {
+ parent::setUp();
+ $this->installEntitySchema('user');
+ }
+
/**
* Tests that correct versions are staged after calling ::begin().
*/
public function testCorrectVersionsStaged() {
$this->setReleaseMetadata(__DIR__ . '/../../fixtures/release-history/drupal.9.8.1-security.xml');
+ // Create a user who will own the stage even after the container is rebuilt.
+ $user = $this->createUser([], NULL, TRUE, ['uid' => 2]);
+ $this->setCurrentUser($user);
+
// Point to a fake site which requires Drupal core via a distribution. The
// lock file should be scanned to determine the core packages, which should
// result in drupal/core-recommended being updated.
@@ -48,8 +65,9 @@ class UpdaterTest extends AutomaticUpdatesKernelTestBase {
$kernel = $this->container->get('kernel');
$kernel->rebuildContainer();
$this->container = $kernel->getContainer();
- // Keep using the mocked path locator.
+ // Keep using the mocked path locator and current user.
$this->container->set('package_manager.path_locator', $locator->reveal());
+ $this->setCurrentUser($user);
// When we call Updater::stage(), the stored project versions should be
// read from state and passed to Composer Stager's Stager service, in the