diff --git a/core/misc/cspell/dictionary.txt b/core/misc/cspell/dictionary.txt
index b86c6c02771ed31aee16ccebf285d6f790606d23..cdd5e311cba2caa025fc31357b03e1c0cf3c8662 100644
--- a/core/misc/cspell/dictionary.txt
+++ b/core/misc/cspell/dictionary.txt
@@ -1597,6 +1597,7 @@ untrusted
unvalidated
unversioned
unwrapper
+unwritable
upcasted
upcasting
updateprogress
diff --git a/core/modules/auto_updates/tests/src/Functional/FileSystemOperationsTest.php b/core/modules/auto_updates/tests/src/Functional/FileSystemOperationsTest.php
deleted file mode 100644
index dcf116bd827b8fcbee21792a1e0cc0052992cab4..0000000000000000000000000000000000000000
--- a/core/modules/auto_updates/tests/src/Functional/FileSystemOperationsTest.php
+++ /dev/null
@@ -1,148 +0,0 @@
-<?php
-
-namespace Drupal\Tests\auto_updates\Functional;
-
-use Drupal\auto_updates\Updater;
-use Drupal\Core\Site\Settings;
-use Drupal\package_manager\PathLocator;
-
-/**
- * Tests handling of files and directories during an update.
- *
- * @group auto_updates
- */
-class FileSystemOperationsTest extends AutoUpdatesFunctionalTestBase {
-
- /**
- * {@inheritdoc}
- */
- protected static $modules = ['auto_updates_test'];
-
- /**
- * {@inheritdoc}
- */
- protected $defaultTheme = 'stark';
-
- /**
- * The updater service under test.
- *
- * @var \Drupal\Tests\auto_updates\Functional\TestUpdater
- */
- private $updater;
-
- /**
- * The full path of the staging directory.
- *
- * @var string
- */
- protected $stageDir;
-
- /**
- * {@inheritdoc}
- */
- protected function setUp(): void {
- parent::setUp();
-
- // Create a mocked path locator that uses the fake site fixture as its
- // active directory, and has a staging area within the site directory for
- // this test.
- $drupal_root = $this->getDrupalRoot();
- /** @var \Drupal\package_manager\PathLocator|\Prophecy\Prophecy\ObjectProphecy $locator */
- $locator = $this->prophesize(PathLocator::class);
- $locator->getActiveDirectory()->willReturn(__DIR__ . '/../../fixtures/fake-site');
- $this->stageDir = implode(DIRECTORY_SEPARATOR, [
- $drupal_root,
- $this->siteDirectory,
- 'stage',
- ]);
- $locator->getProjectRoot()->willReturn($drupal_root);
- $locator->getWebRoot()->willReturn('');
-
- $this->updater = new TestUpdater(
- $locator->reveal(),
- $this->container->get('package_manager.beginner'),
- $this->container->get('package_manager.stager'),
- $this->container->get('package_manager.committer'),
- $this->container->get('file_system'),
- $this->container->get('event_dispatcher'),
- $this->container->get('tempstore.shared')
- );
- $this->updater::$stagingRoot = $this->stageDir;
-
- // Use the public and private files directories in the fake site fixture.
- $settings = Settings::getAll();
- $settings['file_public_path'] = 'files/public';
- $settings['file_private_path'] = 'files/private';
- new Settings($settings);
-
- // Updater::begin() will trigger update validators, such as
- // \Drupal\auto_updates\Validator\UpdateVersionValidator, that need to
- // fetch release metadata. We need to ensure that those HTTP request(s)
- // succeed, so set them up to point to our fake release metadata.
- $this->setReleaseMetadata(__DIR__ . '/../../fixtures/release-history/drupal.9.8.1-security.xml');
- $this->setCoreVersion('9.8.0');
-
- $this->drupalLogin($this->rootUser);
- $this->checkForUpdates();
- $this->drupalLogout();
- }
-
- /**
- * Tests that certain files and directories are not staged.
- *
- * @covers \Drupal\auto_updates\Updater::getExclusions
- */
- public function testExclusions(): void {
- $stage_id = $this->updater->begin(['drupal' => '9.8.1']);
- $this->assertFileDoesNotExist("$this->stageDir/$stage_id/sites/default/settings.php");
- $this->assertFileDoesNotExist("$this->stageDir/$stage_id/sites/default/settings.local.php");
- $this->assertFileDoesNotExist("$this->stageDir/$stage_id/sites/default/services.yml");
- // A file in sites/default, that isn't one of the site-specific settings
- // files, should be staged.
- $this->assertFileExists("$this->stageDir/$stage_id/sites/default/staged.txt");
- $this->assertDirectoryDoesNotExist("$this->stageDir/$stage_id/sites/simpletest");
- $this->assertDirectoryDoesNotExist("$this->stageDir/$stage_id/files/public");
- $this->assertDirectoryDoesNotExist("$this->stageDir/$stage_id/files/private");
- // A file that's in the general files directory, but not in the public or
- // private directories, should be staged.
- $this->assertFileExists("$this->stageDir/$stage_id/files/staged.txt");
- }
-
- /**
- * Tests that the staging directory is properly cleaned up.
- *
- * @covers \Drupal\auto_updates\Cleaner
- */
- public function testClean(): void {
- $stage_id = $this->updater->begin(['drupal' => '9.8.1']);
- // Make the staged site directory read-only, so we can test that it will be
- // made writable on clean-up.
- $this->assertTrue(chmod("$this->stageDir/$stage_id/sites/default", 0400));
- $this->assertIsNotWritable("$this->stageDir/$stage_id/sites/default/staged.txt");
- // If the site directory is not writable, this will throw an exception.
- $this->updater->destroy();
- $this->assertDirectoryDoesNotExist($this->stageDir);
- }
-
-}
-
-/**
- * A test-only version of the updater.
- */
-class TestUpdater extends Updater {
-
- /**
- * The directory where staging areas will be created.
- *
- * @var string
- */
- public static $stagingRoot;
-
- /**
- * {@inheritdoc}
- */
- protected static function getStagingRoot(): string {
- return static::$stagingRoot ?: parent::getStagingRoot();
- }
-
-}
diff --git a/core/modules/package_manager/src/Stage.php b/core/modules/package_manager/src/Stage.php
index 6d4678383085849e26e49e7c0fb4d27c2beb4545..711da3a019d7204793ef8712db41ac73e5f1a488 100644
--- a/core/modules/package_manager/src/Stage.php
+++ b/core/modules/package_manager/src/Stage.php
@@ -4,6 +4,7 @@
use Drupal\Component\FileSystem\FileSystem;
use Drupal\Component\Utility\Crypt;
+use Drupal\Core\File\Exception\FileException;
use Drupal\Core\File\FileSystemInterface;
use Drupal\Core\TempStore\SharedTempStoreFactory;
use Drupal\package_manager\Event\PostApplyEvent;
@@ -290,12 +291,17 @@ public function destroy(bool $force = FALSE): void {
// Delete all directories in parent staging directory.
$parent_stage_dir = static::getStagingRoot();
if (is_dir($parent_stage_dir)) {
- // @todo Ensure that even if attempting to delete this directory throws an
- // exception the stage is still marked as available in
- // https://www.drupal.org/i/3258048.
- $this->fileSystem->deleteRecursive($parent_stage_dir, function (string $path): void {
- $this->fileSystem->chmod($path, 0777);
- });
+ try {
+ $this->fileSystem->deleteRecursive($parent_stage_dir, function (string $path): void {
+ $this->fileSystem->chmod($path, 0777);
+ });
+ }
+ catch (FileException $e) {
+ // Deliberately swallow the exception so that the stage will be marked
+ // as available and the post-destroy event will be fired, even if the
+ // staging area can't actually be deleted. The file system service logs
+ // the exception, so we don't need to do anything else here.
+ }
}
$this->markAsAvailable();
$this->dispatch(new PostDestroyEvent($this));
diff --git a/core/modules/package_manager/tests/modules/package_manager_bypass/src/PackageManagerBypassServiceProvider.php b/core/modules/package_manager/tests/modules/package_manager_bypass/src/PackageManagerBypassServiceProvider.php
index 3394cf4e159b39d300273529e119dc94929346c3..3f7bf47d387368d0b1fbd280f46d62929dfd35ec 100644
--- a/core/modules/package_manager/tests/modules/package_manager_bypass/src/PackageManagerBypassServiceProvider.php
+++ b/core/modules/package_manager/tests/modules/package_manager_bypass/src/PackageManagerBypassServiceProvider.php
@@ -4,7 +4,6 @@
use Drupal\Core\DependencyInjection\ContainerBuilder;
use Drupal\Core\DependencyInjection\ServiceProviderBase;
-use Drupal\Core\Site\Settings;
use Symfony\Component\DependencyInjection\Reference;
/**
@@ -18,30 +17,19 @@ class PackageManagerBypassServiceProvider extends ServiceProviderBase {
public function alter(ContainerBuilder $container) {
parent::alter($container);
- // By default, bypass the Composer Stager library. This can be disabled for
- // tests that want to use the real library, but only need to disable
- // validators.
- if (Settings::get('package_manager_bypass_stager', TRUE)) {
- $container->getDefinition('package_manager.beginner')
- ->setClass(Beginner::class);
- $container->getDefinition('package_manager.stager')
- ->setClass(Stager::class);
-
- $container->register('package_manager_bypass.committer')
- ->setClass(Committer::class)
- ->setPublic(FALSE)
- ->setDecoratedService('package_manager.committer')
- ->setArguments([
- new Reference('package_manager_bypass.committer.inner'),
- ])
- ->setProperty('_serviceId', 'package_manager.committer');
- }
-
- // Allow functional tests to disable specific validators as necessary.
- // Kernel tests can override the ::register() method and modify the
- // container directly.
- $validators = Settings::get('package_manager_bypass_validators', []);
- array_walk($validators, [$container, 'removeDefinition']);
+ $container->getDefinition('package_manager.beginner')
+ ->setClass(Beginner::class);
+ $container->getDefinition('package_manager.stager')
+ ->setClass(Stager::class);
+
+ $container->register('package_manager_bypass.committer')
+ ->setClass(Committer::class)
+ ->setPublic(FALSE)
+ ->setDecoratedService('package_manager.committer')
+ ->setArguments([
+ new Reference('package_manager_bypass.committer.inner'),
+ ])
+ ->setProperty('_serviceId', 'package_manager.committer');
}
}
diff --git a/core/modules/package_manager/tests/src/Kernel/ExcludedPathsTest.php b/core/modules/package_manager/tests/src/Kernel/ExcludedPathsTest.php
index 776c0ff9a0b1737a4c9d78f5ebcbd3528ea682ec..33dd8f5e169025836ddd8bd21efcb2c9fbfc5428 100644
--- a/core/modules/package_manager/tests/src/Kernel/ExcludedPathsTest.php
+++ b/core/modules/package_manager/tests/src/Kernel/ExcludedPathsTest.php
@@ -26,18 +26,17 @@ class ExcludedPathsTest extends PackageManagerKernelTestBase {
protected function setUp(): void {
parent::setUp();
- // Normally, package_manager_bypass will disable all the actual staging
- // operations. In this case, we want to perform them so that we can be sure
- // that files are staged as expected.
- $this->setSetting('package_manager_bypass_stager', FALSE);
// The private stream wrapper is only registered if this setting is set.
// @see \Drupal\Core\CoreServiceProvider::register()
$this->setSetting('file_private_path', 'private');
-
- // Rebuild the container to make the new settings take effect.
- $kernel = $this->container->get('kernel');
- $kernel->rebuildContainer();
- $this->container = $kernel->getContainer();
+ // In this test, we want to perform the actual staging operations so that we
+ // can be sure that files are staged as expected. This will also rebuild
+ // the container, enabling the private stream wrapper.
+ $this->container->get('module_installer')->uninstall([
+ 'package_manager_bypass',
+ ]);
+ // Ensure we have an up-to-date container.
+ $this->container = $this->container->get('kernel')->getContainer();
// Mock a SQLite database connection so we can test that the subscriber will
// exclude the database files.
diff --git a/core/modules/package_manager/tests/src/Kernel/StageOwnershipTest.php b/core/modules/package_manager/tests/src/Kernel/StageOwnershipTest.php
index 99339a969614da6e496a947e2f282abb6ba5779b..a5c76c7b7af7abe78f3865dd6915770b29ae99a6 100644
--- a/core/modules/package_manager/tests/src/Kernel/StageOwnershipTest.php
+++ b/core/modules/package_manager/tests/src/Kernel/StageOwnershipTest.php
@@ -2,11 +2,16 @@
namespace Drupal\Tests\package_manager\Kernel;
+use Drupal\Core\File\FileSystem;
+use Drupal\Core\Logger\RfcLogLevel;
+use Drupal\package_manager\Event\PostDestroyEvent;
use Drupal\package_manager\Event\PreCreateEvent;
use Drupal\package_manager\Exception\StageException;
use Drupal\package_manager\Exception\StageOwnershipException;
use Drupal\package_manager_test_validation\TestSubscriber;
use Drupal\Tests\user\Traits\UserCreationTrait;
+use Prophecy\Argument;
+use Psr\Log\LoggerInterface;
/**
* Tests that ownership of the stage is enforced.
@@ -32,6 +37,7 @@ class StageOwnershipTest extends PackageManagerKernelTestBase {
protected function setUp(): void {
parent::setUp();
$this->installSchema('system', ['sequences']);
+ $this->installSchema('user', ['users_data']);
$this->installEntitySchema('user');
$this->registerPostUpdateFunctions();
}
@@ -221,4 +227,70 @@ public function testForceDestroy(): void {
$not_owned->destroy(TRUE);
}
+ /**
+ * Tests that the stage is available if ::destroy() has a file system error.
+ */
+ public function testStageDestroyedWithFileSystemError(): void {
+ // Enable the Composer Stager library, since we will actually want to create
+ // the stage directory.
+ $this->container->get('module_installer')->uninstall([
+ 'package_manager_bypass',
+ ]);
+ // Ensure we have an up-to-date container.
+ $this->container = $this->container->get('kernel')->getContainer();
+ $this->createTestProject();
+
+ $logger_channel = $this->container->get('logger.channel.file');
+ $arguments = [
+ $this->container->get('stream_wrapper_manager'),
+ $this->container->get('settings'),
+ $logger_channel,
+ ];
+ $this->container->set('file_system', new class (...$arguments) extends FileSystem {
+
+ /**
+ * {@inheritdoc}
+ */
+ public function chmod($uri, $mode = NULL) {
+ // Normally, the stage will call this method as it tries to make
+ // everything in the staging area writable so it can be deleted. We
+ // don't wan't to do that in this test, since we're specifically testing
+ // what happens when we try to delete a staging area with unwritable
+ // files.
+ }
+
+ });
+
+ $stage = $this->createStage();
+ $this->assertTrue($stage->isAvailable());
+ $stage->create();
+ $this->assertFalse($stage->isAvailable());
+
+ // Make the stage directory unwritable, which should prevent files in it
+ // from being deleted.
+ $dir = $stage->getStageDirectory();
+ chmod($dir, 0400);
+ $this->assertDirectoryIsNotWritable($dir);
+
+ // Mock a logger to prove that a file system error was raised while trying
+ // to delete the stage directory.
+ $logger = $this->prophesize(LoggerInterface::class);
+ $logger->log(
+ RfcLogLevel::ERROR,
+ "Failed to unlink file '%path'.",
+ Argument::withEntry('%path', "$dir/composer.json")
+ )->shouldBeCalled();
+ $logger_channel->addLogger($logger->reveal());
+
+ // Listen to the post-destroy event so we can confirm that it was fired, and
+ // the stage was made available, despite the file system error.
+ $stage_available = NULL;
+ $this->container->get('event_dispatcher')
+ ->addListener(PostDestroyEvent::class, function (PostDestroyEvent $event) use (&$stage_available): void {
+ $stage_available = $event->getStage()->isAvailable();
+ });
+ $stage->destroy();
+ $this->assertTrue($stage_available);
+ }
+
}