From 5e2745b58454143a8324ce6ac9f826c04695e053 Mon Sep 17 00:00:00 2001 From: "yash.rode" <yash.rode@3685174.no-reply.drupal.org> Date: Tue, 4 Oct 2022 18:13:48 +0000 Subject: [PATCH] Issue #3310914 by yash.rode, phenaproxima, tedbow: LockFileValidator should listen to StatusCheckEvent --- .../src/Validator/LockFileValidator.php | 7 +++++ .../src/Kernel/LockFileValidatorTest.php | 30 +++++++++++++++---- .../Kernel/PackageManagerKernelTestBase.php | 6 +++- 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/package_manager/src/Validator/LockFileValidator.php b/package_manager/src/Validator/LockFileValidator.php index 9ab722607d..6f2f7facdc 100644 --- a/package_manager/src/Validator/LockFileValidator.php +++ b/package_manager/src/Validator/LockFileValidator.php @@ -10,6 +10,7 @@ use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Event\PreCreateEvent; use Drupal\package_manager\Event\PreOperationStageEvent; use Drupal\package_manager\Event\PreRequireEvent; +use Drupal\package_manager\Event\StatusCheckEvent; use Drupal\package_manager\PathLocator; /** @@ -104,6 +105,11 @@ final class LockFileValidator implements PreOperationStageValidatorInterface { * {@inheritdoc} */ public function validateStagePreOperation(PreOperationStageEvent $event): void { + // Early return if the stage is not already created. + if ($event instanceof StatusCheckEvent && $event->getStage()->isAvailable()) { + return; + } + // Ensure we can get a current hash of the lock file. $active_hash = $this->getLockFileHash($this->pathLocator->getProjectRoot()); if (empty($active_hash)) { @@ -152,6 +158,7 @@ final class LockFileValidator implements PreOperationStageValidatorInterface { PreCreateEvent::class => 'storeHash', PreRequireEvent::class => 'validateStagePreOperation', PreApplyEvent::class => 'validateStagePreOperation', + StatusCheckEvent::class => 'validateStagePreOperation', PostDestroyEvent::class => 'deleteHash', ]; } diff --git a/package_manager/tests/src/Kernel/LockFileValidatorTest.php b/package_manager/tests/src/Kernel/LockFileValidatorTest.php index bd68bd57fe..f8f4724444 100644 --- a/package_manager/tests/src/Kernel/LockFileValidatorTest.php +++ b/package_manager/tests/src/Kernel/LockFileValidatorTest.php @@ -41,7 +41,10 @@ class LockFileValidatorTest extends PackageManagerKernelTestBase { unlink($this->activeDir . '/composer.lock'); $no_lock = ValidationResult::createError(['Could not hash the active lock file.']); - $this->assertResults([$no_lock], PreCreateEvent::class); + $stage = $this->assertResults([$no_lock], PreCreateEvent::class); + // The stage was not created successfully, so the status check should be + // clear. + $this->assertStatusCheckResults([], $stage); } /** @@ -75,7 +78,9 @@ class LockFileValidatorTest extends PackageManagerKernelTestBase { $result = ValidationResult::createError([ 'Unexpected changes were detected in composer.lock, which indicates that other Composer operations were performed since this Package Manager operation started. This can put the code base into an unreliable state and therefore is not allowed.', ]); - $this->assertResults([$result], $event_class); + $stage = $this->assertResults([$result], $event_class); + // A status check should agree that there is an error here. + $this->assertStatusCheckResults([$result], $stage); } /** @@ -94,7 +99,9 @@ class LockFileValidatorTest extends PackageManagerKernelTestBase { $result = ValidationResult::createError([ 'Could not hash the active lock file.', ]); - $this->assertResults([$result], $event_class); + $stage = $this->assertResults([$result], $event_class); + // A status check should agree that there is an error here. + $this->assertStatusCheckResults([$result], $stage); } /** @@ -116,7 +123,9 @@ class LockFileValidatorTest extends PackageManagerKernelTestBase { $result = ValidationResult::createError([ 'Could not retrieve stored hash of the active lock file.', ]); - $this->assertResults([$result], $event_class); + $stage = $this->assertResults([$result], $event_class); + // A status check should agree that there is an error here. + $this->assertStatusCheckResults([$result], $stage); } /** @@ -129,7 +138,18 @@ class LockFileValidatorTest extends PackageManagerKernelTestBase { $result = ValidationResult::createError([ 'There are no pending Composer operations.', ]); - $this->assertResults([$result], PreApplyEvent::class); + $stage = $this->assertResults([$result], PreApplyEvent::class); + // A status check shouldn't produce raise any errors, because it's only + // during pre-apply that we care if there are any pending Composer + // operations. + $this->assertStatusCheckResults([], $stage); + } + + /** + * Tests StatusCheckEvent when the stage is available. + */ + public function testStatusCheckAvailableStage():void { + $this->assertStatusCheckResults([]); } /** diff --git a/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php b/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php index a46ba08a35..ebcc20a7a5 100644 --- a/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php +++ b/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php @@ -148,8 +148,11 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase { * @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. + * + * @return \Drupal\package_manager\Stage + * The stage that was used to collect the validation results. */ - protected function assertResults(array $expected_results, string $event_class = NULL): void { + protected function assertResults(array $expected_results, string $event_class = NULL): Stage { $stage = $this->createStage(); try { @@ -170,6 +173,7 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase { $this->assertNotEmpty($event_class); $this->assertInstanceOf($event_class, $e->event); } + return $stage; } /** -- GitLab