From d581997c5cda0f5b2cee3d8392a604b1dab3b9a2 Mon Sep 17 00:00:00 2001 From: Ted Bowman <41201-tedbow@users.noreply.drupalcode.org> Date: Thu, 31 Aug 2023 17:32:27 +0000 Subject: [PATCH] Issue #3382942 by phenaproxima, tedbow, TravisCarden: Eliminate dependencies on Composer Stager internals --- package_manager/package_manager.services.yml | 33 +++++++------- .../src/Event/CollectPathsToExcludeEvent.php | 20 ++++----- package_manager/src/Event/PreApplyEvent.php | 24 ++++------ package_manager/src/Event/PreCreateEvent.php | 24 ++++------ .../src/Event/StatusCheckEvent.php | 43 +++++++++--------- package_manager/src/ExecutableFinder.php | 5 +-- package_manager/src/FileSyncerFactory.php | 2 +- package_manager/src/ImmutablePathList.php | 41 +++++++++++++++++ .../src/PackageManagerServiceProvider.php | 12 ----- package_manager/src/ProcessFactory.php | 5 +-- package_manager/src/StageBase.php | 29 ++++++------ package_manager/src/StatusCheckTrait.php | 16 ++----- .../src/TranslatableStringFactory.php | 5 +-- .../src/Validator/SymlinkValidator.php | 14 ++++-- .../fixture_manipulator.services.yml | 8 ++-- .../package_manager_bypass.services.yml | 14 +++--- .../src/LoggingBeginner.php | 2 +- .../src/LoggingCommitter.php | 2 +- .../PackageManagerBypassServiceProvider.php | 10 +++-- .../tests/src/Kernel/ExecutableFinderTest.php | 11 +---- .../Kernel/PackageManagerKernelTestBase.php | 26 +---------- .../Kernel/PathExcluder/GitExcluderTest.php | 8 ++-- .../tests/src/Kernel/StageBaseTest.php | 7 +-- .../tests/src/Kernel/StageEventsTest.php | 6 ++- .../tests/src/Kernel/StatusCheckTraitTest.php | 27 ++++++++---- .../src/Traits/FixtureManipulatorTrait.php | 4 +- .../Unit/StageNotInActiveValidatorTest.php | 3 +- .../tests/src/Unit/StatusCheckEventTest.php | 30 ------------- scripts/commit-code-check.sh | 15 +++++-- tests/src/Kernel/ConsoleUpdateStageTest.php | 44 ++++++++++++++----- tests/src/Traits/ComposerStagerTestTrait.php | 1 + 31 files changed, 239 insertions(+), 252 deletions(-) create mode 100644 package_manager/src/ImmutablePathList.php delete mode 100644 package_manager/tests/src/Unit/StatusCheckEventTest.php diff --git a/package_manager/package_manager.services.yml b/package_manager/package_manager.services.yml index bc657dd3cd..90089e6987 100644 --- a/package_manager/package_manager.services.yml +++ b/package_manager/package_manager.services.yml @@ -10,20 +10,23 @@ services: # Basic infrastructure services for Composer Stager, overridden by us to # provide additional functionality. - Drupal\package_manager\ProcessFactory: - public: false Drupal\package_manager\ExecutableFinder: public: false + decorates: 'PhpTuf\ComposerStager\API\Finder\Service\ExecutableFinderInterface' + Drupal\package_manager\FileSyncerFactory: + public: false + decorates: 'PhpTuf\ComposerStager\API\FileSyncer\Factory\FileSyncerFactoryInterface' + Drupal\package_manager\ProcessFactory: + public: false + decorates: 'PhpTuf\ComposerStager\API\Process\Factory\ProcessFactoryInterface' Drupal\package_manager\TranslatableStringFactory: public: false - PhpTuf\ComposerStager\API\Finder\Service\ExecutableFinderInterface: - alias: 'Drupal\package_manager\ExecutableFinder' - PhpTuf\ComposerStager\API\Process\Factory\ProcessFactoryInterface: - alias: 'Drupal\package_manager\ProcessFactory' + decorates: 'PhpTuf\ComposerStager\API\Translation\Factory\TranslatableFactoryInterface' + Drupal\package_manager\NoSymlinksPointToADirectory: + public: false + decorates: 'PhpTuf\ComposerStager\API\Precondition\Service\NoSymlinksPointToADirectoryInterface' PhpTuf\ComposerStager\API\FileSyncer\Service\FileSyncerInterface: - factory: ['@Drupal\package_manager\FileSyncerFactory', 'create'] - PhpTuf\ComposerStager\API\Translation\Factory\TranslatableFactoryInterface: - alias: 'Drupal\package_manager\TranslatableStringFactory' + factory: ['@PhpTuf\ComposerStager\API\FileSyncer\Factory\FileSyncerFactoryInterface', 'create'] logger.channel.package_manager: parent: logger.channel_base arguments: @@ -34,15 +37,9 @@ services: - 'package_manager_change_log' # Services provided to Drupal by Package Manager. - package_manager.beginner: - class: PhpTuf\ComposerStager\Internal\Core\Beginner - PhpTuf\ComposerStager\API\Core\BeginnerInterface: '@package_manager.beginner' - package_manager.stager: - class: PhpTuf\ComposerStager\Internal\Core\Stager - PhpTuf\ComposerStager\API\Core\StagerInterface: '@package_manager.stager' - package_manager.committer: - class: PhpTuf\ComposerStager\Internal\Core\Committer - PhpTuf\ComposerStager\API\Core\CommitterInterface: '@package_manager.committer' + package_manager.beginner: '@PhpTuf\ComposerStager\API\Core\BeginnerInterface' + package_manager.stager: '@PhpTuf\ComposerStager\API\Core\StagerInterface' + package_manager.committer: '@PhpTuf\ComposerStager\API\Core\CommitterInterface' package_manager.path_locator: class: Drupal\package_manager\PathLocator arguments: diff --git a/package_manager/src/Event/CollectPathsToExcludeEvent.php b/package_manager/src/Event/CollectPathsToExcludeEvent.php index aeb7be56ad..f6e6c65dab 100644 --- a/package_manager/src/Event/CollectPathsToExcludeEvent.php +++ b/package_manager/src/Event/CollectPathsToExcludeEvent.php @@ -7,8 +7,8 @@ namespace Drupal\package_manager\Event; use Drupal\package_manager\StageBase; use Drupal\package_manager\PathLocator; use PhpTuf\ComposerStager\API\Path\Factory\PathFactoryInterface; +use PhpTuf\ComposerStager\API\Path\Factory\PathListFactoryInterface; use PhpTuf\ComposerStager\API\Path\Value\PathListInterface; -use PhpTuf\ComposerStager\Internal\Path\Value\PathList; /** * Defines an event that collects paths to exclude. @@ -18,13 +18,6 @@ use PhpTuf\ComposerStager\Internal\Path\Value\PathList; */ final class CollectPathsToExcludeEvent extends StageEvent implements PathListInterface { - /** - * The list of paths to exclude. - * - * @var \PhpTuf\ComposerStager\API\Path\Value\PathListInterface - */ - protected PathListInterface $pathList; - /** * Constructs a CollectPathsToExcludeEvent object. * @@ -34,14 +27,19 @@ final class CollectPathsToExcludeEvent extends StageEvent implements PathListInt * The path locator service. * @param \PhpTuf\ComposerStager\API\Path\Factory\PathFactoryInterface $pathFactory * The path factory service. + * @param \PhpTuf\ComposerStager\API\Path\Value\PathListInterface|null $pathList + * (optional) The list of paths to exclude. */ public function __construct( StageBase $stage, private readonly PathLocator $pathLocator, - private readonly PathFactoryInterface $pathFactory + private readonly PathFactoryInterface $pathFactory, + private ?PathListInterface $pathList = NULL, ) { parent::__construct($stage); - $this->pathList = new PathList(); + + $this->pathList ??= \Drupal::service(PathListFactoryInterface::class) + ->create(); } /** @@ -55,7 +53,7 @@ final class CollectPathsToExcludeEvent extends StageEvent implements PathListInt * {@inheritdoc} */ public function getAll(): array { - return $this->pathList->getAll(); + return array_unique($this->pathList->getAll()); } /** diff --git a/package_manager/src/Event/PreApplyEvent.php b/package_manager/src/Event/PreApplyEvent.php index 721584df05..5cef0946a7 100644 --- a/package_manager/src/Event/PreApplyEvent.php +++ b/package_manager/src/Event/PreApplyEvent.php @@ -4,7 +4,9 @@ declare(strict_types = 1); namespace Drupal\package_manager\Event; +use Drupal\package_manager\ImmutablePathList; use Drupal\package_manager\StageBase; +use PhpTuf\ComposerStager\API\Path\Value\PathListInterface; /** * Event fired before staged changes are synced to the active directory. @@ -12,36 +14,26 @@ use Drupal\package_manager\StageBase; final class PreApplyEvent extends PreOperationStageEvent { /** - * Paths to exclude from the update. + * The list of paths to ignore in the active and stage directories. * - * @var string[] + * @var \Drupal\package_manager\ImmutablePathList */ - private array $excludedPaths = []; - - /** - * Returns the paths to exclude from the current operation. - * - * @return string[] - * The paths to exclude. - */ - public function getExcludedPaths(): array { - return array_unique($this->excludedPaths); - } + public readonly ImmutablePathList $excludedPaths; /** * Constructs a PreApplyEvent object. * * @param \Drupal\package_manager\StageBase $stage * The stage which fired this event. - * @param string[] $paths_to_exclude + * @param \PhpTuf\ComposerStager\API\Path\Value\PathListInterface $excluded_paths * The list of paths to exclude. These will not be copied from the stage * directory to the active directory, nor be deleted from the active * directory if they exist, when the stage directory is copied back into * the active directory. */ - public function __construct(StageBase $stage, array $paths_to_exclude) { + public function __construct(StageBase $stage, PathListInterface $excluded_paths) { parent::__construct($stage); - $this->excludedPaths = $paths_to_exclude; + $this->excludedPaths = new ImmutablePathList($excluded_paths); } } diff --git a/package_manager/src/Event/PreCreateEvent.php b/package_manager/src/Event/PreCreateEvent.php index e7468a5981..be26b1f235 100644 --- a/package_manager/src/Event/PreCreateEvent.php +++ b/package_manager/src/Event/PreCreateEvent.php @@ -4,7 +4,9 @@ declare(strict_types = 1); namespace Drupal\package_manager\Event; +use Drupal\package_manager\ImmutablePathList; use Drupal\package_manager\StageBase; +use PhpTuf\ComposerStager\API\Path\Value\PathListInterface; /** * Event fired before a stage directory is created. @@ -12,34 +14,24 @@ use Drupal\package_manager\StageBase; final class PreCreateEvent extends PreOperationStageEvent { /** - * Paths to exclude from the update. + * The list of paths to exclude from the stage directory. * - * @var string[] + * @var \Drupal\package_manager\ImmutablePathList */ - private array $excludedPaths = []; - - /** - * Returns the paths to exclude from the current operation. - * - * @return string[] - * The paths to exclude. - */ - public function getExcludedPaths(): array { - return array_unique($this->excludedPaths); - } + public readonly ImmutablePathList $excludedPaths; /** * Constructs a PreCreateEvent object. * * @param \Drupal\package_manager\StageBase $stage * The stage which fired this event. - * @param string[] $paths_to_exclude + * @param \PhpTuf\ComposerStager\API\Path\Value\PathListInterface $excluded_paths * The list of paths to exclude. These will not be copied into the stage * directory when it is created. */ - public function __construct(StageBase $stage, array $paths_to_exclude) { + public function __construct(StageBase $stage, PathListInterface $excluded_paths) { parent::__construct($stage); - $this->excludedPaths = $paths_to_exclude; + $this->excludedPaths = new ImmutablePathList($excluded_paths); } } diff --git a/package_manager/src/Event/StatusCheckEvent.php b/package_manager/src/Event/StatusCheckEvent.php index c50629af1c..10834fb002 100644 --- a/package_manager/src/Event/StatusCheckEvent.php +++ b/package_manager/src/Event/StatusCheckEvent.php @@ -5,9 +5,10 @@ declare(strict_types = 1); namespace Drupal\package_manager\Event; use Drupal\Core\StringTranslation\TranslatableMarkup; +use Drupal\package_manager\ImmutablePathList; use Drupal\package_manager\StageBase; use Drupal\package_manager\ValidationResult; -use Drupal\system\SystemManager; +use PhpTuf\ComposerStager\API\Path\Value\PathListInterface; /** * Event fired to check the status of the system to use Package Manager. @@ -18,36 +19,38 @@ use Drupal\system\SystemManager; final class StatusCheckEvent extends PreOperationStageEvent { /** - * Returns paths to exclude or NULL if a base requirement is not fulfilled. + * The paths to exclude, or NULL if there was an error collecting them. * - * @return string[]|null - * The paths to exclude, or NULL if a base requirement is not fulfilled. + * @var \Drupal\package_manager\ImmutablePathList|null * - * @throws \LogicException - * Thrown if the excluded paths are NULL and no errors have been added to - * this event. + * @see ::__construct() */ - public function getExcludedPaths(): ?array { - if (isset($this->pathsToExclude)) { - return array_unique($this->pathsToExclude); - } - - if (empty($this->getResults(SystemManager::REQUIREMENT_ERROR))) { - throw new \LogicException('$paths_to_exclude should only be NULL if the error that caused the paths to not be collected was added to the status check event.'); - } - return NULL; - } + public readonly ?ImmutablePathList $excludedPaths; /** * Constructs a StatusCheckEvent object. * * @param \Drupal\package_manager\StageBase $stage * The stage which fired this event. - * @param string[]|null $pathsToExclude - * The list of paths to exclude, or NULL if they could not be collected. + * @param \PhpTuf\ComposerStager\API\Path\Value\PathListInterface|\Throwable $excluded_paths + * The list of paths to exclude or, if an error occurred while they were + * being collected, the throwable from that error. If this is a throwable, + * it will be converted to a validation error. */ - public function __construct(StageBase $stage, private ?array $pathsToExclude) { + public function __construct(StageBase $stage, PathListInterface|\Throwable $excluded_paths) { parent::__construct($stage); + + // If there was an error collecting the excluded paths, convert it to a + // validation error so we can still run status checks that don't need to + // examine the list of excluded paths. + if ($excluded_paths instanceof \Throwable) { + $this->addErrorFromThrowable($excluded_paths); + $excluded_paths = NULL; + } + else { + $excluded_paths = new ImmutablePathList($excluded_paths); + } + $this->excludedPaths = $excluded_paths; } /** diff --git a/package_manager/src/ExecutableFinder.php b/package_manager/src/ExecutableFinder.php index 731a751c6d..9e924bed44 100644 --- a/package_manager/src/ExecutableFinder.php +++ b/package_manager/src/ExecutableFinder.php @@ -6,7 +6,6 @@ namespace Drupal\package_manager; use Drupal\Core\Config\ConfigFactoryInterface; use PhpTuf\ComposerStager\API\Finder\Service\ExecutableFinderInterface; -use PhpTuf\ComposerStager\Internal\Finder\Service\ExecutableFinder as StagerExecutableFinder; /** * An executable finder which looks for executable paths in configuration. @@ -21,13 +20,13 @@ final class ExecutableFinder implements ExecutableFinderInterface { /** * Constructs an ExecutableFinder object. * - * @param \PhpTuf\ComposerStager\Internal\Finder\Service\ExecutableFinder $decorated + * @param \PhpTuf\ComposerStager\API\Finder\Service\ExecutableFinderInterface $decorated * The decorated executable finder. * @param \Drupal\Core\Config\ConfigFactoryInterface $configFactory * The config factory service. */ public function __construct( - private readonly StagerExecutableFinder $decorated, + private readonly ExecutableFinderInterface $decorated, private readonly ConfigFactoryInterface $configFactory ) {} diff --git a/package_manager/src/FileSyncerFactory.php b/package_manager/src/FileSyncerFactory.php index 9ba699e74b..3f2ffdb9cf 100644 --- a/package_manager/src/FileSyncerFactory.php +++ b/package_manager/src/FileSyncerFactory.php @@ -18,7 +18,7 @@ use PhpTuf\ComposerStager\API\FileSyncer\Service\RsyncFileSyncerInterface; * at any time without warning. External code should not interact with this * class. */ -final class FileSyncerFactory { +final class FileSyncerFactory implements FileSyncerFactoryInterface { /** * Constructs a FileSyncerFactory object. diff --git a/package_manager/src/ImmutablePathList.php b/package_manager/src/ImmutablePathList.php new file mode 100644 index 0000000000..dc61e744f1 --- /dev/null +++ b/package_manager/src/ImmutablePathList.php @@ -0,0 +1,41 @@ +<?php + +declare(strict_types = 1); + +namespace Drupal\package_manager; + +use PhpTuf\ComposerStager\API\Path\Value\PathListInterface; + +/** + * Defines a path list that cannot be changed. + * + * @internal + * This is an internal part of Package Manager and may be changed or removed + * at any time without warning. External code should not interact with this + * class. + */ +final class ImmutablePathList implements PathListInterface { + + /** + * Constructs an ImmutablePathList object. + * + * @param \PhpTuf\ComposerStager\API\Path\Value\PathListInterface $decorated + * The decorated path list. + */ + public function __construct(private readonly PathListInterface $decorated) {} + + /** + * {@inheritdoc} + */ + public function add(string ...$paths): never { + throw new \LogicException('Immutable path lists cannot be changed.'); + } + + /** + * {@inheritdoc} + */ + public function getAll(): array { + return $this->decorated->getAll(); + } + +} diff --git a/package_manager/src/PackageManagerServiceProvider.php b/package_manager/src/PackageManagerServiceProvider.php index 8eaf814855..a883fb232f 100644 --- a/package_manager/src/PackageManagerServiceProvider.php +++ b/package_manager/src/PackageManagerServiceProvider.php @@ -7,8 +7,6 @@ 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; /** * Defines dynamic container services for Package Manager. @@ -121,16 +119,6 @@ final class PackageManagerServiceProvider extends ServiceProviderBase { } } // END: DELETE FROM CORE MERGE REQUEST - // Decorate certain Composer Stager preconditions. - $container->register(NoSymlinksPointToADirectory::class) - ->setPublic(FALSE) - ->setAutowired(TRUE) - ->setDecoratedService(NoSymlinksPointToADirectoryInterface::class); - - $container->register(FileSyncerFactory::class) - ->setPublic(FALSE) - ->setAutowired(TRUE) - ->setDecoratedService(FileSyncerFactoryInterface::class); } } diff --git a/package_manager/src/ProcessFactory.php b/package_manager/src/ProcessFactory.php index 92a1e9f8a9..3e605046c2 100644 --- a/package_manager/src/ProcessFactory.php +++ b/package_manager/src/ProcessFactory.php @@ -8,7 +8,6 @@ use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\File\FileSystemInterface; use PhpTuf\ComposerStager\API\Process\Factory\ProcessFactoryInterface; use PhpTuf\ComposerStager\API\Process\Service\ProcessInterface; -use PhpTuf\ComposerStager\Internal\Process\Factory\ProcessFactory as StagerProcessFactory; // cspell:ignore BINDIR @@ -29,13 +28,13 @@ final class ProcessFactory implements ProcessFactoryInterface { * The file system service. * @param \Drupal\Core\Config\ConfigFactoryInterface $configFactory * The config factory service. - * @param \PhpTuf\ComposerStager\Internal\Process\Factory\ProcessFactory $decorated + * @param \PhpTuf\ComposerStager\API\Process\Factory\ProcessFactoryInterface $decorated * The decorated process factory service. */ public function __construct( private readonly FileSystemInterface $fileSystem, private readonly ConfigFactoryInterface $configFactory, - private readonly StagerProcessFactory $decorated, + private readonly ProcessFactoryInterface $decorated, ) {} /** diff --git a/package_manager/src/StageBase.php b/package_manager/src/StageBase.php index 1dbe6c8d42..c49c1a7beb 100644 --- a/package_manager/src/StageBase.php +++ b/package_manager/src/StageBase.php @@ -34,7 +34,7 @@ use PhpTuf\ComposerStager\API\Core\StagerInterface; use PhpTuf\ComposerStager\API\Exception\InvalidArgumentException; use PhpTuf\ComposerStager\API\Exception\PreconditionException; use PhpTuf\ComposerStager\API\Path\Factory\PathFactoryInterface; -use PhpTuf\ComposerStager\Internal\Path\Value\PathList; +use PhpTuf\ComposerStager\API\Path\Value\PathListInterface; use Psr\Log\LoggerAwareInterface; use Psr\Log\LoggerAwareTrait; use Psr\Log\NullLogger; @@ -254,7 +254,7 @@ abstract class StageBase implements LoggerAwareInterface { /** * Collects paths that Composer Stager should exclude. * - * @return string[] + * @return \PhpTuf\ComposerStager\API\Path\Value\PathListInterface * A list of paths that Composer Stager should exclude when creating the * stage directory and applying staged changes to the active directory. * @@ -264,15 +264,14 @@ abstract class StageBase implements LoggerAwareInterface { * @see ::create() * @see ::apply() */ - protected function getPathsToExclude(): array { + protected function getPathsToExclude(): PathListInterface { $event = new CollectPathsToExcludeEvent($this, $this->pathLocator, $this->pathFactory); try { - $this->eventDispatcher->dispatch($event); + return $this->eventDispatcher->dispatch($event); } catch (\Throwable $e) { $this->rethrowAsStageException($e); } - return $event->getAll(); } /** @@ -324,13 +323,14 @@ abstract class StageBase implements LoggerAwareInterface { $active_dir = $this->pathFactory->create($this->pathLocator->getProjectRoot()); $stage_dir = $this->pathFactory->create($this->getStageDirectory()); - $event = new PreCreateEvent($this, $this->getPathsToExclude()); + $excluded_paths = $this->getPathsToExclude(); + $event = new PreCreateEvent($this, $excluded_paths); // If an error occurs and we won't be able to create the stage, mark it as // available. $this->dispatch($event, [$this, 'markAsAvailable']); try { - $this->beginner->begin($active_dir, $stage_dir, new PathList(...$event->getExcludedPaths()), NULL, $timeout); + $this->beginner->begin($active_dir, $stage_dir, $excluded_paths, NULL, $timeout); } catch (\Throwable $error) { $this->destroy(); @@ -442,23 +442,24 @@ abstract class StageBase implements LoggerAwareInterface { $active_dir = $this->pathFactory->create($this->pathLocator->getProjectRoot()); $stage_dir = $this->pathFactory->create($this->getStageDirectory()); + $excluded_paths = $this->getPathsToExclude(); + // Exclude the failure file from the commit operation. + $excluded_paths->add( + str_replace($active_dir->absolute() . DIRECTORY_SEPARATOR, '', $this->failureMarker->getPath()), + ); + // If an error occurs while dispatching the events, ensure that ::destroy() // doesn't think we're in the middle of applying the staged changes to the // active directory. - $event = new PreApplyEvent($this, $this->getPathsToExclude()); + $event = new PreApplyEvent($this, $excluded_paths); $this->tempStore->set(self::TEMPSTORE_APPLY_TIME_KEY, $this->time->getRequestTime()); $this->dispatch($event, $this->setNotApplying(...)); // Create a marker file so that we can tell later on if the commit failed. $this->failureMarker->write($this, $this->getFailureMarkerMessage()); - // Exclude the failure file from the commit operation. - $paths_to_exclude = new PathList(...$event->getExcludedPaths()); - $paths_to_exclude->add( - str_replace($this->pathLocator->getProjectRoot() . DIRECTORY_SEPARATOR, '', $this->failureMarker->getPath()), - ); try { - $this->committer->commit($stage_dir, $active_dir, $paths_to_exclude, NULL, $timeout); + $this->committer->commit($stage_dir, $active_dir, $excluded_paths, NULL, $timeout); } catch (InvalidArgumentException | PreconditionException $e) { // The commit operation has not started yet, so we can clear the failure diff --git a/package_manager/src/StatusCheckTrait.php b/package_manager/src/StatusCheckTrait.php index 23386235b8..d4a1799b3e 100644 --- a/package_manager/src/StatusCheckTrait.php +++ b/package_manager/src/StatusCheckTrait.php @@ -42,22 +42,12 @@ trait StatusCheckTrait { try { $paths_to_exclude_event = new CollectPathsToExcludeEvent($stage, $path_locator, $path_factory); $event_dispatcher->dispatch($paths_to_exclude_event); - $event = new StatusCheckEvent($stage, $paths_to_exclude_event->getAll()); } catch (\Throwable $throwable) { - // We can dispatch the status check event without the paths to exclude, - // but it must be set explicitly to NULL, to allow those status checks to - // run that do not need the paths to exclude. - $event = new StatusCheckEvent($stage, NULL); - // Add the error that was encountered so that regardless of any other - // validation errors BaseRequirementsFulfilledValidator will stop the - // event propagation after the base requirement validators have run. - // @see \Drupal\package_manager\Validator\BaseRequirementsFulfilledValidator - $event->addErrorFromThrowable($throwable, t('Unable to collect the paths to exclude.')); + $paths_to_exclude_event = $throwable; } - - $event_dispatcher->dispatch($event); - return $event->getResults(); + $event = new StatusCheckEvent($stage, $paths_to_exclude_event); + return $event_dispatcher->dispatch($event)->getResults(); } } diff --git a/package_manager/src/TranslatableStringFactory.php b/package_manager/src/TranslatableStringFactory.php index 8b2f6a2282..c0557eec97 100644 --- a/package_manager/src/TranslatableStringFactory.php +++ b/package_manager/src/TranslatableStringFactory.php @@ -9,7 +9,6 @@ use PhpTuf\ComposerStager\API\Translation\Factory\TranslatableFactoryInterface; use PhpTuf\ComposerStager\API\Translation\Service\DomainOptionsInterface; use PhpTuf\ComposerStager\API\Translation\Value\TranslatableInterface; use PhpTuf\ComposerStager\API\Translation\Value\TranslationParametersInterface; -use PhpTuf\ComposerStager\Internal\Translation\Factory\TranslatableFactory as StagerTranslatableFactory; /** * Creates translatable strings that can interoperate with Composer Stager. @@ -24,13 +23,13 @@ final class TranslatableStringFactory implements TranslatableFactoryInterface { /** * Constructs a TranslatableStringFactory object. * - * @param \PhpTuf\ComposerStager\Internal\Translation\Factory\TranslatableFactory $decorated + * @param \PhpTuf\ComposerStager\API\Translation\Factory\TranslatableFactoryInterface $decorated * The decorated translatable factory service. * @param \Drupal\Core\StringTranslation\TranslationInterface $translation * The string translation service. */ public function __construct( - private readonly StagerTranslatableFactory $decorated, + private readonly TranslatableFactoryInterface $decorated, private readonly TranslationInterface $translation, ) {} diff --git a/package_manager/src/Validator/SymlinkValidator.php b/package_manager/src/Validator/SymlinkValidator.php index d3381f302f..29ce772ec6 100644 --- a/package_manager/src/Validator/SymlinkValidator.php +++ b/package_manager/src/Validator/SymlinkValidator.php @@ -9,7 +9,7 @@ use Drupal\package_manager\Event\PreRequireEvent; use Drupal\package_manager\PathLocator; use PhpTuf\ComposerStager\API\Exception\PreconditionException; use PhpTuf\ComposerStager\API\Path\Factory\PathFactoryInterface; -use PhpTuf\ComposerStager\Internal\Path\Value\PathList; +use PhpTuf\ComposerStager\API\Path\Factory\PathListFactoryInterface; use PhpTuf\ComposerStager\API\Precondition\Service\NoUnsupportedLinksExistInterface; use Symfony\Component\EventDispatcher\EventSubscriberInterface; @@ -36,11 +36,14 @@ final class SymlinkValidator implements EventSubscriberInterface { * The Composer Stager precondition that this validator wraps. * @param \PhpTuf\ComposerStager\API\Path\Factory\PathFactoryInterface $pathFactory * The path factory service. + * @param \PhpTuf\ComposerStager\API\Path\Factory\PathListFactoryInterface $pathListFactory + * The path list factory service. */ public function __construct( private readonly PathLocator $pathLocator, private readonly NoUnsupportedLinksExistInterface $precondition, private readonly PathFactoryInterface $pathFactory, + private readonly PathListFactoryInterface $pathListFactory, ) {} /** @@ -65,17 +68,20 @@ final class SymlinkValidator implements EventSubscriberInterface { } $stage_dir = $this->pathFactory->create($stage_dir); - $excluded_paths = $event->getExcludedPaths(); // Return early if no excluded paths were collected because this validator // is dependent on knowing which paths to exclude when searching for // symlinks. // @see \Drupal\package_manager\StatusCheckTrait::runStatusCheck() - if ($excluded_paths === NULL) { + if ($event->excludedPaths === NULL) { return; } + // The list of excluded paths is immutable, but the precondition may need to + // mutate it, so convert it back to a normal, mutable path list. + $exclusions = $this->pathListFactory->create(...$event->excludedPaths->getAll()); + try { - $this->precondition->assertIsFulfilled($active_dir, $stage_dir, new PathList(...$excluded_paths)); + $this->precondition->assertIsFulfilled($active_dir, $stage_dir, $exclusions); } catch (PreconditionException $e) { $event->addErrorFromThrowable($e); diff --git a/package_manager/tests/modules/fixture_manipulator/fixture_manipulator.services.yml b/package_manager/tests/modules/fixture_manipulator/fixture_manipulator.services.yml index 862187ca84..f83263a213 100644 --- a/package_manager/tests/modules/fixture_manipulator/fixture_manipulator.services.yml +++ b/package_manager/tests/modules/fixture_manipulator/fixture_manipulator.services.yml @@ -1,5 +1,5 @@ services: - fixture_manipulator.stage_manipulator: - class: Drupal\fixture_manipulator\StageFixtureManipulator - arguments: ['@state', '@fixture_manipulator.stage_manipulator.inner'] - decorates: 'package_manager.beginner' + _defaults: + autowire: true + Drupal\fixture_manipulator\StageFixtureManipulator: + decorates: 'PhpTuf\ComposerStager\API\Core\BeginnerInterface' diff --git a/package_manager/tests/modules/package_manager_bypass/package_manager_bypass.services.yml b/package_manager/tests/modules/package_manager_bypass/package_manager_bypass.services.yml index 7881bb9a11..b5fea63ec7 100644 --- a/package_manager/tests/modules/package_manager_bypass/package_manager_bypass.services.yml +++ b/package_manager/tests/modules/package_manager_bypass/package_manager_bypass.services.yml @@ -1,9 +1,7 @@ services: - package_manager_bypass.beginner: - class: Drupal\package_manager_bypass\LoggingBeginner - arguments: ['@state', '@package_manager_bypass.beginner.inner' ] - decorates: 'package_manager.beginner' - package_manager_bypass.committer: - class: Drupal\package_manager_bypass\LoggingCommitter - arguments: [ '@state', '@package_manager_bypass.committer.inner' ] - decorates: 'package_manager.committer' + _defaults: + autowire: true + Drupal\package_manager_bypass\LoggingBeginner: + decorates: 'PhpTuf\ComposerStager\API\Core\BeginnerInterface' + Drupal\package_manager_bypass\LoggingCommitter: + decorates: 'PhpTuf\ComposerStager\API\Core\CommitterInterface' diff --git a/package_manager/tests/modules/package_manager_bypass/src/LoggingBeginner.php b/package_manager/tests/modules/package_manager_bypass/src/LoggingBeginner.php index 8ae0152d25..a14543bedc 100644 --- a/package_manager/tests/modules/package_manager_bypass/src/LoggingBeginner.php +++ b/package_manager/tests/modules/package_manager_bypass/src/LoggingBeginner.php @@ -45,7 +45,7 @@ final class LoggingBeginner implements BeginnerInterface { * {@inheritdoc} */ public function begin(PathInterface $activeDir, PathInterface $stagingDir, ?PathListInterface $exclusions = NULL, ?OutputCallbackInterface $callback = NULL, ?int $timeout = ProcessInterface::DEFAULT_TIMEOUT): void { - $this->saveInvocationArguments($activeDir, $stagingDir, $exclusions, $timeout); + $this->saveInvocationArguments($activeDir, $stagingDir, $exclusions?->getAll(), $timeout); $this->throwExceptionIfSet(); $this->inner->begin($activeDir, $stagingDir, $exclusions, $callback, $timeout); } diff --git a/package_manager/tests/modules/package_manager_bypass/src/LoggingCommitter.php b/package_manager/tests/modules/package_manager_bypass/src/LoggingCommitter.php index 1e6a9d8932..41bda89e97 100644 --- a/package_manager/tests/modules/package_manager_bypass/src/LoggingCommitter.php +++ b/package_manager/tests/modules/package_manager_bypass/src/LoggingCommitter.php @@ -45,7 +45,7 @@ final class LoggingCommitter implements CommitterInterface { * {@inheritdoc} */ public function commit(PathInterface $stagingDir, PathInterface $activeDir, ?PathListInterface $exclusions = NULL, ?OutputCallbackInterface $callback = NULL, ?int $timeout = ProcessInterface::DEFAULT_TIMEOUT): void { - $this->saveInvocationArguments($stagingDir, $activeDir, $exclusions, $timeout); + $this->saveInvocationArguments($stagingDir, $activeDir, $exclusions?->getAll(), $timeout); $this->throwExceptionIfSet(); $this->inner->commit($stagingDir, $activeDir, $exclusions, $callback, $timeout); } 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 f6ab530a2f..18e7c60c40 100644 --- a/package_manager/tests/modules/package_manager_bypass/src/PackageManagerBypassServiceProvider.php +++ b/package_manager/tests/modules/package_manager_bypass/src/PackageManagerBypassServiceProvider.php @@ -7,6 +7,7 @@ namespace Drupal\package_manager_bypass; use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\Core\DependencyInjection\ServiceProviderBase; use Drupal\Core\Site\Settings; +use PhpTuf\ComposerStager\API\Core\StagerInterface; use Symfony\Component\DependencyInjection\Parameter; use Symfony\Component\DependencyInjection\Reference; @@ -23,19 +24,22 @@ final class PackageManagerBypassServiceProvider extends ServiceProviderBase { public function alter(ContainerBuilder $container) { parent::alter($container); - $state = new Reference('state'); // By default, \Drupal\package_manager_bypass\NoOpStager is applied, except // when a test opts out by setting this setting to FALSE. // @see \Drupal\package_manager_bypass\NoOpStager::setLockFileShouldChange() if (Settings::get('package_manager_bypass_composer_stager', TRUE)) { - $container->getDefinition('package_manager.stager')->setClass(NoOpStager::class)->setArguments([$state]); + $container->register(NoOpStager::class) + ->setClass(NoOpStager::class) + ->setPublic(FALSE) + ->setAutowired(TRUE) + ->setDecoratedService(StagerInterface::class); } $container->getDefinition('package_manager.path_locator') ->setClass(MockPathLocator::class) ->setAutowired(FALSE) ->setArguments([ - $state, + new Reference('state'), new Parameter('app.root'), new Reference('config.factory'), new Reference('file_system'), diff --git a/package_manager/tests/src/Kernel/ExecutableFinderTest.php b/package_manager/tests/src/Kernel/ExecutableFinderTest.php index 82402899b4..c1ee4ef5d1 100644 --- a/package_manager/tests/src/Kernel/ExecutableFinderTest.php +++ b/package_manager/tests/src/Kernel/ExecutableFinderTest.php @@ -7,8 +7,6 @@ namespace Drupal\Tests\package_manager\Kernel; use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\package_manager\ExecutableFinder; use PhpTuf\ComposerStager\API\Finder\Service\ExecutableFinderInterface; -use PhpTuf\ComposerStager\Internal\Finder\Service\ExecutableFinder as StagerExecutableFinder; -use PhpTuf\ComposerStager\API\Translation\Factory\TranslatableFactoryInterface; use Symfony\Component\Process\ExecutableFinder as SymfonyExecutableFinder; /** @@ -23,7 +21,7 @@ class ExecutableFinderTest extends PackageManagerKernelTestBase { */ public function register(ContainerBuilder $container) { // Mock a Symfony executable finder that always returns /dev/null. - $symfony_executable_finder = new class extends SymfonyExecutableFinder { + $container->set(SymfonyExecutableFinder::class, new class extends SymfonyExecutableFinder { /** * {@inheritdoc} @@ -32,12 +30,7 @@ class ExecutableFinderTest extends PackageManagerKernelTestBase { return '/dev/null'; } - }; - $container->getDefinition(ExecutableFinder::class) - ->setArgument('$decorated', new StagerExecutableFinder( - $symfony_executable_finder, - $this->createMock(TranslatableFactoryInterface::class), - )); + }); } /** diff --git a/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php b/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php index 8199cf67e9..b391c243be 100644 --- a/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php +++ b/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php @@ -12,7 +12,6 @@ use Drupal\Core\Site\Settings; use Drupal\fixture_manipulator\StageFixtureManipulator; use Drupal\KernelTests\KernelTestBase; use Drupal\package_manager\Event\PreApplyEvent; -use Drupal\package_manager\Event\PreCreateEvent; use Drupal\package_manager\Event\PreOperationStageEvent; use Drupal\package_manager\Exception\StageEventException; use Drupal\package_manager\FailureMarker; @@ -30,7 +29,7 @@ use GuzzleHttp\Handler\MockHandler; use GuzzleHttp\HandlerStack; use GuzzleHttp\Psr7\Response; use GuzzleHttp\Psr7\Utils; -use PhpTuf\ComposerStager\Internal\Path\Factory\PathFactory; +use PhpTuf\ComposerStager\API\Path\Factory\PathFactoryInterface; use Psr\Http\Message\RequestInterface; use Symfony\Component\Filesystem\Filesystem; @@ -182,7 +181,7 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase { $this->container->get('event_dispatcher'), $this->container->get('tempstore.shared'), $this->container->get('datetime.time'), - new PathFactory(), + $this->container->get(PathFactoryInterface::class), $this->container->get(FailureMarker::class) ); } @@ -443,27 +442,6 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase { $this->assertValidationResultsEqual($expected_results, $event->getResults(), NULL, $stage_dir); } - /** - * Creates a StageEventException from an array of validation results. - * - * @param \Drupal\package_manager\ValidationResult[] $expected_results - * The validation results. Note that only errors will be added to the event; - * warnings will be ignored. - * @param string $event_class - * (optional) The event which raised the exception. Defaults to - * PreCreateEvent. - * @param \Drupal\package_manager\StageBase $stage - * (optional) The stage which caused the exception. - * - * @return \Drupal\package_manager\Exception\StageEventException - * An exception with the given validation results. - */ - protected function createStageEventExceptionFromResults(array $expected_results, string $event_class = PreCreateEvent::class, StageBase $stage = NULL): StageEventException { - $event = new $event_class($stage ?? $this->createStage(), []); - array_walk($expected_results, $event->addResult(...)); - return new StageEventException($event); - } - } /** diff --git a/package_manager/tests/src/Kernel/PathExcluder/GitExcluderTest.php b/package_manager/tests/src/Kernel/PathExcluder/GitExcluderTest.php index a102694689..7cd2638892 100644 --- a/package_manager/tests/src/Kernel/PathExcluder/GitExcluderTest.php +++ b/package_manager/tests/src/Kernel/PathExcluder/GitExcluderTest.php @@ -74,7 +74,7 @@ class GitExcluderTest extends PackageManagerKernelTestBase { 'modules/example/.git', ]; foreach ($excluded_paths as $excluded_path) { - $this->assertContains($excluded_path, $beginner_args[0][2]->getAll()); + $this->assertContains($excluded_path, $beginner_args[0][2]); } $not_excluded_paths = [ 'modules/contrib/package_known_to_composer_removed_later/.git', @@ -82,7 +82,7 @@ class GitExcluderTest extends PackageManagerKernelTestBase { 'different_installer_path/package_known_to_composer/.git', ]; foreach ($not_excluded_paths as $not_excluded_path) { - $this->assertNotContains($not_excluded_path, $beginner_args[0][2]->getAll()); + $this->assertNotContains($not_excluded_path, $beginner_args[0][2]); } } @@ -130,9 +130,9 @@ class GitExcluderTest extends PackageManagerKernelTestBase { // new .git folder in stage directory that either composer is aware of it or // the developer knows what they are doing. foreach ($excluded_paths as $excluded_path) { - $this->assertContains($excluded_path, $committer_args[0][2]->getAll()); + $this->assertContains($excluded_path, $committer_args[0][2]); } - $this->assertNotContains('modules/unknown_to_composer_in_stage/.git', $committer_args[0][2]->getAll()); + $this->assertNotContains('modules/unknown_to_composer_in_stage/.git', $committer_args[0][2]); } } diff --git a/package_manager/tests/src/Kernel/StageBaseTest.php b/package_manager/tests/src/Kernel/StageBaseTest.php index 2911f5d937..64a0918fd9 100644 --- a/package_manager/tests/src/Kernel/StageBaseTest.php +++ b/package_manager/tests/src/Kernel/StageBaseTest.php @@ -228,7 +228,6 @@ class StageBaseTest extends PackageManagerKernelTestBase { // simulate an attempt to destroy the stage while it's being applied, for // testing purposes. $event->stage->destroy($force); - // @see \PhpTuf\ComposerStager\Internal\Precondition\Service\StagingDirDoesNotExist LoggingCommitter::setException( new PreconditionException( $this->prophesize(PreconditionInterface::class)->reveal(), @@ -658,7 +657,7 @@ class StageBaseTest extends PackageManagerKernelTestBase { // the event. $asserted = FALSE; $assert_excluded = function (object $event) use (&$asserted): void { - $this->assertContains('exclude/me', $event->getExcludedPaths()); + $this->assertContains('exclude/me', $event->excludedPaths->getAll()); // Use this to confirm that this listener was actually called. $asserted = TRUE; }; @@ -683,9 +682,7 @@ class StageBaseTest extends PackageManagerKernelTestBase { $committer = $this->container->get('package_manager.committer'); $committer_args = $committer->getInvocationArguments(); $this->assertCount(1, $committer_args); - /** @var \PhpTuf\ComposerStager\API\Path\Value\PathListInterface $path_list */ - $path_list = $committer_args[0][2]; - $this->assertContains('PACKAGE_MANAGER_FAILURE.yml', $path_list->getAll()); + $this->assertContains('PACKAGE_MANAGER_FAILURE.yml', $committer_args[0][2]); } /** diff --git a/package_manager/tests/src/Kernel/StageEventsTest.php b/package_manager/tests/src/Kernel/StageEventsTest.php index 1ba4a48f35..474d94ed46 100644 --- a/package_manager/tests/src/Kernel/StageEventsTest.php +++ b/package_manager/tests/src/Kernel/StageEventsTest.php @@ -18,6 +18,7 @@ use Drupal\package_manager\Event\StageEvent; use Drupal\package_manager\Event\StatusCheckEvent; use Drupal\package_manager\Exception\StageEventException; use Drupal\package_manager\ValidationResult; +use PhpTuf\ComposerStager\API\Path\Value\PathListInterface; use Symfony\Component\EventDispatcher\EventSubscriberInterface; /** @@ -168,16 +169,17 @@ class StageEventsTest extends PackageManagerKernelTestBase implements EventSubsc $warning = ValidationResult::createWarning([ t('The path ahead is scary...'), ]); + $excluded_paths = $this->createMock(PathListInterface::class); // Status check events can accept both errors and warnings. - $event = new StatusCheckEvent($stage, NULL); + $event = new StatusCheckEvent($stage, $excluded_paths); $event->addResult($error); $event->addResult($warning); $this->assertSame([$error, $warning], $event->getResults()); // Other stage events will accept errors, but throw an exception if you try // to add a warning. - $event = new PreCreateEvent($stage, []); + $event = new PreCreateEvent($stage, $excluded_paths); $event->addResult($error); $this->assertSame([$error], $event->getResults()); $this->expectException(\InvalidArgumentException::class); diff --git a/package_manager/tests/src/Kernel/StatusCheckTraitTest.php b/package_manager/tests/src/Kernel/StatusCheckTraitTest.php index ba6e71e4a9..58e83e945b 100644 --- a/package_manager/tests/src/Kernel/StatusCheckTraitTest.php +++ b/package_manager/tests/src/Kernel/StatusCheckTraitTest.php @@ -28,7 +28,7 @@ class StatusCheckTraitTest extends PackageManagerKernelTestBase { $status_check_called = FALSE; $this->addEventTestListener(function (StatusCheckEvent $event) use (&$status_check_called): void { - $this->assertContains('/junk/drawer', $event->getExcludedPaths()); + $this->assertContains('/junk/drawer', $event->excludedPaths->getAll()); $status_check_called = TRUE; }, StatusCheckEvent::class); $this->runStatusCheck($this->createStage(), $this->container->get('event_dispatcher')); @@ -39,14 +39,23 @@ class StatusCheckTraitTest extends PackageManagerKernelTestBase { * Tests that any error will be added to the status check event. */ public function testNoErrorIfPathsToExcludeCannotBeCollected(): void { - $this->addEventTestListener(function (): void { - throw new \Exception('Not a chance, friend.'); - }, CollectPathsToExcludeEvent::class); - $result = ValidationResult::createError( - [t('Not a chance, friend.')], - t('Unable to collect the paths to exclude.'), - ); - $this->assertStatusCheckResults([$result]); + $e = new \Exception('Not a chance, friend.'); + + $listener = function () use ($e): never { + throw $e; + }; + $this->addEventTestListener($listener, CollectPathsToExcludeEvent::class); + + $excluded_paths_are_null = FALSE; + $listener = function (StatusCheckEvent $event) use (&$excluded_paths_are_null): void { + $excluded_paths_are_null = is_null($event->excludedPaths); + }; + $this->addEventTestListener($listener, StatusCheckEvent::class); + + $this->assertStatusCheckResults([ + ValidationResult::createErrorFromThrowable($e), + ]); + $this->assertTrue($excluded_paths_are_null); } } diff --git a/package_manager/tests/src/Traits/FixtureManipulatorTrait.php b/package_manager/tests/src/Traits/FixtureManipulatorTrait.php index f8428cc5dc..c65e670c5a 100644 --- a/package_manager/tests/src/Traits/FixtureManipulatorTrait.php +++ b/package_manager/tests/src/Traits/FixtureManipulatorTrait.php @@ -2,6 +2,8 @@ namespace Drupal\Tests\package_manager\Traits; +use Drupal\fixture_manipulator\StageFixtureManipulator; + /** * A trait for common fixture manipulator functions. */ @@ -14,7 +16,7 @@ trait FixtureManipulatorTrait { * The stage fixture manipulator service. */ protected function getStageFixtureManipulator() { - return $this->container->get('fixture_manipulator.stage_manipulator'); + return $this->container->get(StageFixtureManipulator::class); } } diff --git a/package_manager/tests/src/Unit/StageNotInActiveValidatorTest.php b/package_manager/tests/src/Unit/StageNotInActiveValidatorTest.php index eb289d96b6..5febb206a7 100644 --- a/package_manager/tests/src/Unit/StageNotInActiveValidatorTest.php +++ b/package_manager/tests/src/Unit/StageNotInActiveValidatorTest.php @@ -11,6 +11,7 @@ use Drupal\package_manager\ValidationResult; use Drupal\package_manager\Validator\StageNotInActiveValidator; use Drupal\Tests\package_manager\Traits\ValidationTestTrait; use Drupal\Tests\UnitTestCase; +use PhpTuf\ComposerStager\API\Path\Value\PathListInterface; use Symfony\Component\Filesystem\Path; /** @@ -43,7 +44,7 @@ class StageNotInActiveValidatorTest extends UnitTestCase { $stage_not_in_active_validator = new StageNotInActiveValidator($path_locator); $stage_not_in_active_validator->setStringTranslation($this->getStringTranslationStub()); - $event = new PreCreateEvent($stage, ['some/path']); + $event = new PreCreateEvent($stage, $this->createMock(PathListInterface::class)); $stage_not_in_active_validator->validate($event); $this->assertValidationResultsEqual($expected, $event->getResults(), $path_locator); } diff --git a/package_manager/tests/src/Unit/StatusCheckEventTest.php b/package_manager/tests/src/Unit/StatusCheckEventTest.php deleted file mode 100644 index fd5777f464..0000000000 --- a/package_manager/tests/src/Unit/StatusCheckEventTest.php +++ /dev/null @@ -1,30 +0,0 @@ -<?php - -declare(strict_types = 1); - -namespace Drupal\Tests\package_manager\Unit; - -use Drupal\package_manager\Event\StatusCheckEvent; -use Drupal\package_manager\StageBase; -use Drupal\Tests\UnitTestCase; - -/** - * @coversDefaultClass \Drupal\package_manager\Event\StatusCheckEvent - * @group package_manager - */ -class StatusCheckEventTest extends UnitTestCase { - - /** - * @covers ::getExcludedPaths - */ - public function testNoPathsNoErrorException(): void { - $event = new StatusCheckEvent( - $this->prophesize(StageBase::class)->reveal(), - NULL - ); - $this->expectException(\LogicException::class); - $this->expectExceptionMessage('$paths_to_exclude should only be NULL if the error that caused the paths to not be collected was added to the status check event.'); - $event->getExcludedPaths(); - } - -} diff --git a/scripts/commit-code-check.sh b/scripts/commit-code-check.sh index 91d0b7bc47..dddeb82d0a 100755 --- a/scripts/commit-code-check.sh +++ b/scripts/commit-code-check.sh @@ -91,23 +91,30 @@ print_results() { fi } -print_title "[1/4] PHPCS" +print_title "[1/5] PHPCS" $CORE_DIRECTORY/vendor/bin/phpcs $MODULE_DIRECTORY -ps --standard="$CORE_DIRECTORY/core/phpcs.xml.dist" print_results $? "PHPCS" -print_title "[2/4] PHPStan" +print_title "[2/5] PHPStan" php -d apc.enabled=0 -d apc.enable_cli=0 $CORE_DIRECTORY/vendor/bin/phpstan analyze --no-progress --configuration="$MODULE_DIRECTORY/phpstan.neon.dist" $MODULE_DIRECTORY print_results $? "PHPStan" -print_title "[3/4] CSpell" +print_title "[3/5] CSpell" cd $CORE_DIRECTORY/core && yarn run -s spellcheck --no-progress --root $MODULE_DIRECTORY -c .cspell.json "**" && cd - print_results $? "CSpell" -print_title "[4/4] eslint:yaml" +print_title "[4/5] eslint:yaml" cd $CORE_DIRECTORY/core && yarn eslint --resolve-plugins-relative-to . --ext .yml $MODULE_DIRECTORY && cd - print_results $? "eslint:yaml" print_separator +print_title "[5/5] Composer Stager internals are not used" +grep -r -n 'ComposerStager\\Internal' $MODULE_DIRECTORY +if [ $? == 0 ]; then + print_results 1 "\\PhpTuf\\ComposerStager\\Internal namespace is not used" +fi +print_separator + if [[ "$FINAL_STATUS" == "1" ]]; then printf "${red}Drupal code quality checks failed.${reset}" if [[ "$DRUPALCI" == "1" ]]; then diff --git a/tests/src/Kernel/ConsoleUpdateStageTest.php b/tests/src/Kernel/ConsoleUpdateStageTest.php index 580948dc43..f6eee1208e 100644 --- a/tests/src/Kernel/ConsoleUpdateStageTest.php +++ b/tests/src/Kernel/ConsoleUpdateStageTest.php @@ -16,6 +16,7 @@ use Drupal\package_manager\Event\PostDestroyEvent; use Drupal\package_manager\Event\PostRequireEvent; use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Event\PreDestroyEvent; +use Drupal\package_manager\Event\PreOperationStageEvent; use Drupal\package_manager\Event\PreRequireEvent; use Drupal\package_manager\Event\PreCreateEvent; use Drupal\package_manager\Event\StageEvent; @@ -322,17 +323,14 @@ END; t('Destroy the stage!'), ]); - $exception = $this->createStageEventExceptionFromResults([$error], $event_class, $stage); - TestSubscriber1::setTestResult($exception->event->getResults(), $event_class); + TestSubscriber1::setTestResult([$error], $event_class); + $exception = $this->createStageEventExceptionFromResults([$error]); } else { - /** @var \Throwable $exception */ $exception = new $exception_class('Destroy the stage!'); TestSubscriber1::setException($exception, $event_class); } - $expected_log_message = $exception->getMessage(); - // Ensure that nothing has been logged yet. $this->assertEmpty($cron_logger->records); $this->assertEmpty($this->logger->records); @@ -340,7 +338,7 @@ END; $this->assertTrue($stage->isAvailable()); $this->runConsoleUpdateStage(); - $logged_by_stage = $this->logger->hasRecord($expected_log_message, (string) RfcLogLevel::ERROR); + $logged_by_stage = $this->logger->hasRecord($exception->getMessage(), (string) RfcLogLevel::ERROR); // To check if the exception was logged by the main cron service, we need // to set up a special predicate, because exceptions logged by cron are // always formatted in a particular way that we don't control. But the @@ -353,7 +351,6 @@ END; } return FALSE; }; - $logged_by_cron = $cron_logger->hasRecordThatPasses($predicate, (string) RfcLogLevel::ERROR); // If a pre-destroy event flags a validation error, it's handled like any @@ -542,8 +539,9 @@ END; $error = ValidationResult::createError([ t('Error while updating!'), ]); - $exception = $this->createStageEventExceptionFromResults([$error], $event_class, $this->container->get(ConsoleUpdateStage::class)); - TestSubscriber1::setTestResult($exception->event->getResults(), $event_class); + TestSubscriber1::setTestResult([$error], $event_class); + $exception_message = $this->createStageEventExceptionFromResults([$error]) + ->getMessage(); $this->runConsoleUpdateStage(); @@ -554,7 +552,7 @@ END; $expected_body = <<<END Drupal core failed to update automatically from 9.8.0 to 9.8.2. The following error was logged: -{$exception->getMessage()} +{$exception_message} No immediate action is needed, but it is recommended that you visit $url to perform the update, or at least check that everything still looks good. @@ -584,7 +582,8 @@ END; t('Error while updating!'), ]); TestSubscriber1::setTestResult([$error], $event_class); - $exception = $this->createStageEventExceptionFromResults([$error], $event_class, $this->container->get(ConsoleUpdateStage::class)); + $exception_message = $this->createStageEventExceptionFromResults([$error]) + ->getMessage(); $this->runConsoleUpdateStage(); @@ -595,7 +594,7 @@ END; $expected_body = <<<END Drupal core failed to update automatically from 9.8.0 to 9.8.1. The following error was logged: -{$exception->getMessage()} +{$exception_message} Your site is running an insecure version of Drupal and should be updated as soon as possible. Visit $url to perform the update. @@ -750,4 +749,25 @@ END; $this->assertNull($this->container->get('state')->get('common_test.cron')); } + /** + * Creates a StageEventException from a particular set of validation results. + * + * @param \Drupal\package_manager\ValidationResult[] $results + * The validation results associated with the exception. + * + * @return \Drupal\package_manager\Exception\StageEventException + * A stage exception carrying the given validation results. + */ + private function createStageEventExceptionFromResults(array $results): StageEventException { + // Different stage events are constructed with different arguments. Rather + // than encode all that here, just create a generic stage event which + // can carry validation results, and use it to generate the exception. + $event = new class ( + $this->container->get(ConsoleUpdateStage::class), + ) extends PreOperationStageEvent {}; + + array_walk($results, $event->addResult(...)); + return new StageEventException($event); + } + } diff --git a/tests/src/Traits/ComposerStagerTestTrait.php b/tests/src/Traits/ComposerStagerTestTrait.php index 8350fc094d..af5775f597 100644 --- a/tests/src/Traits/ComposerStagerTestTrait.php +++ b/tests/src/Traits/ComposerStagerTestTrait.php @@ -32,6 +32,7 @@ trait ComposerStagerTestTrait { * {@see \PhpTuf\ComposerStager\API\Translation\Service\DomainOptionsInterface}. * * @return \PhpTuf\ComposerStager\API\Translation\Value\TranslatableInterface + * A message that can be translated by Composer Stager. */ protected function createComposeStagerMessage( string $message, -- GitLab