Issue #3254207 by kunal.sachdev, phenaproxima, tedbow: Only allow 1 patch...

Issue #3254207 by kunal.sachdev, phenaproxima, tedbow: Only allow 1 patch release update increment in Cron

src/Validator/UpdateVersionValidator.php

@@ -95,12 +95,14 @@ class UpdateVersionValidator implements EventSubscriberInterface {
       '@to_version' => $to_version_string,
       '@from_version' => $from_version_string,
     ];
+    $from_version_extra = $from_version->getVersionExtra();
+    $to_version_extra = $to_version->getVersionExtra();
     if (Semver::satisfies($to_version_string, "< $from_version_string")) {
       $event->addError([
         $this->t('Update version @to_version is lower than @from_version, downgrading is not supported.', $variables),
       ]);
     }
-    elseif ($from_version->getVersionExtra() === 'dev') {
+    elseif ($from_version_extra === 'dev') {
       $event->addError([
         $this->t('Drupal cannot be automatically updated from its current version, @from_version, to the recommended version, @to_version, because automatic updates from a dev version to any other version are not supported.', $variables),
       ]);
@@ -122,6 +124,30 @@ class UpdateVersionValidator implements EventSubscriberInterface {
         ]);
       }
     }
+    elseif ($stage instanceof CronUpdater) {
+      if ($from_version_extra || $to_version_extra) {
+        if ($from_version_extra) {
+          $messages[] = $this->t('Drupal cannot be automatically updated during cron from its current version, @from_version, because Automatic Updates only supports updating from stable versions during cron.', $variables);
+          $event->addError($messages);
+        }
+        if ($to_version_extra) {
+          // Because we do not support updating to a new minor version during
+          // cron it is probably impossible to update from a stable version to
+          // a unstable/pre-release version, but we should check this condition
+          // just in case.
+          $messages[] = $this->t('Drupal cannot be automatically updated during cron to the recommended version, @to_version, because Automatic Updates only supports updating to stable versions during cron.', $variables);
+          $event->addError($messages);
+        }
+      }
+      else {
+        $to_patch_version = (int) $this->getPatchVersion($to_version_string);
+        $from_patch_version = (int) $this->getPatchVersion($from_version_string);
+        if ($from_patch_version + 1 !== $to_patch_version) {
+          $messages[] = $this->t('Drupal cannot be automatically updated during cron from its current version, @from_version, to the recommended version, @to_version, because Automatic Updates only supports 1 patch version update during cron.', $variables);
+          $event->addError($messages);
+        }
+      }
+    }
   }
 
   /**
@@ -134,4 +160,26 @@ class UpdateVersionValidator implements EventSubscriberInterface {
     ];
   }
 
+  /**
+   * Gets the patch number for a version string.
+   *
+   * @todo Move this method to \Drupal\Core\Extension\ExtensionVersion in
+   *   https://www.drupal.org/i/3261744.
+   *
+   * @param string $version_string
+   *   The version string.
+   *
+   * @return string
+   *   The patch number.
+   */
+  private function getPatchVersion(string $version_string): string {
+    $version_extra = ExtensionVersion::createFromVersionString($version_string)
+      ->getVersionExtra();
+    if ($version_extra) {
+      $version_string = str_replace("-$version_extra", '', $version_string);
+    }
+    $version_parts = explode('.', $version_string);
+    return $version_parts[2];
+  }
+
 }

tests/fixtures/release-history/drupal.9.8.1.xml → tests/fixtures/release-history/drupal.9.8.2.xml

@@ -10,6 +10,18 @@
    <term><name>Projects</name><value>Drupal project</value></term>
   </terms>
 <releases>
+  <release>
+    <name>Drupal 9.8.2</name>
+    <version>9.8.2</version>
+    <status>published</status>
+    <release_link>http://example.com/drupal-9-8-2-release</release_link>
+    <download_link>http://example.com/drupal-9-8-2.tar.gz</download_link>
+    <date>1250425521</date>
+    <terms>
+      <term><name>Release type</name><value>New features</value></term>
+      <term><name>Release type</name><value>Bug fixes</value></term>
+    </terms>
+  </release>
  <release>
   <name>Drupal 9.8.1</name>
   <version>9.8.1</version>
@@ -34,5 +46,17 @@
      <term><name>Release type</name><value>Bug fixes</value></term>
    </terms>
  </release>
