diff --git a/src/CronUpdater.php b/src/CronUpdater.php index 744c5c31d253fcf069fb0418729c26a996b18140..d85adf21f6129d92170cc11514c44f0e1272389a 100644 --- a/src/CronUpdater.php +++ b/src/CronUpdater.php @@ -118,14 +118,15 @@ class CronUpdater extends Updater { /** * {@inheritdoc} */ - public function begin(array $project_versions, ?int $timeout = 300): string { - // Prevent mischievous callers from starting an update even if unattended - // updates are disabled. To start an update programmatically, use - // \Drupal\automatic_updates\Updater::begin(). - if ($this->getMode() === static::DISABLED) { - throw new \LogicException('Unattended updates are disabled.'); - } - return parent::begin($project_versions, $timeout); + final public function begin(array $project_versions, ?int $timeout = 300): string { + // Unattended updates should never be started using this method. They should + // only be done by ::handleCron(), which has a strong opinion about which + // release to update to. Throwing an exception here is just to enforce this + // boundary. To update to a specific version of core, use + // \Drupal\automatic_updates\Updater::begin() (which is called in + // ::performUpdate() to start the update to the target version of core + // chosen by ::handleCron()). + throw new \BadMethodCallException(__METHOD__ . '() cannot be called directly.'); } /** @@ -148,7 +149,8 @@ class CronUpdater extends Updater { // handle any exceptions or validation errors consistently, and destroy the // stage regardless of whether the update succeeds. try { - $this->begin(['drupal' => $target_version], $timeout); + // @see ::begin() + parent::begin(['drupal' => $target_version], $timeout); $this->stage(); $this->apply(); diff --git a/tests/src/Kernel/CronUpdaterTest.php b/tests/src/Kernel/CronUpdaterTest.php index 3aec58c37590403f39459b8b6485d5e8869757fe..ab16c9dd4a4247d1d4e4c8c12be1fa308d53abc7 100644 --- a/tests/src/Kernel/CronUpdaterTest.php +++ b/tests/src/Kernel/CronUpdaterTest.php @@ -342,14 +342,10 @@ class CronUpdaterTest extends AutomaticUpdatesKernelTestBase { } /** - * Tests that the cron updater throws an exception if started while disabled. + * Tests that CronUpdater::begin() unconditionally throws an exception. */ - public function testExceptionWhenDisabled(): void { - $this->config('automatic_updates.settings') - ->set('cron', CronUpdater::DISABLED) - ->save(); - - $this->expectExceptionMessage('Unattended updates are disabled.'); + public function testBeginThrowsException(): void { + $this->expectExceptionMessage(CronUpdater::class . '::begin() cannot be called directly.'); $this->container->get('automatic_updates.cron_updater') ->begin(['drupal' => '9.8.1']); } diff --git a/tests/src/Kernel/ReadinessValidation/StagedDatabaseUpdateValidatorTest.php b/tests/src/Kernel/ReadinessValidation/StagedDatabaseUpdateValidatorTest.php index 8b294d01aec4ee396591cd4e196f90a504e6ec6e..086575a13c46b318558d124520f9002905bc8589 100644 --- a/tests/src/Kernel/ReadinessValidation/StagedDatabaseUpdateValidatorTest.php +++ b/tests/src/Kernel/ReadinessValidation/StagedDatabaseUpdateValidatorTest.php @@ -2,9 +2,10 @@ namespace Drupal\Tests\automatic_updates\Kernel\ReadinessValidation; -use Drupal\package_manager\Exception\StageValidationException; -use Drupal\package_manager\ValidationResult; +use Drupal\Core\Logger\RfcLogLevel; +use Drupal\package_manager\Event\PreApplyEvent; use Drupal\Tests\automatic_updates\Kernel\AutomaticUpdatesKernelTestBase; +use Psr\Log\Test\TestLogger; /** * @covers \Drupal\automatic_updates\Validator\StagedDatabaseUpdateValidator @@ -25,18 +26,24 @@ class StagedDatabaseUpdateValidatorTest extends AutomaticUpdatesKernelTestBase { */ private const SUFFIXES = ['install', 'post_update.php']; + /** + * The test logger channel. + * + * @var \Psr\Log\Test\TestLogger + */ + private $logger; + /** * {@inheritdoc} */ protected function setUp(): void { parent::setUp(); - $this->createTestProject(); - /** @var \Drupal\Tests\automatic_updates\Kernel\TestCronUpdater $updater */ - $updater = $this->container->get('automatic_updates.cron_updater'); - $updater->begin(['drupal' => '9.8.1']); - $updater->stage(); + $this->logger = new TestLogger(); + $this->container->get('logger.factory') + ->get('automatic_updates') + ->addLogger($this->logger); } /** @@ -80,21 +87,27 @@ class StagedDatabaseUpdateValidatorTest extends AutomaticUpdatesKernelTestBase { $this->assertFalse($this->container->get('module_handler')->moduleExists('views')); $this->assertFalse($this->container->get('theme_handler')->themeExists('automatic_updates_theme_with_updates')); - // Create bogus staged versions of Views' and - // Automatic Updates Theme with Updates .install and .post_update.php files. - // Since these extensions are not installed, the changes should not raise - // any validation errors. - $updater = $this->container->get('automatic_updates.cron_updater'); - $module_list = $this->container->get('extension.list.module')->getList(); - $theme_list = $this->container->get('extension.list.theme')->getList(); - $module_dir = $updater->getStageDirectory() . '/' . $module_list['views']->getPath(); - $theme_dir = $updater->getStageDirectory() . '/' . $theme_list['automatic_updates_theme_with_updates']->getPath(); - foreach (static::SUFFIXES as $suffix) { - file_put_contents("$module_dir/views.$suffix", $this->randomString()); - file_put_contents("$theme_dir/automatic_updates_theme_with_updates.$suffix", $this->randomString()); - } + $listener = function (PreApplyEvent $event): void { + // Create bogus staged versions of Views' and + // Automatic Updates Theme with Updates .install and .post_update.php + // files. Since these extensions are not installed, the changes should not + // raise any validation errors. + $dir = $event->getStage()->getStageDirectory(); + $module_list = $this->container->get('extension.list.module')->getList(); + $theme_list = $this->container->get('extension.list.theme')->getList(); + $module_dir = $dir . '/' . $module_list['views']->getPath(); + $theme_dir = $dir . '/' . $theme_list['automatic_updates_theme_with_updates']->getPath(); + foreach (static::SUFFIXES as $suffix) { + file_put_contents("$module_dir/views.$suffix", $this->randomString()); + file_put_contents("$theme_dir/automatic_updates_theme_with_updates.$suffix", $this->randomString()); + } + }; + $this->container->get('event_dispatcher') + ->addListener(PreApplyEvent::class, $listener, PHP_INT_MAX); - $updater->apply(); + $this->container->get('cron')->run(); + // There should not have been any errors. + $this->assertFalse($this->logger->hasRecords(RfcLogLevel::ERROR)); } /** @@ -124,73 +137,61 @@ class StagedDatabaseUpdateValidatorTest extends AutomaticUpdatesKernelTestBase { * @dataProvider providerFileChanged */ public function testFileChanged(string $suffix, bool $delete): void { - /** @var \Drupal\automatic_updates\CronUpdater $updater */ - $updater = $this->container->get('automatic_updates.cron_updater'); - $theme_installer = $this->container->get('theme_installer'); - $theme_installer->install(['automatic_updates_theme_with_updates']); - $theme = $this->container->get('theme_handler') - ->getTheme('automatic_updates_theme_with_updates'); - $module_file = $updater->getStageDirectory() . "/core/modules/system/system.$suffix"; - $theme_file = $updater->getStageDirectory() . "/{$theme->getPath()}/automatic_updates_theme_with_updates.$suffix"; - if ($delete) { - unlink($module_file); - unlink($theme_file); - } - else { - file_put_contents($module_file, $this->randomString()); - file_put_contents($theme_file, $this->randomString()); - } - - $expected_results = [ - ValidationResult::createError(['System', 'Automatic Updates Theme With Updates'], t('The update cannot proceed because possible database updates have been detected in the following extensions.')), - ]; + $listener = function (PreApplyEvent $event) use ($suffix, $delete): void { + $dir = $event->getStage()->getStageDirectory(); + $theme_installer = $this->container->get('theme_installer'); + $theme_installer->install(['automatic_updates_theme_with_updates']); + $theme = $this->container->get('theme_handler') + ->getTheme('automatic_updates_theme_with_updates'); + $module_file = "$dir/core/modules/system/system.$suffix"; + $theme_file = "$dir/{$theme->getPath()}/{$theme->getName()}.$suffix"; + if ($delete) { + unlink($module_file); + unlink($theme_file); + } + else { + file_put_contents($module_file, $this->randomString()); + file_put_contents($theme_file, $this->randomString()); + } + }; + $this->container->get('event_dispatcher') + ->addListener(PreApplyEvent::class, $listener, PHP_INT_MAX); - try { - $updater->apply(); - $this->fail('Expected a validation error.'); - } - catch (StageValidationException $e) { - $this->assertValidationResultsEqual($expected_results, $e->getResults()); - } + $this->container->get('cron')->run(); + $this->assertTrue($this->logger->hasRecordThatContains("The update cannot proceed because possible database updates have been detected in the following extensions.\nSystem\nAutomatic Updates Theme With Updates", RfcLogLevel::ERROR)); } /** * Tests that an error is raised if install or post-update files are added. */ public function testUpdatesAddedInStage(): void { - $module = $this->container->get('module_handler') - ->getModule('package_manager_bypass'); - $theme_installer = $this->container->get('theme_installer'); - $theme_installer->install(['automatic_updates_theme']); - $theme = $this->container->get('theme_handler') - ->getTheme('automatic_updates_theme'); + $listener = function (PreApplyEvent $event): void { + $module = $this->container->get('module_handler') + ->getModule('package_manager_bypass'); + $theme_installer = $this->container->get('theme_installer'); + $theme_installer->install(['automatic_updates_theme']); + $theme = $this->container->get('theme_handler') + ->getTheme('automatic_updates_theme'); - /** @var \Drupal\automatic_updates\CronUpdater $updater */ - $updater = $this->container->get('automatic_updates.cron_updater'); + $dir = $event->getStage()->getStageDirectory(); - foreach (static::SUFFIXES as $suffix) { - $module_file = sprintf('%s/%s/%s.%s', $updater->getStageDirectory(), $module->getPath(), $module->getName(), $suffix); - $theme_file = sprintf('%s/%s/%s.%s', $updater->getStageDirectory(), $theme->getPath(), $theme->getName(), $suffix); - // The files we're creating shouldn't already exist in the staging area - // unless it's a file we actually ship, which is a scenario covered by - // ::testFileChanged(). - $this->assertFileDoesNotExist($module_file); - $this->assertFileDoesNotExist($theme_file); - file_put_contents($module_file, $this->randomString()); - file_put_contents($theme_file, $this->randomString()); - } - - $expected_results = [ - ValidationResult::createError(['Package Manager Bypass', 'Automatic Updates Theme'], t('The update cannot proceed because possible database updates have been detected in the following extensions.')), - ]; + foreach (static::SUFFIXES as $suffix) { + $module_file = sprintf('%s/%s/%s.%s', $dir, $module->getPath(), $module->getName(), $suffix); + $theme_file = sprintf('%s/%s/%s.%s', $dir, $theme->getPath(), $theme->getName(), $suffix); + // The files we're creating shouldn't already exist in the staging area + // unless it's a file we actually ship, which is a scenario covered by + // ::testFileChanged(). + $this->assertFileDoesNotExist($module_file); + $this->assertFileDoesNotExist($theme_file); + file_put_contents($module_file, $this->randomString()); + file_put_contents($theme_file, $this->randomString()); + } + }; + $this->container->get('event_dispatcher') + ->addListener(PreApplyEvent::class, $listener, PHP_INT_MAX); - try { - $updater->apply(); - $this->fail('Expected a validation error.'); - } - catch (StageValidationException $e) { - $this->assertValidationResultsEqual($expected_results, $e->getResults()); - } + $this->container->get('cron')->run(); + $this->assertTrue($this->logger->hasRecordThatContains("The update cannot proceed because possible database updates have been detected in the following extensions.\nPackage Manager Bypass\nAutomatic Updates Theme", RfcLogLevel::ERROR)); } } diff --git a/tests/src/Kernel/ReadinessValidation/VersionPolicyValidatorTest.php b/tests/src/Kernel/ReadinessValidation/VersionPolicyValidatorTest.php index a218c05637d4e4f561fc9b54e4976ae387e7e75c..24a70e2bf28c320335dc00742edfe13b61bf222e 100644 --- a/tests/src/Kernel/ReadinessValidation/VersionPolicyValidatorTest.php +++ b/tests/src/Kernel/ReadinessValidation/VersionPolicyValidatorTest.php @@ -215,10 +215,8 @@ class VersionPolicyValidatorTest extends AutomaticUpdatesKernelTestBase { return [ 'valid target, dev snapshot installed' => [ - ['automatic_updates.updater', 'automatic_updates.cron_updater'], '9.8.0-dev', "$metadata_dir/drupal.9.8.1-security.xml", - [CronUpdater::SECURITY, CronUpdater::ALL], ['drupal' => '9.8.1'], [ $this->createValidationResult('9.8.0-dev', '9.8.1', [ @@ -229,10 +227,8 @@ class VersionPolicyValidatorTest extends AutomaticUpdatesKernelTestBase { // The following cases can only happen by explicitly supplying the updater // with an invalid target version. 'downgrade' => [ - ['automatic_updates.updater', 'automatic_updates.cron_updater'], '9.8.1', "$metadata_dir/drupal.9.8.2.xml", - [CronUpdater::SECURITY, CronUpdater::ALL], ['drupal' => '9.8.0'], [ $this->createValidationResult('9.8.1', '9.8.0', [ @@ -241,10 +237,8 @@ class VersionPolicyValidatorTest extends AutomaticUpdatesKernelTestBase { ], ], 'major version upgrade' => [ - ['automatic_updates.updater', 'automatic_updates.cron_updater'], '8.9.1', "$metadata_dir/drupal.9.8.2.xml", - [CronUpdater::SECURITY, CronUpdater::ALL], ['drupal' => '9.8.2'], [ $this->createValidationResult('8.9.1', '9.8.2', [ @@ -253,10 +247,8 @@ class VersionPolicyValidatorTest extends AutomaticUpdatesKernelTestBase { ], ], 'unsupported target version' => [ - ['automatic_updates.updater', 'automatic_updates.cron_updater'], '9.8.0', "$metadata_dir/drupal.9.8.2-unsupported_unpublished.xml", - [CronUpdater::SECURITY, CronUpdater::ALL], ['drupal' => '9.8.1'], [ $this->createValidationResult('9.8.0', '9.8.1', [ @@ -265,58 +257,19 @@ class VersionPolicyValidatorTest extends AutomaticUpdatesKernelTestBase { ], ], // This case proves that an attended update to a normal non-security - // release is allowed regardless of how cron is configured... + // release is allowed regardless of how cron is configured. 'attended update to normal release' => [ - ['automatic_updates.updater'], '9.8.1', "$metadata_dir/drupal.9.8.2.xml", - [CronUpdater::SECURITY, CronUpdater::ALL], ['drupal' => '9.8.2'], [], ], - // ...and these two cases prove that an unattended update to a normal - // non-security release is only allowed if cron is configured to allow - // all updates. - 'unattended update to normal release, security only in cron' => [ - ['automatic_updates.cron_updater'], - '9.8.1', - "$metadata_dir/drupal.9.8.2.xml", - [CronUpdater::SECURITY], - ['drupal' => '9.8.2'], - [ - $this->createValidationResult('9.8.1', '9.8.2', [ - 'Drupal cannot be automatically updated during cron from 9.8.1 to 9.8.2 because 9.8.2 is not a security release.', - ]), - ], - ], - 'unattended update to normal release, all allowed in cron' => [ - ['automatic_updates.cron_updater'], - '9.8.1', - "$metadata_dir/drupal.9.8.2.xml", - [CronUpdater::ALL], - ['drupal' => '9.8.2'], - [], - ], - // These three cases prove that updating across minor versions of Drupal + // These two cases prove that updating across minor versions of Drupal // core is only allowed for attended updates when a specific configuration // flag is set. - 'unattended update to next minor' => [ - ['automatic_updates.cron_updater'], - '9.7.9', - "$metadata_dir/drupal.9.8.2.xml", - [CronUpdater::SECURITY, CronUpdater::ALL], - ['drupal' => '9.8.2'], - [ - $this->createValidationResult('9.7.9', '9.8.2', [ - 'Drupal cannot be automatically updated from 9.7.9 to 9.8.2 because automatic updates from one minor version to another are not supported during cron.', - ]), - ], - ], 'attended update to next minor not allowed' => [ - ['automatic_updates.updater'], '9.7.9', "$metadata_dir/drupal.9.8.2.xml", - [CronUpdater::SECURITY, CronUpdater::ALL], ['drupal' => '9.8.2'], [ $this->createValidationResult('9.7.9', '9.8.2', [ @@ -325,10 +278,8 @@ class VersionPolicyValidatorTest extends AutomaticUpdatesKernelTestBase { ], ], 'attended update to next minor allowed' => [ - ['automatic_updates.updater'], '9.7.9', "$metadata_dir/drupal.9.8.2.xml", - [CronUpdater::SECURITY, CronUpdater::ALL], ['drupal' => '9.8.2'], [], TRUE, @@ -336,74 +287,22 @@ class VersionPolicyValidatorTest extends AutomaticUpdatesKernelTestBase { // If attended updates across minor versions are allowed, it's okay to // update from an unsupported minor version. 'attended update from unsupported minor allowed' => [ - ['automatic_updates.updater'], '9.7.9', "$metadata_dir/drupal.9.8.1-security.xml", - [CronUpdater::SECURITY, CronUpdater::ALL], ['drupal' => '9.8.1'], [], TRUE, ], - // Unattended updates to unstable versions are not allowed. - 'unattended update to unstable version' => [ - ['automatic_updates.cron_updater'], - '9.8.0', - "$metadata_dir/drupal.9.8.2-older-sec-release.xml", - [CronUpdater::SECURITY, CronUpdater::ALL], - ['drupal' => '9.8.1-beta1'], - [ - $this->createValidationResult('9.8.0', '9.8.1-beta1', [ - 'Drupal cannot be automatically updated during cron to the recommended version, 9.8.1-beta1, because it is not a stable version.', - ]), - ], - ], - // Unattended updates from an unsupported minor are never allowed, but - // the messaging will vary depending on whether attended updates across - // minor versions are allowed. - 'unattended update from unsupported minor, minor updates forbidden' => [ - ['automatic_updates.cron_updater'], - '9.7.9', - "$metadata_dir/drupal.9.8.1-security.xml", - [CronUpdater::SECURITY, CronUpdater::ALL], - ['drupal' => '9.8.1'], - [ - $this->createValidationResult('9.7.9', '9.8.1', [ - 'The currently installed version of Drupal core, 9.7.9, is not in a supported minor version. Your site will not be automatically updated during cron until it is updated to a supported minor version.', - 'See the <a href="/admin/reports/updates">available updates page</a> for available updates.', - ]), - ], - FALSE, - ], - 'unattended update from unsupported minor, minor updates allowed' => [ - ['automatic_updates.cron_updater'], - '9.7.9', - "$metadata_dir/drupal.9.8.1-security.xml", - [CronUpdater::SECURITY, CronUpdater::ALL], - ['drupal' => '9.8.1'], - [ - $this->createValidationResult('9.7.9', '9.8.1', [ - 'The currently installed version of Drupal core, 9.7.9, is not in a supported minor version. Your site will not be automatically updated during cron until it is updated to a supported minor version.', - 'Use the <a href="/admin/modules/automatic-update">update form</a> to update to a supported version.', - ]), - ], - TRUE, - ], ]; } /** * Tests validation of explicitly specified target versions. * - * @param string[] $updaters - * The IDs of the updater services to test. * @param string $installed_version * The installed version of Drupal core. * @param string $release_metadata * The path of the core release metadata to serve to the update system. - * @param string[] $cron_modes - * The modes for unattended updates. Can contain - * \Drupal\automatic_updates\CronUpdater::SECURITY or - * \Drupal\automatic_updates\CronUpdater::ALL. * @param string[] $project_versions * The desired project versions that should be passed to the updater. * @param \Drupal\package_manager\ValidationResult[] $expected_results @@ -414,31 +313,26 @@ class VersionPolicyValidatorTest extends AutomaticUpdatesKernelTestBase { * * @dataProvider providerApi */ - public function testApi(array $updaters, string $installed_version, string $release_metadata, array $cron_modes, array $project_versions, array $expected_results, bool $allow_minor_updates = FALSE): void { + public function testApi(string $installed_version, string $release_metadata, array $project_versions, array $expected_results, bool $allow_minor_updates = FALSE): void { $this->setCoreVersion($installed_version); $this->setReleaseMetadata(['drupal' => $release_metadata]); - foreach ($cron_modes as $cron_mode) { - $this->config('automatic_updates.settings') - ->set('cron', $cron_mode) - ->set('allow_core_minor_updates', $allow_minor_updates) - ->save(); + $this->config('automatic_updates.settings') + ->set('allow_core_minor_updates', $allow_minor_updates) + ->save(); - foreach ($updaters as $updater) { - /** @var \Drupal\automatic_updates\Updater $updater */ - $updater = $this->container->get($updater); + /** @var \Drupal\automatic_updates\Updater $updater */ + $updater = $this->container->get('automatic_updates.updater'); - try { - $updater->begin($project_versions); - // Ensure that we did not, in fact, expect any errors. - $this->assertEmpty($expected_results); - // Reset the updater for the next iteration of the loop. - $updater->destroy(); - } - catch (StageValidationException $e) { - $this->assertValidationResultsEqual($expected_results, $e->getResults()); - } - } + try { + $updater->begin($project_versions); + // Ensure that we did not, in fact, expect any errors. + $this->assertEmpty($expected_results); + // Reset the updater for the next iteration of the loop. + $updater->destroy(); + } + catch (StageValidationException $e) { + $this->assertValidationResultsEqual($expected_results, $e->getResults()); } }