diff --git a/package_manager/package_manager.api.php b/package_manager/package_manager.api.php index 5adbc6711ea613deb031075fd25e7bcfe21d73fa..532989ebe1101cd0af4596b5dcc5c494985acbff 100644 --- a/package_manager/package_manager.api.php +++ b/package_manager/package_manager.api.php @@ -100,15 +100,6 @@ * active directory. This event may be dispatched multiple times during the * stage life cycle. * - * - \Drupal\package_manager\Event\PreDestroyEvent - * Dispatched before the stage directory is deleted and the stage releases its - * ownership. This event is dispatched only once during the stage life cycle. - * - * - \Drupal\package_manager\Event\PostDestroyEvent - * Dispatched after the stage directory is deleted and the stage has released - * its ownership. This event is dispatched only once during the stage life - * cycle. - * * @section sec_stage_api Stage API: Public methods * The public API of any stage consists of the following methods: * @@ -140,9 +131,9 @@ * be claimed by its owner to call this method. * * - \Drupal\package_manager\StageBase::destroy() - * Destroys the stage directory, releases ownership, and dispatches pre- and - * post-destroy events. It is possible to destroy the stage without having - * claimed it first, but this shouldn't be done unless absolutely necessary. + * Destroys the stage directory, and releases ownership. It is possible to + * destroy the stage without having claimed it first, but this shouldn't be + * done unless absolutely necessary. * * - \Drupal\package_manager\StageBase::stageDirectoryExists() * Determines if the stage directory exists and returns a boolean accordingly. diff --git a/package_manager/src/Event/PostDestroyEvent.php b/package_manager/src/Event/PostDestroyEvent.php deleted file mode 100644 index b68d214f9edeba60afb7b20d4fdf0e0b11b709e8..0000000000000000000000000000000000000000 --- a/package_manager/src/Event/PostDestroyEvent.php +++ /dev/null @@ -1,16 +0,0 @@ -<?php - -declare(strict_types = 1); - -namespace Drupal\package_manager\Event; - -/** - * Event fired after the stage directory is destroyed. - * - * If the stage is being force destroyed, $this->stage may be an object of a - * different class than the one that originally created the stage directory. - * - * @see \Drupal\package_manager\StageBase::destroy() - */ -final class PostDestroyEvent extends StageEvent { -} diff --git a/package_manager/src/Event/PreDestroyEvent.php b/package_manager/src/Event/PreDestroyEvent.php deleted file mode 100644 index d51e2a76e9f3668882423317aec6bf6325bdf109..0000000000000000000000000000000000000000 --- a/package_manager/src/Event/PreDestroyEvent.php +++ /dev/null @@ -1,16 +0,0 @@ -<?php - -declare(strict_types = 1); - -namespace Drupal\package_manager\Event; - -/** - * Event fired before the stage directory is destroyed. - * - * If the stage is being force destroyed, $this->stage may be an object of a - * different class than the one that originally created the stage directory. - * - * @see \Drupal\package_manager\StageBase::destroy() - */ -final class PreDestroyEvent extends PreOperationStageEvent { -} diff --git a/package_manager/src/StageBase.php b/package_manager/src/StageBase.php index 6b55b1ff3e59db9a1ebd930eb68460e42800317b..b94b0dfc92a4f564caebf874744c5d4d668379c6 100644 --- a/package_manager/src/StageBase.php +++ b/package_manager/src/StageBase.php @@ -16,11 +16,9 @@ use Drupal\Core\TempStore\SharedTempStoreFactory; use Drupal\package_manager\Event\CollectPathsToExcludeEvent; use Drupal\package_manager\Event\PostApplyEvent; use Drupal\package_manager\Event\PostCreateEvent; -use Drupal\package_manager\Event\PostDestroyEvent; use Drupal\package_manager\Event\PostRequireEvent; use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Event\PreCreateEvent; -use Drupal\package_manager\Event\PreDestroyEvent; use Drupal\package_manager\Event\PreOperationStageEvent; use Drupal\package_manager\Event\PreRequireEvent; use Drupal\package_manager\Event\StageEvent; @@ -532,7 +530,6 @@ abstract class StageBase implements LoggerAwareInterface { throw new StageException($this, 'Cannot destroy the stage directory while it is being applied to the active directory.'); } - $this->dispatch(new PreDestroyEvent($this)); $staging_root = $this->getStagingRoot(); // If the stage root directory exists, delete it and everything in it. if (file_exists($staging_root)) { @@ -543,15 +540,14 @@ abstract class StageBase implements LoggerAwareInterface { } catch (FileException) { // Deliberately swallow the exception so that the stage will be marked - // as available and the post-destroy event will be fired, even if the - // stage directory can't actually be deleted. The file system service - // logs the exception, so we don't need to do anything else here. + // as available, even if the stage directory can't actually be deleted. + // The file system service logs the exception, so we don't need to do + // anything else here. } } $this->storeDestroyInfo($force, $message); $this->markAsAvailable(); - $this->dispatch(new PostDestroyEvent($this)); } /** diff --git a/package_manager/src/Validator/LockFileValidator.php b/package_manager/src/Validator/LockFileValidator.php index e1637de81f8c845aeab10dcbee5bca2be96b56e0..819f690742fa36e7d854bde51d1d2b0bf1830725 100644 --- a/package_manager/src/Validator/LockFileValidator.php +++ b/package_manager/src/Validator/LockFileValidator.php @@ -6,7 +6,7 @@ namespace Drupal\package_manager\Validator; use Drupal\Core\State\StateInterface; use Drupal\Core\StringTranslation\StringTranslationTrait; -use Drupal\package_manager\Event\PostDestroyEvent; +use Drupal\package_manager\Event\PostApplyEvent; use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Event\PreCreateEvent; use Drupal\package_manager\Event\PreOperationStageEvent; @@ -176,7 +176,7 @@ final class LockFileValidator implements EventSubscriberInterface { PreRequireEvent::class => 'validate', PreApplyEvent::class => 'validate', StatusCheckEvent::class => 'validate', - PostDestroyEvent::class => 'deleteHash', + PostApplyEvent::class => 'deleteHash', ]; } diff --git a/package_manager/tests/modules/package_manager_test_event_logger/src/EventSubscriber/EventLogSubscriber.php b/package_manager/tests/modules/package_manager_test_event_logger/src/EventSubscriber/EventLogSubscriber.php index 7ff4e1a666274039b3151a454c0ab2ac0553576f..17e9dd3ac632bc83e33b437bb020fd372bf68eba 100644 --- a/package_manager/tests/modules/package_manager_test_event_logger/src/EventSubscriber/EventLogSubscriber.php +++ b/package_manager/tests/modules/package_manager_test_event_logger/src/EventSubscriber/EventLogSubscriber.php @@ -7,11 +7,9 @@ namespace Drupal\package_manager_test_event_logger\EventSubscriber; use Drupal\package_manager\Event\CollectPathsToExcludeEvent; use Drupal\package_manager\Event\PostApplyEvent; use Drupal\package_manager\Event\PostCreateEvent; -use Drupal\package_manager\Event\PostDestroyEvent; use Drupal\package_manager\Event\PostRequireEvent; use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Event\PreCreateEvent; -use Drupal\package_manager\Event\PreDestroyEvent; use Drupal\package_manager\Event\PreRequireEvent; use Drupal\package_manager\Event\StageEvent; use Drupal\package_manager\PathLocator; @@ -80,8 +78,6 @@ final class EventLogSubscriber implements EventSubscriberInterface { PostRequireEvent::class => ['logEventInfo', PHP_INT_MAX], PreApplyEvent::class => ['logEventInfo', PHP_INT_MAX], PostApplyEvent::class => ['logEventInfo', PHP_INT_MAX], - PreDestroyEvent::class => ['logEventInfo', PHP_INT_MAX], - PostDestroyEvent::class => ['logEventInfo', PHP_INT_MAX], ]; } 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 260c0761ab483c025584d6db70009bd5c5ca5074..1e75a1251eca340dcc9a5fe9496e044f5d232124 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 @@ -7,11 +7,9 @@ namespace Drupal\package_manager_test_validation\EventSubscriber; use Drupal\Core\State\StateInterface; use Drupal\package_manager\Event\PostApplyEvent; use Drupal\package_manager\Event\PostCreateEvent; -use Drupal\package_manager\Event\PostDestroyEvent; use Drupal\package_manager\Event\PostRequireEvent; use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Event\PreCreateEvent; -use Drupal\package_manager\Event\PreDestroyEvent; use Drupal\package_manager\Event\PreRequireEvent; use Drupal\package_manager\Event\StageEvent; use Drupal\package_manager\Event\StatusCheckEvent; @@ -160,8 +158,6 @@ class TestSubscriber implements EventSubscriberInterface { PostRequireEvent::class => ['handleEvent', $priority], PreApplyEvent::class => ['handleEvent', $priority], PostApplyEvent::class => ['handleEvent', $priority], - PreDestroyEvent::class => ['handleEvent', $priority], - PostDestroyEvent::class => ['handleEvent', $priority], StatusCheckEvent::class => ['handleEvent', $priority], ]; } diff --git a/package_manager/tests/src/Build/TemplateProjectTestBase.php b/package_manager/tests/src/Build/TemplateProjectTestBase.php index c26fc22bc55967f0d45321ed8e966c6bef5b30c3..c471b315cd0d7931d492fc71e4d0dd7701c51fd9 100644 --- a/package_manager/tests/src/Build/TemplateProjectTestBase.php +++ b/package_manager/tests/src/Build/TemplateProjectTestBase.php @@ -607,7 +607,7 @@ END; * (optional) The expected stage events that should have been fired in the * order in which they should have been fired. Events can be specified more * that once if they will be fired multiple times. If there are no events - * specified all life cycle events from PreCreateEvent to PostDestroyEvent + * specified all life cycle events from PreCreateEvent to PostApplyEvent * will be asserted. * @param int $wait * (optional) How many seconds to wait for the events to be fired. Defaults diff --git a/package_manager/tests/src/Kernel/StageEventsTest.php b/package_manager/tests/src/Kernel/StageEventsTest.php index 474d94ed46445f414c85e69b678acab821f03165..516400a1f93d8f4d99f08447d1519e305206f0cb 100644 --- a/package_manager/tests/src/Kernel/StageEventsTest.php +++ b/package_manager/tests/src/Kernel/StageEventsTest.php @@ -7,11 +7,9 @@ namespace Drupal\Tests\package_manager\Kernel; use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\package_manager\Event\PostApplyEvent; use Drupal\package_manager\Event\PostCreateEvent; -use Drupal\package_manager\Event\PostDestroyEvent; use Drupal\package_manager\Event\PostRequireEvent; use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Event\PreCreateEvent; -use Drupal\package_manager\Event\PreDestroyEvent; use Drupal\package_manager\Event\PreOperationStageEvent; use Drupal\package_manager\Event\PreRequireEvent; use Drupal\package_manager\Event\StageEvent; @@ -75,8 +73,6 @@ class StageEventsTest extends PackageManagerKernelTestBase implements EventSubsc PostRequireEvent::class => 'handleEvent', PreApplyEvent::class => 'handleEvent', PostApplyEvent::class => 'handleEvent', - PreDestroyEvent::class => 'handleEvent', - PostDestroyEvent::class => 'handleEvent', ]; } @@ -112,8 +108,6 @@ class StageEventsTest extends PackageManagerKernelTestBase implements EventSubsc PostRequireEvent::class, PreApplyEvent::class, PostApplyEvent::class, - PreDestroyEvent::class, - PostDestroyEvent::class, ]); } @@ -128,7 +122,6 @@ class StageEventsTest extends PackageManagerKernelTestBase implements EventSubsc 'PreCreateEvent' => [PreCreateEvent::class], 'PreRequireEvent' => [PreRequireEvent::class], 'PreApplyEvent' => [PreApplyEvent::class], - 'PreDestroyEvent' => [PreDestroyEvent::class], ]; } diff --git a/package_manager/tests/src/Kernel/StageOwnershipTest.php b/package_manager/tests/src/Kernel/StageOwnershipTest.php index e0f3a6a6394b716f6f40f7bcf599960255e22c11..201d4a34bff1a684ddf6562762bb84c7f5eaa108 100644 --- a/package_manager/tests/src/Kernel/StageOwnershipTest.php +++ b/package_manager/tests/src/Kernel/StageOwnershipTest.php @@ -6,7 +6,6 @@ namespace Drupal\Tests\package_manager\Kernel; use Drupal\Core\File\FileSystem; use Drupal\Core\Logger\RfcLogLevel; -use Drupal\package_manager\Event\PostDestroyEvent; use Drupal\package_manager\Event\PreCreateEvent; use Drupal\package_manager\Exception\StageException; use Drupal\package_manager\Exception\StageOwnershipException; @@ -318,14 +317,9 @@ class StageOwnershipTest extends PackageManagerKernelTestBase { $logger = new TestLogger(); $logger_channel->addLogger($logger); - // Listen to the post-destroy event so we can confirm that it was fired, and - // the stage was made available, despite the file system error. - $stage_available = NULL; - $this->addEventTestListener(function (PostDestroyEvent $event) use (&$stage_available): void { - $stage_available = $event->stage->isAvailable(); - }, PostDestroyEvent::class); + // Confirm that the stage was made available, despite the file system error. $stage->destroy(); - $this->assertTrue($stage_available); + $this->assertTrue($stage->isAvailable()); // A file system error should have been logged while trying to delete the // stage directory. diff --git a/src/ConsoleUpdateStage.php b/src/ConsoleUpdateStage.php index 7e2f09b52315f5af1fed427c59b2b9b58eff00f5..9131c624224f543608009bec937987b39bcb1bde 100644 --- a/src/ConsoleUpdateStage.php +++ b/src/ConsoleUpdateStage.php @@ -318,17 +318,7 @@ class ConsoleUpdateStage extends UpdateStage { $this->logger->error($e->getMessage()); } $this->lock->release('cron'); - - // If any pre-destroy event subscribers raise validation errors, ensure they - // are formatted and logged. But if any pre- or post-destroy event - // subscribers throw another exception, don't bother catching it, since it - // will be caught and handled by the main cron service. - try { - $this->destroy(); - } - catch (StageEventException $e) { - $this->logger->error($e->getMessage()); - } + $this->destroy(); } } diff --git a/tests/src/Build/CoreUpdateTest.php b/tests/src/Build/CoreUpdateTest.php index 2c03f72d6081390525001a1babf74936e37f5d9c..49060aa5a31603ff86559de8ce33bd35c39df7be 100644 --- a/tests/src/Build/CoreUpdateTest.php +++ b/tests/src/Build/CoreUpdateTest.php @@ -9,11 +9,9 @@ use Drupal\automatic_updates\ConsoleUpdateStage; use Drupal\automatic_updates\UpdateStage; use Drupal\package_manager\Event\PostApplyEvent; use Drupal\package_manager\Event\PostCreateEvent; -use Drupal\package_manager\Event\PostDestroyEvent; use Drupal\package_manager\Event\PostRequireEvent; use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Event\PreCreateEvent; -use Drupal\package_manager\Event\PreDestroyEvent; use Drupal\package_manager\Event\PreRequireEvent; use Drupal\Tests\WebAssert; use Symfony\Component\Process\Process; @@ -126,8 +124,6 @@ class CoreUpdateTest extends UpdateTestBase { PostRequireEvent::class, PreApplyEvent::class, PostApplyEvent::class, - PreDestroyEvent::class, - PostDestroyEvent::class, ], message: 'Error response: ' . $file_contents ); @@ -231,16 +227,12 @@ class CoreUpdateTest extends UpdateTestBase { // The stage will first destroy the stage made above before going through // stage lifecycle events for the cron update. $expected_events = [ - PreDestroyEvent::class, - PostDestroyEvent::class, PreCreateEvent::class, PostCreateEvent::class, PreRequireEvent::class, PostRequireEvent::class, PreApplyEvent::class, PostApplyEvent::class, - PreDestroyEvent::class, - PostDestroyEvent::class, ]; $this->assertExpectedStageEventsFired(ConsoleUpdateStage::class, $expected_events, 360); $this->assertCronUpdateSuccessful(); diff --git a/tests/src/Functional/UpdateErrorTest.php b/tests/src/Functional/UpdateErrorTest.php index 2111614ce2689a35a1b8d3cc653a4c24cbdd1b56..971573f863f4551212426d8e2fcc23ce8f1efa5d 100644 --- a/tests/src/Functional/UpdateErrorTest.php +++ b/tests/src/Functional/UpdateErrorTest.php @@ -6,11 +6,9 @@ namespace Drupal\Tests\automatic_updates\Functional; use Drupal\package_manager\Event\PostApplyEvent; use Drupal\package_manager\Event\PostCreateEvent; -use Drupal\package_manager\Event\PostDestroyEvent; use Drupal\package_manager\Event\PostRequireEvent; use Drupal\package_manager\Event\PreApplyEvent; use Drupal\package_manager\Event\PreCreateEvent; -use Drupal\package_manager\Event\PreDestroyEvent; use Drupal\package_manager\Event\PreOperationStageEvent; use Drupal\package_manager\Event\PreRequireEvent; use Drupal\package_manager\Event\StatusCheckEvent; @@ -236,8 +234,6 @@ class UpdateErrorTest extends UpdaterFormTestBase { $this->assertContains($event, [ PreApplyEvent::class, PostApplyEvent::class, - PreDestroyEvent::class, - PostDestroyEvent::class, ]); // Try to finish the update. $page->pressButton('Continue'); @@ -252,43 +248,9 @@ class UpdateErrorTest extends UpdaterFormTestBase { // message should be visible. $this->assertStringContainsString('/admin/automatic-update-ready/', $session->getCurrentUrl()); $assert_session->statusMessageContains($expected_message, 'error'); - return; - } - - // If we got any further, ensure we were expecting an exception during the - // destroy phase. - $this->assertStringEndsWith('DestroyEvent', $event); - // We should have been forwarded to the main update page, and the exception - // message should be visible. - $assert_session->addressEquals('/admin/reports/updates'); - $assert_session->statusMessageContains($expected_message, 'error'); - - // If the exception was thrown during PreDestroyEvent, the stage was not - // destroyed, so pretend there's another available update, and ensure that - // the user cannot update to it before they delete the existing update. - if ($event === PreDestroyEvent::class) { - $this->mockActiveCoreVersion('9.8.0'); - $this->checkForUpdates(); - $this->drupalGet('/admin/reports/updates/update'); - - $assert_session->statusMessageContains('Cannot begin an update because another Composer operation is currently in progress.', 'error'); - $assert_session->buttonNotExists('Update to 9.8.1'); - $assert_session->buttonExists('Delete existing update'); } } - /** - * {@inheritdoc} - */ - protected function tearDown(): void { - // Ensure that any pre- or post-destroy exception we set up during testing - // does not interfere with the parent class' ability to destroy the stage. - TestSubscriber::setException(NULL, PreDestroyEvent::class); - TestSubscriber::setException(NULL, PostDestroyEvent::class); - - parent::tearDown(); - } - /** * Data provider for ::testUpdateStoppedByEventSubscriber(). * @@ -304,8 +266,6 @@ class UpdateErrorTest extends UpdaterFormTestBase { PostRequireEvent::class, PreApplyEvent::class, PostApplyEvent::class, - PreDestroyEvent::class, - PostDestroyEvent::class, ]; $data = []; foreach ($events as $event) { diff --git a/tests/src/Kernel/ConsoleUpdateStageTest.php b/tests/src/Kernel/ConsoleUpdateStageTest.php index 9953c86b0e71446622923ee47d0be724619f3e07..f017a3bc1e35dccebb1fb79487a427c4b057bb0d 100644 --- a/tests/src/Kernel/ConsoleUpdateStageTest.php +++ b/tests/src/Kernel/ConsoleUpdateStageTest.php @@ -12,10 +12,8 @@ use Drupal\Core\Logger\RfcLogLevel; use Drupal\Core\Url; use Drupal\package_manager\Event\PostApplyEvent; use Drupal\package_manager\Event\PostCreateEvent; -use Drupal\package_manager\Event\PostDestroyEvent; use Drupal\package_manager\Event\PostRequireEvent; use Drupal\package_manager\Event\PreApplyEvent; -use Drupal\package_manager\Event\PreDestroyEvent; use Drupal\package_manager\Event\PreOperationStageEvent; use Drupal\package_manager\Event\PreRequireEvent; use Drupal\package_manager\Event\PreCreateEvent; @@ -250,14 +248,6 @@ END; PostApplyEvent::class, 'Exception', ], - 'pre-destroy exception' => [ - PreDestroyEvent::class, - 'Exception', - ], - 'post-destroy exception' => [ - PostDestroyEvent::class, - 'Exception', - ], // Only pre-operation events can add validation results. // @see \Drupal\package_manager\Event\PreOperationStageEvent // @see \Drupal\package_manager\Stage::dispatch() @@ -273,10 +263,6 @@ END; PreApplyEvent::class, StageEventException::class, ], - 'pre-destroy validation error' => [ - PreDestroyEvent::class, - StageEventException::class, - ], ]; } @@ -306,9 +292,9 @@ END; 'drupal' => __DIR__ . "/../../../package_manager/tests/fixtures/release-history/drupal.9.8.1-security.xml", ]); - // If the pre- or post-destroy events throw an exception, it will not be - // caught by the cron update runner, but it *will* be caught by the main cron - // service, which will log it as a cron error that we'll want to check for. + // If an exception is thrown during destroy, it will not be caught by the + // cron update runner, but it *will* be caught by the main cron service, + // which will log it as a cron error that we'll want to check for. $cron_logger = new TestLogger(); $this->container->get('logger.factory') ->get('cron') @@ -341,36 +327,10 @@ END; $this->runConsoleUpdateStage(); $logged_by_stage = $this->logger->hasRecord($exception->getMessage(), (string) RfcLogLevel::ERROR); - // To check if the exception was logged by the main cron service, we need - // to set up a special predicate, because exceptions logged by cron are - // always formatted in a particular way that we don't control. But the - // original exception object is stored with the log entry, so we look for - // that and confirm that its message is the same. - // @see watchdog_exception() - $predicate = function (array $record) use ($exception): bool { - if (isset($record['context']['exception'])) { - return $record['context']['exception']->getMessage() === $exception->getMessage(); - } - return FALSE; - }; - $logged_by_cron = $cron_logger->hasRecordThatPasses($predicate, (string) RfcLogLevel::ERROR); - - // If a pre-destroy event flags a validation error, it's handled like any - // other event (logged by the cron update runner, but not the main cron - // service). But if a pre- or post-destroy event throws an exception, the - // cron update runner won't try to catch it. Instead, it will be caught and - // logged by the main cron service. - if ($event_class === PreDestroyEvent::class || $event_class === PostDestroyEvent::class) { - // If the pre-destroy event throws an exception or flags a validation - // error, the stage won't be destroyed. But, once the post-destroy event - // is fired, the stage should be fully destroyed and marked as available. - $this->assertSame($event_class === PostDestroyEvent::class, $stage->isAvailable()); - } - else { - $this->assertTrue($stage->isAvailable()); - } + + $this->assertTrue($stage->isAvailable()); $this->assertTrue($logged_by_stage); - $this->assertFalse($logged_by_cron); + $this->assertEmpty($cron_logger->records); } /**