From 9d34ebe0acd35b534a601ca15c77748d98a04001 Mon Sep 17 00:00:00 2001
From: "kunal.sachdev" <kunal.sachdev@3685163.no-reply.drupal.org>
Date: Tue, 4 Oct 2022 17:27:11 +0000
Subject: [PATCH] Issue #3312981 by kunal.sachdev, phenaproxima: Move
XdebugValidator into Package Manager
---
automatic_updates.services.yml | 2 +
package_manager/package_manager.install | 7 ---
package_manager/package_manager.services.yml | 4 ++
.../src/Validator/XdebugValidator.php | 45 +++++++++++++++
.../tests/src/Kernel/XdebugValidatorTest.php | 30 ++++++++++
src/Validator/XdebugValidator.php | 57 ++++++++++++-------
.../AutomaticUpdatesFunctionalTestBase.php | 1 +
.../Kernel/AutomaticUpdatesKernelTestBase.php | 1 +
.../XdebugValidatorTest.php | 2 +-
9 files changed, 122 insertions(+), 27 deletions(-)
create mode 100644 package_manager/src/Validator/XdebugValidator.php
create mode 100644 package_manager/tests/src/Kernel/XdebugValidatorTest.php
diff --git a/automatic_updates.services.yml b/automatic_updates.services.yml
index 85bec42ebb..9b26b15583 100644
--- a/automatic_updates.services.yml
+++ b/automatic_updates.services.yml
@@ -135,6 +135,8 @@ services:
- { 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:
diff --git a/package_manager/package_manager.install b/package_manager/package_manager.install
index fbb5e3564d..03e207a886 100644
--- a/package_manager/package_manager.install
+++ b/package_manager/package_manager.install
@@ -20,13 +20,6 @@ function package_manager_requirements(string $phase) {
'severity' => REQUIREMENT_ERROR,
];
}
- if ($phase === 'runtime' && extension_loaded('xdebug')) {
- $requirements['package_manager_xdebug'] = [
- 'title' => t('Xdebug enabled'),
- 'description' => t('Xdebug is enabled, which may have a negative performance impact on Package Manager and any modules that use it.'),
- 'severity' => REQUIREMENT_WARNING,
- ];
- }
// If we're able to check for the presence of the failure marker at all, do it
// irrespective of the current run phase. If the failure marker is there, the
diff --git a/package_manager/package_manager.services.yml b/package_manager/package_manager.services.yml
index 3db8249aac..cd785118a5 100644
--- a/package_manager/package_manager.services.yml
+++ b/package_manager/package_manager.services.yml
@@ -193,6 +193,10 @@ services:
class: Drupal\package_manager\Validator\SupportedReleaseValidator
tags:
- { name: event_subscriber }
+ package_manager.validator.xdebug:
+ class: Drupal\package_manager\Validator\XdebugValidator
+ tags:
+ - { name: event_subscriber }
package_manager.update_processor:
class: Drupal\package_manager\PackageManagerUpdateProcessor
arguments: [ '@config.factory', '@queue', '@update.fetcher', '@state', '@private_key', '@keyvalue', '@keyvalue.expirable' ]
diff --git a/package_manager/src/Validator/XdebugValidator.php b/package_manager/src/Validator/XdebugValidator.php
new file mode 100644
index 0000000000..a8bd959ae8
--- /dev/null
+++ b/package_manager/src/Validator/XdebugValidator.php
@@ -0,0 +1,45 @@
+<?php
+
+namespace Drupal\package_manager\Validator;
+
+use Drupal\Core\StringTranslation\StringTranslationTrait;
+use Drupal\package_manager\Event\StatusCheckEvent;
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+
+/**
+ * Performs validation if Xdebug is enabled.
+ *
+ * @internal
+ * This is an internal part of Package Manager and may be changed or removed
+ * at any time without warning. External code should not interact with this
+ * class.
+ */
+final class XdebugValidator implements EventSubscriberInterface {
+
+ use StringTranslationTrait;
+
+ /**
+ * Adds warning if Xdebug is enabled.
+ *
+ * @param \Drupal\package_manager\Event\StatusCheckEvent $event
+ * The event object.
+ */
+ public function checkForXdebug(StatusCheckEvent $event): void {
+ if (function_exists('xdebug_break')) {
+ $messages = [
+ $this->t('Xdebug is enabled, which may have a negative performance impact on Package Manager and any modules that use it.'),
+ ];
+ $event->addWarning($messages);
+ }
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public static function getSubscribedEvents() {
+ return [
+ StatusCheckEvent::class => 'checkForXdebug',
+ ];
+ }
+
+}
diff --git a/package_manager/tests/src/Kernel/XdebugValidatorTest.php b/package_manager/tests/src/Kernel/XdebugValidatorTest.php
new file mode 100644
index 0000000000..eaf9cbfc42
--- /dev/null
+++ b/package_manager/tests/src/Kernel/XdebugValidatorTest.php
@@ -0,0 +1,30 @@
+<?php
+
+namespace Drupal\Tests\package_manager\Kernel;
+
+use Drupal\package_manager\ValidationResult;
+
+/**
+ * @covers \Drupal\package_manager\Validator\XdebugValidator
+ *
+ * @group package_manager
+ */
+class XdebugValidatorTest extends PackageManagerKernelTestBase {
+
+ /**
+ * Tests warnings and/or errors if Xdebug is enabled.
+ */
+ public function testXdebugValidation(): void {
+ // Ensure the validator will think Xdebug is enabled.
+ if (!function_exists('xdebug_break')) {
+ // @codingStandardsIgnoreLine
+ eval('function xdebug_break() {}');
+ }
+
+ $result = ValidationResult::createWarning([
+ 'Xdebug is enabled, which may have a negative performance impact on Package Manager and any modules that use it.',
+ ]);
+ $this->assertStatusCheckResults([$result]);
+ }
+
+}
diff --git a/src/Validator/XdebugValidator.php b/src/Validator/XdebugValidator.php
index 3a6d581c9e..e8d842331b 100644
--- a/src/Validator/XdebugValidator.php
+++ b/src/Validator/XdebugValidator.php
@@ -2,13 +2,14 @@
namespace Drupal\automatic_updates\Validator;
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Drupal\automatic_updates\CronUpdater;
use Drupal\automatic_updates\Event\ReadinessCheckEvent;
use Drupal\automatic_updates\Updater;
-use Drupal\Core\StringTranslation\StringTranslationTrait;
use Drupal\package_manager\Event\PreCreateEvent;
use Drupal\package_manager\Event\PreOperationStageEvent;
-use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+use Drupal\package_manager\Event\StatusCheckEvent;
+use Drupal\package_manager\Validator\XdebugValidator as PackageManagerXdebugValidator;
/**
* Performs validation if Xdebug is enabled.
@@ -20,36 +21,54 @@ use Symfony\Component\EventDispatcher\EventSubscriberInterface;
*/
final class XdebugValidator implements EventSubscriberInterface {
- use StringTranslationTrait;
+ /**
+ * The Package Manager validator we're wrapping.
+ *
+ * @var \Drupal\package_manager\Validator\XdebugValidator
+ */
+ private $packageManagerValidator;
/**
- * Performs validation if Xdebug is enabled.
+ * Constructs an XdebugValidator object.
*
- * If Xdebug is enabled, cron updates are prevented. For other updates, only
- * a warning is flagged during readiness checks.
+ * @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;
+ }
+
+ /**
+ * Performs validation if Xdebug is enabled.
*
* @param \Drupal\package_manager\Event\PreOperationStageEvent $event
* The event object.
*/
public function checkForXdebug(PreOperationStageEvent $event): void {
+ $stage = $event->getStage();
+
// We only want to do this check if the stage belongs to Automatic Updates.
- if (!($event->getStage() instanceof Updater)) {
+ if (!($stage instanceof Updater)) {
return;
}
- if (function_exists('xdebug_break')) {
- $messages = [
- $this->t('Xdebug is enabled, which may cause timeout errors.'),
- ];
-
- if ($event->getStage() instanceof CronUpdater) {
- // Cron updates are not allowed if Xdebug is enabled.
- $event->addError($messages);
+ $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());
}
- elseif ($event instanceof ReadinessCheckEvent) {
- // For non-cron updates provide a warning but do not stop updates from
- // executing.
- $event->addWarning($messages);
+ }
+ elseif ($event instanceof ReadinessCheckEvent) {
+ // For non-cron updates provide a warning but do not stop updates from
+ // executing.
+ foreach ($results as $result) {
+ $event->addWarning($result->getMessages(), $result->getSummary());
}
}
}
diff --git a/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php b/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php
index aca41c1655..97cc989455 100644
--- a/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php
+++ b/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php
@@ -39,6 +39,7 @@ abstract class AutomaticUpdatesFunctionalTestBase extends BrowserTestBase {
'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 9daf998a37..828193a302 100644
--- a/tests/src/Kernel/AutomaticUpdatesKernelTestBase.php
+++ b/tests/src/Kernel/AutomaticUpdatesKernelTestBase.php
@@ -40,6 +40,7 @@ abstract class AutomaticUpdatesKernelTestBase extends PackageManagerKernelTestBa
}
// 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();
// By default, pretend we're running Drupal core 9.8.0 and a non-security
diff --git a/tests/src/Kernel/ReadinessValidation/XdebugValidatorTest.php b/tests/src/Kernel/ReadinessValidation/XdebugValidatorTest.php
index b0996ebf57..61df8a6233 100644
--- a/tests/src/Kernel/ReadinessValidation/XdebugValidatorTest.php
+++ b/tests/src/Kernel/ReadinessValidation/XdebugValidatorTest.php
@@ -44,7 +44,7 @@ class XdebugValidatorTest extends AutomaticUpdatesKernelTestBase {
* Tests warnings and/or errors if Xdebug is enabled.
*/
public function testXdebugValidation(): void {
- $message = 'Xdebug is enabled, which may cause timeout errors.';
+ $message = 'Xdebug is enabled, which may have a negative performance impact on Package Manager and any modules that use it.';
$config = $this->config('automatic_updates.settings');
// If cron updates are disabled, the readiness check message should only be
--
GitLab