From 6cc69f45bbf22a6d545b34c955b487c8f71f28fc Mon Sep 17 00:00:00 2001
From: phenaproxima <phenaproxima@205645.no-reply.drupal.org>
Date: Wed, 1 Dec 2021 18:09:50 +0000
Subject: [PATCH] Issue #3252097 by phenaproxima, tedbow: Remove
PathLocator::getStageDirectory()
---
package_manager/package_manager.services.yml | 1 -
package_manager/src/PathLocator.php | 31 +---------------
package_manager/src/Stage.php | 16 +++++++--
.../src/Functional/ExcludedPathsTest.php | 25 ++++++++++---
.../src/Kernel/DiskSpaceValidatorTest.php | 1 -
.../src/Kernel/LockFileValidatorTest.php | 3 --
.../WritableFileSystemValidatorTest.php | 1 -
.../Functional/FileSystemOperationsTest.php | 27 ++++++++++++--
.../StagedProjectsValidatorTest.php | 36 +++++++++++++++++--
tests/src/Kernel/UpdaterTest.php | 1 -
10 files changed, 93 insertions(+), 49 deletions(-)
diff --git a/package_manager/package_manager.services.yml b/package_manager/package_manager.services.yml
index cc7e58d625..803c02b7c6 100644
--- a/package_manager/package_manager.services.yml
+++ b/package_manager/package_manager.services.yml
@@ -77,7 +77,6 @@ services:
package_manager.path_locator:
class: Drupal\package_manager\PathLocator
arguments:
- - '@config.factory'
- '%app.root%'
# Validators.
diff --git a/package_manager/src/PathLocator.php b/package_manager/src/PathLocator.php
index 982022e3e4..8e14b31156 100644
--- a/package_manager/src/PathLocator.php
+++ b/package_manager/src/PathLocator.php
@@ -3,21 +3,12 @@
namespace Drupal\package_manager;
use Composer\Autoload\ClassLoader;
-use Drupal\Component\FileSystem\FileSystem;
-use Drupal\Core\Config\ConfigFactoryInterface;
/**
* Computes file system paths that are needed to stage code changes.
*/
class PathLocator {
- /**
- * The config factory service.
- *
- * @var \Drupal\Core\Config\ConfigFactoryInterface
- */
- protected $configFactory;
-
/**
* The absolute path of the running Drupal code base.
*
@@ -28,13 +19,10 @@ class PathLocator {
/**
* Constructs a PathLocator object.
*
- * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
- * The config factory service.
* @param string $app_root
* The absolute path of the running Drupal code base.
*/
- public function __construct(ConfigFactoryInterface $config_factory, string $app_root) {
- $this->configFactory = $config_factory;
+ public function __construct(string $app_root) {
$this->appRoot = $app_root;
}
@@ -48,23 +36,6 @@ class PathLocator {
return $this->getProjectRoot();
}
- /**
- * Returns the path of the directory where changes should be staged.
- *
- * This directory may be made world-writeable for clean-up, so it should be
- * somewhere that doesn't put the Drupal installation at risk. Each staging
- * area will use a sub-directory with a random name.
- *
- * @return string
- * The absolute path of the directory where changes should be staged.
- */
- public function getStageDirectory(): string {
- // Append the site ID to the directory in order to support parallel test
- // runs, or multiple sites hosted on the same server.
- $site_id = $this->configFactory->get('system.site')->get('uuid');
- return FileSystem::getOsTemporaryDirectory() . DIRECTORY_SEPARATOR . '.package_manager_' . $site_id;
- }
-
/**
* Returns the absolute path of the project root.
*
diff --git a/package_manager/src/Stage.php b/package_manager/src/Stage.php
index 5024089da6..abaa72ef42 100644
--- a/package_manager/src/Stage.php
+++ b/package_manager/src/Stage.php
@@ -2,6 +2,7 @@
namespace Drupal\package_manager;
+use Drupal\Component\FileSystem\FileSystem;
use Drupal\Component\Utility\Crypt;
use Drupal\Core\File\FileSystemInterface;
use Drupal\Core\TempStore\SharedTempStoreFactory;
@@ -281,7 +282,7 @@ class Stage {
$this->dispatch(new PreDestroyEvent($this));
// Delete all directories in parent staging directory.
- $parent_stage_dir = $this->pathLocator->getStageDirectory();
+ $parent_stage_dir = static::getStagingRoot();
if (is_dir($parent_stage_dir)) {
$this->fileSystem->deleteRecursive($parent_stage_dir, function (string $path): void {
$this->fileSystem->chmod($path, 0777);
@@ -438,7 +439,18 @@ class Stage {
if (!$this->lock) {
throw new \LogicException(__METHOD__ . '() cannot be called because the stage has not been created or claimed.');
}
- return $this->pathLocator->getStageDirectory() . DIRECTORY_SEPARATOR . $this->lock[0];
+ return static::getStagingRoot() . DIRECTORY_SEPARATOR . $this->lock[0];
+ }
+
+ /**
+ * Returns the directory where staging areas will be created.
+ *
+ * @return string
+ * The absolute path of the directory containing the staging areas managed
+ * by this class.
+ */
+ protected static function getStagingRoot(): string {
+ return FileSystem::getOsTemporaryDirectory() . DIRECTORY_SEPARATOR . '.package_manager';
}
}
diff --git a/package_manager/tests/src/Functional/ExcludedPathsTest.php b/package_manager/tests/src/Functional/ExcludedPathsTest.php
index f3ccffdc64..f9e53994df 100644
--- a/package_manager/tests/src/Functional/ExcludedPathsTest.php
+++ b/package_manager/tests/src/Functional/ExcludedPathsTest.php
@@ -59,11 +59,9 @@ class ExcludedPathsTest extends BrowserTestBase {
*/
public function testExcludedPaths(): void {
$active_dir = __DIR__ . '/../../fixtures/fake_site';
- $parent_stage_dir = $this->siteDirectory . '/stage';
$path_locator = $this->prophesize(PathLocator::class);
$path_locator->getActiveDirectory()->willReturn($active_dir);
- $path_locator->getStageDirectory()->willReturn($parent_stage_dir);
$site_path = 'sites/example.com';
@@ -92,7 +90,7 @@ class ExcludedPathsTest extends BrowserTestBase {
$property->setAccessible(TRUE);
$property->setValue($subscriber, $database->reveal());
- $stage = new Stage(
+ $stage = new class(
$path_locator->reveal(),
$this->container->get('package_manager.beginner'),
$this->container->get('package_manager.stager'),
@@ -100,8 +98,25 @@ class ExcludedPathsTest extends BrowserTestBase {
$this->container->get('file_system'),
$this->container->get('event_dispatcher'),
$this->container->get('tempstore.shared'),
- );
- $stage_dir = $parent_stage_dir . DIRECTORY_SEPARATOR . $stage->create();
+ ) extends Stage {
+
+ /**
+ * The directory where staging areas will be created.
+ *
+ * @var string
+ */
+ public static $stagingRoot;
+
+ /**
+ * {@inheritdoc}
+ */
+ protected static function getStagingRoot(): string {
+ return static::$stagingRoot;
+ }
+
+ };
+ $stage::$stagingRoot = $this->siteDirectory . '/stage';
+ $stage_dir = $stage::$stagingRoot . DIRECTORY_SEPARATOR . $stage->create();
$this->assertDirectoryExists($stage_dir);
$this->assertDirectoryNotExists("$stage_dir/sites/simpletest");
diff --git a/package_manager/tests/src/Kernel/DiskSpaceValidatorTest.php b/package_manager/tests/src/Kernel/DiskSpaceValidatorTest.php
index bb969e1a71..c4ee293ee0 100644
--- a/package_manager/tests/src/Kernel/DiskSpaceValidatorTest.php
+++ b/package_manager/tests/src/Kernel/DiskSpaceValidatorTest.php
@@ -167,7 +167,6 @@ class DiskSpaceValidatorTest extends PackageManagerKernelTestBase {
$path_locator = $this->prophesize('\Drupal\package_manager\PathLocator');
$path_locator->getProjectRoot()->willReturn('root');
$path_locator->getActiveDirectory()->willReturn('root');
- $path_locator->getStageDirectory()->willReturn('/fake/stage/dir');
$path_locator->getVendorDirectory()->willReturn('vendor');
$this->container->set('package_manager.path_locator', $path_locator->reveal());
diff --git a/package_manager/tests/src/Kernel/LockFileValidatorTest.php b/package_manager/tests/src/Kernel/LockFileValidatorTest.php
index 298c103ad7..62c0142462 100644
--- a/package_manager/tests/src/Kernel/LockFileValidatorTest.php
+++ b/package_manager/tests/src/Kernel/LockFileValidatorTest.php
@@ -26,13 +26,10 @@ class LockFileValidatorTest extends PackageManagerKernelTestBase {
$vendor = vfsStream::newDirectory('vendor');
$this->vfsRoot->addChild($vendor);
- $stage = vfsStream::newDirectory('stage');
- $this->vfsRoot->addChild($stage);
$path_locator = $this->prophesize(PathLocator::class);
$path_locator->getActiveDirectory()->willReturn($this->vfsRoot->url());
$path_locator->getProjectRoot()->willReturn($this->vfsRoot->url());
- $path_locator->getStageDirectory()->willReturn($stage->url());
$path_locator->getWebRoot()->willReturn('');
$path_locator->getVendorDirectory()->willReturn($vendor->url());
$this->container->set('package_manager.path_locator', $path_locator->reveal());
diff --git a/package_manager/tests/src/Kernel/WritableFileSystemValidatorTest.php b/package_manager/tests/src/Kernel/WritableFileSystemValidatorTest.php
index 0b2398d6a1..cae0606c83 100644
--- a/package_manager/tests/src/Kernel/WritableFileSystemValidatorTest.php
+++ b/package_manager/tests/src/Kernel/WritableFileSystemValidatorTest.php
@@ -111,7 +111,6 @@ class WritableFileSystemValidatorTest extends PackageManagerKernelTestBase {
$path_locator = $this->prophesize(PathLocator::class);
$path_locator->getActiveDirectory()->willReturn($root->url());
- $path_locator->getStageDirectory()->willReturn('/fake/stage/dir');
$path_locator->getWebRoot()->willReturn('');
$path_locator->getVendorDirectory()->willReturn($vendor->url());
$this->container->set('package_manager.path_locator', $path_locator->reveal());
diff --git a/tests/src/Functional/FileSystemOperationsTest.php b/tests/src/Functional/FileSystemOperationsTest.php
index 1c29e1fda8..3bb2bdb3f7 100644
--- a/tests/src/Functional/FileSystemOperationsTest.php
+++ b/tests/src/Functional/FileSystemOperationsTest.php
@@ -26,7 +26,7 @@ class FileSystemOperationsTest extends AutomaticUpdatesFunctionalTestBase {
/**
* The updater service under test.
*
- * @var \Drupal\automatic_updates\Updater
+ * @var \Drupal\Tests\automatic_updates\Functional\TestUpdater
*/
private $updater;
@@ -55,11 +55,10 @@ class FileSystemOperationsTest extends AutomaticUpdatesFunctionalTestBase {
$this->siteDirectory,
'stage',
]);
- $locator->getStageDirectory()->willReturn($this->stageDir);
$locator->getProjectRoot()->willReturn($drupal_root);
$locator->getWebRoot()->willReturn('');
- $this->updater = new Updater(
+ $this->updater = new TestUpdater(
$locator->reveal(),
$this->container->get('package_manager.beginner'),
$this->container->get('package_manager.stager'),
@@ -68,6 +67,7 @@ class FileSystemOperationsTest extends AutomaticUpdatesFunctionalTestBase {
$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();
@@ -125,3 +125,24 @@ class FileSystemOperationsTest extends AutomaticUpdatesFunctionalTestBase {
}
}
+
+/**
+ * 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/tests/src/Kernel/ReadinessValidation/StagedProjectsValidatorTest.php b/tests/src/Kernel/ReadinessValidation/StagedProjectsValidatorTest.php
index aa9a25e809..1b21d13605 100644
--- a/tests/src/Kernel/ReadinessValidation/StagedProjectsValidatorTest.php
+++ b/tests/src/Kernel/ReadinessValidation/StagedProjectsValidatorTest.php
@@ -2,6 +2,7 @@
namespace Drupal\Tests\automatic_updates\Kernel\ReadinessValidation;
+use Drupal\automatic_updates\Updater;
use Drupal\Core\DependencyInjection\ContainerBuilder;
use Drupal\package_manager\Event\PreApplyEvent;
use Drupal\package_manager\Exception\StageValidationException;
@@ -26,6 +27,16 @@ class StagedProjectsValidatorTest extends AutomaticUpdatesKernelTestBase {
'package_manager_bypass',
];
+ /**
+ * {@inheritdoc}
+ */
+ public function register(ContainerBuilder $container) {
+ parent::register($container);
+
+ $container->getDefinition('automatic_updates.updater')
+ ->setClass(TestUpdater::class);
+ }
+
/**
* {@inheritdoc}
*/
@@ -62,12 +73,12 @@ class StagedProjectsValidatorTest extends AutomaticUpdatesKernelTestBase {
// subdirectory using the stage ID after it is created below.
$vendor = vfsStream::newDirectory('au_stage');
$this->vfsRoot->addChild($vendor);
- $locator->getStageDirectory()->willReturn($this->vfsRoot->url() . DIRECTORY_SEPARATOR . 'au_stage');
+ TestUpdater::$stagingRoot = $vendor->url();
}
else {
// If we are testing non-existent staging directory we can use the path
// directly.
- $locator->getStageDirectory()->willReturn($stage_dir);
+ TestUpdater::$stagingRoot = $stage_dir;
}
$this->container->set('package_manager.path_locator', $locator->reveal());
@@ -213,3 +224,24 @@ class StagedProjectsValidatorTest extends AutomaticUpdatesKernelTestBase {
}
}
+
+/**
+ * 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/tests/src/Kernel/UpdaterTest.php b/tests/src/Kernel/UpdaterTest.php
index 4d2a172f80..034514a3b9 100644
--- a/tests/src/Kernel/UpdaterTest.php
+++ b/tests/src/Kernel/UpdaterTest.php
@@ -52,7 +52,6 @@ class UpdaterTest extends AutomaticUpdatesKernelTestBase {
$locator->getActiveDirectory()->willReturn($fixture_dir);
$locator->getProjectRoot()->willReturn($fixture_dir);
$locator->getVendorDirectory()->willReturn($fixture_dir);
- $locator->getStageDirectory()->willReturn('/tmp');
$this->container->set('package_manager.path_locator', $locator->reveal());
$id = $this->container->get('automatic_updates.updater')->begin([
--
GitLab