From 7f45a8ab8a5797e93503a54dd2744dcbbc568bcb Mon Sep 17 00:00:00 2001
From: phenaproxima <phenaproxima@205645.no-reply.drupal.org>
Date: Fri, 23 Sep 2022 15:49:59 +0000
Subject: [PATCH] Issue #3298444 by phenaproxima, omkar.podey: Symlink
validator should delegate to Composer Stager's precondition
---
package_manager/package_manager.services.yml | 4 +-
.../src/PathExcluder/GitExcluder.php | 3 +-
.../src/Validator/SymlinkValidator.php | 158 +++++++--------
.../Kernel/PathExcluder/GitExcluderTest.php | 24 ---
.../tests/src/Kernel/SymlinkValidatorTest.php | 183 +++++-------------
5 files changed, 121 insertions(+), 251 deletions(-)
diff --git a/package_manager/package_manager.services.yml b/package_manager/package_manager.services.yml
index cb63a5f1b1..ad96bab6dc 100644
--- a/package_manager/package_manager.services.yml
+++ b/package_manager/package_manager.services.yml
@@ -105,8 +105,10 @@ services:
package_manager.validator.symlink:
class: Drupal\package_manager\Validator\SymlinkValidator
arguments:
- - '@module_handler'
- '@package_manager.path_locator'
+ - '@PhpTuf\ComposerStager\Domain\Service\Precondition\CodebaseContainsNoSymlinksInterface'
+ - '@PhpTuf\ComposerStager\Infrastructure\Factory\Path\PathFactoryInterface'
+ - '@module_handler'
tags:
- { name: event_subscriber }
package_manager.validator.duplicate_info_file:
diff --git a/package_manager/src/PathExcluder/GitExcluder.php b/package_manager/src/PathExcluder/GitExcluder.php
index c713ddd03e..5ec84d0680 100644
--- a/package_manager/src/PathExcluder/GitExcluder.php
+++ b/package_manager/src/PathExcluder/GitExcluder.php
@@ -56,8 +56,7 @@ final class GitExcluder implements EventSubscriberInterface {
->directories()
->name('.git')
->ignoreVCS(FALSE)
- ->ignoreDotFiles(FALSE)
- ->ignoreUnreadableDirs();
+ ->ignoreDotFiles(FALSE);
$paths = [];
foreach ($finder as $git_directory) {
diff --git a/package_manager/src/Validator/SymlinkValidator.php b/package_manager/src/Validator/SymlinkValidator.php
index 40eab73939..b8b5550826 100644
--- a/package_manager/src/Validator/SymlinkValidator.php
+++ b/package_manager/src/Validator/SymlinkValidator.php
@@ -10,7 +10,9 @@ use Drupal\package_manager\Event\PreCreateEvent;
use Drupal\package_manager\Event\PreOperationStageEvent;
use Drupal\package_manager\Event\StatusCheckEvent;
use Drupal\package_manager\PathLocator;
-use Symfony\Component\Finder\Finder;
+use PhpTuf\ComposerStager\Domain\Exception\PreconditionException;
+use PhpTuf\ComposerStager\Domain\Service\Precondition\CodebaseContainsNoSymlinksInterface;
+use PhpTuf\ComposerStager\Infrastructure\Factory\Path\PathFactoryInterface;
/**
* Flags errors if the project root or staging area contain symbolic links.
@@ -27,13 +29,6 @@ class SymlinkValidator implements PreOperationStageValidatorInterface {
use StringTranslationTrait;
- /**
- * The module handler service.
- *
- * @var \Drupal\Core\Extension\ModuleHandlerInterface
- */
- protected $moduleHandler;
-
/**
* The path locator service.
*
@@ -42,77 +37,85 @@ class SymlinkValidator implements PreOperationStageValidatorInterface {
protected $pathLocator;
/**
- * Constructs a SymlinkValidator object.
+ * The Composer Stager precondition that this validator wraps.
*
- * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler
- * The module handler service.
- * @param \Drupal\package_manager\PathLocator $path_locator
- * The path locator service.
+ * @var \PhpTuf\ComposerStager\Domain\Service\Precondition\CodebaseContainsNoSymlinksInterface
*/
- public function __construct(ModuleHandlerInterface $module_handler, PathLocator $path_locator) {
- $this->moduleHandler = $module_handler;
- $this->pathLocator = $path_locator;
- }
+ protected $precondition;
/**
- * {@inheritdoc}
+ * The path factory service.
+ *
+ * @var \PhpTuf\ComposerStager\Infrastructure\Factory\Path\PathFactoryInterface
*/
- public function validateStagePreOperation(PreOperationStageEvent $event): void {
- $dir = $this->pathLocator->getProjectRoot();
-
- if ($this->hasLinks($dir)) {
- $this->addError('Symbolic links were found in the active directory, which are not supported at this time.', $event);
- }
- }
+ protected $pathFactory;
/**
- * Checks if the staging area has any symbolic links.
+ * The module handler service.
*
- * @param \Drupal\package_manager\Event\PreApplyEvent $event
- * The event object.
+ * @var \Drupal\Core\Extension\ModuleHandlerInterface
*/
- public function preApply(PreApplyEvent $event): void {
- $dir = $event->getStage()->getStageDirectory();
-
- if ($this->hasLinks($dir)) {
- $this->addError('Symbolic links were found in the staging area, which are not supported at this time.', $event);
- }
- }
+ protected $moduleHandler;
/**
- * Recursively checks if a directory has any symbolic links.
- *
- * @param string $dir
- * The path of the directory to check.
+ * Constructs a SymlinkValidator object.
*
- * @return bool
- * TRUE if the directory contains any symbolic links, FALSE otherwise.
+ * @param \Drupal\package_manager\PathLocator $path_locator
+ * The path locator service.
+ * @param \PhpTuf\ComposerStager\Domain\Service\Precondition\CodebaseContainsNoSymlinksInterface $precondition
+ * The Composer Stager precondition that this validator wraps.
+ * @param \PhpTuf\ComposerStager\Infrastructure\Factory\Path\PathFactoryInterface $path_factory
+ * The path factory service.
+ * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler
+ * The module handler service.
*/
- protected function hasLinks(string $dir): bool {
- // Finder::filter() explicitly requires a closure, so create one from
- // ::isLink() so that we can still override it for testing purposes.
- $is_link = \Closure::fromCallable([$this, 'isLink']);
-
- // Finder::hasResults() is more efficient than count() because it will
- // return early if there is a match.
- return Finder::create()
- ->in($dir)
- ->filter($is_link)
- ->ignoreUnreadableDirs()
- ->hasResults();
+ public function __construct(PathLocator $path_locator, CodebaseContainsNoSymlinksInterface $precondition, PathFactoryInterface $path_factory, ModuleHandlerInterface $module_handler) {
+ $this->pathLocator = $path_locator;
+ $this->precondition = $precondition;
+ $this->pathFactory = $path_factory;
+ $this->moduleHandler = $module_handler;
}
/**
- * Checks if a file or directory is a symbolic link.
- *
- * @param \SplFileInfo $file
- * A value object for the file or directory.
- *
- * @return bool
- * TRUE if the file or directory is a symbolic link, FALSE otherwise.
+ * {@inheritdoc}
*/
- protected function isLink(\SplFileInfo $file): bool {
- return $file->isLink();
+ public function validateStagePreOperation(PreOperationStageEvent $event): void {
+ $active_dir = $this->pathFactory->create($this->pathLocator->getProjectRoot());
+
+ // The precondition requires us to pass both an active and stage directory,
+ // so if the stage hasn't been created or claimed yet, use the directory
+ // that contains this file, which contains only a few files and no symlinks,
+ // as the stage directory. The precondition itself doesn't care if the
+ // directory actually exists or not.
+ try {
+ $stage_dir = $event->getStage()->getStageDirectory();
+ }
+ catch (\LogicException $e) {
+ $stage_dir = __DIR__;
+ }
+ $stage_dir = $this->pathFactory->create($stage_dir);
+
+ try {
+ $this->precondition->assertIsFulfilled($active_dir, $stage_dir);
+ }
+ catch (PreconditionException $e) {
+ $message = $e->getMessage();
+
+ // If the Help module is enabled, append a link to Package Manager's help
+ // page.
+ // @see package_manager_help()
+ if ($this->moduleHandler->moduleExists('help')) {
+ $url = Url::fromRoute('help.page', ['name' => 'package_manager'])
+ ->setOption('fragment', 'package-manager-faq-symlinks-found')
+ ->toString();
+
+ $message = $this->t('@message See <a href=":package-manager-help">the help page</a> for information on how to resolve the problem.', [
+ '@message' => $message,
+ ':package-manager-help' => $url,
+ ]);
+ }
+ $event->addError([$message]);
+ }
}
/**
@@ -121,38 +124,9 @@ class SymlinkValidator implements PreOperationStageValidatorInterface {
public static function getSubscribedEvents() {
return [
PreCreateEvent::class => 'validateStagePreOperation',
+ PreApplyEvent::class => 'validateStagePreOperation',
StatusCheckEvent::class => 'validateStagePreOperation',
- PreApplyEvent::class => [
- ['validateStagePreOperation'],
- ['preApply'],
- ],
];
}
- /**
- * Adds a validation error to a given event.
- *
- * @param string $message
- * The error message. If the Help module is enabled, a link to Package
- * Manager's help page will be appended.
- * @param \Drupal\package_manager\Event\PreApplyEvent|\Drupal\package_manager\Event\PreOperationStageEvent $event
- * The event to add the error to.
- *
- * @see package_manager_help()
- */
- protected function addError(string $message, $event): void {
- if ($this->moduleHandler->moduleExists('help')) {
- $url = Url::fromRoute('help.page', ['name' => 'package_manager'])
- ->setOption('fragment', 'package-manager-faq-symlinks-found')
- ->toString();
-
- $message = $this->t('@message See <a href=":package-manager-help">the help page</a> for information on how to resolve the problem.', [
- '@message' => $message,
- ':package-manager-help' => $url,
- ]);
- }
-
- $event->addError([$message]);
- }
-
}
diff --git a/package_manager/tests/src/Kernel/PathExcluder/GitExcluderTest.php b/package_manager/tests/src/Kernel/PathExcluder/GitExcluderTest.php
index 0966b1fa55..4da701a076 100644
--- a/package_manager/tests/src/Kernel/PathExcluder/GitExcluderTest.php
+++ b/package_manager/tests/src/Kernel/PathExcluder/GitExcluderTest.php
@@ -2,7 +2,6 @@
namespace Drupal\Tests\package_manager\Kernel\PathExcluder;
-use Drupal\package_manager_bypass\Beginner;
use Drupal\Tests\package_manager\Kernel\PackageManagerKernelTestBase;
/**
@@ -23,29 +22,6 @@ class GitExcluderTest extends PackageManagerKernelTestBase {
parent::setUp();
}
- /**
- * Tests that unreadable directories are ignored by the event subscriber.
- */
- public function testUnreadableDirectoriesAreIgnored(): void {
- $active_dir = $this->container->get('package_manager.path_locator')
- ->getProjectRoot();
-
- // Create an unreadable directory within the active directory, which will
- // raise an exception as the event subscriber tries to scan for .git
- // directories...unless unreadable directories are being ignored, as they
- // should be.
- $unreadable_dir = $active_dir . '/unreadable';
- mkdir($unreadable_dir, 0000);
- $this->assertDirectoryIsNotReadable($unreadable_dir);
-
- // Don't mirror the active directory into the virtual staging area, since
- // the active directory contains an unreadable directory which will cause
- // an exception.
- Beginner::setFixturePath(NULL);
-
- $this->createStage()->create();
- }
-
/**
* Tests that Git directories are excluded from staging operations.
*/
diff --git a/package_manager/tests/src/Kernel/SymlinkValidatorTest.php b/package_manager/tests/src/Kernel/SymlinkValidatorTest.php
index fdda7d469e..acf84ca7f1 100644
--- a/package_manager/tests/src/Kernel/SymlinkValidatorTest.php
+++ b/package_manager/tests/src/Kernel/SymlinkValidatorTest.php
@@ -5,9 +5,10 @@ namespace Drupal\Tests\package_manager\Kernel;
use Drupal\Core\DependencyInjection\ContainerBuilder;
use Drupal\Core\Url;
use Drupal\package_manager\Event\PreCreateEvent;
-use Drupal\package_manager\Exception\StageValidationException;
use Drupal\package_manager\ValidationResult;
-use Drupal\package_manager\Validator\SymlinkValidator;
+use PhpTuf\ComposerStager\Domain\Exception\PreconditionException;
+use PhpTuf\ComposerStager\Domain\Service\Precondition\CodebaseContainsNoSymlinksInterface;
+use Prophecy\Argument;
/**
* @covers \Drupal\package_manager\Validator\SymlinkValidator
@@ -17,144 +18,75 @@ use Drupal\package_manager\Validator\SymlinkValidator;
class SymlinkValidatorTest extends PackageManagerKernelTestBase {
/**
- * {@inheritdoc}
+ * The mocked precondition that checks for symlinks.
+ *
+ * @var \PhpTuf\ComposerStager\Domain\Service\Precondition\CodebaseContainsNoSymlinksInterface|\Prophecy\Prophecy\ObjectProphecy
*/
- public function register(ContainerBuilder $container) {
- parent::register($container);
-
- $container->getDefinition('package_manager.validator.symlink')
- ->setClass(TestSymlinkValidator::class);
- }
+ private $precondition;
/**
- * Tests that a symlink in the project root raises an error.
+ * {@inheritdoc}
*/
- public function testSymlinkInProjectRoot(): void {
- $result = ValidationResult::createError([
- 'Symbolic links were found in the active directory, which are not supported at this time.',
- ]);
-
- $active_dir = $this->container->get('package_manager.path_locator')
- ->getProjectRoot();
- // @see \Drupal\Tests\package_manager\Kernel\TestSymlinkValidator::isLink()
- touch($active_dir . '/modules/a_link');
- $this->assertStatusCheckResults([$result]);
- $this->assertResults([$result], PreCreateEvent::class);
-
- $this->enableModules(['help']);
- $this->assertStatusCheckResults($this->addHelpTextToResults([$result]));
- $this->assertResultsWithHelp([$result], PreCreateEvent::class);
+ protected function setUp(): void {
+ $this->precondition = $this->prophesize(CodebaseContainsNoSymlinksInterface::class);
+ parent::setUp();
}
/**
- * Tests that a symlink in the staging area raises an error.
- *
- * @dataProvider providerHelpEnabledOrNot
- */
- public function testSymlinkInStagingArea(bool $enable_help): void {
- $expected_results = [ValidationResult::createError([
- 'Symbolic links were found in the staging area, which are not supported at this time.',
- ]),
- ];
-
- if ($enable_help) {
- $this->enableModules(['help']);
- $expected_results = $this->addHelpTextToResults($expected_results);
- }
-
- $stage = $this->createStage();
- $stage->create();
- $stage->require(['composer/semver:^3']);
-
- // @see \Drupal\Tests\package_manager\Kernel\TestSymlinkValidator::isLink()
- touch($stage->getStageDirectory() . '/modules/a_link');
-
- try {
- $stage->apply();
- $this->fail('Expected a validation error.');
- }
- catch (StageValidationException $e) {
- $this->assertValidationResultsEqual($expected_results, $e->getResults());
- }
- }
-
- /**
- * Tests that symlinks in the project root and staging area raise an error.
- *
- * @dataProvider providerHelpEnabledOrNot
+ * {@inheritdoc}
*/
- public function testSymlinkInProjectRootAndStagingArea(bool $enable_help): void {
- $expected_results = [
- ValidationResult::createError([
- 'Symbolic links were found in the active directory, which are not supported at this time.',
- ]),
- ValidationResult::createError([
- 'Symbolic links were found in the staging area, which are not supported at this time.',
- ]),
- ];
-
- if ($enable_help) {
- $this->enableModules(['help']);
- $expected_results = $this->addHelpTextToResults($expected_results);
- }
-
- $stage = $this->createStage();
- $stage->create();
- $stage->require(['composer/semver:^3']);
-
- $active_dir = $this->container->get('package_manager.path_locator')
- ->getProjectRoot();
- // @see \Drupal\Tests\package_manager\Kernel\TestSymlinkValidator::isLink()
- touch($active_dir . '/modules/a_link');
- touch($stage->getStageDirectory() . '/modules/a_link');
+ public function register(ContainerBuilder $container) {
+ parent::register($container);
- try {
- $stage->apply();
- $this->fail('Expected a validation error.');
- }
- catch (StageValidationException $e) {
- $this->assertValidationResultsEqual($expected_results, $e->getResults());
- }
+ $container->getDefinition('package_manager.validator.symlink')
+ ->setArgument('$precondition', $this->precondition->reveal());
}
/**
- * Data provider for test methods that test with and without the Help module.
+ * Data provider for ::testSymlink().
*
* @return array[]
* The test cases.
*/
- public function providerHelpEnabledOrNot() {
+ public function providerSymlink(): array {
return [
- 'help_module_enabled' => [TRUE],
- 'help_module_disabled' => [FALSE],
+ 'no symlinks' => [FALSE],
+ 'symlinks' => [TRUE],
];
}
/**
- * Asserts that a set of validation results link to the Package Manager help.
+ * Tests that the validator invokes Composer Stager's symlink precondition.
+ *
+ * @param bool $symlinks_exist
+ * Whether or not the precondition will detect symlinks.
*
- * @param \Drupal\package_manager\ValidationResult[] $expected_results
- * The expected validation results.
- * @param string|null $event_class
- * (optional) The class of the event which should return the results. Must
- * be passed if $expected_results is not empty.
+ * @dataProvider providerSymlink
*/
- private function assertResultsWithHelp(array $expected_results, string $event_class = NULL): void {
- $expected_results = $this->addHelpTextToResults($expected_results);
+ public function testSymlink(bool $symlinks_exist): void {
+ $arguments = Argument::cetera();
+ // The precondition should always be invoked.
+ $this->precondition->assertIsFulfilled($arguments)->shouldBeCalled();
+
+ if ($symlinks_exist) {
+ $exception = new PreconditionException($this->precondition->reveal(), 'Symlinks were found.');
+ $this->precondition->assertIsFulfilled($arguments)->willThrow($exception);
+
+ $expected_results = [
+ ValidationResult::createError([
+ $exception->getMessage(),
+ ]),
+ ];
+ }
+ else {
+ $expected_results = [];
+ }
+
$this->assertStatusCheckResults($expected_results);
- $this->assertResults($expected_results, $event_class);
- }
+ $this->assertResults($expected_results, PreCreateEvent::class);
+
+ $this->enableModules(['help']);
- /**
- * Adds help text to results messages.
- *
- * @param \Drupal\package_manager\ValidationResult[] $results
- * The expected validation results.
- *
- * @return array
- * The new results.
- */
- public function addHelpTextToResults(array $results): array {
$url = Url::fromRoute('help.page', ['name' => 'package_manager'])
->setOption('fragment', 'package-manager-faq-symlinks-found')
->toString();
@@ -164,25 +96,12 @@ class SymlinkValidatorTest extends PackageManagerKernelTestBase {
$map = function (string $message) use ($url): string {
return $message . ' See <a href="' . $url . '">the help page</a> for information on how to resolve the problem.';
};
- foreach ($results as $index => $result) {
+ foreach ($expected_results as $index => $result) {
$messages = array_map($map, $result->getMessages());
- $results[$index] = ValidationResult::createError($messages);
+ $expected_results[$index] = ValidationResult::createError($messages);
}
- return $results;
- }
-
-}
-
-/**
- * A test validator that considers anything named 'a_link' to be a symlink.
- */
-class TestSymlinkValidator extends SymlinkValidator {
-
- /**
- * {@inheritdoc}
- */
- protected function isLink(\SplFileInfo $file): bool {
- return $file->getBasename() === 'a_link' || parent::isLink($file);
+ $this->assertStatusCheckResults($expected_results);
+ $this->assertResults($expected_results, PreCreateEvent::class);
}
}
--
GitLab