From fd4863269ff441a6887596a0059e3348214de5cc Mon Sep 17 00:00:00 2001
From: "kunal.sachdev" <kunal.sachdev@3685163.no-reply.drupal.org>
Date: Tue, 9 Nov 2021 19:35:37 +0000
Subject: [PATCH] Issue #3242724 by kunal.sachdev, phenaproxima, tedbow: Ensure
cron runs frequently enough to run updates
---
automatic_updates.services.yml | 10 +
src/Validator/CronFrequencyValidator.php | 184 ++++++++++++++++++
.../Kernel/AutomaticUpdatesKernelTestBase.php | 5 +
.../CronFrequencyValidatorTest.php | 167 ++++++++++++++++
4 files changed, 366 insertions(+)
create mode 100644 src/Validator/CronFrequencyValidator.php
create mode 100644 tests/src/Kernel/ReadinessValidation/CronFrequencyValidatorTest.php
diff --git a/automatic_updates.services.yml b/automatic_updates.services.yml
index d4f600641c..d0d414912d 100644
--- a/automatic_updates.services.yml
+++ b/automatic_updates.services.yml
@@ -70,3 +70,13 @@ services:
class: Drupal\automatic_updates\Validator\CoreComposerValidator
tags:
- { name: event_subscriber }
+ automatic_updates.cron_frequency_validator:
+ class: Drupal\automatic_updates\Validator\CronFrequencyValidator
+ arguments:
+ - '@config.factory'
+ - '@module_handler'
+ - '@state'
+ - '@datetime.time'
+ - '@string_translation'
+ tags:
+ - { name: event_subscriber }
diff --git a/src/Validator/CronFrequencyValidator.php b/src/Validator/CronFrequencyValidator.php
new file mode 100644
index 0000000000..df2a647dcf
--- /dev/null
+++ b/src/Validator/CronFrequencyValidator.php
@@ -0,0 +1,184 @@
+<?php
+
+namespace Drupal\automatic_updates\Validator;
+
+use Drupal\automatic_updates\CronUpdater;
+use Drupal\automatic_updates\Event\ReadinessCheckEvent;
+use Drupal\Component\Datetime\TimeInterface;
+use Drupal\Core\Config\ConfigFactoryInterface;
+use Drupal\Core\Extension\ModuleHandlerInterface;
+use Drupal\Core\State\StateInterface;
+use Drupal\Core\StringTranslation\StringTranslationTrait;
+use Drupal\Core\StringTranslation\TranslationInterface;
+use Drupal\Core\Url;
+use Drupal\package_manager\ValidationResult;
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+
+/**
+ * Validates that cron runs frequently enough to perform automatic updates.
+ */
+class CronFrequencyValidator implements EventSubscriberInterface {
+
+ use StringTranslationTrait;
+
+ /**
+ * The error-level interval between cron runs, in seconds.
+ *
+ * If cron runs less frequently than this, an error will be raised during
+ * validation. Defaults to 24 hours.
+ *
+ * @var int
+ */
+ protected const ERROR_INTERVAL = 86400;
+
+ /**
+ * The warning-level interval between cron runs, in seconds.
+ *
+ * If cron runs less frequently than this, a warning will be raised during
+ * validation. Defaults to 3 hours.
+ *
+ * @var int
+ */
+ protected const WARNING_INTERVAL = 10800;
+
+ /**
+ * The cron frequency, in hours, to suggest in errors or warnings.
+ *
+ * @var int
+ */
+ protected const SUGGESTED_INTERVAL = self::WARNING_INTERVAL / 3600;
+
+ /**
+ * The config factory service.
+ *
+ * @var \Drupal\Core\Config\ConfigFactoryInterface
+ */
+ protected $configFactory;
+
+ /**
+ * The module handler.
+ *
+ * @var \Drupal\Core\Extension\ModuleHandlerInterface
+ */
+ protected $moduleHandler;
+
+ /**
+ * The state service.
+ *
+ * @var \Drupal\Core\State\StateInterface
+ */
+ protected $state;
+
+ /**
+ * The time service.
+ *
+ * @var \Drupal\Component\Datetime\TimeInterface
+ */
+ protected $time;
+
+ /**
+ * CronFrequencyValidator constructor.
+ *
+ * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
+ * The config factory service.
+ * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler
+ * The module handler.
+ * @param \Drupal\Core\State\StateInterface $state
+ * The state service.
+ * @param \Drupal\Component\Datetime\TimeInterface $time
+ * The time service.
+ * @param \Drupal\Core\StringTranslation\TranslationInterface $translation
+ * The translation service.
+ */
+ public function __construct(ConfigFactoryInterface $config_factory, ModuleHandlerInterface $module_handler, StateInterface $state, TimeInterface $time, TranslationInterface $translation) {
+ $this->configFactory = $config_factory;
+ $this->moduleHandler = $module_handler;
+ $this->state = $state;
+ $this->time = $time;
+ $this->setStringTranslation($translation);
+ }
+
+ /**
+ * Validates that cron runs frequently enough to perform automatic updates.
+ *
+ * @param \Drupal\automatic_updates\Event\ReadinessCheckEvent $event
+ * The event object.
+ */
+ public function checkCronFrequency(ReadinessCheckEvent $event): void {
+ $cron_enabled = $this->configFactory->get('automatic_updates.settings')
+ ->get('cron');
+
+ // If automatic updates are disabled during cron, there's nothing we need
+ // to validate.
+ if ($cron_enabled === CronUpdater::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\automatic_updates\Event\ReadinessCheckEvent $event
+ * The event object.
+ */
+ protected function validateAutomatedCron(ReadinessCheckEvent $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) {
+ $error = ValidationResult::createError([$message]);
+ $event->addValidationResult($error);
+ }
+ elseif ($interval > static::WARNING_INTERVAL) {
+ $warning = ValidationResult::createWarning([$message]);
+ $event->addValidationResult($warning);
+ }
+ }
+
+ /**
+ * Validates the cron frequency according to the last cron run time.
+ *
+ * @param \Drupal\automatic_updates\Event\ReadinessCheckEvent $event
+ * The event object.
+ */
+ protected function validateLastCronRun(ReadinessCheckEvent $event): void {
+ // Determine when cron last ran. If not known, use the time that Drupal was
+ // installed, defaulting to the beginning of the Unix epoch.
+ $cron_last = $this->state->get('system.cron_last', $this->state->get('install_time', 0));
+
+ // @todo Should we allow a little extra time in case the server job takes
+ // longer than expected? Otherwise a server setup with a 3-hour cron job
+ // will always give this warning. Maybe this isn't necessary because the
+ // last cron run time is recorded after cron runs. Address this in
+ // https://www.drupal.org/project/automatic_updates/issues/3248544.
+ if ($this->time->getRequestTime() - $cron_last > static::WARNING_INTERVAL) {
+ $error = ValidationResult::createError([
+ $this->t('Cron has not run recently. For more information, see the online handbook entry for <a href=":cron-handbook">configuring cron jobs</a> to run at least every @frequency hours.', [
+ ':cron-handbook' => 'https://www.drupal.org/cron',
+ '@frequency' => static::SUGGESTED_INTERVAL,
+ ]),
+ ]);
+ $event->addValidationResult($error);
+ }
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public static function getSubscribedEvents() {
+ return [
+ ReadinessCheckEvent::class => 'checkCronFrequency',
+ ];
+ }
+
+}
diff --git a/tests/src/Kernel/AutomaticUpdatesKernelTestBase.php b/tests/src/Kernel/AutomaticUpdatesKernelTestBase.php
index 5def1a571f..276a4d2bed 100644
--- a/tests/src/Kernel/AutomaticUpdatesKernelTestBase.php
+++ b/tests/src/Kernel/AutomaticUpdatesKernelTestBase.php
@@ -60,6 +60,11 @@ abstract class AutomaticUpdatesKernelTestBase extends KernelTestBase {
// update to 9.8.1 is available.
$this->setCoreVersion('9.8.0');
$this->setReleaseMetadata(__DIR__ . '/../../fixtures/release-history/drupal.9.8.1.xml');
+
+ // Set a last cron run time so that the cron frequency validator will run
+ // from a sane state.
+ // @see \Drupal\automatic_updates\Validator\CronFrequencyValidator
+ $this->container->get('state')->set('system.cron_last', time());
}
/**
diff --git a/tests/src/Kernel/ReadinessValidation/CronFrequencyValidatorTest.php b/tests/src/Kernel/ReadinessValidation/CronFrequencyValidatorTest.php
new file mode 100644
index 0000000000..25026fba17
--- /dev/null
+++ b/tests/src/Kernel/ReadinessValidation/CronFrequencyValidatorTest.php
@@ -0,0 +1,167 @@
+<?php
+
+namespace Drupal\Tests\automatic_updates\Kernel\ReadinessValidation;
+
+use Drupal\automatic_updates\CronUpdater;
+use Drupal\automatic_updates\Event\ReadinessCheckEvent;
+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
+ *
+ * @group automatic_updates
+ */
+class CronFrequencyValidatorTest extends AutomaticUpdatesKernelTestBase {
+
+ /**
+ * {@inheritdoc}
+ */
+ protected static $modules = [
+ 'automatic_updates',
+ 'package_manager',
+ 'package_manager_bypass',
+ ];
+
+ /**
+ * Tests that nothing is validated if updates are disabled during cron.
+ */
+ public function testNoValidationIfCronDisabled(): void {
+ $this->config('automatic_updates.settings')
+ ->set('cron', CronUpdater::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('string_translation')
+ ) extends CronFrequencyValidator {
+
+ /**
+ * {@inheritdoc}
+ */
+ protected function validateAutomatedCron(ReadinessCheckEvent $event): void {
+ throw new AssertionFailedError(__METHOD__ . '() should not have been called.');
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ protected function validateLastCronRun(ReadinessCheckEvent $event): void {
+ throw new AssertionFailedError(__METHOD__ . '() should not have been called.');
+ }
+
+ };
+ $this->container->set('automatic_updates.cron_frequency_validator', $validator);
+ $this->assertCheckerResultsFromManager([], TRUE);
+ }
+
+ /**
+ * Data provider for ::testLastCronRunValidation().
+ *
+ * @return array[]
+ * Sets of arguments to pass to the test method.
+ */
+ public function providerLastCronRunValidation(): array {
+ $error = ValidationResult::createError([
+ '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.',
+ ]);
+
+ return [
+ 'cron never ran' => [
+ 0,
+ [$error],
+ ],
+ 'cron ran four hours ago' => [
+ time() - 14400,
+ [$error],
+ ],
+ 'cron ran an hour ago' => [
+ time() - 3600,
+ [],
+ ],
+ ];
+ }
+
+ /**
+ * Tests validation based on the last cron run time.
+ *
+ * @param int $last_run
+ * A timestamp of the last time cron ran.
+ * @param \Drupal\package_manager\ValidationResult[] $expected_results
+ * The expected validation results.
+ *
+ * @dataProvider providerLastCronRunValidation
+ */
+ public function testLastCronRunValidation(int $last_run, array $expected_results): void {
+ $this->container->get('state')->set('system.cron_last', $last_run);
+ $this->assertCheckerResultsFromManager($expected_results, TRUE);
+
+ // After running cron, any errors or warnings should be gone.
+ $this->container->get('cron')->run();
+ $this->assertCheckerResultsFromManager([], TRUE);
+ }
+
+ /**
+ * Data provider for ::testAutomatedCronValidation().
+ *
+ * @return array[]
+ * Sets of arguments to pass to the test method.
+ */
+ public function providerAutomatedCronValidation(): array {
+ return [
+ 'default configuration' => [
+ NULL,
+ [],
+ ],
+ 'every 6 hours' => [
+ 21600,
+ [
+ ValidationResult::createWarning([
+ '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([
+ '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);
+ }
+
+}
--
GitLab