From f4a868012ee17384e9f7635831a654b6ba26e7fd Mon Sep 17 00:00:00 2001
From: omkar podey <58183-omkar.podey@users.noreply.drupalcode.org>
Date: Fri, 9 Dec 2022 02:35:52 +0000
Subject: [PATCH] Issue #3321256 by omkar.podey, tedbow, Wim Leers: Fix race
condition in \Drupal\automatic_updates_test_cron\Enabler
---
automatic_updates.install | 32 ++++++++++++++-
.../Validator/UpdateReleaseValidatorTest.php | 5 +++
config/install/automatic_updates.settings.yml | 2 +-
src/CronUpdater.php | 12 ------
.../automatic_updates_test_cron.module | 4 ++
.../automatic_updates_test_cron.services.yml | 5 ---
.../src/Enabler.php | 39 -------------------
tests/src/Build/UpdateTestBase.php | 7 +++-
.../AutomaticUpdatesFunctionalTestBase.php | 5 ++-
tests/src/Functional/StatusCheckTest.php | 11 ++++--
tests/src/Functional/UpdatePathTest.php | 5 +++
.../Kernel/AutomaticUpdatesKernelTestBase.php | 20 +++++++---
tests/src/Kernel/CronUpdaterTest.php | 19 +++------
.../StatusCheckFailureEmailTest.php | 2 +
.../Kernel/StatusCheck/StatusCheckerTest.php | 6 +++
.../ValidationResultDisplayTraitTest.php | 5 +++
16 files changed, 94 insertions(+), 85 deletions(-)
delete mode 100644 tests/modules/automatic_updates_test_cron/automatic_updates_test_cron.services.yml
delete mode 100644 tests/modules/automatic_updates_test_cron/src/Enabler.php
diff --git a/automatic_updates.install b/automatic_updates.install
index 20e7e83776..c0aadef4dc 100644
--- a/automatic_updates.install
+++ b/automatic_updates.install
@@ -7,7 +7,9 @@
declare(strict_types = 1);
+use Drupal\automatic_updates\CronUpdater;
use Drupal\automatic_updates\Validation\StatusCheckRequirements;
+use Drupal\system\SystemManager;
/**
* Implements hook_uninstall().
@@ -24,7 +26,19 @@ function automatic_updates_requirements($phase) {
// Check that site is ready to perform automatic updates.
/** @var \Drupal\automatic_updates\Validation\StatusCheckRequirements $status_check_requirement */
$status_check_requirement = \Drupal::classResolver(StatusCheckRequirements::class);
- return $status_check_requirement->getRequirements();
+ $requirements = $status_check_requirement->getRequirements();
+
+ // Check that site has cron updates enabled or not.
+ // @todo Remove in https://www.drupal.org/project/automatic_updates/issues/3284443
+ if (\Drupal::configFactory()->get('automatic_updates.settings')->get('cron') !== CronUpdater::DISABLED) {
+ $requirements['automatic_updates_cron'] = [
+ 'title' => t('Cron installs updates automatically'),
+ 'severity' => SystemManager::REQUIREMENT_WARNING,
+ 'value' => t('Enabled. This is NOT an officially supported feature of the Automatic Updates module at this time. Use at your own risk.'),
+ ];
+ }
+
+ return $requirements;
}
}
@@ -38,3 +52,19 @@ function automatic_updates_update_9001(): void {
$key_value->rename('readiness_validation_last_run', 'status_check_last_run');
$key_value->rename('readiness_check_timestamp', 'status_check_timestamp');
}
+
+/**
+ * Sets cron setting to \Drupal\automatic_updates\CronUpdater::DISABLED.
+ *
+ * This is necessary until TUF support is available on drupal.org. This used to
+ * be hardcoded, but that caused complex race conditions in tests. Hence, it is
+ * now just configured.
+ *
+ * @see automatic_updates_requirements()
+ * @see https://www.drupal.org/project/automatic_updates/issues/3284443
+ */
+function automatic_updates_update_9002(): void {
+ /** @var \Drupal\Core\Config\Config $config */
+ $config = \Drupal::service('config.factory')->getEditable('automatic_updates.settings');
+ $config->set('cron', CronUpdater::DISABLED)->save();
+}
diff --git a/automatic_updates_extensions/tests/src/Kernel/Validator/UpdateReleaseValidatorTest.php b/automatic_updates_extensions/tests/src/Kernel/Validator/UpdateReleaseValidatorTest.php
index c075b8e389..fa5bdb51a6 100644
--- a/automatic_updates_extensions/tests/src/Kernel/Validator/UpdateReleaseValidatorTest.php
+++ b/automatic_updates_extensions/tests/src/Kernel/Validator/UpdateReleaseValidatorTest.php
@@ -16,6 +16,11 @@ use Drupal\Tests\automatic_updates_extensions\Kernel\AutomaticUpdatesExtensionsK
*/
class UpdateReleaseValidatorTest extends AutomaticUpdatesExtensionsKernelTestBase {
+ /**
+ * {@inheritdoc}
+ */
+ protected static $modules = ['automatic_updates'];
+
/**
* Data provider for testPreCreateException().
*
diff --git a/config/install/automatic_updates.settings.yml b/config/install/automatic_updates.settings.yml
index 31043e05ce..2649653a1e 100644
--- a/config/install/automatic_updates.settings.yml
+++ b/config/install/automatic_updates.settings.yml
@@ -1,3 +1,3 @@
-cron: security
+cron: disable
allow_core_minor_updates: false
status_check_mail: errors_only
diff --git a/src/CronUpdater.php b/src/CronUpdater.php
index c7b2836cd6..c46932b70c 100644
--- a/src/CronUpdater.php
+++ b/src/CronUpdater.php
@@ -26,15 +26,6 @@ use Symfony\Component\HttpFoundation\Response;
*/
class CronUpdater extends Updater {
- /**
- * Whether or not cron updates are hard-disabled.
- *
- * @var bool
- *
- * @todo Remove this when TUF integration is stable.
- */
- private static $disabled = TRUE;
-
/**
* All automatic updates are disabled.
*
@@ -337,9 +328,6 @@ class CronUpdater extends Updater {
* during cron.
*/
final public function getMode(): string {
- if (self::$disabled) {
- return static::DISABLED;
- }
$mode = $this->configFactory->get('automatic_updates.settings')->get('cron');
return $mode ?: CronUpdater::SECURITY;
}
diff --git a/tests/modules/automatic_updates_test_cron/automatic_updates_test_cron.module b/tests/modules/automatic_updates_test_cron/automatic_updates_test_cron.module
index af8284e440..2716bb75f6 100644
--- a/tests/modules/automatic_updates_test_cron/automatic_updates_test_cron.module
+++ b/tests/modules/automatic_updates_test_cron/automatic_updates_test_cron.module
@@ -7,6 +7,8 @@
* @todo Move into automatic_updates when TUF integration is stable.
*/
+declare(strict_types=1);
+
use Drupal\package_manager\ProjectInfo;
use Drupal\automatic_updates\CronUpdater;
use Drupal\Core\Extension\ExtensionVersion;
@@ -15,6 +17,8 @@ use Drupal\update\ProjectSecurityData;
/**
* Implements hook_form_FORM_ID_alter() for 'update_settings' form.
+ *
+ * @todo Move to automatic_updates in https://www.drupal.org/i/3322361
*/
function automatic_updates_test_cron_form_update_settings_alter(array &$form, FormStateInterface $form_state, string $form_id) {
$project_info = new ProjectInfo('drupal');
diff --git a/tests/modules/automatic_updates_test_cron/automatic_updates_test_cron.services.yml b/tests/modules/automatic_updates_test_cron/automatic_updates_test_cron.services.yml
deleted file mode 100644
index b8e5c8bddd..0000000000
--- a/tests/modules/automatic_updates_test_cron/automatic_updates_test_cron.services.yml
+++ /dev/null
@@ -1,5 +0,0 @@
-services:
- automatic_updates_test_cron.enabler:
- class: Drupal\automatic_updates_test_cron\Enabler
- tags:
- - { name: event_subscriber }
diff --git a/tests/modules/automatic_updates_test_cron/src/Enabler.php b/tests/modules/automatic_updates_test_cron/src/Enabler.php
deleted file mode 100644
index da799f7e3e..0000000000
--- a/tests/modules/automatic_updates_test_cron/src/Enabler.php
+++ /dev/null
@@ -1,39 +0,0 @@
-<?php
-
-declare(strict_types = 1);
-
-namespace Drupal\automatic_updates_test_cron;
-
-use Drupal\automatic_updates\CronUpdater;
-use Symfony\Component\EventDispatcher\EventSubscriberInterface;
-use Symfony\Component\HttpKernel\KernelEvents;
-
-/**
- * Enables automatic updates during cron.
- *
- * @todo Remove this when TUF integration is stable.
- */
-class Enabler implements EventSubscriberInterface {
-
- /**
- * {@inheritdoc}
- */
- public static function getSubscribedEvents(): array {
- return [
- KernelEvents::REQUEST => 'enableCron',
- ];
- }
-
- /**
- * Enables automatic updates during cron.
- */
- public function enableCron(): void {
- if (class_exists(CronUpdater::class)) {
- $reflector = new \ReflectionClass(CronUpdater::class);
- $reflector = $reflector->getProperty('disabled');
- $reflector->setAccessible(TRUE);
- $reflector->setValue(NULL, FALSE);
- }
- }
-
-}
diff --git a/tests/src/Build/UpdateTestBase.php b/tests/src/Build/UpdateTestBase.php
index ddafd13da5..839b3f5553 100644
--- a/tests/src/Build/UpdateTestBase.php
+++ b/tests/src/Build/UpdateTestBase.php
@@ -19,12 +19,15 @@ abstract class UpdateTestBase extends TemplateProjectTestBase {
*/
protected function createTestProject(string $template): void {
parent::createTestProject($template);
-
+ // @todo Remove in https://www.drupal.org/project/automatic_updates/issues/3284443
+ $code = <<<END
+\$config['automatic_updates.settings']['cron'] = 'security';
+END;
+ $this->writeSettings($code);
// Install Automatic Updates, and other modules needed for testing.
$this->installModules([
'automatic_updates',
'automatic_updates_test_api',
- 'automatic_updates_test_cron',
]);
}
diff --git a/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php b/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php
index fdd355e03d..5bf4eeb488 100644
--- a/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php
+++ b/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php
@@ -4,6 +4,7 @@ declare(strict_types = 1);
namespace Drupal\Tests\automatic_updates\Functional;
+use Drupal\automatic_updates\CronUpdater;
use Drupal\Core\Site\Settings;
use Drupal\package_manager_bypass\Beginner;
use Drupal\package_manager_bypass\Stager;
@@ -23,7 +24,7 @@ abstract class AutomaticUpdatesFunctionalTestBase extends BrowserTestBase {
* {@inheritdoc}
*/
protected static $modules = [
- 'automatic_updates_test_cron',
+ 'automatic_updates',
'automatic_updates_test_disable_validators',
'package_manager_bypass',
];
@@ -54,6 +55,8 @@ abstract class AutomaticUpdatesFunctionalTestBase extends BrowserTestBase {
parent::setUp();
$this->disableValidators($this->disableValidators);
$this->useFixtureDirectoryAsActive(__DIR__ . '/../../../package_manager/tests/fixtures/fake_site');
+ // @todo Remove in https://www.drupal.org/project/automatic_updates/issues/3284443
+ $this->config('automatic_updates.settings')->set('cron', CronUpdater::SECURITY)->save();
}
/**
diff --git a/tests/src/Functional/StatusCheckTest.php b/tests/src/Functional/StatusCheckTest.php
index 84db352a40..bfa9e3c59c 100644
--- a/tests/src/Functional/StatusCheckTest.php
+++ b/tests/src/Functional/StatusCheckTest.php
@@ -5,6 +5,7 @@ declare(strict_types = 1);
namespace Drupal\Tests\automatic_updates\Functional;
use Behat\Mink\Element\NodeElement;
+use Drupal\automatic_updates\CronUpdater;
use Drupal\automatic_updates\StatusCheckMailer;
use Drupal\automatic_updates_test\Datetime\TestTime;
use Drupal\automatic_updates_test\EventSubscriber\TestSubscriber1;
@@ -330,10 +331,9 @@ class StatusCheckTest extends AutomaticUpdatesFunctionalTestBase {
// Confirm status check messages are not displayed when cron updates are
// disabled.
- $this->drupalGet(Url::fromRoute('update.settings'));
- $edit['automatic_updates_cron'] = 'disable';
- $this->submitForm($edit, 'Save configuration');
- $this->drupalGet(Url::fromRoute($admin_route));
+ $this->config('automatic_updates.settings')->set('cron', CronUpdater::DISABLED)->save();
+ $this->drupalGet('admin/structure');
+ $this->checkForMetaRefresh();
$assert->pageTextNotContains(static::$warningsExplanation);
$assert->pageTextNotContains($expected_results[0]->getMessages()[0]);
}
@@ -344,6 +344,7 @@ class StatusCheckTest extends AutomaticUpdatesFunctionalTestBase {
public function testStatusCheckAfterInstall(): void {
$assert = $this->assertSession();
$this->drupalLogin($this->checkerRunnerUser);
+ $this->container->get('module_installer')->uninstall(['automatic_updates']);
$this->drupalGet('admin/reports/status');
$assert->pageTextNotContains('Update readiness checks');
@@ -351,6 +352,8 @@ class StatusCheckTest extends AutomaticUpdatesFunctionalTestBase {
// We have to install the automatic_updates_test module because it provides
// the functionality to retrieve our fake release history metadata.
$this->container->get('module_installer')->install(['automatic_updates', 'automatic_updates_test']);
+ // @todo Remove in https://www.drupal.org/project/automatic_updates/issues/3284443
+ $this->config('automatic_updates.settings')->set('cron', CronUpdater::SECURITY)->save();
$this->drupalGet('admin/reports/status');
$this->assertNoErrors(TRUE);
diff --git a/tests/src/Functional/UpdatePathTest.php b/tests/src/Functional/UpdatePathTest.php
index 2f5139acb8..b697f42c18 100644
--- a/tests/src/Functional/UpdatePathTest.php
+++ b/tests/src/Functional/UpdatePathTest.php
@@ -4,6 +4,7 @@ declare(strict_types = 1);
namespace Drupal\Tests\automatic_updates\Functional;
+use Drupal\automatic_updates\CronUpdater;
use Drupal\automatic_updates\StatusCheckMailer;
use Drupal\FunctionalTests\Update\UpdatePathTestBase;
@@ -55,8 +56,12 @@ class UpdatePathTest extends UpdatePathTestBase {
}
$this->assertEmpty($this->config('automatic_updates.settings')->get('status_check_mail'));
+ $this->assertSame(CronUpdater::SECURITY, $this->config('automatic_updates.settings')->get('cron'));
+
$this->runUpdates();
+ $this->assertSame(CronUpdater::DISABLED, $this->config('automatic_updates.settings')->get('cron'));
+
// TRICKY: we do expect `readiness_validation_last_run` to have been renamed
// to `status_check_last_run`, but then
// automatic_updates_post_update_create_status_check_mail_config() should
diff --git a/tests/src/Kernel/AutomaticUpdatesKernelTestBase.php b/tests/src/Kernel/AutomaticUpdatesKernelTestBase.php
index ad69a1c284..aa8670c2af 100644
--- a/tests/src/Kernel/AutomaticUpdatesKernelTestBase.php
+++ b/tests/src/Kernel/AutomaticUpdatesKernelTestBase.php
@@ -23,10 +23,18 @@ abstract class AutomaticUpdatesKernelTestBase extends PackageManagerKernelTestBa
/**
* {@inheritdoc}
+ *
+ * TRICKY: due to the way that automatic_updates forcibly disables cron-based
+ * updating for the end user, we need to override the current default
+ * configuration BEFORE the module is installed. This triggers config schema
+ * exceptions. Since none of these tests are interacting with configuration
+ * anyway, this is a reasonable temporary workaround.
+ *
+ * @see ::setUp()
+ * @see https://www.drupal.org/project/automatic_updates/issues/3284443
+ * @todo Remove in https://www.drupal.org/project/automatic_updates/issues/3284443
*/
- protected static $modules = [
- 'automatic_updates_test_cron',
- ];
+ protected $strictConfigSchema = FALSE;
/**
* {@inheritdoc}
@@ -46,6 +54,9 @@ abstract class AutomaticUpdatesKernelTestBase extends PackageManagerKernelTestBa
$this->disableValidators[] = 'automatic_updates.validator.xdebug';
$this->disableValidators[] = 'package_manager.validator.xdebug';
parent::setUp();
+ // Enable cron updates, which will eventually be the default.
+ // @todo Remove in https://www.drupal.org/project/automatic_updates/issues/3284443
+ $this->config('automatic_updates.settings')->set('cron', CronUpdater::SECURITY)->save();
// By default, pretend we're running Drupal core 9.8.0 and a non-security
// update to 9.8.1 is available.
@@ -56,9 +67,6 @@ abstract class AutomaticUpdatesKernelTestBase extends PackageManagerKernelTestBa
// from a sane state.
// @see \Drupal\automatic_updates\Validator\CronFrequencyValidator
$this->container->get('state')->set('system.cron_last', time());
-
- // @todo Remove this when TUF integration is stable.
- $this->container->get('automatic_updates_test_cron.enabler')->enableCron();
}
/**
diff --git a/tests/src/Kernel/CronUpdaterTest.php b/tests/src/Kernel/CronUpdaterTest.php
index 9078970cf6..8a6b30252c 100644
--- a/tests/src/Kernel/CronUpdaterTest.php
+++ b/tests/src/Kernel/CronUpdaterTest.php
@@ -7,7 +7,6 @@ namespace Drupal\Tests\automatic_updates\Kernel;
use Drupal\automatic_updates\CronUpdater;
use Drupal\automatic_updates_test\EventSubscriber\TestSubscriber1;
use Drupal\Core\DependencyInjection\ContainerBuilder;
-use Drupal\Core\Form\FormState;
use Drupal\Core\Logger\RfcLogLevel;
use Drupal\Core\Url;
use Drupal\package_manager\Event\PostApplyEvent;
@@ -24,9 +23,8 @@ use Drupal\package_manager_bypass\Committer;
use Drupal\Tests\automatic_updates\Traits\EmailNotificationsTestTrait;
use Drupal\Tests\package_manager\Traits\PackageManagerBypassTestTrait;
use Drupal\Tests\user\Traits\UserCreationTrait;
-use Drupal\update\UpdateSettingsForm;
-use ColinODell\PsrTestLogger\TestLogger;
use Prophecy\Argument;
+use ColinODell\PsrTestLogger\TestLogger;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
/**
@@ -53,7 +51,7 @@ class CronUpdaterTest extends AutomaticUpdatesKernelTestBase {
/**
* The test logger.
*
- * @var ColinODell\PsrTestLogger\TestLogger
+ * @var \ColinODell\PsrTestLogger\TestLogger
*/
private $logger;
@@ -151,16 +149,7 @@ class CronUpdaterTest extends AutomaticUpdatesKernelTestBase {
$this->setCoreVersion('9.8.0');
update_get_available(TRUE);
- // Submit the configuration form programmatically, to prove our alterations
- // work as expected.
- $form_builder = $this->container->get('form_builder');
- $form_state = new FormState();
- $form = $form_builder->buildForm(UpdateSettingsForm::class, $form_state);
- // Ensure that the version ranges in the setting's description, which are
- // computed dynamically, look correct.
- $this->assertStringContainsString('Automatic updates are only supported for 9.8.x versions of Drupal core. Drupal 9.8 will receive security updates until 9.10.0 is released.', (string) $form['automatic_updates_cron']['#description']);
- $form_state->setValue('automatic_updates_cron', $setting);
- $form_builder->submitForm(UpdateSettingsForm::class, $form_state);
+ $this->config('automatic_updates.settings')->set('cron', $setting)->save();
// Since we're just trying to ensure that all of Package Manager's services
// are called as expected, disable validation by replacing the event
@@ -258,6 +247,8 @@ class CronUpdaterTest extends AutomaticUpdatesKernelTestBase {
*/
public function testStageDestroyedOnError(string $event_class, string $exception_class): void {
$this->installConfig('automatic_updates');
+ // @todo Remove in https://www.drupal.org/project/automatic_updates/issues/3284443
+ $this->config('automatic_updates.settings')->set('cron', CronUpdater::SECURITY)->save();
$this->setCoreVersion('9.8.0');
// Ensure that there is a security release to which we should update.
$this->setReleaseMetadata([
diff --git a/tests/src/Kernel/StatusCheck/StatusCheckFailureEmailTest.php b/tests/src/Kernel/StatusCheck/StatusCheckFailureEmailTest.php
index b6e37b0173..61e631f7ff 100644
--- a/tests/src/Kernel/StatusCheck/StatusCheckFailureEmailTest.php
+++ b/tests/src/Kernel/StatusCheck/StatusCheckFailureEmailTest.php
@@ -53,6 +53,8 @@ class StatusCheckFailureEmailTest extends AutomaticUpdatesKernelTestBase {
$this->installSchema('user', ['users_data']);
$this->installConfig('automatic_updates');
+ // @todo Remove in https://www.drupal.org/project/automatic_updates/issues/3284443
+ $this->config('automatic_updates.settings')->set('cron', CronUpdater::SECURITY)->save();
$this->setUpEmailRecipients();
// Allow stored available update data to live for a very, very long time.
diff --git a/tests/src/Kernel/StatusCheck/StatusCheckerTest.php b/tests/src/Kernel/StatusCheck/StatusCheckerTest.php
index 4199491448..c08a0d3b60 100644
--- a/tests/src/Kernel/StatusCheck/StatusCheckerTest.php
+++ b/tests/src/Kernel/StatusCheck/StatusCheckerTest.php
@@ -36,6 +36,12 @@ class StatusCheckerTest extends AutomaticUpdatesKernelTestBase {
$this->setCoreVersion('9.8.2');
$this->installEntitySchema('user');
$this->installSchema('user', ['users_data']);
+
+ // Undoes the override in parent::setUp(), to allow the module to be
+ // installed, which every other test methods in this class does. Without
+ // this \Drupal\Core\Config\PreExistingConfigException is thrown.
+ // @todo Remove in https://www.drupal.org/project/automatic_updates/issues/3284443
+ $this->config('automatic_updates.settings')->delete();
}
/**
diff --git a/tests/src/Kernel/ValidationResultDisplayTraitTest.php b/tests/src/Kernel/ValidationResultDisplayTraitTest.php
index ee2b4bfdbe..7bdbec570d 100644
--- a/tests/src/Kernel/ValidationResultDisplayTraitTest.php
+++ b/tests/src/Kernel/ValidationResultDisplayTraitTest.php
@@ -20,6 +20,11 @@ class ValidationResultDisplayTraitTest extends AutomaticUpdatesKernelTestBase {
use ValidationResultDisplayTrait;
use StringTranslationTrait;
+ /**
+ * {@inheritdoc}
+ */
+ protected static $modules = ['automatic_updates'];
+
/**
* @covers ::displayResults
*/
--
GitLab