diff --git a/automatic_updates.services.yml b/automatic_updates.services.yml index f1c157c354ff2022dd043ce2695c3dbca0169241..70c5bc3404ebd8d6c8edb2b8117f859c7928d658 100644 --- a/automatic_updates.services.yml +++ b/automatic_updates.services.yml @@ -90,12 +90,6 @@ services: - '@string_translation' tags: - { name: event_subscriber } - automatic_updates.validator.xdebug: - class: Drupal\automatic_updates\Validator\XdebugValidator - arguments: - - '@package_manager.validator.xdebug' - tags: - - { name: event_subscriber } automatic_updates.validator.version_policy: class: Drupal\automatic_updates\Validator\VersionPolicyValidator arguments: diff --git a/package_manager/src/Validator/XdebugValidator.php b/package_manager/src/Validator/XdebugValidator.php index 1894c06ae1e4ea55bcfa3bc27cb8bf88df6506bb..d9983474b10662caf0dea22b6d9d15aa8dbc1ee6 100644 --- a/package_manager/src/Validator/XdebugValidator.php +++ b/package_manager/src/Validator/XdebugValidator.php @@ -16,7 +16,7 @@ use Symfony\Component\EventDispatcher\EventSubscriberInterface; * at any time without warning. External code should not interact with this * class. */ -final class XdebugValidator implements EventSubscriberInterface { +class XdebugValidator implements EventSubscriberInterface { use StringTranslationTrait; @@ -26,13 +26,26 @@ final class XdebugValidator implements EventSubscriberInterface { * @param \Drupal\package_manager\Event\StatusCheckEvent $event * The event object. */ - public function checkForXdebug(StatusCheckEvent $event): void { + public function validateXdebugOff(StatusCheckEvent $event): void { + $warning = $this->checkForXdebug(); + if ($warning) { + $event->addWarning($warning); + } + } + + /** + * Checks if Xdebug is enabled and returns a warning if it is. + * + * @return array|null + * Returns an array of warnings or null if Xdebug isn't detected. + */ + protected function checkForXdebug(): ?array { if (function_exists('xdebug_break')) { - $messages = [ + return [ $this->t('Xdebug is enabled, which may have a negative performance impact on Package Manager and any modules that use it.'), ]; - $event->addWarning($messages); } + return NULL; } /** @@ -40,7 +53,7 @@ final class XdebugValidator implements EventSubscriberInterface { */ public static function getSubscribedEvents(): array { return [ - StatusCheckEvent::class => 'checkForXdebug', + StatusCheckEvent::class => 'validateXdebugOff', ]; } diff --git a/src/AutomaticUpdatesServiceProvider.php b/src/AutomaticUpdatesServiceProvider.php new file mode 100644 index 0000000000000000000000000000000000000000..b229f354e5717c9c13f0a6b01c3912a39592f6d9 --- /dev/null +++ b/src/AutomaticUpdatesServiceProvider.php @@ -0,0 +1,27 @@ +<?php + +declare(strict_types = 1); + +namespace Drupal\automatic_updates; + +use Drupal\automatic_updates\Validator\XdebugValidator; +use Drupal\Core\DependencyInjection\ContainerBuilder; +use Drupal\Core\DependencyInjection\ServiceProviderBase; + +/** + * Modifies container services for Automatic Updates. + */ +class AutomaticUpdatesServiceProvider extends ServiceProviderBase { + + /** + * {@inheritdoc} + */ + public function alter(ContainerBuilder $container) { + $service_id = 'package_manager.validator.xdebug'; + if ($container->hasDefinition($service_id)) { + $container->getDefinition($service_id) + ->setClass(XdebugValidator::class); + } + } + +} diff --git a/src/Validator/XdebugValidator.php b/src/Validator/XdebugValidator.php index ec6ff6513cb1028fbd16e227f7084797e6daf4f3..3051ad81a3f70392705fd967338de1e2a5367c50 100644 --- a/src/Validator/XdebugValidator.php +++ b/src/Validator/XdebugValidator.php @@ -6,7 +6,6 @@ namespace Drupal\automatic_updates\Validator; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Drupal\automatic_updates\CronUpdater; -use Drupal\automatic_updates\Updater; use Drupal\package_manager\Event\PreCreateEvent; use Drupal\package_manager\Event\PreOperationStageEvent; use Drupal\package_manager\Event\StatusCheckEvent; @@ -20,24 +19,7 @@ use Drupal\package_manager\Validator\XdebugValidator as PackageManagerXdebugVali * at any time without warning. External code should not interact with this * class. */ -final class XdebugValidator implements EventSubscriberInterface { - - /** - * The Package Manager validator we're wrapping. - * - * @var \Drupal\package_manager\Validator\XdebugValidator - */ - private $packageManagerValidator; - - /** - * Constructs an XdebugValidator object. - * - * @param \Drupal\package_manager\Validator\XdebugValidator $package_manager_validator - * The Package Manager validator we're wrapping. - */ - public function __construct(PackageManagerXdebugValidator $package_manager_validator) { - $this->packageManagerValidator = $package_manager_validator; - } +final class XdebugValidator extends PackageManagerXdebugValidator implements EventSubscriberInterface { /** * Performs validation if Xdebug is enabled. @@ -45,31 +27,17 @@ final class XdebugValidator implements EventSubscriberInterface { * @param \Drupal\package_manager\Event\PreOperationStageEvent $event * The event object. */ - public function checkForXdebug(PreOperationStageEvent $event): void { + public function validateXdebugOff(PreOperationStageEvent $event): void { $stage = $event->getStage(); + $warning = $this->checkForXdebug(); - // We only want to do this check if the stage belongs to Automatic Updates. - if (!($stage instanceof Updater)) { - return; - } - - $status_check = new StatusCheckEvent($stage, []); - $this->packageManagerValidator->checkForXdebug($status_check); - $results = $status_check->getResults(); - if (empty($results)) { - return; - } - elseif ($stage instanceof CronUpdater) { - // Cron updates are not allowed if Xdebug is enabled. - foreach ($results as $result) { - $event->addError($result->getMessages(), $result->getSummary()); + if ($warning) { + if ($stage instanceof CronUpdater) { + // Cron updates are not allowed if Xdebug is enabled. + $event->addError([$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.")]); } - } - elseif ($event instanceof StatusCheckEvent) { - // For non-cron updates provide a warning but do not stop updates from - // executing. - foreach ($results as $result) { - $event->addWarning($result->getMessages(), $result->getSummary()); + elseif ($event instanceof StatusCheckEvent) { + $event->addWarning($warning); } } } @@ -79,8 +47,8 @@ final class XdebugValidator implements EventSubscriberInterface { */ public static function getSubscribedEvents(): array { return [ - PreCreateEvent::class => 'checkForXdebug', - StatusCheckEvent::class => 'checkForXdebug', + PreCreateEvent::class => 'validateXdebugOff', + StatusCheckEvent::class => 'validateXdebugOff', ]; } diff --git a/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php b/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php index b1168db288498480eef5776d4d9db066b1a9e3ed..608b5311c4a491c506826222bce897e95d57cfdd 100644 --- a/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php +++ b/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php @@ -60,7 +60,6 @@ abstract class AutomaticUpdatesFunctionalTestBase extends BrowserTestBase { 'automatic_updates.composer_executable_validator', 'package_manager.validator.composer_executable', // Always allow tests to run with Xdebug on. - 'automatic_updates.validator.xdebug', 'package_manager.validator.xdebug', ]; diff --git a/tests/src/Kernel/AutomaticUpdatesKernelTestBase.php b/tests/src/Kernel/AutomaticUpdatesKernelTestBase.php index aa8670c2afb9da762e188182efd7863373b1d476..800ec5f403cebbf4dc9a8909d2637ccfd91e40da 100644 --- a/tests/src/Kernel/AutomaticUpdatesKernelTestBase.php +++ b/tests/src/Kernel/AutomaticUpdatesKernelTestBase.php @@ -51,7 +51,6 @@ abstract class AutomaticUpdatesKernelTestBase extends PackageManagerKernelTestBa $this->disableValidators[] = 'automatic_updates.validator.symlink'; } // Always disable the Xdebug validator to allow test to run with Xdebug on. - $this->disableValidators[] = 'automatic_updates.validator.xdebug'; $this->disableValidators[] = 'package_manager.validator.xdebug'; parent::setUp(); // Enable cron updates, which will eventually be the default. diff --git a/tests/src/Kernel/StatusCheck/XdebugValidatorTest.php b/tests/src/Kernel/StatusCheck/XdebugValidatorTest.php index a7006fe850dfd1e8a5e280a8bf87d9fcdea46af0..1ef9e2269a0f210f2f0066608c9840d73207d15b 100644 --- a/tests/src/Kernel/StatusCheck/XdebugValidatorTest.php +++ b/tests/src/Kernel/StatusCheck/XdebugValidatorTest.php @@ -7,6 +7,7 @@ namespace Drupal\Tests\automatic_updates\Kernel\StatusCheck; use Drupal\automatic_updates\CronUpdater; use Drupal\Core\Logger\RfcLogLevel; use Drupal\Core\StringTranslation\StringTranslationTrait; +use Drupal\package_manager\StatusCheckTrait; use Drupal\package_manager\ValidationResult; use Drupal\Tests\automatic_updates\Kernel\AutomaticUpdatesKernelTestBase; use Drupal\Tests\package_manager\Traits\PackageManagerBypassTestTrait; @@ -21,6 +22,7 @@ class XdebugValidatorTest extends AutomaticUpdatesKernelTestBase { use PackageManagerBypassTestTrait; use StringTranslationTrait; + use StatusCheckTrait; /** * {@inheritdoc} @@ -40,7 +42,7 @@ class XdebugValidatorTest extends AutomaticUpdatesKernelTestBase { // The parent class unconditionally disables the Xdebug validator we're // testing, so undo that here. - $validator = $this->container->get('automatic_updates.validator.xdebug'); + $validator = $this->container->get('package_manager.validator.xdebug'); $this->container->get('event_dispatcher')->addSubscriber($validator); } @@ -48,12 +50,26 @@ class XdebugValidatorTest extends AutomaticUpdatesKernelTestBase { * Tests warnings and/or errors if Xdebug is enabled. */ public function testXdebugValidation(): void { - $message = 'Xdebug is enabled, which may have a negative performance impact on Package Manager and any modules that use it.'; + $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."); $config = $this->config('automatic_updates.settings'); // If cron updates are disabled, the status check message should only be // a warning. $config->set('cron', CronUpdater::DISABLED)->save(); + + // Tests that other projects which depend on Package manager also get the + // warning. + $stage = $this->createStage(); + $this->assertUpdateStagedTimes(0); + $stage->create(); + $stage->require(['drupal/random']); + $this->assertUpdateStagedTimes(1); + $event_dispatcher = \Drupal::service('event_dispatcher'); + $result = $this->runStatusCheck($stage, $event_dispatcher, TRUE); + $this->assertSame($message->getUntranslatedString(), $result[0]->getMessages()[0]->getUntranslatedString()); + $stage->destroy(TRUE); + $result = ValidationResult::createWarning([$message]); $this->assertCheckerResultsFromManager([$result], TRUE); @@ -63,7 +79,7 @@ class XdebugValidatorTest extends AutomaticUpdatesKernelTestBase { // If cron updates are enabled the status check message should be an // error. - $result = ValidationResult::createError([$message]); + $result = ValidationResult::createError([$error]); $this->assertCheckerResultsFromManager([$result], TRUE); // Trying to do the update during cron should fail with an error. @@ -73,8 +89,9 @@ class XdebugValidatorTest extends AutomaticUpdatesKernelTestBase { ->addLogger($logger); $this->container->get('cron')->run(); - $this->assertUpdateStagedTimes(0); - $this->assertTrue($logger->hasRecordThatMatches("/$message/", (string) RfcLogLevel::ERROR)); + // Assert there was not another update staged during cron. + $this->assertUpdateStagedTimes(1); + $this->assertTrue($logger->hasRecordThatMatches("/$error/", (string) RfcLogLevel::ERROR)); } }