From 2fd5f316fe1c33beb97b70a8f29ab05e7b34f9ba Mon Sep 17 00:00:00 2001 From: phenaproxima <phenaproxima@205645.no-reply.drupal.org> Date: Thu, 11 Aug 2022 18:35:30 +0000 Subject: [PATCH] Issue #3303143 by phenaproxima: Move getStagingRoot() to PathLocator --- .../tests/src/Functional/UpdaterFormTest.php | 6 +- package_manager/package_manager.services.yml | 2 + package_manager/src/PathLocator.php | 47 +++++++++++++++- package_manager/src/Stage.php | 3 +- .../PackageManagerBypassServiceProvider.php | 11 ++-- .../src/PathLocator.php | 21 +++++-- .../Kernel/PackageManagerKernelTestBase.php | 2 +- .../tests/src/Unit/PathLocatorTest.php | 55 +++++++++++++++++++ .../AutomaticUpdatesFunctionalTestBase.php | 2 +- 9 files changed, 132 insertions(+), 17 deletions(-) create mode 100644 package_manager/tests/src/Unit/PathLocatorTest.php diff --git a/automatic_updates_extensions/tests/src/Functional/UpdaterFormTest.php b/automatic_updates_extensions/tests/src/Functional/UpdaterFormTest.php index c37a5a5d73..0e7b82b8ea 100644 --- a/automatic_updates_extensions/tests/src/Functional/UpdaterFormTest.php +++ b/automatic_updates_extensions/tests/src/Functional/UpdaterFormTest.php @@ -80,7 +80,7 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase { $fixture_dir = __DIR__ . '/../../fixtures/two_projects'; Beginner::setFixturePath($fixture_dir); $this->container->get('package_manager.path_locator') - ->setPaths($fixture_dir, $fixture_dir . '/vendor', ''); + ->setPaths($fixture_dir, $fixture_dir . '/vendor', '', NULL); $this->drupalLogin($user); $this->drupalPlaceBlock('local_tasks_block', ['primary' => TRUE]); } @@ -227,7 +227,7 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase { $fixture_dir = __DIR__ . '/../../fixtures/one_project'; Beginner::setFixturePath($fixture_dir); $this->container->get('package_manager.path_locator') - ->setPaths($fixture_dir, $fixture_dir . '/vendor', ''); + ->setPaths($fixture_dir, $fixture_dir . '/vendor', '', NULL); $assert = $this->assertSession(); $user = $this->createUser( [ @@ -246,7 +246,7 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase { $fixture_dir = __DIR__ . '/../../fixtures/no_project'; Beginner::setFixturePath($fixture_dir); $this->container->get('package_manager.path_locator') - ->setPaths($fixture_dir, $fixture_dir . '/vendor', ''); + ->setPaths($fixture_dir, $fixture_dir . '/vendor', '', NULL); $this->getSession()->reload(); $assert->pageTextContains('Updates were found, but they must be performed manually. See the list of available updates for more information.'); $this->assertNoUpdates(); diff --git a/package_manager/package_manager.services.yml b/package_manager/package_manager.services.yml index 669a87c8e5..5e21b98712 100644 --- a/package_manager/package_manager.services.yml +++ b/package_manager/package_manager.services.yml @@ -43,6 +43,8 @@ services: class: Drupal\package_manager\PathLocator arguments: - '%app.root%' + - '@config.factory' + - '@file_system' # Validators. package_manager.validator.composer_executable: diff --git a/package_manager/src/PathLocator.php b/package_manager/src/PathLocator.php index e381ff25c6..723eb1b920 100644 --- a/package_manager/src/PathLocator.php +++ b/package_manager/src/PathLocator.php @@ -3,6 +3,8 @@ namespace Drupal\package_manager; use Composer\Autoload\ClassLoader; +use Drupal\Core\Config\ConfigFactoryInterface; +use Drupal\Core\File\FileSystemInterface; /** * Computes file system paths that are needed to stage code changes. @@ -16,14 +18,42 @@ class PathLocator { */ protected $appRoot; + /** + * The config factory service. + * + * @var \Drupal\Core\Config\ConfigFactoryInterface + */ + protected $configFactory; + + /** + * The file system service. + * + * @var \Drupal\Core\File\FileSystemInterface + */ + protected $fileSystem; + /** * Constructs a PathLocator object. * * @param string $app_root * The absolute path of the running Drupal code base. + * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory + * The config factory service. + * @param \Drupal\Core\File\FileSystemInterface $file_system + * The file system service. */ - public function __construct(string $app_root) { + public function __construct(string $app_root, ConfigFactoryInterface $config_factory = NULL, FileSystemInterface $file_system = NULL) { $this->appRoot = $app_root; + if (empty($config_factory)) { + @trigger_error('Calling ' . __METHOD__ . '() without the $config_factory argument is deprecated in automatic_updates:2.0.1 and will be required before automatic_updates:3.0.0. See https://www.drupal.org/node/3300008.', E_USER_DEPRECATED); + $config_factory = \Drupal::configFactory(); + } + $this->configFactory = $config_factory; + if (empty($file_system)) { + @trigger_error('Calling ' . __METHOD__ . '() without the $file_system argument is deprecated in automatic_updates:2.0.1 and will be required before automatic_updates:3.0.0. See https://www.drupal.org/node/3300008.', E_USER_DEPRECATED); + $file_system = \Drupal::service('file_system'); + } + $this->fileSystem = $file_system; } /** @@ -84,4 +114,19 @@ class PathLocator { return trim($web_root, DIRECTORY_SEPARATOR); } + /** + * Returns the directory where staging areas will be created. + * + * The staging root may be affected by site settings, so stages may wish to + * cache the value returned by this method, to ensure that they use the same + * staging root throughout their life cycle. + * + * @return string + * The absolute path of the directory where staging areas should be created. + */ + public function getStagingRoot(): string { + $site_id = $this->configFactory->get('system.site')->get('uuid'); + return $this->fileSystem->getTempDirectory() . DIRECTORY_SEPARATOR . '.package_manager' . $site_id; + } + } diff --git a/package_manager/src/Stage.php b/package_manager/src/Stage.php index d712457c05..fa6e9ec412 100644 --- a/package_manager/src/Stage.php +++ b/package_manager/src/Stage.php @@ -625,8 +625,7 @@ class Stage { // cycle. $dir = $this->tempStore->get(self::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; + $dir = $this->pathLocator->getStagingRoot(); $this->tempStore->set(self::TEMPSTORE_STAGING_ROOT_KEY, $dir); } return $dir; diff --git a/package_manager/tests/modules/package_manager_bypass/src/PackageManagerBypassServiceProvider.php b/package_manager/tests/modules/package_manager_bypass/src/PackageManagerBypassServiceProvider.php index b526d5242c..029b54b5f3 100644 --- a/package_manager/tests/modules/package_manager_bypass/src/PackageManagerBypassServiceProvider.php +++ b/package_manager/tests/modules/package_manager_bypass/src/PackageManagerBypassServiceProvider.php @@ -19,8 +19,9 @@ class PackageManagerBypassServiceProvider extends ServiceProviderBase { public function alter(ContainerBuilder $container) { parent::alter($container); + $state = new Reference('state'); $arguments = [ - new Reference('state'), + $state, new Reference(Filesystem::class), ]; if (Settings::get('package_manager_bypass_composer_stager', TRUE)) { @@ -34,9 +35,11 @@ class PackageManagerBypassServiceProvider extends ServiceProviderBase { } } - $container->getDefinition('package_manager.path_locator') - ->setClass(PathLocator::class) - ->addArgument($arguments[0]); + $definition = $container->getDefinition('package_manager.path_locator') + ->setClass(PathLocator::class); + $arguments = $definition->getArguments(); + array_unshift($arguments, $state); + $definition->setArguments($arguments); } } diff --git a/package_manager/tests/modules/package_manager_bypass/src/PathLocator.php b/package_manager/tests/modules/package_manager_bypass/src/PathLocator.php index 8d985308b6..869e4b9f37 100644 --- a/package_manager/tests/modules/package_manager_bypass/src/PathLocator.php +++ b/package_manager/tests/modules/package_manager_bypass/src/PathLocator.php @@ -20,13 +20,13 @@ class PathLocator extends BasePathLocator { /** * Constructs a PathLocator object. * - * @param string $app_root - * The Drupal application root. * @param \Drupal\Core\State\StateInterface $state * The state service. + * @param mixed ...$arguments + * Additional arguments to pass to the parent constructor. */ - public function __construct(string $app_root, StateInterface $state) { - parent::__construct($app_root); + public function __construct(StateInterface $state, ...$arguments) { + parent::__construct(...$arguments); $this->state = $state; } @@ -51,6 +51,13 @@ class PathLocator extends BasePathLocator { return $this->state->get(static::class . ' web', parent::getWebRoot()); } + /** + * {@inheritdoc} + */ + public function getStagingRoot(): string { + return $this->state->get(static::class . ' stage', parent::getStagingRoot()); + } + /** * Sets the paths to return. * @@ -61,11 +68,15 @@ class PathLocator extends BasePathLocator { * @param string|null $web_root * The web root, relative to the project root, or NULL to defer to the * parent class. + * @param string|null $staging_root + * The absolute path of the staging root, or NULL to defer to the parent + * class. */ - public function setPaths(?string $project_root, ?string $vendor_dir, ?string $web_root): void { + public function setPaths(?string $project_root, ?string $vendor_dir, ?string $web_root, ?string $staging_root): void { $this->state->set(static::class . ' root', $project_root); $this->state->set(static::class . ' vendor', $vendor_dir); $this->state->set(static::class . ' web', $web_root); + $this->state->set(static::class . ' stage', $staging_root); } } diff --git a/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php b/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php index f4a0f22fd9..3106049198 100644 --- a/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php +++ b/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php @@ -215,7 +215,7 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase { $active_dir = $active_dir->url(); /** @var \Drupal\package_manager_bypass\PathLocator $path_locator */ $path_locator = $this->container->get('package_manager.path_locator'); - $path_locator->setPaths($active_dir, $active_dir . '/vendor', ''); + $path_locator->setPaths($active_dir, $active_dir . '/vendor', '', NULL); // Ensure the active directory will be copied into the virtual staging area. Beginner::setFixturePath($active_dir); diff --git a/package_manager/tests/src/Unit/PathLocatorTest.php b/package_manager/tests/src/Unit/PathLocatorTest.php new file mode 100644 index 0000000000..0c67904263 --- /dev/null +++ b/package_manager/tests/src/Unit/PathLocatorTest.php @@ -0,0 +1,55 @@ +<?php + +namespace Drupal\Tests\package_manager\Unit; + +use Drupal\Core\DependencyInjection\ContainerBuilder; +use Drupal\Core\File\FileSystemInterface; +use Drupal\package_manager\PathLocator; +use Drupal\Tests\UnitTestCase; + +/** + * @coversDefaultClass \Drupal\package_manager\PathLocator + * + * @group package_manager + */ +class PathLocatorTest extends UnitTestCase { + + /** + * @covers ::getStagingRoot + */ + public function testStagingRoot(): void { + $config_factory = $this->getConfigFactoryStub([ + 'system.site' => [ + 'uuid' => 'my_site_id', + ], + ]); + $file_system = $this->prophesize(FileSystemInterface::class); + $file_system->getTempDirectory()->willReturn('/path/to/temp'); + + $path_locator = new PathLocator( + '/path/to/drupal', + $config_factory, + $file_system->reveal() + ); + $this->assertSame('/path/to/temp/.package_managermy_site_id', $path_locator->getStagingRoot()); + } + + /** + * Tests that deprecations are raised for missing constructor arguments. + * + * @group legacy + */ + public function testConstructorDeprecations(): void { + $container = new ContainerBuilder(); + $container->set('config.factory', $this->getConfigFactoryStub()); + $container->set('file_system', $this->createMock(FileSystemInterface::class)); + \Drupal::setContainer($container); + + $this->expectDeprecation('Calling ' . PathLocator::class . '::__construct() without the $config_factory argument is deprecated in automatic_updates:2.0.1 and will be required before automatic_updates:3.0.0. See https://www.drupal.org/node/3300008.'); + new PathLocator('/path/to/drupal', NULL, $container->get('file_system')); + + $this->expectDeprecation('Calling ' . PathLocator::class . '::__construct() without the $file_system argument is deprecated in automatic_updates:2.0.1 and will be required before automatic_updates:3.0.0. See https://www.drupal.org/node/3300008.'); + new PathLocator('/path/to/drupal', $container->get('config.factory')); + } + +} diff --git a/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php b/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php index 9f056d460b..1d352dec7a 100644 --- a/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php +++ b/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php @@ -54,7 +54,7 @@ abstract class AutomaticUpdatesFunctionalTestBase extends BrowserTestBase { Beginner::setFixturePath($fixture_dir); $this->container->get('package_manager.path_locator') - ->setPaths($fixture_dir, $fixture_dir . '/vendor', ''); + ->setPaths($fixture_dir, $fixture_dir . '/vendor', '', NULL); } /** -- GitLab