Issue #3285669 by phenaproxima, tedbow: VersionPolicyValidator should not try...

Issue #3285669 by phenaproxima, tedbow: VersionPolicyValidator should not try to detect the target version unless the update will be unattended

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}
    */

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.
    *

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