From a68c4bea2a4f8531b4298c7ea020fac9cbfa009d Mon Sep 17 00:00:00 2001 From: Ted Bowman <41201-tedbow@users.noreply.drupalcode.org> Date: Tue, 20 Dec 2022 19:53:16 +0000 Subject: [PATCH] Issue #3321684 by kunal.sachdev, tedbow, Wim Leers: Most validators that subscribe to PreCreate to check readiness to update should also subscribe to PreApply --- .../tests/src/Functional/UpdaterFormTest.php | 26 ++- .../Validator/ComposerExecutableValidator.php | 2 + .../Validator/ComposerJsonExistsValidator.php | 2 + .../Validator/ComposerPatchesValidator.php | 2 + .../Validator/ComposerSettingsValidator.php | 2 + .../src/Validator/DiskSpaceValidator.php | 2 + .../Validator/EnvironmentSupportValidator.php | 2 + .../src/Validator/MultisiteValidator.php | 2 + .../src/Validator/PendingUpdatesValidator.php | 2 + .../src/Validator/SettingsValidator.php | 2 + .../Validator/WritableFileSystemValidator.php | 13 +- .../tests/src/Build/PackageUpdateTest.php | 3 + .../ComposerExecutableValidatorTest.php | 67 ++++++- .../ComposerJsonExistsValidatorTest.php | 29 +++ .../Kernel/ComposerPatchesValidatorTest.php | 53 +++++- .../Kernel/ComposerSettingsValidatorTest.php | 24 +++ .../src/Kernel/DiskSpaceValidatorTest.php | 31 ++++ .../EnvironmentSupportValidatorTest.php | 49 +++++ .../src/Kernel/MultisiteValidatorTest.php | 29 +++ .../Kernel/PendingUpdatesValidatorTest.php | 25 +++ .../src/Kernel/SettingsValidatorTest.php | 22 +++ .../WritableFileSystemValidatorTest.php | 33 ++++ src/Validator/CronServerValidator.php | 2 + src/Validator/XdebugValidator.php | 2 + tests/src/Functional/UpdaterFormTest.php | 32 +++- .../StatusCheck/CronServerValidatorTest.php | 175 ++++++++++++------ .../StatusCheck/XdebugValidatorTest.php | 42 ++++- 27 files changed, 587 insertions(+), 88 deletions(-) diff --git a/automatic_updates_extensions/tests/src/Functional/UpdaterFormTest.php b/automatic_updates_extensions/tests/src/Functional/UpdaterFormTest.php index 85c4656671..95d33ed18c 100644 --- a/automatic_updates_extensions/tests/src/Functional/UpdaterFormTest.php +++ b/automatic_updates_extensions/tests/src/Functional/UpdaterFormTest.php @@ -299,28 +299,38 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase { $this->container->get('state')->set('automatic_updates_test.new_update', TRUE); $page->pressButton('Continue'); $this->checkForMetaRefresh(); + $assert_session->pageTextContainsOnce('An error has occurred.'); + $assert_session->pageTextContainsOnce('Please continue to the error page'); + $page->clickLink('the error page'); + $assert_session->pageTextContains('Some modules have database schema updates to install. You should run the database update script immediately.'); + $assert_session->linkExists('database update script'); + $assert_session->linkByHrefExists('/update.php'); + $page->clickLink('database update script'); $this->assertSession()->addressEquals('/update.php'); - $assert_session->pageTextNotContains('Possible database updates have been detected in the following extensions.'); - $assert_session->pageTextContainsOnce('Please apply database updates to complete the update process.'); + $assert_session->pageTextNotContains('Possible database updates have been detected in the following extension'); $page->clickLink('Continue'); // @see automatic_updates_update_1191934() $assert_session->pageTextContains('Dynamic automatic_updates_update_1191934'); $page->clickLink('Apply pending updates'); $this->checkForMetaRefresh(); $assert_session->pageTextContains('Updates were attempted.'); + // PendingUpdatesValidator prevented the update to complete, so the status + // checks weren't run. + $this->drupalGet('/admin'); + $assert_session->pageTextContains('Your site has not recently run an update readiness check. Run readiness checks now.'); } else { $page->pressButton('Continue'); $this->checkForMetaRefresh(); $assert_session->addressEquals('/admin/reports/updates'); $assert_session->pageTextContainsOnce('Update complete!'); + // Status checks should display errors on admin page. + $this->drupalGet('/admin'); + // Confirm that the status checks were run and the new error is displayed. + $assert_session->statusMessageContains('Error before continue.', 'error'); + $assert_session->statusMessageContains(static::$errorsExplanation, 'error'); + $assert_session->pageTextNotContains('Your site has not recently run an update readiness check. Run readiness checks now.'); } - // Status checks should display errors on admin page. - $this->drupalGet('/admin'); - // Confirm that the status checks were run and the new error is displayed. - $assert_session->statusMessageContains('Error before continue.', 'error'); - $assert_session->statusMessageContains(static::$errorsExplanation, 'error'); - $assert_session->pageTextNotContains('Your site has not recently run an update readiness check. Run readiness checks now.'); } /** diff --git a/package_manager/src/Validator/ComposerExecutableValidator.php b/package_manager/src/Validator/ComposerExecutableValidator.php index 785a7cbde8..ff626a3a28 100644 --- a/package_manager/src/Validator/ComposerExecutableValidator.php +++ b/package_manager/src/Validator/ComposerExecutableValidator.php @@ -7,6 +7,7 @@ namespace Drupal\package_manager\Validator; use Composer\Semver\Semver; use Drupal\Core\Extension\ModuleHandlerInterface; use Drupal\Core\Url; +use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Event\PreCreateEvent; use Drupal\package_manager\Event\PreOperationStageEvent; use Drupal\Core\StringTranslation\StringTranslationTrait; @@ -167,6 +168,7 @@ class ComposerExecutableValidator implements EventSubscriberInterface { public static function getSubscribedEvents(): array { return [ PreCreateEvent::class => 'validateStagePreOperation', + PreApplyEvent::class => 'validateStagePreOperation', StatusCheckEvent::class => 'validateStagePreOperation', ]; } diff --git a/package_manager/src/Validator/ComposerJsonExistsValidator.php b/package_manager/src/Validator/ComposerJsonExistsValidator.php index 9542c70004..c10fe88fde 100644 --- a/package_manager/src/Validator/ComposerJsonExistsValidator.php +++ b/package_manager/src/Validator/ComposerJsonExistsValidator.php @@ -6,6 +6,7 @@ namespace Drupal\package_manager\Validator; use Drupal\Core\StringTranslation\StringTranslationTrait; use Drupal\Core\StringTranslation\TranslationInterface; +use Drupal\package_manager\Event\PreApplyEvent; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Drupal\package_manager\Event\PreCreateEvent; use Drupal\package_manager\Event\PreOperationStageEvent; @@ -52,6 +53,7 @@ final class ComposerJsonExistsValidator implements EventSubscriberInterface { // @see \Drupal\package_manager\Validator\EnvironmentSupportValidator return [ PreCreateEvent::class => ['validateComposerJson', 190], + PreApplyEvent::class => ['validateComposerJson', 190], StatusCheckEvent::class => ['validateComposerJson', 190], ]; } diff --git a/package_manager/src/Validator/ComposerPatchesValidator.php b/package_manager/src/Validator/ComposerPatchesValidator.php index 464b19fbc2..65ca7c4c8d 100644 --- a/package_manager/src/Validator/ComposerPatchesValidator.php +++ b/package_manager/src/Validator/ComposerPatchesValidator.php @@ -5,6 +5,7 @@ declare(strict_types = 1); namespace Drupal\package_manager\Validator; use Drupal\Core\StringTranslation\StringTranslationTrait; +use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Event\PreCreateEvent; use Drupal\package_manager\Event\PreOperationStageEvent; use Drupal\package_manager\Event\StatusCheckEvent; @@ -46,6 +47,7 @@ class ComposerPatchesValidator implements EventSubscriberInterface { public static function getSubscribedEvents(): array { return [ PreCreateEvent::class => 'validateStagePreOperation', + PreApplyEvent::class => 'validateStagePreOperation', StatusCheckEvent::class => 'validateStagePreOperation', ]; } diff --git a/package_manager/src/Validator/ComposerSettingsValidator.php b/package_manager/src/Validator/ComposerSettingsValidator.php index 8a712d278a..747a2dfa69 100644 --- a/package_manager/src/Validator/ComposerSettingsValidator.php +++ b/package_manager/src/Validator/ComposerSettingsValidator.php @@ -6,6 +6,7 @@ namespace Drupal\package_manager\Validator; use Drupal\Core\StringTranslation\StringTranslationTrait; use Drupal\Core\StringTranslation\TranslationInterface; +use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Event\PreCreateEvent; use Drupal\package_manager\Event\PreOperationStageEvent; use Drupal\package_manager\Event\StatusCheckEvent; @@ -57,6 +58,7 @@ final class ComposerSettingsValidator implements EventSubscriberInterface { public static function getSubscribedEvents(): array { return [ PreCreateEvent::class => 'validateStagePreOperation', + PreApplyEvent::class => 'validateStagePreOperation', StatusCheckEvent::class => 'validateStagePreOperation', ]; } diff --git a/package_manager/src/Validator/DiskSpaceValidator.php b/package_manager/src/Validator/DiskSpaceValidator.php index 2295e9fcc4..80c35cb8df 100644 --- a/package_manager/src/Validator/DiskSpaceValidator.php +++ b/package_manager/src/Validator/DiskSpaceValidator.php @@ -4,6 +4,7 @@ declare(strict_types = 1); namespace Drupal\package_manager\Validator; +use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Event\PreCreateEvent; use Drupal\package_manager\Event\PreOperationStageEvent; use Drupal\Component\FileSystem\FileSystem; @@ -169,6 +170,7 @@ class DiskSpaceValidator implements EventSubscriberInterface { public static function getSubscribedEvents(): array { return [ PreCreateEvent::class => 'validateStagePreOperation', + PreApplyEvent::class => 'validateStagePreOperation', StatusCheckEvent::class => 'validateStagePreOperation', ]; } diff --git a/package_manager/src/Validator/EnvironmentSupportValidator.php b/package_manager/src/Validator/EnvironmentSupportValidator.php index 090a8d1da4..d1338bb308 100644 --- a/package_manager/src/Validator/EnvironmentSupportValidator.php +++ b/package_manager/src/Validator/EnvironmentSupportValidator.php @@ -6,6 +6,7 @@ namespace Drupal\package_manager\Validator; use Drupal\Core\Link; use Drupal\Core\StringTranslation\StringTranslationTrait; +use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Event\PreCreateEvent; use Drupal\package_manager\Event\PreOperationStageEvent; use Drupal\package_manager\Event\StatusCheckEvent; @@ -66,6 +67,7 @@ final class EnvironmentSupportValidator implements EventSubscriberInterface { public static function getSubscribedEvents(): array { return [ PreCreateEvent::class => ['validateStagePreOperation', 200], + PreApplyEvent::class => ['validateStagePreOperation', 200], StatusCheckEvent::class => ['validateStagePreOperation', 200], ]; } diff --git a/package_manager/src/Validator/MultisiteValidator.php b/package_manager/src/Validator/MultisiteValidator.php index b4437f89a4..968c2cf59f 100644 --- a/package_manager/src/Validator/MultisiteValidator.php +++ b/package_manager/src/Validator/MultisiteValidator.php @@ -6,6 +6,7 @@ namespace Drupal\package_manager\Validator; use Drupal\Core\StringTranslation\StringTranslationTrait; use Drupal\Core\StringTranslation\TranslationInterface; +use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Event\PreCreateEvent; use Drupal\package_manager\Event\PreOperationStageEvent; use Drupal\package_manager\Event\StatusCheckEvent; @@ -77,6 +78,7 @@ final class MultisiteValidator implements EventSubscriberInterface { public static function getSubscribedEvents(): array { return [ PreCreateEvent::class => 'validateStagePreOperation', + PreApplyEvent::class => 'validateStagePreOperation', StatusCheckEvent::class => 'validateStagePreOperation', ]; } diff --git a/package_manager/src/Validator/PendingUpdatesValidator.php b/package_manager/src/Validator/PendingUpdatesValidator.php index a508d9420a..507157b9af 100644 --- a/package_manager/src/Validator/PendingUpdatesValidator.php +++ b/package_manager/src/Validator/PendingUpdatesValidator.php @@ -4,6 +4,7 @@ declare(strict_types = 1); namespace Drupal\package_manager\Validator; +use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Event\PreCreateEvent; use Drupal\package_manager\Event\PreOperationStageEvent; use Drupal\Core\StringTranslation\StringTranslationTrait; @@ -92,6 +93,7 @@ final class PendingUpdatesValidator implements EventSubscriberInterface { return [ PreCreateEvent::class => 'validateStagePreOperation', StatusCheckEvent::class => 'validateStagePreOperation', + PreApplyEvent::class => 'validateStagePreOperation', ]; } diff --git a/package_manager/src/Validator/SettingsValidator.php b/package_manager/src/Validator/SettingsValidator.php index be93a75283..697b0201fd 100644 --- a/package_manager/src/Validator/SettingsValidator.php +++ b/package_manager/src/Validator/SettingsValidator.php @@ -7,6 +7,7 @@ namespace Drupal\package_manager\Validator; use Drupal\Core\Site\Settings; use Drupal\Core\StringTranslation\StringTranslationTrait; use Drupal\Core\StringTranslation\TranslationInterface; +use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Event\PreCreateEvent; use Drupal\package_manager\Event\PreOperationStageEvent; use Drupal\package_manager\Event\StatusCheckEvent; @@ -51,6 +52,7 @@ final class SettingsValidator implements EventSubscriberInterface { public static function getSubscribedEvents(): array { return [ PreCreateEvent::class => 'validateStagePreOperation', + PreApplyEvent::class => 'validateStagePreOperation', StatusCheckEvent::class => 'validateStagePreOperation', ]; } diff --git a/package_manager/src/Validator/WritableFileSystemValidator.php b/package_manager/src/Validator/WritableFileSystemValidator.php index 9dc7d54228..96c7af0259 100644 --- a/package_manager/src/Validator/WritableFileSystemValidator.php +++ b/package_manager/src/Validator/WritableFileSystemValidator.php @@ -5,6 +5,7 @@ declare(strict_types = 1); namespace Drupal\package_manager\Validator; use Drupal\Core\StringTranslation\TranslationInterface; +use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Event\PreCreateEvent; use Drupal\package_manager\Event\PreOperationStageEvent; use Drupal\Core\StringTranslation\StringTranslationTrait; @@ -71,8 +72,15 @@ class WritableFileSystemValidator implements EventSubscriberInterface { $messages[] = $this->t('The vendor directory "@dir" is not writable.', ['@dir' => $dir]); } - // Ensure the stage root directory is writable. If it doesn't exist, ensure - // we will be able to create it. + // During pre-apply don't check whether the staging root is writable. + if ($event instanceof PreApplyEvent) { + if ($messages) { + $event->addError($messages, $this->t('The file system is not writable.')); + } + return; + } + // Ensure the staging root is writable. If it doesn't exist, ensure we will + // be able to create it. $dir = $this->pathLocator->getStagingRoot(); if (!file_exists($dir)) { $dir = dirname($dir); @@ -99,6 +107,7 @@ class WritableFileSystemValidator implements EventSubscriberInterface { public static function getSubscribedEvents(): array { return [ PreCreateEvent::class => 'validateStagePreOperation', + PreApplyEvent::class => 'validateStagePreOperation', StatusCheckEvent::class => 'validateStagePreOperation', ]; } diff --git a/package_manager/tests/src/Build/PackageUpdateTest.php b/package_manager/tests/src/Build/PackageUpdateTest.php index bdeec35957..c6c593eea0 100644 --- a/package_manager/tests/src/Build/PackageUpdateTest.php +++ b/package_manager/tests/src/Build/PackageUpdateTest.php @@ -34,6 +34,7 @@ class PackageUpdateTest extends TemplateProjectTestBase { // Change both modules' upstream version. $this->addRepository('alpha', $this->copyFixtureToTempDirectory(__DIR__ . '/../../fixtures/alpha/1.1.0')); $this->addRepository('updated_module', $this->copyFixtureToTempDirectory(__DIR__ . '/../../fixtures/updated_module/1.1.0')); + // Make .git folder // Use the API endpoint to create a stage and update updated_module to // 1.1.0. Even though both modules have version 1.1.0 available, only @@ -56,6 +57,8 @@ class PackageUpdateTest extends TemplateProjectTestBase { $mink = $this->getMink(); $mink->assertSession()->statusCodeEquals(200); + + $file_contents = $mink->getSession()->getPage()->getContent(); $file_contents = json_decode($file_contents, TRUE); diff --git a/package_manager/tests/src/Kernel/ComposerExecutableValidatorTest.php b/package_manager/tests/src/Kernel/ComposerExecutableValidatorTest.php index d8879992fa..70e6a8bac2 100644 --- a/package_manager/tests/src/Kernel/ComposerExecutableValidatorTest.php +++ b/package_manager/tests/src/Kernel/ComposerExecutableValidatorTest.php @@ -6,6 +6,7 @@ namespace Drupal\Tests\package_manager\Kernel; use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\Core\Url; +use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Event\PreCreateEvent; use Drupal\package_manager\Validator\ComposerExecutableValidator; use Drupal\package_manager\ValidationResult; @@ -72,6 +73,33 @@ class ComposerExecutableValidatorTest extends PackageManagerKernelTestBase { $this->assertResults([$error], PreCreateEvent::class); } + /** + * Tests error on pre-apply if the Composer executable isn't found. + */ + public function testErrorIfComposerNotFoundDuringPreApply(): void { + // Setting command output which doesn't raise error for pre-create event. + TestComposerExecutableValidator::setCommandOutput("Composer version 2.2.12"); + $exception = new IOException("This is your regularly scheduled error."); + + $listener = function () use ($exception): void { + TestComposerExecutableValidator::setCommandOutput($exception); + }; + $this->container->get('event_dispatcher')->addListener(PreApplyEvent::class, $listener, PHP_INT_MAX); + + // The validator should translate that exception into an error. + $error = ValidationResult::createError([ + $exception->getMessage(), + ]); + $stage = $this->assertResults([$error], PreApplyEvent::class); + $stage->destroy(TRUE); + + // Setting command output which doesn't raise error for pre-create event. + TestComposerExecutableValidator::setCommandOutput("Composer version 2.2.12"); + $this->enableModules(['help']); + $this->container->get('event_dispatcher')->addListener(PreApplyEvent::class, $listener, PHP_INT_MAX); + $this->assertResultsWithHelp([$error], PreApplyEvent::class, FALSE); + } + /** * Data provider for testComposerVersionValidation(). * @@ -180,6 +208,36 @@ class ComposerExecutableValidatorTest extends PackageManagerKernelTestBase { $this->assertResultsWithHelp($expected_results, PreCreateEvent::class); } + /** + * Tests validation of various Composer versions on pre-apply. + * + * @param string $reported_version + * The version of Composer that `composer --version` should report. + * @param \Drupal\package_manager\ValidationResult[] $expected_results + * The expected validation results. + * + * @dataProvider providerComposerVersionValidation + */ + public function testComposerVersionValidationDuringPreApply(string $reported_version, array $expected_results): void { + // Setting command output which doesn't raise error for pre-create event. + TestComposerExecutableValidator::setCommandOutput("Composer version 2.2.12"); + $listener = function () use ($reported_version): void { + TestComposerExecutableValidator::setCommandOutput("Composer version $reported_version"); + }; + $this->container->get('event_dispatcher')->addListener(PreApplyEvent::class, $listener, PHP_INT_MAX); + + // If the validator can't find a recognized, supported version of Composer, + // it should produce errors. + $stage = $this->assertResults($expected_results, PreApplyEvent::class); + $stage->destroy(TRUE); + + // Setting command output which doesn't raise error for pre-create event. + TestComposerExecutableValidator::setCommandOutput("Composer version 2.2.12"); + $this->enableModules(['help']); + $this->container->get('event_dispatcher')->addListener(PreApplyEvent::class, $listener, PHP_INT_MAX); + $this->assertResultsWithHelp($expected_results, PreApplyEvent::class, FALSE); + } + /** * Asserts that a set of validation results link to the Package Manager help. * @@ -188,8 +246,11 @@ class ComposerExecutableValidatorTest extends PackageManagerKernelTestBase { * @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. + * @param bool $assert_status_check + * (optional) Whether the status checks should be asserted. Defaults to + * TRUE. */ - private function assertResultsWithHelp(array $expected_results, string $event_class = NULL): void { + private function assertResultsWithHelp(array $expected_results, string $event_class = NULL, bool $assert_status_check = TRUE): void { $url = Url::fromRoute('help.page', ['name' => 'package_manager']) ->setOption('fragment', 'package-manager-faq-composer-not-found') ->toString(); @@ -203,7 +264,9 @@ class ComposerExecutableValidatorTest extends PackageManagerKernelTestBase { $messages = array_map($map, $result->getMessages()); $expected_results[$index] = ValidationResult::createError($messages); } - $this->assertStatusCheckResults($expected_results); + if ($assert_status_check) { + $this->assertStatusCheckResults($expected_results); + } $this->assertResults($expected_results, $event_class); } diff --git a/package_manager/tests/src/Kernel/ComposerJsonExistsValidatorTest.php b/package_manager/tests/src/Kernel/ComposerJsonExistsValidatorTest.php index 0bb39c908c..9a261c6d89 100644 --- a/package_manager/tests/src/Kernel/ComposerJsonExistsValidatorTest.php +++ b/package_manager/tests/src/Kernel/ComposerJsonExistsValidatorTest.php @@ -4,6 +4,7 @@ declare(strict_types = 1); namespace Drupal\Tests\package_manager\Kernel; +use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Event\PreCreateEvent; use Drupal\package_manager\Event\StatusCheckEvent; use Drupal\package_manager\ValidationResult; @@ -39,4 +40,32 @@ class ComposerJsonExistsValidatorTest extends PackageManagerKernelTestBase { $this->assertResults([$result], PreCreateEvent::class); } + /** + * Tests that active composer.json is not present during pre-apply. + */ + public function testComposerRequirementDuringPreApply(): void { + $result = ValidationResult::createError([ + 'No composer.json file can be found at vfs://root/active', + ]); + $this->container->get('event_dispatcher')->addListener( + PreApplyEvent::class, + function (): void { + unlink($this->container->get('package_manager.path_locator') + ->getProjectRoot() . '/composer.json'); + }, + PHP_INT_MAX + ); + $this->container->get('event_dispatcher')->addListener( + PreApplyEvent::class, + function (): void { + $this->fail('Event propagation should have been stopped during ' . PreApplyEvent::class . '.'); + }, + // Execute this listener immediately after the tested validator, which + // uses priority 190. This ensures informative test failures. + // @see \Drupal\package_manager\Validator\ComposerJsonExistsValidator::getSubscribedEvents() + 189 + ); + $this->assertResults([$result], PreApplyEvent::class); + } + } diff --git a/package_manager/tests/src/Kernel/ComposerPatchesValidatorTest.php b/package_manager/tests/src/Kernel/ComposerPatchesValidatorTest.php index c9d854c2db..ffb0220691 100644 --- a/package_manager/tests/src/Kernel/ComposerPatchesValidatorTest.php +++ b/package_manager/tests/src/Kernel/ComposerPatchesValidatorTest.php @@ -4,7 +4,8 @@ declare(strict_types = 1); namespace Drupal\Tests\package_manager\Kernel; -use Composer\Json\JsonFile; +use Drupal\fixture_manipulator\ActiveFixtureManipulator; +use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Event\PreCreateEvent; use Drupal\package_manager\ValidationResult; @@ -24,15 +25,7 @@ class ComposerPatchesValidatorTest extends PackageManagerKernelTestBase { $dir = $this->container->get('package_manager.path_locator') ->getProjectRoot(); - // Simulate that the patcher is installed in the active directory. - $file = new JsonFile($dir . '/vendor/composer/installed.json'); - $this->assertTrue($file->exists()); - $data = $file->read(); - $data['packages'][] = [ - 'name' => 'cweagans/composer-patches', - 'version' => '1.0.0', - ]; - $file->write($data); + $this->installPatcherInActive($dir); // Because ComposerUtility reads composer.json and passes it to the Composer // factory as an array, Composer will assume that the configuration is @@ -44,4 +37,44 @@ class ComposerPatchesValidatorTest extends PackageManagerKernelTestBase { $this->assertResults([$error], PreCreateEvent::class); } + /** + * Tests that the patcher configuration is validated during pre-apply. + */ + public function testErrorDuringPreApply(): void { + // Simulate an active directory where the patcher is installed, but there's + // no composer-exit-on-patch-failure flag. + $dir = $this->container->get('package_manager.path_locator') + ->getProjectRoot(); + + $this->container->get('event_dispatcher')->addListener( + PreApplyEvent::class, + function () use ($dir): void { + $this->installPatcherInActive($dir); + }, + PHP_INT_MAX + ); + // Because ComposerUtility reads composer.json and passes it to the Composer + // factory as an array, Composer will assume that the configuration is + // coming from a config.json file, even if one doesn't exist. + $error = ValidationResult::createError([ + "The <code>cweagans/composer-patches</code> plugin is installed, but the <code>composer-exit-on-patch-failure</code> key is not set to <code>true</code> in the <code>extra</code> section of $dir/config.json.", + ]); + $this->assertResults([$error], PreApplyEvent::class); + } + + /** + * Simulates that the patcher is installed in the active directory. + * + * @param string $dir + * The active directory. + */ + private function installPatcherInActive(string $dir): void { + (new ActiveFixtureManipulator()) + ->addPackage([ + 'name' => 'cweagans/composer-patches', + 'version' => '1.0.0', + 'type' => 'composer-plugin', + ])->commitChanges(); + } + } diff --git a/package_manager/tests/src/Kernel/ComposerSettingsValidatorTest.php b/package_manager/tests/src/Kernel/ComposerSettingsValidatorTest.php index 751a11563c..e41720a27a 100644 --- a/package_manager/tests/src/Kernel/ComposerSettingsValidatorTest.php +++ b/package_manager/tests/src/Kernel/ComposerSettingsValidatorTest.php @@ -5,6 +5,7 @@ declare(strict_types = 1); namespace Drupal\Tests\package_manager\Kernel; use Drupal\Component\Serialization\Json; +use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Event\PreCreateEvent; use Drupal\package_manager\ValidationResult; @@ -68,4 +69,27 @@ class ComposerSettingsValidatorTest extends PackageManagerKernelTestBase { $this->assertResults($expected_results, PreCreateEvent::class); } + /** + * Tests that Composer's secure-http setting is validated during pre-apply. + * + * @param string $contents + * The contents of the composer.json file. + * @param \Drupal\package_manager\ValidationResult[] $expected_results + * The expected validation results, if any. + * + * @dataProvider providerSecureHttpValidation + */ + public function testSecureHttpValidationDuringPreApply(string $contents, array $expected_results): void { + $this->container->get('event_dispatcher')->addListener( + PreApplyEvent::class, + function () use ($contents): void { + $active_dir = $this->container->get('package_manager.path_locator') + ->getProjectRoot(); + file_put_contents("$active_dir/composer.json", $contents); + }, + PHP_INT_MAX + ); + $this->assertResults($expected_results, PreApplyEvent::class); + } + } diff --git a/package_manager/tests/src/Kernel/DiskSpaceValidatorTest.php b/package_manager/tests/src/Kernel/DiskSpaceValidatorTest.php index 8fcfd844c6..982bf45693 100644 --- a/package_manager/tests/src/Kernel/DiskSpaceValidatorTest.php +++ b/package_manager/tests/src/Kernel/DiskSpaceValidatorTest.php @@ -4,6 +4,7 @@ declare(strict_types = 1); namespace Drupal\Tests\package_manager\Kernel; +use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Event\PreCreateEvent; use Drupal\package_manager\ValidationResult; use Drupal\Component\Utility\Bytes; @@ -154,4 +155,34 @@ class DiskSpaceValidatorTest extends PackageManagerKernelTestBase { $this->assertResults($expected_results, PreCreateEvent::class); } + /** + * Tests disk space validation during pre-apply. + * + * @param bool $shared_disk + * Whether the root and vendor directories are on the same logical disk. + * @param array $free_space + * The free space that should be reported for various paths. The keys + * are the paths, and the values are the free space that should be reported, + * in a format that can be parsed by + * \Drupal\Component\Utility\Bytes::toNumber(). + * @param \Drupal\package_manager\ValidationResult[] $expected_results + * The expected validation results. + * + * @dataProvider providerDiskSpaceValidation + */ + public function testDiskSpaceValidationDuringPreApply(bool $shared_disk, array $free_space, array $expected_results): void { + $this->container->get('event_dispatcher')->addListener( + PreApplyEvent::class, + function () use ($shared_disk, $free_space): void { + /** @var \Drupal\Tests\package_manager\Kernel\TestDiskSpaceValidator $validator */ + $validator = $this->container->get('package_manager.validator.disk_space'); + $validator->sharedDisk = $shared_disk; + $validator->freeSpace = array_map([Bytes::class, 'toNumber'], $free_space); + }, + PHP_INT_MAX + ); + + $this->assertResults($expected_results, PreApplyEvent::class); + } + } diff --git a/package_manager/tests/src/Kernel/EnvironmentSupportValidatorTest.php b/package_manager/tests/src/Kernel/EnvironmentSupportValidatorTest.php index f20e597543..a1d0afaef3 100644 --- a/package_manager/tests/src/Kernel/EnvironmentSupportValidatorTest.php +++ b/package_manager/tests/src/Kernel/EnvironmentSupportValidatorTest.php @@ -4,6 +4,7 @@ declare(strict_types = 1); namespace Drupal\Tests\package_manager\Kernel; +use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Event\PreCreateEvent; use Drupal\package_manager\Event\StatusCheckEvent; use Drupal\package_manager\ValidationResult; @@ -41,6 +42,35 @@ class EnvironmentSupportValidatorTest extends PackageManagerKernelTestBase { $this->assertResults([$result], PreCreateEvent::class); } + /** + * Tests an invalid URL in the environment support variable during pre-apply. + */ + public function testInvalidUrlDuringPreApply(): void { + $this->container->get('event_dispatcher')->addListener( + PreApplyEvent::class, + function (): void { + putenv(EnvironmentSupportValidator::VARIABLE_NAME . '=broken/url.org'); + }, + PHP_INT_MAX + ); + + $result = ValidationResult::createError([ + 'Package Manager is not supported by your environment.', + ]); + + $this->container->get('event_dispatcher')->addListener( + PreApplyEvent::class, + function (): void { + $this->fail('Event propagation should have been stopped during ' . PreApplyEvent::class . '.'); + }, + // Execute this listener immediately after the tested validator, which + // uses priority 200. This ensures informative test failures. + // @see \Drupal\package_manager\Validator\EnvironmentSupportValidator::getSubscribedEvents() + 199 + ); + $this->assertResults([$result], PreApplyEvent::class); + } + /** * Tests that the validation message links to the provided URL. */ @@ -55,4 +85,23 @@ class EnvironmentSupportValidatorTest extends PackageManagerKernelTestBase { $this->assertResults([$result], PreCreateEvent::class); } + /** + * Tests that the validation message links to the provided URL during pre-apply. + */ + public function testValidUrlDuringPreApply(): void { + $url = 'http://www.example.com'; + $this->container->get('event_dispatcher')->addListener( + PreApplyEvent::class, + function () use ($url): void { + putenv(EnvironmentSupportValidator::VARIABLE_NAME . '=' . $url); + }, + PHP_INT_MAX + ); + + $result = ValidationResult::createError([ + '<a href="' . $url . '">Package Manager is not supported by your environment.</a>', + ]); + $this->assertResults([$result], PreApplyEvent::class); + } + } diff --git a/package_manager/tests/src/Kernel/MultisiteValidatorTest.php b/package_manager/tests/src/Kernel/MultisiteValidatorTest.php index 7db26d1a85..1bd1760a9a 100644 --- a/package_manager/tests/src/Kernel/MultisiteValidatorTest.php +++ b/package_manager/tests/src/Kernel/MultisiteValidatorTest.php @@ -4,6 +4,7 @@ declare(strict_types = 1); namespace Drupal\Tests\package_manager\Kernel; +use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Event\PreCreateEvent; use Drupal\package_manager\ValidationResult; @@ -60,4 +61,32 @@ class MultisiteValidatorTest extends PackageManagerKernelTestBase { $this->assertResults($expected_results, PreCreateEvent::class); } + /** + * Tests that an error is flagged if run in a multisite during pre-apply. + * + * @param bool $is_multisite + * Whether the validator will be in a multisite. + * @param \Drupal\package_manager\ValidationResult[] $expected_results + * The expected validation results. + * + * @dataProvider providerMultisite + */ + public function testMultisiteDuringPreApply(bool $is_multisite, array $expected_results = []): void { + $this->container->get('event_dispatcher')->addListener( + PreApplyEvent::class, + function () use ($is_multisite): void { + // If we should simulate a multisite, ensure there is a sites.php in the + // test project. + // @see \Drupal\package_manager\Validator\MultisiteValidator::isMultisite() + if ($is_multisite) { + $project_root = $this->container->get('package_manager.path_locator') + ->getProjectRoot(); + touch($project_root . '/sites/sites.php'); + } + }, + PHP_INT_MAX + ); + $this->assertResults($expected_results, PreApplyEvent::class); + } + } diff --git a/package_manager/tests/src/Kernel/PendingUpdatesValidatorTest.php b/package_manager/tests/src/Kernel/PendingUpdatesValidatorTest.php index ec565b3283..b9aadc24f8 100644 --- a/package_manager/tests/src/Kernel/PendingUpdatesValidatorTest.php +++ b/package_manager/tests/src/Kernel/PendingUpdatesValidatorTest.php @@ -5,6 +5,7 @@ declare(strict_types = 1); namespace Drupal\Tests\package_manager\Kernel; use Drupal\package_manager\Event\PreCreateEvent; +use Drupal\package_manager\Exception\StageValidationException; use Drupal\package_manager\ValidationResult; /** @@ -64,4 +65,28 @@ class PendingUpdatesValidatorTest extends PackageManagerKernelTestBase { $this->assertResults([$result], PreCreateEvent::class); } + /** + * Tests that pending updates stop an operation from being applied. + */ + public function testPendingUpdateAfterStaged(): void { + $this->registerPostUpdateFunctions(); + + $stage = $this->createStage(); + $stage->create(); + $stage->require(['drupal/core:9.8.1']); + // Make an additional post-update function available; the update registry + // will think it's pending. + require_once __DIR__ . '/../../fixtures/post_update.php'; + $result = ValidationResult::createError([ + 'Some modules have database schema updates to install. You should run the <a href="/update.php">database update script</a> immediately.', + ]); + try { + $stage->apply(); + $this->fail('Able to apply update even though there is pending update.'); + } + catch (StageValidationException $exception) { + $this->assertValidationResultsEqual([$result], $exception->getResults()); + } + } + } diff --git a/package_manager/tests/src/Kernel/SettingsValidatorTest.php b/package_manager/tests/src/Kernel/SettingsValidatorTest.php index 49fd7f819b..ef6fa98cd9 100644 --- a/package_manager/tests/src/Kernel/SettingsValidatorTest.php +++ b/package_manager/tests/src/Kernel/SettingsValidatorTest.php @@ -4,6 +4,7 @@ declare(strict_types = 1); namespace Drupal\Tests\package_manager\Kernel; +use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Event\PreCreateEvent; use Drupal\package_manager\ValidationResult; @@ -45,4 +46,25 @@ class SettingsValidatorTest extends PackageManagerKernelTestBase { $this->assertResults($expected_results, PreCreateEvent::class); } + /** + * Tests settings validation during pre-apply. + * + * @param bool $setting + * The value of the update_fetch_with_http_fallback setting. + * @param \Drupal\package_manager\ValidationResult[] $expected_results + * The expected validation results. + * + * @dataProvider providerSettingsValidation + */ + public function testSettingsValidationDuringPreApply(bool $setting, array $expected_results): void { + $this->container->get('event_dispatcher')->addListener( + PreApplyEvent::class, + function () use ($setting): void { + $this->setSetting('update_fetch_with_http_fallback', $setting); + }, + PHP_INT_MAX + ); + $this->assertResults($expected_results, PreApplyEvent::class); + } + } diff --git a/package_manager/tests/src/Kernel/WritableFileSystemValidatorTest.php b/package_manager/tests/src/Kernel/WritableFileSystemValidatorTest.php index 25ece01ebc..d8cfec3e02 100644 --- a/package_manager/tests/src/Kernel/WritableFileSystemValidatorTest.php +++ b/package_manager/tests/src/Kernel/WritableFileSystemValidatorTest.php @@ -4,6 +4,7 @@ declare(strict_types = 1); namespace Drupal\Tests\package_manager\Kernel; +use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Event\PreCreateEvent; use Drupal\package_manager\ValidationResult; use Symfony\Component\Filesystem\Filesystem; @@ -90,6 +91,38 @@ class WritableFileSystemValidatorTest extends PackageManagerKernelTestBase { $this->assertResults($expected_results, PreCreateEvent::class); } + /** + * Tests the file system permissions validator during pre-apply. + * + * @param int $root_permissions + * The file permissions for the root folder. + * @param int $vendor_permissions + * The file permissions for the vendor folder. + * @param array $expected_results + * The expected validation results. + * + * @dataProvider providerWritable + */ + public function testWritableDuringPreApply(int $root_permissions, int $vendor_permissions, array $expected_results): void { + $this->container->get('event_dispatcher')->addListener( + PreApplyEvent::class, + function () use ($root_permissions, $vendor_permissions): void { + $path_locator = $this->container->get('package_manager.path_locator'); + + // We need to set the vendor directory's permissions first because, in + // the virtual project, it's located inside the project root. + $this->assertTrue(chmod($path_locator->getVendorDirectory(), $vendor_permissions)); + $this->assertTrue(chmod($path_locator->getProjectRoot(), $root_permissions)); + + // During pre-apply we don't care whether the staging root is writable. + $this->assertTrue(chmod($path_locator->getStagingRoot(), 0444)); + }, + PHP_INT_MAX + ); + + $this->assertResults($expected_results, PreApplyEvent::class); + } + /** * Data provider for ::testStagingRootPermissions(). * diff --git a/src/Validator/CronServerValidator.php b/src/Validator/CronServerValidator.php index e03f4966e7..4e4829db30 100644 --- a/src/Validator/CronServerValidator.php +++ b/src/Validator/CronServerValidator.php @@ -9,6 +9,7 @@ use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\Extension\ModuleHandlerInterface; use Drupal\Core\StringTranslation\StringTranslationTrait; use Drupal\Core\Url; +use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Event\PreCreateEvent; use Drupal\package_manager\Event\PreOperationStageEvent; use Drupal\package_manager\Event\StatusCheckEvent; @@ -118,6 +119,7 @@ final class CronServerValidator implements EventSubscriberInterface { public static function getSubscribedEvents(): array { return [ PreCreateEvent::class => 'checkServer', + PreApplyEvent::class => 'checkServer', StatusCheckEvent::class => 'checkServer', ]; } diff --git a/src/Validator/XdebugValidator.php b/src/Validator/XdebugValidator.php index 3051ad81a3..f8b66288d1 100644 --- a/src/Validator/XdebugValidator.php +++ b/src/Validator/XdebugValidator.php @@ -4,6 +4,7 @@ declare(strict_types = 1); namespace Drupal\automatic_updates\Validator; +use Drupal\package_manager\Event\PreApplyEvent; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Drupal\automatic_updates\CronUpdater; use Drupal\package_manager\Event\PreCreateEvent; @@ -48,6 +49,7 @@ final class XdebugValidator extends PackageManagerXdebugValidator implements Eve public static function getSubscribedEvents(): array { return [ PreCreateEvent::class => 'validateXdebugOff', + PreApplyEvent::class => 'validateXdebugOff', StatusCheckEvent::class => 'validateXdebugOff', ]; } diff --git a/tests/src/Functional/UpdaterFormTest.php b/tests/src/Functional/UpdaterFormTest.php index 1fac661510..081dc9db41 100644 --- a/tests/src/Functional/UpdaterFormTest.php +++ b/tests/src/Functional/UpdaterFormTest.php @@ -633,11 +633,17 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase { // Confirm the site remains in maintenance more when redirected to // update.php. $this->assertTrue($state->get('system.maintenance_mode')); + $assert_session->pageTextContainsOnce('An error has occurred.'); + $assert_session->pageTextContainsOnce('Please continue to the error page'); + $page->clickLink('the error page'); + $assert_session->pageTextContains('Some modules have database schema updates to install. You should run the database update script immediately.'); + $assert_session->linkExists('database update script'); + $assert_session->linkByHrefExists('/update.php'); + $page->clickLink('database update script'); $assert_session->addressEquals('/update.php'); $assert_session->pageTextNotContains($cached_message); $assert_session->pageTextNotContains(reset($messages)); $assert_session->pageTextNotContains($possible_update_message); - $assert_session->pageTextContainsOnce('Please apply database updates to complete the update process.'); $this->assertTrue($state->get('system.maintenance_mode')); $page->clickLink('Continue'); // @see automatic_updates_update_1191934() @@ -771,28 +777,38 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase { $this->container->get('state')->set('automatic_updates_test.new_update', TRUE); $page->pressButton('Continue'); $this->checkForMetaRefresh(); + $assert_session->pageTextContainsOnce('An error has occurred.'); + $assert_session->pageTextContainsOnce('Please continue to the error page'); + $page->clickLink('the error page'); + $assert_session->pageTextContains('Some modules have database schema updates to install. You should run the database update script immediately.'); + $assert_session->linkExists('database update script'); + $assert_session->linkByHrefExists('/update.php'); + $page->clickLink('database update script'); $this->assertSession()->addressEquals('/update.php'); $assert_session->pageTextNotContains('Possible database updates have been detected in the following extension'); - $assert_session->pageTextContainsOnce('Please apply database updates to complete the update process.'); $page->clickLink('Continue'); // @see automatic_updates_update_1191934() $assert_session->pageTextContains('Dynamic automatic_updates_update_1191934'); $page->clickLink('Apply pending updates'); $this->checkForMetaRefresh(); $assert_session->pageTextContains('Updates were attempted.'); + // PendingUpdatesValidator prevented the update to complete, so the status + // checks weren't run. + $this->drupalGet('/admin'); + $assert_session->pageTextContains('Your site has not recently run an update readiness check. Run readiness checks now.'); } else { $page->pressButton('Continue'); $this->checkForMetaRefresh(); $assert_session->addressEquals('/admin/reports/updates'); $assert_session->pageTextContainsOnce('Update complete!'); + // Status checks should display errors on admin page. + $this->drupalGet('/admin'); + // Confirm that the status checks were run and the new error is displayed. + $assert_session->statusMessageContains('Error before continue.', 'error'); + $assert_session->statusMessageContains(static::$errorsExplanation, 'error'); + $assert_session->pageTextNotContains('Your site has not recently run an update readiness check. Run readiness checks now.'); } - // Status checks should display errors on admin page. - $this->drupalGet('/admin'); - // Confirm that the status checks were run and the new error is displayed. - $assert_session->statusMessageContains('Error before continue.', 'error'); - $assert_session->statusMessageContains(static::$errorsExplanation, 'error'); - $assert_session->pageTextNotContains('Your site has not recently run an update readiness check. Run readiness checks now.'); } /** diff --git a/tests/src/Kernel/StatusCheck/CronServerValidatorTest.php b/tests/src/Kernel/StatusCheck/CronServerValidatorTest.php index fcac9f57e6..83ffcec5aa 100644 --- a/tests/src/Kernel/StatusCheck/CronServerValidatorTest.php +++ b/tests/src/Kernel/StatusCheck/CronServerValidatorTest.php @@ -8,10 +8,12 @@ use Drupal\automatic_updates\CronUpdater; use Drupal\automatic_updates\Validator\CronServerValidator; use Drupal\Core\Logger\RfcLogLevel; use Drupal\Core\Url; +use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Exception\StageValidationException; use Drupal\package_manager\ValidationResult; use Drupal\Tests\automatic_updates\Kernel\AutomaticUpdatesKernelTestBase; use ColinODell\PsrTestLogger\TestLogger; +use Drupal\Tests\package_manager\Traits\PackageManagerBypassTestTrait; /** * @covers \Drupal\automatic_updates\Validator\CronServerValidator @@ -20,6 +22,8 @@ use ColinODell\PsrTestLogger\TestLogger; */ class CronServerValidatorTest extends AutomaticUpdatesKernelTestBase { + use PackageManagerBypassTestTrait; + /** * {@inheritdoc} */ @@ -35,59 +39,65 @@ class CronServerValidatorTest extends AutomaticUpdatesKernelTestBase { $error = ValidationResult::createError([ 'Your site appears to be running on the built-in PHP web server on port 80. Drupal cannot be automatically updated with this configuration unless the site can also be reached on an alternate port.', ]); - - return [ - 'PHP server with alternate port' => [ + // Add all the test cases where there no expected results for all cron + // modes. + foreach ([CronUpdater::DISABLED, CronUpdater::SECURITY, CronUpdater::ALL] as $cron_mode) { + $test_cases["PHP server with alternate port, cron $cron_mode"] = [ TRUE, 'cli-server', - [CronUpdater::DISABLED, CronUpdater::SECURITY, CronUpdater::ALL], + $cron_mode, [], - ], - 'PHP server with same port, cron enabled' => [ - FALSE, - 'cli-server', - [CronUpdater::SECURITY, CronUpdater::ALL], - [$error], - ], - 'PHP server with same port, cron disabled' => [ - FALSE, - 'cli-server', - [CronUpdater::DISABLED], - [], - ], - 'other server with alternate port' => [ + ]; + $test_cases[" 'other server with alternate port, cron $cron_mode"] = [ TRUE, 'nginx', - [CronUpdater::DISABLED, CronUpdater::SECURITY, CronUpdater::ALL], + $cron_mode, [], - ], - 'other server with same port' => [ + ]; + $test_cases["other server with same port, cron $cron_mode"] = [ FALSE, 'nginx', - [CronUpdater::DISABLED, CronUpdater::SECURITY, CronUpdater::ALL], + $cron_mode, [], - ], + ]; + } + // If the PHP server is used with the same port and cron is enabled an error + // will be flagged. + foreach ([CronUpdater::SECURITY, CronUpdater::ALL] as $cron_mode) { + $test_cases["PHP server with same port, cron $cron_mode"] = [ + FALSE, + 'cli-server', + $cron_mode, + [$error], + ]; + } + $test_cases["PHP server with same port, cron disabled"] = [ + FALSE, + 'cli-server', + CronUpdater::DISABLED, + [], ]; + return $test_cases; } /** - * Tests server validation for unattended updates. + * Tests server validation during pre-create for unattended updates. * * @param bool $alternate_port * Whether or not an alternate port should be set. * @param string $server_api * The value of the PHP_SAPI constant, as known to the validator. - * @param string[] $cron_modes - * The cron modes to test with. Can contain any of + * @param string $cron_mode + * The cron mode to test with. Can be any of * \Drupal\automatic_updates\CronUpdater::DISABLED, - * \Drupal\automatic_updates\CronUpdater::SECURITY, and + * \Drupal\automatic_updates\CronUpdater::SECURITY, or * \Drupal\automatic_updates\CronUpdater::ALL. * @param \Drupal\package_manager\ValidationResult[] $expected_results * The expected validation results. * * @dataProvider providerCronServerValidation */ - public function testCronServerValidation(bool $alternate_port, string $server_api, array $cron_modes, array $expected_results): void { + public function testCronServerValidationDuringPreCreate(bool $alternate_port, string $server_api, string $cron_mode, array $expected_results): void { $request = $this->container->get('request_stack')->getCurrentRequest(); $this->assertNotEmpty($request); $this->assertSame(80, $request->getPort()); @@ -96,28 +106,87 @@ class CronServerValidatorTest extends AutomaticUpdatesKernelTestBase { $property->setAccessible(TRUE); $property->setValue(NULL, $server_api); - foreach ($cron_modes as $mode) { - $this->config('automatic_updates.settings') - ->set('cron', $mode) - ->set('cron_port', $alternate_port ? 2501 : 0) - ->save(); - - $this->assertCheckerResultsFromManager($expected_results, TRUE); + $this->config('automatic_updates.settings') + ->set('cron', $cron_mode) + ->set('cron_port', $alternate_port ? 2501 : 0) + ->save(); + + $this->assertCheckerResultsFromManager($expected_results, TRUE); + + $logger = new TestLogger(); + $this->container->get('logger.factory') + ->get('automatic_updates') + ->addLogger($logger); + + // If errors were expected, cron should not have run. + $this->container->get('cron')->run(); + if ($expected_results) { + // Assert the update was not staged to ensure the error was flagged in + // PreCreateEvent and not PreApplyEvent. + $this->assertUpdateStagedTimes(0); + $error = new StageValidationException($expected_results); + $this->assertTrue($logger->hasRecord($error->getMessage(), (string) RfcLogLevel::ERROR)); + } + else { + $this->assertFalse($logger->hasRecords((string) RfcLogLevel::ERROR)); + } + } - $logger = new TestLogger(); - $this->container->get('logger.factory') - ->get('automatic_updates') - ->addLogger($logger); + /** + * Tests server validation during pre-apply for unattended updates. + * + * @param bool $alternate_port + * Whether or not an alternate port should be set. + * @param string $server_api + * The value of the PHP_SAPI constant, as known to the validator. + * @param string $cron_mode + * The cron mode to test with. Can be any of + * \Drupal\automatic_updates\CronUpdater::DISABLED, + * \Drupal\automatic_updates\CronUpdater::SECURITY, or + * \Drupal\automatic_updates\CronUpdater::ALL. + * @param \Drupal\package_manager\ValidationResult[] $expected_results + * The expected validation results. + * + * @dataProvider providerCronServerValidation + */ + public function testCronServerValidationDuringPreApply(bool $alternate_port, string $server_api, string $cron_mode, array $expected_results): void { + $request = $this->container->get('request_stack')->getCurrentRequest(); + $this->assertNotEmpty($request); + $this->assertSame(80, $request->getPort()); - // If errors were expected, cron should not have run. - $this->container->get('cron')->run(); - if ($expected_results) { - $error = new StageValidationException($expected_results); - $this->assertTrue($logger->hasRecord($error->getMessage(), (string) RfcLogLevel::ERROR)); - } - else { - $this->assertFalse($logger->hasRecords((string) RfcLogLevel::ERROR)); - } + $logger = new TestLogger(); + $this->container->get('logger.factory') + ->get('automatic_updates') + ->addLogger($logger); + + $this->config('automatic_updates.settings') + ->set('cron', $cron_mode) + ->save(); + + // Add a listener to change the $server_api and $alternate_port settings + // during PreApplyEvent. We set $cron_mode above because this determines + // whether updates will actually be run in cron. + $this->container->get('event_dispatcher')->addListener( + PreApplyEvent::class, + function () use ($alternate_port, $server_api): void { + $property = new \ReflectionProperty(CronServerValidator::class, 'serverApi'); + $property->setAccessible(TRUE); + $property->setValue(NULL, $server_api); + $this->config('automatic_updates.settings') + ->set('cron_port', $alternate_port ? 2501 : 0) + ->save(); + }, + PHP_INT_MAX + ); + // If errors were expected, cron should not have run. + $this->container->get('cron')->run(); + if ($expected_results) { + $this->assertUpdateStagedTimes(1); + $error = new StageValidationException($expected_results); + $this->assertTrue($logger->hasRecord($error->getMessage(), (string) RfcLogLevel::ERROR)); + } + else { + $this->assertFalse($logger->hasRecords((string) RfcLogLevel::ERROR)); } } @@ -128,17 +197,17 @@ class CronServerValidatorTest extends AutomaticUpdatesKernelTestBase { * Whether or not an alternate port should be set. * @param string $server_api * The value of the PHP_SAPI constant, as known to the validator. - * @param string[] $cron_modes - * The cron modes to test with. Can contain any of + * @param string $cron_mode + * The cron mode to test with. Can contain be of * \Drupal\automatic_updates\CronUpdater::DISABLED, - * \Drupal\automatic_updates\CronUpdater::SECURITY, and + * \Drupal\automatic_updates\CronUpdater::SECURITY, or * \Drupal\automatic_updates\CronUpdater::ALL. * @param \Drupal\package_manager\ValidationResult[] $expected_results * The expected validation results. * * @dataProvider providerCronServerValidation */ - public function testHelpLink(bool $alternate_port, string $server_api, array $cron_modes, array $expected_results): void { + public function testHelpLink(bool $alternate_port, string $server_api, string $cron_mode, array $expected_results): void { $this->enableModules(['help']); $url = Url::fromRoute('help.page') @@ -153,7 +222,7 @@ class CronServerValidatorTest extends AutomaticUpdatesKernelTestBase { } $expected_results[$i] = ValidationResult::createError($messages); } - $this->testCronServerValidation($alternate_port, $server_api, $cron_modes, $expected_results); + $this->testCronServerValidationDuringPreApply($alternate_port, $server_api, $cron_mode, $expected_results); } } diff --git a/tests/src/Kernel/StatusCheck/XdebugValidatorTest.php b/tests/src/Kernel/StatusCheck/XdebugValidatorTest.php index 1ef9e2269a..ff06a9d7d4 100644 --- a/tests/src/Kernel/StatusCheck/XdebugValidatorTest.php +++ b/tests/src/Kernel/StatusCheck/XdebugValidatorTest.php @@ -8,6 +8,7 @@ use Drupal\automatic_updates\CronUpdater; use Drupal\Core\Logger\RfcLogLevel; use Drupal\Core\StringTranslation\StringTranslationTrait; use Drupal\package_manager\StatusCheckTrait; +use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\ValidationResult; use Drupal\Tests\automatic_updates\Kernel\AutomaticUpdatesKernelTestBase; use Drupal\Tests\package_manager\Traits\PackageManagerBypassTestTrait; @@ -33,11 +34,6 @@ class XdebugValidatorTest extends AutomaticUpdatesKernelTestBase { * {@inheritdoc} */ protected function setUp(): void { - // Ensure the validator will think Xdebug is enabled. - if (!function_exists('xdebug_break')) { - // @codingStandardsIgnoreLine - eval('function xdebug_break() {}'); - } parent::setUp(); // The parent class unconditionally disables the Xdebug validator we're @@ -50,6 +46,7 @@ class XdebugValidatorTest extends AutomaticUpdatesKernelTestBase { * Tests warnings and/or errors if Xdebug is enabled. */ public function testXdebugValidation(): void { + $this->simulateXdebugEnabled(); $message = $this->t('Xdebug is enabled, which may have a negative performance impact on Package Manager and any modules that use it.'); $error = $this->t("Xdebug is enabled, currently Cron Updates are not allowed while it is enabled. If Xdebug is not disabled you will not receive security and other updates during cron."); @@ -94,4 +91,39 @@ class XdebugValidatorTest extends AutomaticUpdatesKernelTestBase { $this->assertTrue($logger->hasRecordThatMatches("/$error/", (string) RfcLogLevel::ERROR)); } + /** + * Tests warnings and/or errors if Xdebug is enabled during pre-apply. + */ + public function testXdebugValidationDuringPreApply(): void { + $listener = function (): void { + $this->simulateXdebugEnabled(); + }; + $this->container->get('event_dispatcher') + ->addListener(PreApplyEvent::class, $listener, PHP_INT_MAX); + $message = "Xdebug is enabled, currently Cron Updates are not allowed while it is enabled. If Xdebug is not disabled you will not receive security and other updates during cron."; + + // The parent class' setUp() method simulates an available security + // update, so ensure that the cron updater will try to update to it. + $this->config('automatic_updates.settings')->set('cron', CronUpdater::SECURITY)->save(); + + // Trying to do the update during cron should fail with an error. + $logger = new TestLogger(); + $this->container->get('logger.factory') + ->get('automatic_updates') + ->addLogger($logger); + $this->container->get('cron')->run(); + $this->assertUpdateStagedTimes(1); + $this->assertTrue($logger->hasRecordThatMatches("/$message/", (string) RfcLogLevel::ERROR)); + } + + /** + * Simulating that xdebug is enabled. + */ + private function simulateXdebugEnabled(): void { + if (!function_exists('xdebug_break')) { + // @codingStandardsIgnoreLine + eval('function xdebug_break() {}'); + } + } + } -- GitLab