diff --git a/automatic_updates.module b/automatic_updates.module index 4e461d2195521460a6045e0bcccc417adce71321..a2d52d94bac102fee8b94a42b84530ab437d5b65 100644 --- a/automatic_updates.module +++ b/automatic_updates.module @@ -9,6 +9,7 @@ declare(strict_types = 1); use Drupal\automatic_updates\BatchProcessor; use Drupal\automatic_updates\CronUpdateStage; +use Drupal\automatic_updates\Validator\AutomatedCronDisabledValidator; use Drupal\Core\Routing\RouteMatchInterface; use Drupal\automatic_updates\Validation\AdminStatusCheckMessages; use Drupal\Core\Url; @@ -172,6 +173,18 @@ function automatic_updates_cron() { return; } + /** @var \Drupal\automatic_updates\Validator\AutomatedCronDisabledValidator $automated_cron_validator */ + $automated_cron_validator = \Drupal::service(AutomatedCronDisabledValidator::class); + if ($automated_cron_validator->hasTerminateBeenCalled()) { + // Running updates and status checks will not work during kernel + // termination. + \Drupal::logger('automatic_updates') + ->info('Unattended automatic updates were triggered by Automated Cron, which is not supported. No update was performed. See the <a href=":url">status report</a> for more information.', [ + ':url' => Url::fromRoute('system.status')->toString(), + ]); + return; + } + /** @var \Drupal\automatic_updates\Validation\StatusChecker $status_checker */ $status_checker = \Drupal::service('automatic_updates.status_checker'); $last_results = $status_checker->getResults(); diff --git a/automatic_updates.services.yml b/automatic_updates.services.yml index 8f2b54527ff92b268ff1aa28adb188aae97635a2..8e0a83cc13b88b3cc7a3c3f181e06c3875075b25 100644 --- a/automatic_updates.services.yml +++ b/automatic_updates.services.yml @@ -67,6 +67,9 @@ services: $lock: '@lock' tags: - { name: event_subscriber } + Drupal\automatic_updates\Validator\AutomatedCronDisabledValidator: + tags: + - { name: event_subscriber } automatic_updates.validator.staged_database_updates: class: Drupal\automatic_updates\Validator\StagedDatabaseUpdateValidator tags: diff --git a/src/Validator/AutomatedCronDisabledValidator.php b/src/Validator/AutomatedCronDisabledValidator.php new file mode 100644 index 0000000000000000000000000000000000000000..8614691712aff5251d8ea99b619940f8461ab0cb --- /dev/null +++ b/src/Validator/AutomatedCronDisabledValidator.php @@ -0,0 +1,89 @@ +<?php + +declare(strict_types = 1); + +namespace Drupal\automatic_updates\Validator; + +use Drupal\automatic_updates\CronUpdateStage; +use Drupal\Core\Extension\ModuleHandlerInterface; +use Drupal\Core\StringTranslation\StringTranslationTrait; +use Drupal\package_manager\Event\StatusCheckEvent; +use Symfony\Component\EventDispatcher\EventSubscriberInterface; +use Symfony\Component\HttpKernel\KernelEvents; + +/** + * Ensures that updates cannot be triggered by Automated Cron. + * + * @internal + * This is an internal part of Automatic Updates and may be changed or removed + * at any time without warning. External code should not interact with this + * class. + */ +final class AutomatedCronDisabledValidator implements EventSubscriberInterface { + + use StringTranslationTrait; + + /** + * Flags whether the KernelEvents::TERMINATE event has been dispatched. + * + * @var bool + */ + private bool $terminateCalled = FALSE; + + /** + * AutomatedCronDisabledValidator constructor. + * + * @param \Drupal\Core\Extension\ModuleHandlerInterface $moduleHandler + * The module handler. + */ + public function __construct( + private readonly ModuleHandlerInterface $moduleHandler, + ) {} + + /** + * Checks that Automated Cron is not going to trigger unattended updates. + * + * @param \Drupal\package_manager\Event\StatusCheckEvent $event + * The event being handled. + */ + public function validateStatusCheck(StatusCheckEvent $event): void { + if ($event->stage instanceof CronUpdateStage && $this->moduleHandler->moduleExists('automated_cron')) { + $event->addWarning([ + $this->t('This site has the Automated Cron module installed. To use unattended automatic updates, please configure cron manually on your hosting environment. The Automatic Updates module will not do anything if it is triggered by Automated Cron. See the <a href=":url">Automated Cron documentation</a> for information.', [ + ':url' => 'https://www.drupal.org/docs/administering-a-drupal-site/cron-automated-tasks/cron-automated-tasks-overview#s-more-reliable-enable-cron-using-external-trigger', + ]), + ]); + } + } + + /** + * {@inheritdoc} + */ + public static function getSubscribedEvents(): array { + return [ + StatusCheckEvent::class => 'validateStatusCheck', + // Ensure this runs before + // \Drupal\automated_cron\EventSubscriber\AutomatedCron::onTerminate(). + KernelEvents::TERMINATE => ['setTerminateCalled', PHP_INT_MAX], + ]; + } + + /** + * Sets a flag is when the kernel terminates. + */ + public function setTerminateCalled(): void { + $this->terminateCalled = TRUE; + } + + /** + * Determines whether the kernel has terminated. + * + * @return bool + * TRUE if the kernel has terminated (i.e., KernelEvents::TERMINATE has been + * handled), otherwise FALSE. + */ + public function hasTerminateBeenCalled(): bool { + return $this->terminateCalled; + } + +} diff --git a/src/Validator/CronFrequencyValidator.php b/src/Validator/CronFrequencyValidator.php index 5fffda02e7292968754857275f7371686547f099..2b1e3a0e8b6770dfb465524fbf6f9a6ab3aa6fe8 100644 --- a/src/Validator/CronFrequencyValidator.php +++ b/src/Validator/CronFrequencyValidator.php @@ -7,11 +7,9 @@ namespace Drupal\automatic_updates\Validator; use Drupal\automatic_updates\CronUpdateStage; use Drupal\Component\Datetime\TimeInterface; use Drupal\Core\Config\ConfigFactoryInterface; -use Drupal\Core\Extension\ModuleHandlerInterface; use Drupal\Core\Lock\LockBackendInterface; use Drupal\Core\State\StateInterface; use Drupal\Core\StringTranslation\StringTranslationTrait; -use Drupal\Core\Url; use Drupal\package_manager\Event\StatusCheckEvent; use Symfony\Component\EventDispatcher\EventSubscriberInterface; @@ -23,7 +21,7 @@ use Symfony\Component\EventDispatcher\EventSubscriberInterface; * at any time without warning. External code should not interact with this * class. */ -class CronFrequencyValidator implements EventSubscriberInterface { +final class CronFrequencyValidator implements EventSubscriberInterface { use StringTranslationTrait; @@ -59,8 +57,6 @@ class CronFrequencyValidator implements EventSubscriberInterface { * * @param \Drupal\Core\Config\ConfigFactoryInterface $configFactory * The config factory service. - * @param \Drupal\Core\Extension\ModuleHandlerInterface $moduleHandler - * The module handler. * @param \Drupal\Core\State\StateInterface $state * The state service. * @param \Drupal\Component\Datetime\TimeInterface $time @@ -70,19 +66,18 @@ class CronFrequencyValidator implements EventSubscriberInterface { */ public function __construct( private readonly ConfigFactoryInterface $configFactory, - private readonly ModuleHandlerInterface $moduleHandler, private readonly StateInterface $state, private readonly TimeInterface $time, private readonly LockBackendInterface $lock, ) {} /** - * Validates that cron runs frequently enough to perform automatic updates. + * Validates the cron frequency according to the last cron run time. * * @param \Drupal\package_manager\Event\StatusCheckEvent $event * The event object. */ - public function checkCronFrequency(StatusCheckEvent $event): void { + public function validateLastCronRun(StatusCheckEvent $event): void { // We only want to do this check if the stage belongs to Automatic Updates. if (!$event->stage instanceof CronUpdateStage) { return; @@ -92,43 +87,6 @@ class CronFrequencyValidator implements EventSubscriberInterface { if ($event->stage->getMode() === CronUpdateStage::DISABLED) { return; } - elseif ($this->moduleHandler->moduleExists('automated_cron')) { - $this->validateAutomatedCron($event); - } - else { - $this->validateLastCronRun($event); - } - } - - /** - * Validates the cron frequency according to Automated Cron settings. - * - * @param \Drupal\package_manager\Event\StatusCheckEvent $event - * The event object. - */ - protected function validateAutomatedCron(StatusCheckEvent $event): void { - $message = $this->t('Cron is not set to run frequently enough. <a href=":configure">Configure it</a> to run at least every @frequency hours or disable automated cron and run it via an external scheduling system.', [ - ':configure' => Url::fromRoute('system.cron_settings')->toString(), - '@frequency' => static::SUGGESTED_INTERVAL, - ]); - - $interval = $this->configFactory->get('automated_cron.settings')->get('interval'); - - if ($interval > static::ERROR_INTERVAL) { - $event->addError([$message]); - } - elseif ($interval > static::WARNING_INTERVAL) { - $event->addWarning([$message]); - } - } - - /** - * Validates the cron frequency according to the last cron run time. - * - * @param \Drupal\package_manager\Event\StatusCheckEvent $event - * The event object. - */ - protected function validateLastCronRun(StatusCheckEvent $event): void { // If cron is running right now, cron is clearly being run recently enough! if (!$this->lock->lockMayBeAvailable('cron')) { return; @@ -152,7 +110,7 @@ class CronFrequencyValidator implements EventSubscriberInterface { */ public static function getSubscribedEvents(): array { return [ - StatusCheckEvent::class => 'checkCronFrequency', + StatusCheckEvent::class => 'validateLastCronRun', ]; } diff --git a/tests/src/Functional/AutomatedCronDisabledValidatorTest.php b/tests/src/Functional/AutomatedCronDisabledValidatorTest.php new file mode 100644 index 0000000000000000000000000000000000000000..41567a3bc7efe3a015bc45b486f45838c146e3ec --- /dev/null +++ b/tests/src/Functional/AutomatedCronDisabledValidatorTest.php @@ -0,0 +1,53 @@ +<?php + +namespace Drupal\Tests\automatic_updates\Functional; + +use Drupal\automatic_updates\CronUpdateStage; + +/** + * Tests that updates are not run by Automated Cron. + * + * @covers \Drupal\automatic_updates\Validator\AutomatedCronDisabledValidator + * @group automatic_updates + */ +class AutomatedCronDisabledValidatorTest extends AutomaticUpdatesFunctionalTestBase { + + /** + * {@inheritdoc} + */ + protected $defaultTheme = 'stark'; + + /** + * {@inheritdoc} + */ + protected static $modules = ['dblog', 'automated_cron']; + + /** + * {@inheritdoc} + */ + protected function setUp(): void { + parent::setUp(); + $this->drupalLogin($this->createUser(['access site reports'])); + } + + /** + * Tests that automatic updates are not triggered by Automated Cron. + */ + public function testAutomatedCronUpdate() { + // Delete the last cron run time, to ensure that Automated Cron will run. + $this->container->get('state')->delete('system.cron_last'); + $this->config('automatic_updates.settings') + ->set('unattended.level', CronUpdateStage::ALL) + ->save(); + + $this->drupalGet('user'); + // The `drupalGet()` will not wait for the HTTP kernel to terminate (i.e., + // the `KernelEvents::TERMINATE` event) to complete. Although this event + // will likely already be completed, wait 1 second to avoid random test + // failures. + sleep(1); + $this->drupalGet('admin/reports/dblog'); + $this->assertSession()->elementAttributeContains('css', 'a[title^="Unattended"]', 'title', 'Unattended automatic updates were triggered by Automated Cron, which is not supported. No update was performed. See the status report for more information.'); + } + +} diff --git a/tests/src/Functional/StatusCheckTest.php b/tests/src/Functional/StatusCheckTest.php index 12aa80d0bcc1090e0fa4a99193dba9bdadfc8771..e934646bd6f4a039d11f7302b038f54fc6672355 100644 --- a/tests/src/Functional/StatusCheckTest.php +++ b/tests/src/Functional/StatusCheckTest.php @@ -128,11 +128,6 @@ class StatusCheckTest extends AutomaticUpdatesFunctionalTestBase { public function testStatusChecksOnStatusReport(): void { $assert = $this->assertSession(); $page = $this->getSession()->getPage(); - - // Ensure automated_cron is disabled before installing automatic_updates. - // This ensures we are testing that automatic_updates runs the checkers when - // the module itself is installed and they weren't run on cron. - $this->assertFalse($this->container->get('module_handler')->moduleExists('automated_cron')); $this->container->get('module_installer')->install(['automatic_updates', 'automatic_updates_test']); // If the site is ready for updates, the users will see the same output @@ -263,10 +258,6 @@ class StatusCheckTest extends AutomaticUpdatesFunctionalTestBase { $assert = $this->assertSession(); $messages_section_selector = '[data-drupal-messages]'; - // Ensure automated_cron is disabled before installing automatic_updates. This - // ensures we are testing that automatic_updates runs the checkers when the - // module itself is installed and they weren't run on cron. - $this->assertFalse($this->container->get('module_handler')->moduleExists('automated_cron')); $this->container->get('module_installer')->install(['automatic_updates', 'automatic_updates_test']); // If site is ready for updates no message will be displayed on admin pages. diff --git a/tests/src/Kernel/StatusCheck/AutomatedCronDisabledValidatorTest.php b/tests/src/Kernel/StatusCheck/AutomatedCronDisabledValidatorTest.php new file mode 100644 index 0000000000000000000000000000000000000000..fdccdeabafd5f5feb577e398791647c7c7ee43ed --- /dev/null +++ b/tests/src/Kernel/StatusCheck/AutomatedCronDisabledValidatorTest.php @@ -0,0 +1,40 @@ +<?php + +declare(strict_types = 1); + +namespace Drupal\Tests\automatic_updates\Kernel\StatusCheck; + +use Drupal\package_manager\ValidationResult; +use Drupal\Tests\automatic_updates\Kernel\AutomaticUpdatesKernelTestBase; + +/** + * @covers \Drupal\automatic_updates\Validator\AutomatedCronDisabledValidator + * @group automatic_updates + * @internal + */ +class AutomatedCronDisabledValidatorTest extends AutomaticUpdatesKernelTestBase { + + /** + * {@inheritdoc} + */ + protected static $modules = ['automatic_updates', 'automated_cron']; + + /** + * Tests that cron updates are not allowed if Automated Cron is enabled. + */ + public function testCronUpdateNotAllowed(): void { + $expected_results = [ + ValidationResult::createWarning([ + t('This site has the Automated Cron module installed. To use unattended automatic updates, please configure cron manually on your hosting environment. The Automatic Updates module will not do anything if it is triggered by Automated Cron. See the <a href=":url">Automated Cron documentation</a> for information.', [ + ':url' => 'https://www.drupal.org/docs/administering-a-drupal-site/cron-automated-tasks/cron-automated-tasks-overview#s-more-reliable-enable-cron-using-external-trigger', + ]), + ]), + ]; + $this->assertCheckerResultsFromManager($expected_results, TRUE); + + // Even after a cron run, we should have the same results. + $this->container->get('cron')->run(); + $this->assertCheckerResultsFromManager($expected_results); + } + +} diff --git a/tests/src/Kernel/StatusCheck/CronFrequencyValidatorTest.php b/tests/src/Kernel/StatusCheck/CronFrequencyValidatorTest.php index bbd2e7dfab0b317cfaf20d71756994eee6ac7c2f..4d9a8337df57ce196269898144a410464ddda07c 100644 --- a/tests/src/Kernel/StatusCheck/CronFrequencyValidatorTest.php +++ b/tests/src/Kernel/StatusCheck/CronFrequencyValidatorTest.php @@ -5,10 +5,8 @@ declare(strict_types = 1); namespace Drupal\Tests\automatic_updates\Kernel\StatusCheck; use Drupal\automatic_updates\CronUpdateStage; -use Drupal\automatic_updates\Validator\CronFrequencyValidator; use Drupal\package_manager\ValidationResult; use Drupal\Tests\automatic_updates\Kernel\AutomaticUpdatesKernelTestBase; -use PHPUnit\Framework\AssertionFailedError; /** * @covers \Drupal\automatic_updates\Validator\CronFrequencyValidator @@ -43,32 +41,17 @@ class CronFrequencyValidatorTest extends AutomaticUpdatesKernelTestBase { $this->config('automatic_updates.settings') ->set('unattended.level', CronUpdateStage::DISABLED) ->save(); - - $validator = new class ( - $this->container->get('config.factory'), - $this->container->get('module_handler'), - $this->container->get('state'), - $this->container->get('datetime.time'), - $this->container->get('lock') - ) extends CronFrequencyValidator { - - /** - * {@inheritdoc} - */ - protected function validateAutomatedCron($event): void { - throw new AssertionFailedError(__METHOD__ . '() should not have been called.'); - } - - /** - * {@inheritdoc} - */ - protected function validateLastCronRun($event): void { - throw new AssertionFailedError(__METHOD__ . '() should not have been called.'); - } - - }; - $this->container->set('automatic_updates.cron_frequency_validator', $validator); + $state = $this->container->get('state'); + $state->delete('system.cron_last'); + $state->delete('install_time'); $this->assertCheckerResultsFromManager([], TRUE); + $this->config('automatic_updates.settings') + ->set('unattended.level', CronUpdateStage::ALL) + ->save(); + $error = ValidationResult::createError([ + t('Cron has not run recently. For more information, see the online handbook entry for <a href="https://www.drupal.org/cron">configuring cron jobs</a> to run at least every 3 hours.'), + ]); + $this->assertCheckerResultsFromManager([$error], TRUE); } /** @@ -117,62 +100,4 @@ class CronFrequencyValidatorTest extends AutomaticUpdatesKernelTestBase { $this->assertCheckerResultsFromManager([], TRUE); } - /** - * Data provider for testAutomatedCronValidation(). - * - * @return mixed[][] - * The test cases. - */ - public function providerAutomatedCronValidation(): array { - return [ - 'default configuration' => [ - NULL, - [], - ], - 'every 6 hours' => [ - 21600, - [ - ValidationResult::createWarning([ - t('Cron is not set to run frequently enough. <a href="/admin/config/system/cron">Configure it</a> to run at least every 3 hours or disable automated cron and run it via an external scheduling system.'), - ]), - ], - ], - 'every 25 hours' => [ - 90000, - [ - ValidationResult::createError([ - t('Cron is not set to run frequently enough. <a href="/admin/config/system/cron">Configure it</a> to run at least every 3 hours or disable automated cron and run it via an external scheduling system.'), - ]), - ], - ], - ]; - } - - /** - * Tests validation based on Automated Cron settings. - * - * @param int|null $interval - * The configured interval for Automated Cron. If NULL, the default value - * will be used. - * @param \Drupal\package_manager\ValidationResult[] $expected_results - * The expected validation results. - * - * @dataProvider providerAutomatedCronValidation - */ - public function testAutomatedCronValidation(?int $interval, array $expected_results): void { - $this->enableModules(['automated_cron']); - $this->installConfig('automated_cron'); - - if (isset($interval)) { - $this->config('automated_cron.settings') - ->set('interval', $interval) - ->save(); - } - $this->assertCheckerResultsFromManager($expected_results, TRUE); - - // Even after running cron, we should have the same results. - $this->container->get('cron')->run(); - $this->assertCheckerResultsFromManager($expected_results); - } - }