+  <release>
+    <name>Drupal 9.8.0-alpha1</name>
+    <version>9.8.0-alpha1</version>
+    <status>published</status>
+    <release_link>http://example.com/drupal-9-8-0-alpha1-release</release_link>
+    <download_link>http://example.com/drupal-9-8-0-alpha1.tar.gz</download_link>
+    <date>1250424521</date>
+    <terms>
+      <term><name>Release type</name><value>New features</value></term>
+      <term><name>Release type</name><value>Bug fixes</value></term>
+    </terms>
+  </release>
 </releases>
 </project>

tests/src/Functional/ReadinessValidationTest.php

@@ -54,7 +54,7 @@ class ReadinessValidationTest extends AutomaticUpdatesFunctionalTestBase {
    */
   protected function setUp(): void {
     parent::setUp();
-    $this->setReleaseMetadata(__DIR__ . '/../../fixtures/release-history/drupal.9.8.1.xml');
+    $this->setReleaseMetadata(__DIR__ . '/../../fixtures/release-history/drupal.9.8.2.xml');
     $this->setCoreVersion('9.8.1');
 
     $this->reportViewerUser = $this->createUser([
@@ -386,9 +386,8 @@ class ReadinessValidationTest extends AutomaticUpdatesFunctionalTestBase {
     // version.
     $this->setCoreVersion('9.8.0');
 
-    // Flag a validation warning, which will be displayed in the messages area,
-    // but not block or abort the update.
-    $results = $this->testResults['checker_1']['1 warning'];
+    // Flag a validation error, which will be displayed in the messages area.
+    $results = $this->testResults['checker_1']['1 error'];
     TestChecker1::setTestResult($results, ReadinessCheckEvent::class);
     $message = $results[0]->getMessages()[0];
 
@@ -398,7 +397,7 @@ class ReadinessValidationTest extends AutomaticUpdatesFunctionalTestBase {
       'package_manager_bypass',
     ]);
 
-    // The warning should be persistently visible, even after the checker stops
+    // The error should be persistently visible, even after the checker stops
     // flagging it.
     $this->drupalGet('/admin/structure');
     $assert_session->pageTextContains($message);
@@ -407,7 +406,10 @@ class ReadinessValidationTest extends AutomaticUpdatesFunctionalTestBase {
     $assert_session->pageTextContains($message);
 
     // Do the update; we don't expect any errors or special conditions to appear
-    // during it.
+    // during it. The Update button is displayed because the form does its own
+    // readiness check (without storing the results), and the checker is no
+    // longer raising an error.
+    // @todo Fine-tune this in https://www.drupal.org/node/3261758.
     $this->drupalGet('/admin/modules/automatic-update');
     $page->pressButton('Update');
     $this->checkForMetaRefresh();

tests/src/Functional/UpdateLockTest.php

@@ -29,7 +29,7 @@ class UpdateLockTest extends AutomaticUpdatesFunctionalTestBase {
   protected function setUp(): void {
     parent::setUp();
 
-    $this->setReleaseMetadata(__DIR__ . '/../../fixtures/release-history/drupal.9.8.1.xml');
+    $this->setReleaseMetadata(__DIR__ . '/../../fixtures/release-history/drupal.9.8.2.xml');
     $this->drupalLogin($this->rootUser);
     $this->checkForUpdates();
   }

tests/src/Kernel/AutomaticUpdatesKernelTestBase.php

@@ -75,8 +75,8 @@ abstract class AutomaticUpdatesKernelTestBase extends KernelTestBase {
 
     // By default, pretend we're running Drupal core 9.8.0 and a non-security
     // update to 9.8.1 is available.
-    $this->setCoreVersion('9.8.0');
-    $this->setReleaseMetadata(__DIR__ . '/../../fixtures/release-history/drupal.9.8.1.xml');
+    $this->setCoreVersion('9.8.1');
+    $this->setReleaseMetadata(__DIR__ . '/../../fixtures/release-history/drupal.9.8.2.xml');
 
     // Set a last cron run time so that the cron frequency validator will run
     // from a sane state.

tests/src/Kernel/CronUpdaterTest.php

@@ -45,7 +45,7 @@ class CronUpdaterTest extends AutomaticUpdatesKernelTestBase {
     return [
       'disabled, normal release' => [
         CronUpdater::DISABLED,
-        "$fixture_dir/drupal.9.8.1.xml",
+        "$fixture_dir/drupal.9.8.2.xml",
         FALSE,
       ],
       'disabled, security release' => [
@@ -60,12 +60,12 @@ class CronUpdaterTest extends AutomaticUpdatesKernelTestBase {
       ],
       'security only, normal release' => [
         CronUpdater::SECURITY,
-        "$fixture_dir/drupal.9.8.1.xml",
+        "$fixture_dir/drupal.9.8.2.xml",
         FALSE,
       ],
       'enabled, normal release' => [
         CronUpdater::ALL,
-        "$fixture_dir/drupal.9.8.1.xml",
+        "$fixture_dir/drupal.9.8.2.xml",
         TRUE,
       ],
       'enabled, security release' => [
@@ -94,6 +94,7 @@ class CronUpdaterTest extends AutomaticUpdatesKernelTestBase {
     // Our form alter does not refresh information on available updates, so
     // ensure that the appropriate update data is loaded beforehand.
     $this->setReleaseMetadata($release_data);
+    $this->setCoreVersion('9.8.0');
     update_get_available(TRUE);
 
     // Submit the configuration form programmatically, to prove our alterations

tests/src/Kernel/ReadinessValidation/ReadinessValidationManagerTest.php

@@ -242,7 +242,7 @@ class ReadinessValidationManagerTest extends AutomaticUpdatesKernelTestBase {
       ->install(['automatic_updates']);
 
     // Ensure there's a simulated core release to update to.
-    $this->setReleaseMetadata(__DIR__ . '/../../../fixtures/release-history/drupal.9.8.1.xml');
+    $this->setReleaseMetadata(__DIR__ . '/../../../fixtures/release-history/drupal.9.8.2.xml');
 
     // The readiness checker should raise a warning, so that the update is not
     // blocked or aborted.

tests/src/Kernel/ReadinessValidation/StagedDatabaseUpdateValidatorTest.php

@@ -41,7 +41,7 @@ class StagedDatabaseUpdateValidatorTest extends AutomaticUpdatesKernelTestBase {
 
     /** @var \Drupal\Tests\automatic_updates\Kernel\TestCronUpdater $updater */
     $updater = $this->container->get('automatic_updates.cron_updater');
-    $updater->begin(['drupal' => '9.8.1']);
+    $updater->begin(['drupal' => '9.8.2']);
     $updater->stage();
 
     $stage_dir = $updater->getStageDirectory();

tests/src/Kernel/ReadinessValidation/UpdateVersionValidatorTest.php

@@ -33,6 +33,10 @@ class UpdateVersionValidatorTest extends AutomaticUpdatesKernelTestBase {
    * Tests an update version that is same major & minor version as the current.
    */
   public function testNoMajorOrMinorUpdates(): void {
+    $this->setCoreVersion('9.8.0');
+    $this->config('automatic_updates.settings')
+      ->set('cron', CronUpdater::DISABLED)
+      ->save();
     $this->assertCheckerResultsFromManager([], TRUE);
   }
 
@@ -42,7 +46,7 @@ class UpdateVersionValidatorTest extends AutomaticUpdatesKernelTestBase {
   public function testMajorUpdates(): void {
     $this->setCoreVersion('8.9.1');
     $result = ValidationResult::createError([
-      'Drupal cannot be automatically updated from its current version, 8.9.1, to the recommended version, 9.8.1, because automatic updates from one major version to another are not supported.',
+      'Drupal cannot be automatically updated from its current version, 8.9.1, to the recommended version, 9.8.2, because automatic updates from one major version to another are not supported.',
     ]);
     $this->assertCheckerResultsFromManager([$result], TRUE);
   }
@@ -162,8 +166,8 @@ class UpdateVersionValidatorTest extends AutomaticUpdatesKernelTestBase {
    * Tests an update version that is a lower version than the current.
    */
   public function testDowngrading(): void {
-    $this->setCoreVersion('9.8.2');
-    $result = ValidationResult::createError(['Update version 9.8.1 is lower than 9.8.2, downgrading is not supported.']);
+    $this->setCoreVersion('9.8.3');
+    $result = ValidationResult::createError(['Update version 9.8.2 is lower than 9.8.3, downgrading is not supported.']);
     $this->assertCheckerResultsFromManager([$result], TRUE);
   }
 
@@ -172,8 +176,79 @@ class UpdateVersionValidatorTest extends AutomaticUpdatesKernelTestBase {
    */
   public function testUpdatesFromDevVersion(): void {
     $this->setCoreVersion('9.8.0-dev');
-    $result = ValidationResult::createError(['Drupal cannot be automatically updated from its current version, 9.8.0-dev, to the recommended version, 9.8.1, because automatic updates from a dev version to any other version are not supported.']);
+    $result = ValidationResult::createError(['Drupal cannot be automatically updated from its current version, 9.8.0-dev, to the recommended version, 9.8.2, because automatic updates from a dev version to any other version are not supported.']);
+    $this->assertCheckerResultsFromManager([$result], TRUE);
+  }
+
+  /**
+   * Tests a cron update two patch releases ahead of the current version.
+   */
+  public function testCronUpdateTwoPatchReleasesAhead(): void {
+    $this->setCoreVersion('9.8.0');
+    $cron = $this->container->get('cron');
+    $config = $this->config('automatic_updates.settings');
+
+    $logger = new TestLogger();
+    $this->container->get('logger.factory')
+      ->get('automatic_updates')
+      ->addLogger($logger);
+
+    // The latest version is two patch releases ahead, so we won't update to it
+    // during cron, even if configuration allows it, and this should be flagged
+    // as an error during readiness checking. Trying to run the update anyway
+    // should raise an error.
+    $config->set('cron', CronUpdater::ALL)->save();
+    $result = ValidationResult::createError(['Drupal cannot be automatically updated during cron from its current version, 9.8.0, to the recommended version, 9.8.2, because Automatic Updates only supports 1 patch version update during cron.']);
+    $this->assertCheckerResultsFromManager([$result], TRUE);
+    $cron->run();
+    $this->assertUpdateStagedTimes(0);
+    $this->assertTrue($logger->hasRecord("<h2>Unable to complete the update because of errors.</h2>Drupal cannot be automatically updated during cron from its current version, 9.8.0, to the recommended version, 9.8.2, because Automatic Updates only supports 1 patch version update during cron.", RfcLogLevel::ERROR));
+
+    // If cron updates are totally disabled, there's no problem here and no
+    // errors should be raised.
+    $config->set('cron', CronUpdater::DISABLED)->save();
+    $this->assertCheckerResultsFromManager([], TRUE);
+
+    // Even if cron is configured to allow security updates only, the update
+    // will be blocked if it's more than one patch version ahead.
+    $config->set('cron', CronUpdater::SECURITY)->save();
+    $cron->run();
+    $this->assertUpdateStagedTimes(0);
+    $this->assertTrue($logger->hasRecord("<h2>Unable to complete the update because of errors.</h2>Drupal cannot be automatically updated during cron from its current version, 9.8.0, to the recommended version, 9.8.2, because Automatic Updates only supports 1 patch version update during cron.", RfcLogLevel::ERROR));
+  }
+
+  /**
+   * Tests a cron update one patch release ahead of the current version.
+   */
+  public function testCronUpdateOnePatchReleaseAhead(): void {
+    $cron = $this->container->get('cron');
+    $this->config('automatic_updates.settings')
+      ->set('cron', CronUpdater::ALL)
+      ->save();
+    $this->assertCheckerResultsFromManager([], TRUE);
+    $cron->run();
+    $this->assertUpdateStagedTimes(1);
+  }
+
+  /**
+   * Tests a cron update where the current version is not stable.
+   */
+  public function testCronUpdateFromUnstableVersion(): void {
+    $this->setCoreVersion('9.8.0-alpha1');
+    $this->config('automatic_updates.settings')
+      ->set('cron', CronUpdater::ALL)
+      ->save();
+    $logger = new TestLogger();
+    $this->container->get('logger.factory')
+      ->get('automatic_updates')
+      ->addLogger($logger);
+    $message = 'Drupal cannot be automatically updated during cron from its current version, 9.8.0-alpha1, because Automatic Updates only supports updating from stable versions during cron.';
+    $result = ValidationResult::createError([$message]);
     $this->assertCheckerResultsFromManager([$result], TRUE);
+
+    $this->container->get('cron')->run();
+    $this->assertUpdateStagedTimes(0);
+    $this->assertTrue($logger->hasRecord("<h2>Unable to complete the update because of errors.</h2>$message", RfcLogLevel::ERROR));
   }
 
 }

tests/src/Kernel/UpdateRecommenderTest.php

@@ -26,7 +26,7 @@ class UpdateRecommenderTest extends AutomaticUpdatesKernelTestBase {
     $recommender = new UpdateRecommender();
     $recommended_release = $recommender->getRecommendedRelease(TRUE);
     $this->assertNotEmpty($recommended_release);
-    $this->assertSame('9.8.1', $recommended_release->getVersion());
+    $this->assertSame('9.8.2', $recommended_release->getVersion());
     // Getting the recommended release again should not trigger another request.
     $this->assertNotEmpty($recommender->getRecommendedRelease());
   }
@@ -35,7 +35,7 @@ class UpdateRecommenderTest extends AutomaticUpdatesKernelTestBase {
    * Tests fetching the recommended release when there is no update available.
    */
   public function testNoUpdateAvailable(): void {
-    $this->setCoreVersion('9.8.1');
+    $this->setCoreVersion('9.8.2');
 
     $recommender = new UpdateRecommender();
     $recommended_release = $recommender->getRecommendedRelease(TRUE);