From e8d5ee442ac1035bc28a1a19f35bae32a76ccd95 Mon Sep 17 00:00:00 2001 From: Travis Carden <42520-TravisCarden@users.noreply.drupalcode.org> Date: Fri, 25 Aug 2023 19:59:26 +0000 Subject: [PATCH] Issue #3382942: Eliminate dependencies on Composer Stager internals --- package_manager/package_manager.services.yml | 2 - package_manager/src/FileSyncerFactory.php | 34 +++++--------- .../src/PackageManagerServiceProvider.php | 6 +++ .../src/Kernel/ComposerInspectorTest.php | 7 ++- .../src/Kernel/FileSyncerFactoryTest.php | 8 ++-- .../Kernel/PackageManagerKernelTestBase.php | 2 + .../tests/src/Kernel/RsyncValidatorTest.php | 6 ++- .../tests/src/Kernel/ServicesTest.php | 3 ++ .../tests/src/Kernel/StageBaseTest.php | 5 +- .../AutomaticUpdatesFunctionalTestBase.php | 2 + .../ComposerStagerOperationFailureTest.php | 3 +- tests/src/Kernel/ConsoleUpdateStageTest.php | 3 +- tests/src/Kernel/UpdateStageTest.php | 3 +- tests/src/Traits/ComposerStagerTestTrait.php | 47 +++++++++++++++++++ 14 files changed, 88 insertions(+), 43 deletions(-) create mode 100644 tests/src/Traits/ComposerStagerTestTrait.php diff --git a/package_manager/package_manager.services.yml b/package_manager/package_manager.services.yml index 23d28fe557..bc657dd3cd 100644 --- a/package_manager/package_manager.services.yml +++ b/package_manager/package_manager.services.yml @@ -14,8 +14,6 @@ services: public: false Drupal\package_manager\ExecutableFinder: public: false - Drupal\package_manager\FileSyncerFactory: - public: false Drupal\package_manager\TranslatableStringFactory: public: false PhpTuf\ComposerStager\API\Finder\Service\ExecutableFinderInterface: diff --git a/package_manager/src/FileSyncerFactory.php b/package_manager/src/FileSyncerFactory.php index cd7beee881..9ba699e74b 100644 --- a/package_manager/src/FileSyncerFactory.php +++ b/package_manager/src/FileSyncerFactory.php @@ -5,11 +5,10 @@ declare(strict_types = 1); namespace Drupal\package_manager; use Drupal\Core\Config\ConfigFactoryInterface; +use PhpTuf\ComposerStager\API\FileSyncer\Factory\FileSyncerFactoryInterface; use PhpTuf\ComposerStager\API\FileSyncer\Service\FileSyncerInterface; -use PhpTuf\ComposerStager\Internal\FileSyncer\Factory\FileSyncerFactory as StagerFileSyncerFactory; -use PhpTuf\ComposerStager\Internal\FileSyncer\Service\PhpFileSyncer; -use PhpTuf\ComposerStager\Internal\FileSyncer\Service\RsyncFileSyncer; -use Symfony\Component\Process\ExecutableFinder; +use PhpTuf\ComposerStager\API\FileSyncer\Service\PhpFileSyncerInterface; +use PhpTuf\ComposerStager\API\FileSyncer\Service\RsyncFileSyncerInterface; /** * A file syncer factory which creates a file syncer according to configuration. @@ -22,32 +21,23 @@ use Symfony\Component\Process\ExecutableFinder; final class FileSyncerFactory { /** - * The decorated file syncer factory. + * Constructs a FileSyncerFactory object. * - * @var \PhpTuf\ComposerStager\Internal\FileSyncer\Factory\FileSyncerFactory - */ - private $decorated; - - /** - * Constructs a FileCopierFactory object. - * - * @param \Symfony\Component\Process\ExecutableFinder $executable_finder - * The Symfony executable finder. - * @param \PhpTuf\ComposerStager\Internal\FileSyncer\Service\PhpFileSyncer $phpFileSyncer + * @param \PhpTuf\ComposerStager\API\FileSyncer\Factory\FileSyncerFactoryInterface $decorated + * The decorated file syncer factory. + * @param \PhpTuf\ComposerStager\API\FileSyncer\Service\PhpFileSyncerInterface $phpFileSyncer * The PHP file syncer service. - * @param \PhpTuf\ComposerStager\Internal\FileSyncer\Service\RsyncFileSyncer $rsyncFileSyncer + * @param \PhpTuf\ComposerStager\API\FileSyncer\Service\RsyncFileSyncerInterface $rsyncFileSyncer * The rsync file syncer service. * @param \Drupal\Core\Config\ConfigFactoryInterface $configFactory * The config factory service. */ public function __construct( - ExecutableFinder $executable_finder, - private readonly PhpFileSyncer $phpFileSyncer, - private readonly RsyncFileSyncer $rsyncFileSyncer, + private readonly FileSyncerFactoryInterface $decorated, + private readonly PhpFileSyncerInterface $phpFileSyncer, + private readonly RsyncFileSyncerInterface $rsyncFileSyncer, private readonly ConfigFactoryInterface $configFactory, - ) { - $this->decorated = new StagerFileSyncerFactory($executable_finder, $phpFileSyncer, $rsyncFileSyncer); - } + ) {} /** * {@inheritdoc} diff --git a/package_manager/src/PackageManagerServiceProvider.php b/package_manager/src/PackageManagerServiceProvider.php index 3caf33e14a..162a745f6b 100644 --- a/package_manager/src/PackageManagerServiceProvider.php +++ b/package_manager/src/PackageManagerServiceProvider.php @@ -7,6 +7,7 @@ namespace Drupal\package_manager; use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\Core\DependencyInjection\ServiceProviderBase; use PhpTuf\ComposerStager\API\Core\BeginnerInterface; +use PhpTuf\ComposerStager\API\FileSyncer\Factory\FileSyncerFactoryInterface; use PhpTuf\ComposerStager\API\Precondition\Service\NoSymlinksPointToADirectoryInterface; /** @@ -124,6 +125,11 @@ final class PackageManagerServiceProvider extends ServiceProviderBase { ->setPublic(FALSE) ->setAutowired(TRUE) ->setDecoratedService(NoSymlinksPointToADirectoryInterface::class); + + $container->register(FileSyncerFactory::class) + ->setPublic(FALSE) + ->setAutowired(TRUE) + ->setDecoratedService(FileSyncerFactoryInterface::class); } } diff --git a/package_manager/tests/src/Kernel/ComposerInspectorTest.php b/package_manager/tests/src/Kernel/ComposerInspectorTest.php index ee3106a38e..4818f37db4 100644 --- a/package_manager/tests/src/Kernel/ComposerInspectorTest.php +++ b/package_manager/tests/src/Kernel/ComposerInspectorTest.php @@ -16,10 +16,9 @@ use Drupal\Tests\package_manager\Traits\InstalledPackagesListTrait; use Drupal\package_manager\PathLocator; use PhpTuf\ComposerStager\API\Exception\PreconditionException; use PhpTuf\ComposerStager\API\Exception\RuntimeException; +use PhpTuf\ComposerStager\API\Path\Factory\PathFactoryInterface; use PhpTuf\ComposerStager\API\Precondition\Service\ComposerIsAvailableInterface; use PhpTuf\ComposerStager\API\Process\Service\ComposerProcessRunnerInterface; -use PhpTuf\ComposerStager\Internal\Path\Factory\PathFactory; -use PhpTuf\ComposerStager\Internal\Translation\Value\TranslatableMessage; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; @@ -155,7 +154,7 @@ class ComposerInspectorTest extends PackageManagerKernelTestBase { $mocked_precondition = $precondition->reveal(); $this->container->set(ComposerIsAvailableInterface::class, $mocked_precondition); - $message = new TranslatableMessage("Well, that didn't work."); + $message = $this->createComposeStagerMessage("Well, that didn't work."); $precondition->assertIsFulfilled(Argument::cetera()) ->willThrow(new PreconditionException($mocked_precondition, $message)) // The result of the precondition is statically cached, so it should only @@ -362,7 +361,7 @@ class ComposerInspectorTest extends PackageManagerKernelTestBase { $inspector = new class ( $this->container->get(ComposerProcessRunnerInterface::class), $this->container->get(ComposerIsAvailableInterface::class), - new PathFactory(), + $this->container->get(PathFactoryInterface::class), ) extends ComposerInspector { /** diff --git a/package_manager/tests/src/Kernel/FileSyncerFactoryTest.php b/package_manager/tests/src/Kernel/FileSyncerFactoryTest.php index 2ff5bb65ae..3422f66c2f 100644 --- a/package_manager/tests/src/Kernel/FileSyncerFactoryTest.php +++ b/package_manager/tests/src/Kernel/FileSyncerFactoryTest.php @@ -7,8 +7,8 @@ namespace Drupal\Tests\package_manager\Kernel; use Drupal\KernelTests\KernelTestBase; use Drupal\Tests\package_manager\Traits\AssertPreconditionsTrait; use PhpTuf\ComposerStager\API\FileSyncer\Service\FileSyncerInterface; -use PhpTuf\ComposerStager\Internal\FileSyncer\Service\PhpFileSyncer; -use PhpTuf\ComposerStager\Internal\FileSyncer\Service\RsyncFileSyncer; +use PhpTuf\ComposerStager\API\FileSyncer\Service\PhpFileSyncerInterface; +use PhpTuf\ComposerStager\API\FileSyncer\Service\RsyncFileSyncerInterface; /** * @covers \Drupal\package_manager\FileSyncerFactory @@ -50,11 +50,11 @@ class FileSyncerFactoryTest extends KernelTestBase { public function testFactory(?string $configured_syncer): void { switch ($configured_syncer) { case 'rsync': - $expected_syncer = RsyncFileSyncer::class; + $expected_syncer = RsyncFileSyncerInterface::class; break; case 'php': - $expected_syncer = PhpFileSyncer::class; + $expected_syncer = PhpFileSyncerInterface::class; break; default: diff --git a/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php b/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php index b094bf13b1..8199cf67e9 100644 --- a/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php +++ b/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php @@ -20,6 +20,7 @@ use Drupal\package_manager\PathLocator; use Drupal\package_manager\StatusCheckTrait; use Drupal\package_manager\Validator\DiskSpaceValidator; use Drupal\package_manager\StageBase; +use Drupal\Tests\automatic_updates\Traits\ComposerStagerTestTrait; use Drupal\Tests\package_manager\Traits\AssertPreconditionsTrait; use Drupal\Tests\package_manager\Traits\FixtureManipulatorTrait; use Drupal\Tests\package_manager\Traits\FixtureUtilityTrait; @@ -41,6 +42,7 @@ use Symfony\Component\Filesystem\Filesystem; abstract class PackageManagerKernelTestBase extends KernelTestBase { use AssertPreconditionsTrait; + use ComposerStagerTestTrait; use FixtureManipulatorTrait; use FixtureUtilityTrait; use StatusCheckTrait; diff --git a/package_manager/tests/src/Kernel/RsyncValidatorTest.php b/package_manager/tests/src/Kernel/RsyncValidatorTest.php index 54b4aae401..6dac105bd9 100644 --- a/package_manager/tests/src/Kernel/RsyncValidatorTest.php +++ b/package_manager/tests/src/Kernel/RsyncValidatorTest.php @@ -10,7 +10,7 @@ use Drupal\package_manager\ValidationResult; use Drupal\package_manager\Validator\RsyncValidator; use PhpTuf\ComposerStager\API\Exception\LogicException; use PhpTuf\ComposerStager\API\Finder\Service\ExecutableFinderInterface; -use PhpTuf\ComposerStager\Internal\Translation\Value\TranslatableMessage; +use PhpTuf\ComposerStager\API\Translation\Factory\TranslatableFactoryInterface; /** * @covers \Drupal\package_manager\Validator\RsyncValidator @@ -121,7 +121,9 @@ class RsyncValidatorTest extends PackageManagerKernelTestBase { * Tests that the stage cannot be created if rsync is selected, but not found. */ public function testPreCreateFailsIfRsyncNotFound(): void { - $message = new TranslatableMessage('Nope!'); + /** @var \PhpTuf\ComposerStager\API\Translation\Factory\TranslatableFactoryInterface $translatable_factory */ + $translatable_factory = $this->container->get(TranslatableFactoryInterface::class); + $message = $translatable_factory->createTranslatableMessage('Nope!'); $this->executableFinder->find('rsync')->willThrow(new LogicException($message)); $result = ValidationResult::createError([ diff --git a/package_manager/tests/src/Kernel/ServicesTest.php b/package_manager/tests/src/Kernel/ServicesTest.php index 862af87396..f4f5ceefdc 100644 --- a/package_manager/tests/src/Kernel/ServicesTest.php +++ b/package_manager/tests/src/Kernel/ServicesTest.php @@ -6,9 +6,11 @@ namespace Drupal\Tests\package_manager\Kernel; use Drupal\KernelTests\KernelTestBase; use Drupal\package_manager\ExecutableFinder; +use Drupal\package_manager\FileSyncerFactory; use Drupal\package_manager\ProcessFactory; use Drupal\package_manager\TranslatableStringFactory; use Drupal\Tests\package_manager\Traits\AssertPreconditionsTrait; +use PhpTuf\ComposerStager\API\FileSyncer\Factory\FileSyncerFactoryInterface; use PhpTuf\ComposerStager\API\Finder\Service\ExecutableFinderInterface; use PhpTuf\ComposerStager\API\Process\Factory\ProcessFactoryInterface; use PhpTuf\ComposerStager\API\Translation\Factory\TranslatableFactoryInterface; @@ -45,6 +47,7 @@ class ServicesTest extends KernelTestBase { // correctly. $overrides = [ ExecutableFinderInterface::class => ExecutableFinder::class, + FileSyncerFactoryInterface::class => FileSyncerFactory::class, ProcessFactoryInterface::class => ProcessFactory::class, TranslatableFactoryInterface::class => TranslatableStringFactory::class, ]; diff --git a/package_manager/tests/src/Kernel/StageBaseTest.php b/package_manager/tests/src/Kernel/StageBaseTest.php index 61e4939a58..2911f5d937 100644 --- a/package_manager/tests/src/Kernel/StageBaseTest.php +++ b/package_manager/tests/src/Kernel/StageBaseTest.php @@ -26,7 +26,6 @@ use PhpTuf\ComposerStager\API\Exception\ExceptionInterface; use PhpTuf\ComposerStager\API\Exception\InvalidArgumentException; use PhpTuf\ComposerStager\API\Exception\PreconditionException; use PhpTuf\ComposerStager\API\Precondition\Service\PreconditionInterface; -use PhpTuf\ComposerStager\Internal\Translation\Value\TranslatableMessage; use Psr\Log\LogLevel; use ColinODell\PsrTestLogger\TestLogger; @@ -233,7 +232,7 @@ class StageBaseTest extends PackageManagerKernelTestBase { LoggingCommitter::setException( new PreconditionException( $this->prophesize(PreconditionInterface::class)->reveal(), - new TranslatableMessage('Stage directory does not exist'), + $this->createComposeStagerMessage('Stage directory does not exist'), ) ); }; @@ -368,7 +367,7 @@ class StageBaseTest extends PackageManagerKernelTestBase { // Composer Stager's exception messages are usually translatable, so they // need to be wrapped by a TranslatableMessage object. if (is_subclass_of($thrown_class, ExceptionInterface::class)) { - $throwable_arguments[0] = new TranslatableMessage($throwable_arguments[0]); + $throwable_arguments[0] = $this->createComposeStagerMessage($throwable_arguments[0]); } // PreconditionException requires a preconditions object. if ($thrown_class === PreconditionException::class) { diff --git a/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php b/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php index c900eee318..61ccd14f8b 100644 --- a/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php +++ b/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php @@ -8,6 +8,7 @@ use Drupal\automatic_updates\CronUpdateRunner; use Drupal\automatic_updates\CommandExecutor; use Drupal\fixture_manipulator\StageFixtureManipulator; use Drupal\package_manager\PathLocator; +use Drupal\Tests\automatic_updates\Traits\ComposerStagerTestTrait; use Drupal\Tests\BrowserTestBase; use Drupal\Tests\package_manager\Traits\AssertPreconditionsTrait; use Drupal\Tests\package_manager\Traits\FixtureManipulatorTrait; @@ -22,6 +23,7 @@ use Symfony\Component\DependencyInjection\ContainerInterface; abstract class AutomaticUpdatesFunctionalTestBase extends BrowserTestBase { use AssertPreconditionsTrait; + use ComposerStagerTestTrait; use FixtureManipulatorTrait; use FixtureUtilityTrait; diff --git a/tests/src/Functional/ComposerStagerOperationFailureTest.php b/tests/src/Functional/ComposerStagerOperationFailureTest.php index 4263a4d6a1..75b96aed6e 100644 --- a/tests/src/Functional/ComposerStagerOperationFailureTest.php +++ b/tests/src/Functional/ComposerStagerOperationFailureTest.php @@ -9,7 +9,6 @@ use Drupal\package_manager_bypass\LoggingCommitter; use Drupal\package_manager_bypass\NoOpStager; use PhpTuf\ComposerStager\API\Exception\InvalidArgumentException; use PhpTuf\ComposerStager\API\Exception\LogicException; -use PhpTuf\ComposerStager\Internal\Translation\Value\TranslatableMessage; /** * @covers \Drupal\automatic_updates\Form\UpdaterForm @@ -39,7 +38,7 @@ class ComposerStagerOperationFailureTest extends UpdaterFormTestBase { $page->hasButton('Update to 9.8.1'); // Make the specified Composer Stager operation class throw an exception. - $message = new TranslatableMessage("Failure from inside $service_class"); + $message = $this->createComposeStagerMessage("Failure from inside $service_class"); $exception = new $exception_class($message); call_user_func([$service_class, 'setException'], $exception); diff --git a/tests/src/Kernel/ConsoleUpdateStageTest.php b/tests/src/Kernel/ConsoleUpdateStageTest.php index 3ea019918c..fc3e1e0749 100644 --- a/tests/src/Kernel/ConsoleUpdateStageTest.php +++ b/tests/src/Kernel/ConsoleUpdateStageTest.php @@ -30,7 +30,6 @@ use Drupal\Tests\user\Traits\UserCreationTrait; use PhpTuf\ComposerStager\API\Exception\InvalidArgumentException; use PhpTuf\ComposerStager\API\Exception\PreconditionException; use PhpTuf\ComposerStager\API\Precondition\Service\PreconditionInterface; -use PhpTuf\ComposerStager\Internal\Translation\Value\TranslatableMessage; use ColinODell\PsrTestLogger\TestLogger; use Symfony\Component\DependencyInjection\Reference; @@ -685,7 +684,7 @@ END; public function testMaintenanceModeAffectedByException(string $exception_class, bool $will_be_in_maintenance_mode): void { $this->getStageFixtureManipulator()->setCorePackageVersion('9.8.1'); - $message = new TranslatableMessage('A fail whale upon your head!'); + $message = $this->createComposeStagerMessage('A fail whale upon your head!'); LoggingCommitter::setException(match ($exception_class) { InvalidArgumentException::class => new InvalidArgumentException($message), diff --git a/tests/src/Kernel/UpdateStageTest.php b/tests/src/Kernel/UpdateStageTest.php index ab47c3ce61..44b4e830ad 100644 --- a/tests/src/Kernel/UpdateStageTest.php +++ b/tests/src/Kernel/UpdateStageTest.php @@ -11,7 +11,6 @@ use Drupal\package_manager_bypass\LoggingCommitter; use Drupal\Tests\user\Traits\UserCreationTrait; use PhpTuf\ComposerStager\API\Exception\ExceptionInterface; use PhpTuf\ComposerStager\API\Exception\InvalidArgumentException; -use PhpTuf\ComposerStager\Internal\Translation\Value\TranslatableMessage; /** * @coversDefaultClass \Drupal\automatic_updates\UpdateStage @@ -187,7 +186,7 @@ class UpdateStageTest extends AutomaticUpdatesKernelTestBase { // Composer Stager's exception messages are usually translatable, so they // need to be wrapped by a TranslatableMessage object. if (is_subclass_of($thrown_class, ExceptionInterface::class)) { - $thrown_message = new TranslatableMessage($thrown_message); + $thrown_message = $this->createComposeStagerMessage($thrown_message); } LoggingCommitter::setException(new $thrown_class($thrown_message, 123)); $this->expectException($expected_class); diff --git a/tests/src/Traits/ComposerStagerTestTrait.php b/tests/src/Traits/ComposerStagerTestTrait.php new file mode 100644 index 0000000000..8350fc094d --- /dev/null +++ b/tests/src/Traits/ComposerStagerTestTrait.php @@ -0,0 +1,47 @@ +<?php + +declare(strict_types = 1); + +namespace Drupal\Tests\automatic_updates\Traits; + +use PhpTuf\ComposerStager\API\Translation\Factory\TranslatableFactoryInterface; +use PhpTuf\ComposerStager\API\Translation\Value\TranslatableInterface; +use PhpTuf\ComposerStager\API\Translation\Value\TranslationParametersInterface; + +/** + * Contains helper methods for testing Composer Stager interactions. + * + * @internal + * + * @property \Symfony\Component\DependencyInjection\ContainerInterface $container + */ +trait ComposerStagerTestTrait { + + /** + * Creates a Composer Stager translatable message. + * + * @param string $message + * A message containing optional placeholders corresponding to parameters (next). Example: + * ```php + * $message = 'Hello, %first_name %last_name.'; + * ``` + * @param \PhpTuf\ComposerStager\API\Translation\Value\TranslationParametersInterface|null $parameters + * Translation parameters. + * @param string|null $domain + * An arbitrary domain for grouping translations or null to use the default. See + * {@see \PhpTuf\ComposerStager\API\Translation\Service\DomainOptionsInterface}. + * + * @return \PhpTuf\ComposerStager\API\Translation\Value\TranslatableInterface + */ + protected function createComposeStagerMessage( + string $message, + ?TranslationParametersInterface $parameters = NULL, + ?string $domain = NULL, + ): TranslatableInterface { + /** @var \PhpTuf\ComposerStager\API\Translation\Factory\TranslatableFactoryInterface $translatable_factory */ + $translatable_factory = $this->container->get(TranslatableFactoryInterface::class); + + return $translatable_factory->createTranslatableMessage($message, $parameters, $domain); + } + +} -- GitLab