From 812c4756c9ce7df988faa7fa438716813a6dfcb7 Mon Sep 17 00:00:00 2001
From: phenaproxima <phenaproxima@205645.no-reply.drupal.org>
Date: Fri, 24 Jun 2022 14:00:15 +0000
Subject: [PATCH] Issue #3285669 by phenaproxima, tedbow:
VersionPolicyValidator should not try to detect the target version unless the
update will be unattended
---
src/CronUpdater.php | 14 +++++++-
src/Validator/VersionPolicyValidator.php | 35 ++++---------------
.../VersionPolicyValidatorTest.php | 31 +++++++---------
3 files changed, 32 insertions(+), 48 deletions(-)
diff --git a/src/CronUpdater.php b/src/CronUpdater.php
index d85adf21f6..3dad8ae033 100644
--- a/src/CronUpdater.php
+++ b/src/CronUpdater.php
@@ -2,6 +2,7 @@
namespace Drupal\automatic_updates;
+use Drupal\automatic_updates_9_3_shim\ProjectRelease;
use Drupal\Core\Language\LanguageManagerInterface;
use Drupal\Core\Logger\LoggerChannelFactoryInterface;
use Drupal\Core\Mail\MailManagerInterface;
@@ -109,12 +110,23 @@ class CronUpdater extends Updater {
return;
}
- $next_release = $this->releaseChooser->getLatestInInstalledMinor($this);
+ $next_release = $this->getTargetRelease();
if ($next_release) {
$this->performUpdate($next_release->getVersion(), $timeout);
}
}
+ /**
+ * Returns the release of Drupal core to update to, if any.
+ *
+ * @return \Drupal\automatic_updates_9_3_shim\ProjectRelease|null
+ * The release of Drupal core to which we will update, or NULL if there is
+ * nothing to update to.
+ */
+ public function getTargetRelease(): ?ProjectRelease {
+ return $this->releaseChooser->getLatestInInstalledMinor($this);
+ }
+
/**
* {@inheritdoc}
*/
diff --git a/src/Validator/VersionPolicyValidator.php b/src/Validator/VersionPolicyValidator.php
index e20086c370..ffcfbde711 100644
--- a/src/Validator/VersionPolicyValidator.php
+++ b/src/Validator/VersionPolicyValidator.php
@@ -2,7 +2,6 @@
namespace Drupal\automatic_updates\Validator;
-use Composer\Semver\Semver;
use Drupal\automatic_updates\CronUpdater;
use Drupal\automatic_updates\Event\ReadinessCheckEvent;
use Drupal\automatic_updates\ProjectInfo;
@@ -205,38 +204,18 @@ final class VersionPolicyValidator implements EventSubscriberInterface {
}
}
elseif ($event instanceof ReadinessCheckEvent) {
- // It's okay if this returns NULL; it means there's nothing to update to.
- return $this->getTargetVersionFromAvailableReleases($updater);
+ if ($updater instanceof CronUpdater) {
+ $target_release = $updater->getTargetRelease();
+ if ($target_release) {
+ return $target_release->getVersion();
+ }
+ }
+ return NULL;
}
// If we got here, something has gone very wrong.
throw $unknown_target;
}
- /**
- * Returns the target version of Drupal from the list of available releases.
- *
- * @param \Drupal\automatic_updates\Updater $updater
- * The updater which will perform the update.
- *
- * @return string|null
- * The target version of Drupal core, or NULL if it could not be determined.
- *
- * @todo Expand this doc comment to explain how the list of available releases
- * is fetched, sorted, and filtered through (i.e., must match the current
- * minor). Maybe reference ProjectInfo::getInstallableReleases().
- */
- private function getTargetVersionFromAvailableReleases(Updater $updater): ?string {
- $installed_version = $this->getInstalledVersion();
-
- foreach (self::getAvailableReleases($updater) as $possible_release) {
- $possible_version = $possible_release->getVersion();
- if (Semver::satisfies($possible_version, "~$installed_version")) {
- return $possible_version;
- }
- }
- return NULL;
- }
-
/**
* Returns the available releases of Drupal core for a given updater.
*
diff --git a/tests/src/Kernel/ReadinessValidation/VersionPolicyValidatorTest.php b/tests/src/Kernel/ReadinessValidation/VersionPolicyValidatorTest.php
index 24a70e2bf2..4363bfb475 100644
--- a/tests/src/Kernel/ReadinessValidation/VersionPolicyValidatorTest.php
+++ b/tests/src/Kernel/ReadinessValidation/VersionPolicyValidatorTest.php
@@ -58,7 +58,7 @@ class VersionPolicyValidatorTest extends AutomaticUpdatesKernelTestBase {
"$metadata_dir/drupal.9.8.1-security.xml",
[CronUpdater::DISABLED, CronUpdater::SECURITY, CronUpdater::ALL],
[
- $this->createValidationResult('9.8.0-dev', '9.8.1', [
+ $this->createValidationResult('9.8.0-dev', NULL, [
'Drupal cannot be automatically updated from the installed version, 9.8.0-dev, because automatic updates from a dev version to any other version are not supported.',
]),
],
@@ -76,7 +76,7 @@ class VersionPolicyValidatorTest extends AutomaticUpdatesKernelTestBase {
"$metadata_dir/drupal.9.8.1-security.xml",
[CronUpdater::SECURITY, CronUpdater::ALL],
[
- $this->createValidationResult('9.8.0-alpha1', '9.8.1', [
+ $this->createValidationResult('9.8.0-alpha1', NULL, [
'Drupal cannot be automatically updated during cron from its current version, 9.8.0-alpha1, because it is not a stable version.',
]),
],
@@ -92,7 +92,7 @@ class VersionPolicyValidatorTest extends AutomaticUpdatesKernelTestBase {
"$metadata_dir/drupal.9.8.1-security.xml",
[CronUpdater::SECURITY, CronUpdater::ALL],
[
- $this->createValidationResult('9.8.0-beta2', '9.8.1', [
+ $this->createValidationResult('9.8.0-beta2', NULL, [
'Drupal cannot be automatically updated during cron from its current version, 9.8.0-beta2, because it is not a stable version.',
]),
],
@@ -108,30 +108,23 @@ class VersionPolicyValidatorTest extends AutomaticUpdatesKernelTestBase {
"$metadata_dir/drupal.9.8.1-security.xml",
[CronUpdater::SECURITY, CronUpdater::ALL],
[
- $this->createValidationResult('9.8.0-rc3', '9.8.1', [
+ $this->createValidationResult('9.8.0-rc3', NULL, [
'Drupal cannot be automatically updated during cron from its current version, 9.8.0-rc3, because it is not a stable version.',
]),
],
],
- // These two cases prove that, if only security updates are allowed
- // during cron, a readiness error is raised if the next available release
- // is not a security release.
- 'update to normal release allowed' => [
+ // This case proves that, if a stable release is installed, there is no
+ // error generated when if the next available release is a normal (i.e.,
+ // non-security) release. If unattended updates are only enabled for
+ // security releases, the next available release will be ignored, and
+ // therefore generate no validation errors, because it's not a security
+ // release.
+ 'update to normal release' => [
'9.8.1',
"$metadata_dir/drupal.9.8.2.xml",
- [CronUpdater::DISABLED, CronUpdater::ALL],
+ [CronUpdater::DISABLED, CronUpdater::SECURITY, CronUpdater::ALL],
[],
],
- 'update to normal release, security only in cron' => [
- '9.8.1',
- "$metadata_dir/drupal.9.8.2.xml",
- [CronUpdater::SECURITY],
- [
- $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.',
- ]),
- ],
- ],
// These three cases prove that updating from an unsupported minor version
// will raise a readiness error if unattended updates are enabled.
// Furthermore, if an error is raised, the messaging will vary depending
--
GitLab