From caa09ddf6d112ca4b2db0e526f9892614da4b62a Mon Sep 17 00:00:00 2001 From: phenaproxima <phenaproxima@205645.no-reply.drupal.org> Date: Tue, 3 May 2022 17:22:24 +0000 Subject: [PATCH] Issue #3278411 by phenaproxima, tedbow: Always use FileSystemInterface::getTempDirectory() to get the temporary directory --- package_manager/src/Stage.php | 23 ++++++++++++++++--- .../tests/src/Kernel/StageTest.php | 18 +++++++++++---- tests/src/Functional/UpdaterFormTest.php | 7 +++--- 3 files changed, 37 insertions(+), 11 deletions(-) diff --git a/package_manager/src/Stage.php b/package_manager/src/Stage.php index 516d08f9d1..6094a71552 100644 --- a/package_manager/src/Stage.php +++ b/package_manager/src/Stage.php @@ -3,7 +3,6 @@ namespace Drupal\package_manager; use Drupal\Component\Datetime\TimeInterface; -use Drupal\Component\FileSystem\FileSystem; use Drupal\Component\Utility\Crypt; use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\File\Exception\FileException; @@ -67,6 +66,15 @@ class Stage { */ protected const TEMPSTORE_METADATA_KEY = 'metadata'; + /** + * The tempstore key under which to store the path of the staging root. + * + * @var string + * + * @see ::getStagingRoot() + */ + protected const TEMPSTORE_STAGING_ROOT_KEY = 'staging_root'; + /** * The tempstore key under which to store the time that ::apply() was called. * @@ -400,6 +408,7 @@ class Stage { protected function markAsAvailable(): void { $this->tempStore->delete(static::TEMPSTORE_METADATA_KEY); $this->tempStore->delete(static::TEMPSTORE_LOCK_KEY); + $this->tempStore->delete(static::TEMPSTORE_STAGING_ROOT_KEY); $this->lock = NULL; } @@ -552,8 +561,16 @@ class Stage { * by this class. */ protected function getStagingRoot(): string { - $site_id = $this->configFactory->get('system.site')->get('uuid'); - return FileSystem::getOsTemporaryDirectory() . DIRECTORY_SEPARATOR . '.package_manager' . $site_id; + // Since the staging root can depend on site settings, store it so that + // things won't break if the settings change during this stage's life + // cycle. + $dir = $this->tempStore->get(static::TEMPSTORE_STAGING_ROOT_KEY); + if (empty($dir)) { + $site_id = $this->configFactory->get('system.site')->get('uuid'); + $dir = $this->fileSystem->getTempDirectory() . DIRECTORY_SEPARATOR . '.package_manager' . $site_id; + $this->tempStore->set(static::TEMPSTORE_STAGING_ROOT_KEY, $dir); + } + return $dir; } /** diff --git a/package_manager/tests/src/Kernel/StageTest.php b/package_manager/tests/src/Kernel/StageTest.php index 5d029db83e..3f0ba3d0cf 100644 --- a/package_manager/tests/src/Kernel/StageTest.php +++ b/package_manager/tests/src/Kernel/StageTest.php @@ -3,8 +3,10 @@ namespace Drupal\Tests\package_manager\Kernel; use Drupal\Component\Datetime\Time; +use Drupal\Component\FileSystem\FileSystem; use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\Core\Extension\ModuleUninstallValidatorException; +use Drupal\Core\Site\Settings; use Drupal\package_manager\Event\PostApplyEvent; use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Event\StageEvent; @@ -63,15 +65,21 @@ class StageTest extends PackageManagerKernelTestBase { $stage = $this->createStage(); $id = $stage->create(); - $this->assertStringEndsWith("/.package_manager$site_id/$id", $stage->getStageDirectory()); + // If the file_temp_path setting is empty, the stage directory should be + // created in the OS's temporary directory. + $this->assertEmpty(Settings::get('file_temp_path')); + $expected_dir = FileSystem::getOsTemporaryDirectory() . "/.package_manager$site_id/$id"; + $this->assertSame($expected_dir, $stage->getStageDirectory()); + // If the file_temp_path setting is changed, the existing stage shouldn't be + // affected... + $this->setSetting('file_temp_path', '/junk/drawer'); + $this->assertSame($expected_dir, $stage->getStageDirectory()); $stage->destroy(); - + // ...but a new stage should be. $stage = $this->createStage(); $another_id = $stage->create(); - // The new stage ID should be unique, but the parent directory should be - // unchanged. $this->assertNotSame($id, $another_id); - $this->assertStringEndsWith("/.package_manager$site_id/$another_id", $stage->getStageDirectory()); + $this->assertSame("/junk/drawer/.package_manager$site_id/$another_id", $stage->getStageDirectory()); } /** diff --git a/tests/src/Functional/UpdaterFormTest.php b/tests/src/Functional/UpdaterFormTest.php index ff2bff414b..01a4346fa2 100644 --- a/tests/src/Functional/UpdaterFormTest.php +++ b/tests/src/Functional/UpdaterFormTest.php @@ -4,7 +4,6 @@ namespace Drupal\Tests\automatic_updates\Functional; use Drupal\automatic_updates\Event\ReadinessCheckEvent; use Drupal\automatic_updates_test\Datetime\TestTime; -use Drupal\Component\FileSystem\FileSystem; use Drupal\package_manager\Event\PostRequireEvent; use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Event\PreCreateEvent; @@ -511,9 +510,11 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase { // Confirm if the staged directory is deleted without using destroy(), then // an error message will be displayed on the page. // @see \Drupal\package_manager\Stage::getStagingRoot() - $dir = FileSystem::getOsTemporaryDirectory() . '/.package_manager' . $this->config('system.site')->get('uuid'); + /** @var \Drupal\Core\File\FileSystemInterface $file_system */ + $file_system = $this->container->get('file_system'); + $dir = $file_system->getTempDirectory() . '/.package_manager' . $this->config('system.site')->get('uuid'); $this->assertDirectoryExists($dir); - $this->container->get('file_system')->deleteRecursive($dir); + $file_system->deleteRecursive($dir); $this->getSession()->reload(); $assert_session = $this->assertSession(); $error_message = 'There was an error loading the pending update. Press the Cancel update button to start over.'; -- GitLab