Skip to content
Snippets Groups Projects
Commit 67bc9def authored by Ted Bowman's avatar Ted Bowman
Browse files

Issue #3227122 by tedbow, phenaproxima: Create a lock system to ensure only 1...

Issue #3227122 by tedbow, phenaproxima: Create a lock system to ensure only 1 update can be attempted at a time
parent 39ecb2e5
No related branches found
No related tags found
No related merge requests found
Showing
with 404 additions and 43 deletions
......@@ -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:
......
......@@ -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.');
}
}
}
......@@ -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();
......
......@@ -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);
}
}
/**
......
......@@ -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.
*/
......
......@@ -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')
);
}
......
<?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);
}
}
......@@ -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']),
......
......@@ -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'),
......
......@@ -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.
*
......
{
"packages": [
{
"name": "drupal/core",
"version": "9.8.0",
"type": "drupal-core"
},
{
"name": "drupal/test_module",
"version": "1.3.0",
......
{
"packages": [
{
"name": "drupal/core",
"version": "9.8.1",
"type": "drupal-core"
},
{
"name": "drupal/test_module",
"version": "1.3.0",
......
{
"packages": [
{
"name": "drupal/core",
"version": "9.8.0",
"type": "drupal-core"
},
{
"name": "drupal/test_module",
"version": "1.3.0",
......
{
"packages": [
{
"name": "drupal/core",
"version": "9.8.1",
"type": "drupal-core"
},
{
"name": "drupal/test_module",
"version": "1.3.0",
......
{
"packages": [
{
"name": "drupal/core",
"version": "9.8.0",
"type": "drupal-core"
},
{
"name": "drupal/test_theme",
"version": "1.3.0",
......
{
"packages": [
{
"name": "drupal/core",
"version": "9.8.1",
"type": "drupal-core"
},
{
"name": "drupal/test_module2",
"version": "1.3.1",
......
{
"packages": [
{
"name": "drupal/core",
"version": "9.8.0",
"type": "drupal-core"
},
{
"name": "drupal/test_module",
"version": "1.3.0",
......
{
"packages": [
{
"name": "drupal/core",
"version": "9.8.1",
"type": "drupal-core"
},
{
"name": "drupal/test_module",
"version": "1.3.1",
......
......@@ -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.
......
<?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');
}
}
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment