diff --git a/package_manager/src/Exception/StageValidationException.php b/package_manager/src/Exception/StageValidationException.php index 2654615bf9060797a3ab93db1b1a27638231f22e..fb07184cff6c02ace08887ab8e34f609e8451b91 100644 --- a/package_manager/src/Exception/StageValidationException.php +++ b/package_manager/src/Exception/StageValidationException.php @@ -21,12 +21,15 @@ class StageValidationException extends StageException { * * @param \Drupal\package_manager\ValidationResult[] $results * Any relevant validation results. + * @param string $message + * (optional) The exception message. Defaults to a plain text representation + * of the validation results. * @param mixed ...$arguments - * Arguments to pass to the parent constructor. + * Additional arguments to pass to the parent constructor. */ - public function __construct(array $results = [], ...$arguments) { + public function __construct(array $results = [], string $message = '', ...$arguments) { $this->results = $results; - parent::__construct(...$arguments); + parent::__construct($message ?: $this->getResultsAsText(), ...$arguments); } /** @@ -39,4 +42,24 @@ class StageValidationException extends StageException { return $this->results; } + /** + * Formats the validation results as plain text. + * + * @return string + * The results, formatted as plain text. + */ + protected function getResultsAsText(): string { + $text = ''; + + foreach ($this->getResults() as $result) { + $messages = $result->getMessages(); + $summary = $result->getSummary(); + if ($summary) { + array_unshift($messages, $summary); + } + $text .= implode("\n", $messages) . "\n"; + } + return $text; + } + } diff --git a/package_manager/tests/modules/package_manager_test_validation/src/EventSubscriber/TestSubscriber.php b/package_manager/tests/modules/package_manager_test_validation/src/EventSubscriber/TestSubscriber.php index f88eb4f9a4de11490593ed85678921c7b06f9cf0..ca317f0f7e4584489ca1d7705a75639d678522e4 100644 --- a/package_manager/tests/modules/package_manager_test_validation/src/EventSubscriber/TestSubscriber.php +++ b/package_manager/tests/modules/package_manager_test_validation/src/EventSubscriber/TestSubscriber.php @@ -54,7 +54,7 @@ class TestSubscriber implements EventSubscriberInterface { * The event class. */ public static function setExit(string $event): void { - \Drupal::state()->set(static::STATE_KEY . ".$event", 'exit'); + \Drupal::state()->set(self::getStateKey($event), 'exit'); } /** @@ -69,7 +69,7 @@ class TestSubscriber implements EventSubscriberInterface { * The event class. */ public static function setTestResult(?array $results, string $event): void { - $key = static::STATE_KEY . '.' . $event; + $key = static::getStateKey($event); $state = \Drupal::state(); if (isset($results)) { @@ -92,7 +92,7 @@ class TestSubscriber implements EventSubscriberInterface { * The event class. */ public static function setException(?\Throwable $error, string $event): void { - $key = static::STATE_KEY . '.' . $event; + $key = self::getStateKey($event); $state = \Drupal::state(); if (isset($error)) { @@ -103,6 +103,20 @@ class TestSubscriber implements EventSubscriberInterface { } } + /** + * Computes the state key to use for a given event class. + * + * @param string $event + * The event class. + * + * @return string + * The state key under which to store the results for the given event. + */ + protected static function getStateKey(string $event) { + $parts = explode('\\', $event); + return static::STATE_KEY . array_pop($parts); + } + /** * Adds validation results to a stage event. * @@ -110,7 +124,7 @@ class TestSubscriber implements EventSubscriberInterface { * The event object. */ public function handleEvent(StageEvent $event): void { - $results = $this->state->get(static::STATE_KEY . '.' . get_class($event), []); + $results = $this->state->get(self::getStateKey(get_class($event)), []); // Record that value of maintenance mode for each event. $this->state->set(get_class($event) . '.' . 'system.maintenance_mode', $this->state->get('system.maintenance_mode')); diff --git a/package_manager/tests/src/Kernel/StageEventsTest.php b/package_manager/tests/src/Kernel/StageEventsTest.php index e154e95d975cafcc20cc4cf54990abf4e4a47238..34cd73eef1e9b04323d6622b902d68df18bf209e 100644 --- a/package_manager/tests/src/Kernel/StageEventsTest.php +++ b/package_manager/tests/src/Kernel/StageEventsTest.php @@ -140,7 +140,7 @@ class StageEventsTest extends PackageManagerKernelTestBase implements EventSubsc $handler = function (StageEvent $event) use ($event_class): void { if (get_class($event) === $event_class) { if ($event instanceof PreOperationStageEvent) { - $event->addError([['Burn, baby, burn']]); + $event->addError(['Burn, baby, burn']); } } }; diff --git a/package_manager/tests/src/Kernel/StageValidationExceptionTest.php b/package_manager/tests/src/Kernel/StageValidationExceptionTest.php new file mode 100644 index 0000000000000000000000000000000000000000..cf5de802a4360cd799946eaa566b3a1f4bb4a865 --- /dev/null +++ b/package_manager/tests/src/Kernel/StageValidationExceptionTest.php @@ -0,0 +1,81 @@ +<?php + +namespace Drupal\Tests\package_manager\Kernel; + +use Drupal\automatic_updates_test\EventSubscriber\TestSubscriber1; +use Drupal\package_manager\Event\PreCreateEvent; +use Drupal\package_manager\Exception\StageValidationException; +use Drupal\package_manager\ValidationResult; + +/** + * @coversDefaultClass \Drupal\package_manager\Exception\StageValidationException + * + * @group package_manager + */ +class StageValidationExceptionTest extends PackageManagerKernelTestBase { + + /** + * {@inheritdoc} + */ + protected static $modules = [ + 'package_manager_test_validation', + ]; + + /** + * Data provider for testErrors(). + * + * @return array[] + * The test cases for testErrors(). + */ + public function providerResultsAsText(): array { + $messages = ['Blam!', 'Kapow!']; + $summary = t('There was sadness.'); + + $result_no_summary = ValidationResult::createError([$messages[0]]); + $result_with_summary = ValidationResult::createError($messages, $summary); + $result_with_summary_message = "{$summary->getUntranslatedString()}\n{$messages[0]}\n{$messages[1]}\n"; + + return [ + '1 result with summary' => [ + [$result_with_summary], + $result_with_summary_message, + ], + '2 results, with summaries' => [ + [$result_with_summary, $result_with_summary], + "$result_with_summary_message$result_with_summary_message", + ], + '1 result without summary' => [ + [$result_no_summary], + $messages[0], + ], + '2 results without summaries' => [ + [$result_no_summary, $result_no_summary], + $messages[0] . "\n" . $messages[0], + ], + '1 result with summary, 1 result without summary' => [ + [$result_with_summary, $result_no_summary], + $result_with_summary_message . $messages[0] . "\n", + ], + ]; + } + + /** + * Tests formatting a set of validation results as plain text. + * + * @param \Drupal\package_manager\ValidationResult[] $validation_results + * The expected validation results which should be logged. + * @param string $expected_message + * The expected exception message. + * + * @dataProvider providerResultsAsText + * + * @covers ::getResultsAsText() + */ + public function testResultsAsText(array $validation_results, string $expected_message): void { + TestSubscriber1::setTestResult($validation_results, PreCreateEvent::class); + $this->expectException(StageValidationException::class); + $this->expectExceptionMessage($expected_message); + $this->createStage()->create(); + } + +} diff --git a/src/BatchProcessor.php b/src/BatchProcessor.php index 6584ff982c2ce959b7495f1822c15d52c5ff3318..d7ab45cbb8df47213b29dd058c7c1ebbc99ae12e 100644 --- a/src/BatchProcessor.php +++ b/src/BatchProcessor.php @@ -49,9 +49,7 @@ class BatchProcessor { * have been recorded. */ protected static function handleException(\Throwable $error, array &$context): void { - $error_messages = [ - $error->getMessage(), - ]; + $error_messages = []; if ($error instanceof StageValidationException) { foreach ($error->getResults() as $result) { @@ -62,6 +60,9 @@ class BatchProcessor { $error_messages = array_merge($error_messages, $messages); } } + else { + $error_messages[] = $error->getMessage(); + } foreach ($error_messages as $error_message) { $context['results']['errors'][] = $error_message; diff --git a/src/CronUpdater.php b/src/CronUpdater.php index cba9e6cc33c86d6db4ec7e67358212f5e36f8eac..7f6400c6c5d2bafd2914006bd40d0e68b1c6dcac 100644 --- a/src/CronUpdater.php +++ b/src/CronUpdater.php @@ -111,7 +111,7 @@ class CronUpdater extends Updater { ); } catch (\Throwable $e) { - $this->handleException($e); + $this->logger->error($e->getMessage()); } // If an error occurred during the pre-create event, the stage will be @@ -129,7 +129,7 @@ class CronUpdater extends Updater { $this->destroy(); } catch (StageValidationException $e) { - $this->handleException($e); + $this->logger->error($e->getMessage()); } } @@ -143,44 +143,4 @@ class CronUpdater extends Updater { return $this->configFactory->get('automatic_updates.settings')->get('cron') === static::DISABLED; } - /** - * Generates a log message from a stage validation exception. - * - * @param \Drupal\package_manager\Exception\StageValidationException $exception - * The validation exception. - * - * @return string - * The formatted log message, including all the validation results. - */ - protected static function formatValidationException(StageValidationException $exception): string { - $log_message = ''; - foreach ($exception->getResults() as $result) { - $summary = $result->getSummary(); - if ($summary) { - $log_message .= "<h3>$summary</h3><ul>"; - foreach ($result->getMessages() as $message) { - $log_message .= "<li>$message</li>"; - } - $log_message .= "</ul>"; - } - else { - $log_message .= ($log_message ? ' ' : '') . $result->getMessages()[0]; - } - } - return "<h2>{$exception->getMessage()}</h2>$log_message"; - } - - /** - * Handles an exception that is caught during an update. - * - * @param \Throwable $e - * The caught exception. - */ - protected function handleException(\Throwable $e): void { - $message = $e instanceof StageValidationException - ? static::formatValidationException($e) - : $e->getMessage(); - $this->logger->error($message); - } - } diff --git a/tests/src/Kernel/AutomaticUpdatesKernelTestBase.php b/tests/src/Kernel/AutomaticUpdatesKernelTestBase.php index 84318805325a6cbca72fa0119a436adf8a4c847a..c9f8ffdd1e94398b2394145cb2d466ec9e813a2b 100644 --- a/tests/src/Kernel/AutomaticUpdatesKernelTestBase.php +++ b/tests/src/Kernel/AutomaticUpdatesKernelTestBase.php @@ -5,7 +5,6 @@ namespace Drupal\Tests\automatic_updates\Kernel; use Drupal\automatic_updates\CronUpdater; use Drupal\automatic_updates\Updater; use Drupal\Core\DependencyInjection\ContainerBuilder; -use Drupal\package_manager\Exception\StageValidationException; use Drupal\Tests\automatic_updates\Traits\ValidationTestTrait; use Drupal\Tests\package_manager\Kernel\PackageManagerKernelTestBase; use Drupal\Tests\package_manager\Kernel\TestStage; @@ -152,11 +151,4 @@ class TestCronUpdater extends CronUpdater { return TestStage::$stagingRoot ?: parent::getStagingRoot(); } - /** - * {@inheritdoc} - */ - public static function formatValidationException(StageValidationException $exception): string { - return parent::formatValidationException($exception); - } - } diff --git a/tests/src/Kernel/CronUpdaterTest.php b/tests/src/Kernel/CronUpdaterTest.php index db15b37a8f8eb02cc43149d6bb88f2b4fac71619..b40f6a1de7f39ec9807d31342c92888ce3d6cb43 100644 --- a/tests/src/Kernel/CronUpdaterTest.php +++ b/tests/src/Kernel/CronUpdaterTest.php @@ -233,46 +233,6 @@ class CronUpdaterTest extends AutomaticUpdatesKernelTestBase { ]; } - /** - * Data provider for testErrors(). - * - * @return array[] - * The test cases for testErrors(). - */ - public function providerErrors(): array { - $messages = [ - 'PreCreate Event Error', - 'PreCreate Event Error 2', - ]; - $summary = 'There were errors in updates'; - $result_no_summary = ValidationResult::createError([$messages[0]]); - $result_with_summary = ValidationResult::createError($messages, t($summary)); - $result_with_summary_message = "<h3>{$summary}</h3><ul><li>{$messages[0]}</li><li>{$messages[1]}</li></ul>"; - - return [ - '1 result with summary' => [ - [$result_with_summary], - $result_with_summary_message, - ], - '2 results with summary' => [ - [$result_with_summary, $result_with_summary], - "$result_with_summary_message$result_with_summary_message", - ], - '1 result without summary' => [ - [$result_no_summary], - $messages[0], - ], - '2 results without summary' => [ - [$result_no_summary, $result_no_summary], - $messages[0] . ' ' . $messages[0], - ], - '1 result with summary, 1 result without summary' => [ - [$result_with_summary, $result_no_summary], - $result_with_summary_message . ' ' . $messages[0], - ], - ]; - } - /** * Tests that the stage is destroyed if an error occurs during a cron update. * @@ -305,15 +265,15 @@ class CronUpdaterTest extends AutomaticUpdatesKernelTestBase { ValidationResult::createError(['Destroy the stage!']), ]; TestSubscriber1::setTestResult($results, $event_class); - $exception = new StageValidationException($results, 'Unable to complete the update because of errors.'); - $expected_log_message = TestCronUpdater::formatValidationException($exception); + $exception = new StageValidationException($results); } else { /** @var \Throwable $exception */ $exception = new $exception_class('Destroy the stage!'); TestSubscriber1::setException($exception, $event_class); - $expected_log_message = $exception->getMessage(); } + $expected_log_message = $exception->getMessage(); + // Ensure that nothing has been logged yet. $this->assertEmpty($cron_logger->records); $this->assertEmpty($this->logger->records); @@ -367,21 +327,4 @@ class CronUpdaterTest extends AutomaticUpdatesKernelTestBase { } } - /** - * Tests errors during a cron update attempt. - * - * @param \Drupal\package_manager\ValidationResult[] $validation_results - * The expected validation results which should be logged. - * @param string $expected_log_message - * The error message should be logged. - * - * @dataProvider providerErrors - */ - public function testErrors(array $validation_results, string $expected_log_message): void { - TestSubscriber1::setTestResult($validation_results, PreCreateEvent::class); - $this->container->get('cron')->run(); - $this->assertUpdateStagedTimes(0); - $this->assertTrue($this->logger->hasRecord("<h2>Unable to complete the update because of errors.</h2>$expected_log_message", RfcLogLevel::ERROR)); - } - }