Contrib: Issue #3260672 by tedbow: Extra lines fail core checks for remove...

Contrib: Issue #3260672 by tedbow: Extra lines fail core checks for remove functionality - project/automatic_updates@50a752f8

core/modules/auto_updates/auto_updates.routing.yml

@@ -13,6 +13,12 @@ auto_updates.confirmation_page:
   requirements:
     _permission: 'administer software updates'
     _access_update_manager: 'TRUE'
+auto_updates.finish:
+  path: '/automatic-update/finish'
+  defaults:
+    _controller: '\Drupal\auto_updates\Controller\UpdateController::onFinish'
+  requirements:
+    _permission: 'administer software updates'
 # Links to our updater form appear in three different sets of local tasks. To ensure the breadcrumbs and paths are
 # consistent with the other local tasks in each set, we need two separate routes to the same form.
 auto_updates.report_update:

core/modules/auto_updates/auto_updates.services.yml

@@ -47,6 +47,7 @@ services:
     class: Drupal\auto_updates\Validator\UpdateVersionValidator
     arguments:
       - '@string_translation'
+      - '@config.factory'
     tags:
       - { name: event_subscriber }
   auto_updates.composer_executable_validator:

core/modules/auto_updates/config/install/auto_updates.settings.yml

 cron: security
+allow_core_minor_updates: false

core/modules/auto_updates/config/schema/auto_updates.schema.yml

@@ -5,3 +5,6 @@ auto_updates.settings:
     cron:
       type: string
       label: 'Enable automatic updates during cron'
+    allow_core_minor_updates:
+      type: boolean
+      label: 'Allow minor level Drupal core updates'

core/modules/auto_updates/src/BatchProcessor.php

@@ -162,12 +162,11 @@ public static function finishStage(bool $success, array $results, array $operati
    *   A list of the operations that had not been completed by the batch API.
    */
   public static function finishCommit(bool $success, array $results, array $operations): ?RedirectResponse {
-
     if ($success) {
-      \Drupal::messenger()->addMessage('Update complete!');
-      // @todo redirect to update.php?
-      return new RedirectResponse(Url::fromRoute('update.status', [],
-        ['absolute' => TRUE])->toString());
+      $url = Url::fromRoute('auto_updates.finish')
+        ->setAbsolute()
+        ->toString();
+      return new RedirectResponse($url);
     }
     static::handleBatchError($results);
     return NULL;

core/modules/auto_updates/src/Controller/UpdateController.php

+<?php
+
+namespace Drupal\auto_updates\Controller;
+
+use Drupal\Core\Controller\ControllerBase;
+use Drupal\Core\Update\UpdateHookRegistry;
+use Drupal\Core\Update\UpdateRegistry;
+use Drupal\Core\Url;
+use Symfony\Component\DependencyInjection\ContainerInterface;
+use Symfony\Component\HttpFoundation\RedirectResponse;
+
+/**
+ * Defines a controller to handle various stages of an automatic update.
+ *
+ * @internal
+ *   Controller classes are internal.
+ */
+class UpdateController extends ControllerBase {
+
+  /**
+   * The update hook registry service.
+   *
+   * @var \Drupal\Core\Update\UpdateHookRegistry
+   */
+  protected $updateHookRegistry;
+
+  /**
+   * The post-update registry service.
+   *
+   * @var \Drupal\Core\Update\UpdateRegistry
+   */
+  protected $postUpdateRegistry;
+
+  /**
+   * Constructs an UpdateController object.
+   *
+   * @param \Drupal\Core\Update\UpdateHookRegistry $update_hook_registry
+   *   The update hook registry service.
+   * @param \Drupal\Core\Update\UpdateRegistry $post_update_registry
+   *   The post-update registry service.
+   */
+  public function __construct(UpdateHookRegistry $update_hook_registry, UpdateRegistry $post_update_registry) {
+    $this->updateHookRegistry = $update_hook_registry;
+    $this->postUpdateRegistry = $post_update_registry;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function create(ContainerInterface $container) {
+    return new static(
+      $container->get('update.update_hook_registry'),
+      $container->get('update.post_update_registry')
+    );
+  }
+
+  /**
+   * Redirects after staged changes are applied to the active directory.
+   *
+   * If there are any pending update hooks or post-updates, the user is sent to
+   * update.php to run those. Otherwise, they are redirected to the status
+   * report.
+   *
+   * @return \Symfony\Component\HttpFoundation\RedirectResponse
+   *   A redirect to the appropriate destination.
+   */
+  public function onFinish(): RedirectResponse {
+    if ($this->pendingUpdatesExist()) {
+      $message = $this->t('Please apply database updates to complete the update process.');
+      $url = Url::fromRoute('system.db_update');
+    }
+    else {
+      $message = $this->t('Update complete!');
+      $url = Url::fromRoute('update.status');
+    }
+    $this->messenger()->addStatus($message);
+    return new RedirectResponse($url->setAbsolute()->toString());
+  }
+
+  /**
+   * Checks if there are any pending database updates.
+   *
+   * @return bool
+   *   TRUE if there are any pending update hooks or post-updates, otherwise
+   *   FALSE.
+   */
+  protected function pendingUpdatesExist(): bool {
+    if ($this->postUpdateRegistry->getPendingUpdateFunctions()) {
+      return TRUE;
+    }
+
+    $modules = array_keys($this->moduleHandler()->getModuleList());
+    foreach ($modules as $module) {
+      if ($this->updateHookRegistry->getAvailableUpdates($module)) {
+        return TRUE;
+      }
+    }
+    return FALSE;
+  }
+
+}

core/modules/auto_updates/src/CronUpdater.php

@@ -4,6 +4,7 @@
 
 use Drupal\Core\Config\ConfigFactoryInterface;
 use Drupal\Core\Logger\LoggerChannelFactoryInterface;
+use Drupal\package_manager\Exception\StageValidationException;
 
 /**
  * Defines a service that updates via cron.
@@ -114,6 +115,10 @@ public function handleCron(): void {
       $this->apply();
       $this->destroy();
     }
+    catch (StageValidationException $e) {
+      $this->logger->error(static::formatValidationException($e));
+      return;
+    }
     catch (\Throwable $e) {
       $this->logger->error($e->getMessage());
       return;
@@ -128,4 +133,31 @@ public function handleCron(): void {
     );
   }
 
+  /**
+   * Generates a log message from a stage validation exception.
+   *
+   * @param \Drupal\package_manager\Exception\StageValidationException $exception
+   *   The validation exception.
+   *
+   * @return string
+   *   The formatted log message, including all the validation results.
+   */
+  protected static function formatValidationException(StageValidationException $exception): string {
+    $log_message = '';
+    foreach ($exception->getResults() as $result) {
+      $summary = $result->getSummary();
+      if ($summary) {
+        $log_message .= "<h3>$summary</h3><ul>";
+        foreach ($result->getMessages() as $message) {
+          $log_message .= "<li>$message</li>";
+        }
+        $log_message .= "</ul>";
+      }
+      else {
+        $log_message .= ($log_message ? ' ' : '') . $result->getMessages()[0];
+      }
+    }
+    return "<h2>{$exception->getMessage()}</h2>$log_message";
+  }
+
 }

core/modules/auto_updates/src/Form/UpdateReady.php

@@ -4,7 +4,9 @@
 
 use Drupal\auto_updates\BatchProcessor;
 use Drupal\auto_updates\Updater;
+use Drupal\auto_updates\Validator\StagedDatabaseUpdateValidator;
 use Drupal\Core\Batch\BatchBuilder;
+use Drupal\Core\Extension\ModuleExtensionList;
 use Drupal\Core\Form\FormBase;
 use Drupal\Core\Form\FormStateInterface;
 use Drupal\Core\Messenger\MessengerInterface;
@@ -34,6 +36,20 @@ class UpdateReady extends FormBase {
    */
   protected $state;
 
+  /**
+   * The module list service.
+   *
+   * @var \Drupal\Core\Extension\ModuleExtensionList
+   */
+  protected $moduleList;
+
+  /**
+   * The staged database update validator service.
+   *
+   * @var \Drupal\auto_updates\Validator\StagedDatabaseUpdateValidator
+   */
+  protected $stagedDatabaseUpdateValidator;
+
   /**
    * Constructs a new UpdateReady object.
    *
@@ -43,11 +59,17 @@ class UpdateReady extends FormBase {
    *   The messenger service.
    * @param \Drupal\Core\State\StateInterface $state
    *   The state service.
+   * @param \Drupal\Core\Extension\ModuleExtensionList $module_list
+   *   The module list service.
+   * @param \Drupal\auto_updates\Validator\StagedDatabaseUpdateValidator $staged_database_update_validator
+   *   The staged database update validator service.
    */
-  public function __construct(Updater $updater, MessengerInterface $messenger, StateInterface $state) {
+  public function __construct(Updater $updater, MessengerInterface $messenger, StateInterface $state, ModuleExtensionList $module_list, StagedDatabaseUpdateValidator $staged_database_update_validator) {
     $this->updater = $updater;
     $this->setMessenger($messenger);
     $this->state = $state;
+    $this->moduleList = $module_list;
+    $this->stagedDatabaseUpdateValidator = $staged_database_update_validator;
   }
 
   /**
@@ -64,7 +86,9 @@ public static function create(ContainerInterface $container) {
     return new static(
       $container->get('auto_updates.updater'),
       $container->get('messenger'),
-      $container->get('state')
+      $container->get('state'),
+      $container->get('extension.list.module'),
+      $container->get('auto_updates.validator.staged_database_updates')
     );
   }
 
@@ -80,6 +104,23 @@ public function buildForm(array $form, FormStateInterface $form_state, string $s
       return $form;
     }
 
+    // Don't check for pending database updates if the form has been submitted,
+    // because we don't want to store the warning in the messenger during form
+    // submit.
+    if (!$form_state->getUserInput()) {
+      // If there are any installed modules with database updates in the staging
+      // area, warn the user that they might be sent to update.php once the
+      // staged changes have been applied.
+      $pending_updates = $this->getModulesWithStagedDatabaseUpdates();
+
+      if ($pending_updates) {
+        $this->messenger()->addWarning($this->t('Possible database updates were detected in the following modules; you may be redirected to the database update page in order to complete the update process.'));
+        foreach ($pending_updates as $info) {
+          $this->messenger()->addWarning($info['name']);
+        }
+      }
+    }
+
     $form['stage_id'] = [
       '#type' => 'value',
       '#value' => $stage_id,
@@ -106,6 +147,20 @@ public function buildForm(array $form, FormStateInterface $form_state, string $s
     return $form;
   }
 
+  /**
+   * Returns info for all installed modules that have staged database updates.
+   *
+   * @return array[]
+   *   The info arrays for the modules which have staged database updates, keyed
+   *   by module machine name.
+   */
+  protected function getModulesWithStagedDatabaseUpdates(): array {
+    $filter = function (string $name): bool {
+      return $this->stagedDatabaseUpdateValidator->hasStagedUpdates($this->updater, $this->moduleList->get($name));
+    };
+    return array_filter($this->moduleList->getAllInstalledInfo(), $filter, ARRAY_FILTER_USE_KEY);
+  }
+
   /**
    * {@inheritdoc}
    */

core/modules/auto_updates/src/Validator/StagedDatabaseUpdateValidator.php

@@ -3,6 +3,7 @@
 namespace Drupal\auto_updates\Validator;
 
 use Drupal\auto_updates\CronUpdater;
+use Drupal\auto_updates\Updater;
 use Drupal\Core\Extension\Extension;
 use Drupal\Core\Extension\ModuleExtensionList;
 use Drupal\Core\StringTranslation\StringTranslationTrait;
@@ -60,22 +61,13 @@ public function checkUpdateHooks(PreApplyEvent $event): void {
       return;
     }
 
-    $active_dir = $this->pathLocator->getProjectRoot();
-    $stage_dir = $stage->getStageDirectory();
-
-    $web_root = $this->pathLocator->getWebRoot();
-    if ($web_root) {
-      $active_dir .= DIRECTORY_SEPARATOR . $web_root;
-      $stage_dir .= DIRECTORY_SEPARATOR . $web_root;
-    }
-
     $invalid_modules = [];
     // Although \Drupal\auto_updates\Validator\StagedProjectsValidator
     // should prevent non-core modules from being added, updated, or removed in
     // the staging area, we check all installed modules so as not to rely on the
     // presence of StagedProjectsValidator.
     foreach ($this->moduleList->getAllInstalledInfo() as $name => $info) {
-      if ($this->hasStagedUpdates($active_dir, $stage_dir, $this->moduleList->get($name))) {
+      if ($this->hasStagedUpdates($stage, $this->moduleList->get($name))) {
         $invalid_modules[] = $info['name'];
       }
     }
@@ -88,10 +80,8 @@ public function checkUpdateHooks(PreApplyEvent $event): void {
   /**
    * Determines if a staged extension has changed update functions.
    *
-   * @param string $active_dir
-   *   The path of the running Drupal code base.
-   * @param string $stage_dir
-   *   The path of the staging area.
+   * @param \Drupal\auto_updates\Updater $updater
+   *   The updater which is controlling the update process.
    * @param \Drupal\Core\Extension\Extension $extension
    *   The extension to check.
    *
@@ -111,7 +101,16 @@ public function checkUpdateHooks(PreApplyEvent $event): void {
    *
    * @see https://www.drupal.org/project/auto_updates/issues/3253828
    */
-  protected function hasStagedUpdates(string $active_dir, string $stage_dir, Extension $extension): bool {
+  public function hasStagedUpdates(Updater $updater, Extension $extension): bool {
+    $active_dir = $this->pathLocator->getProjectRoot();
+    $stage_dir = $updater->getStageDirectory();
+
+    $web_root = $this->pathLocator->getWebRoot();
+    if ($web_root) {
+      $active_dir .= DIRECTORY_SEPARATOR . $web_root;
+      $stage_dir .= DIRECTORY_SEPARATOR . $web_root;
+    }
+
     $active_hashes = $this->getHashes($active_dir, $extension);
     $staged_hashes = $this->getHashes($stage_dir, $extension);
 

core/modules/auto_updates/src/Validator/UpdateVersionValidator.php

@@ -3,8 +3,10 @@
 namespace Drupal\auto_updates\Validator;
 
 use Composer\Semver\Semver;
+use Drupal\auto_updates\CronUpdater;
 use Drupal\auto_updates\Event\ReadinessCheckEvent;
 use Drupal\auto_updates\Updater;
+use Drupal\Core\Config\ConfigFactoryInterface;
 use Drupal\package_manager\Event\PreCreateEvent;
 use Drupal\package_manager\Event\PreOperationStageEvent;
 use Drupal\Core\Extension\ExtensionVersion;
@@ -19,14 +21,24 @@ class UpdateVersionValidator implements EventSubscriberInterface {
 
   use StringTranslationTrait;
 
+  /**
+   * The config factory service.
+   *
+   * @var \Drupal\Core\Config\ConfigFactoryInterface
+   */
+  protected $configFactory;
+
   /**
    * Constructs a UpdateVersionValidation object.
    *
    * @param \Drupal\Core\StringTranslation\TranslationInterface $translation
    *   The translation service.
+   * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
+   *   The config factory service.
    */
-  public function __construct(TranslationInterface $translation) {
+  public function __construct(TranslationInterface $translation, ConfigFactoryInterface $config_factory) {
     $this->setStringTranslation($translation);
+    $this->configFactory = $config_factory;
   }
 
   /**
@@ -45,7 +57,7 @@ protected function getCoreVersion(): string {
   }
 
   /**
-   * Validates that core is not being updated to another minor or major version.
+   * Validates that core is being updated within an allowed version range.
    *
    * @param \Drupal\package_manager\Event\PreOperationStageEvent $event
    *   The event object.
@@ -79,33 +91,36 @@ public function checkUpdateVersion(PreOperationStageEvent $event): void {
     $core_package_name = reset($core_package_names);
     $to_version_string = $package_versions[$core_package_name];
     $to_version = ExtensionVersion::createFromVersionString($to_version_string);
+    $variables = [
+      '@to_version' => $to_version_string,
+      '@from_version' => $from_version_string,
+    ];
     if (Semver::satisfies($to_version_string, "< $from_version_string")) {
-      $messages[] = $this->t('Update version @to_version is lower than @from_version, downgrading is not supported.', [
-        '@to_version' => $to_version_string,
-        '@from_version' => $from_version_string,
+      $event->addError([
+        $this->t('Update version @to_version is lower than @from_version, downgrading is not supported.', $variables),
       ]);
-      $event->addError($messages);
     }
     elseif ($from_version->getVersionExtra() === 'dev') {
-      $messages[] = $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.', [
-        '@to_version' => $to_version_string,
-        '@from_version' => $from_version_string,
+      $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),
       ]);
-      $event->addError($messages);
     }
     elseif ($from_version->getMajorVersion() !== $to_version->getMajorVersion()) {
-      $messages[] = $this->t('Drupal cannot be automatically updated from its current version, @from_version, to the recommended version, @to_version, because automatic updates from one major version to another are not supported.', [
-        '@to_version' => $to_version_string,
-        '@from_version' => $from_version_string,
+      $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 one major version to another are not supported.', $variables),
       ]);
-      $event->addError($messages);
     }
     elseif ($from_version->getMinorVersion() !== $to_version->getMinorVersion()) {
-      $messages[] = $this->t('Drupal cannot be automatically updated from its current version, @from_version, to the recommended version, @to_version, because automatic updates from one minor version to another are not supported.', [
-        '@from_version' => $this->getCoreVersion(),
-        '@to_version' => $package_versions[$core_package_name],
-      ]);
-      $event->addError($messages);
+      if (!$this->configFactory->get('auto_updates.settings')->get('allow_core_minor_updates')) {
+        $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 one minor version to another are not supported.', $variables),
+        ]);
+      }
+      elseif ($stage instanceof CronUpdater) {
+        $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 one minor version to another are not supported during cron.', $variables),
+        ]);
+      }
     }
   }
 

core/modules/auto_updates/tests/fixtures/fake-site/vendor/composer/installed.json

+{
+    "packages": [
+        {
+            "name": "drupal/test-distribution",
+            "version": "1.0.0",
+            "require": {
+                "drupal/core-recommended": "*"
+            }
+        },
+        {
+            "name": "drupal/core-recommended",
+            "version": "9.8.0",
+            "require": {
+                "drupal/core": "9.8.0"
+            }
+        },
+        {
+            "name": "drupal/core",
+            "version": "9.8.0"
+        },
+        {
+            "name": "drupal/core-dev",
+            "version": "9.8.0"
+        }
+    ],
+    "dev": true,
+    "dev-package-names": [
+        "drupal/core-dev"
+    ]
+}

core/modules/auto_updates/tests/fixtures/project_staged_validation/new_project_added/active/composer.lock → core/modules/auto_updates/tests/fixtures/project_staged_validation/new_project_added/active/vendor/composer/installed.json

@@ -15,9 +15,7 @@
       "description": "This project is removed but there should be no error because it is not a Drupal project.",
       "version": "1.3.1",
       "type": "library"
-    }
-  ],
-  "packages-dev": [
+    },
     {
       "name": "drupal/dev-test_module",
       "version": "1.3.0",
@@ -29,6 +27,10 @@
       "version": "1.3.1",
       "type": "library"
     }
-
+  ],
+  "dev": true,
+  "dev-package-names": [
+    "drupal/dev-test_module",
+    "other/dev-removed"
   ]
 }

core/modules/auto_updates/tests/fixtures/project_staged_validation/new_project_added/staged/composer.lock → core/modules/auto_updates/tests/fixtures/project_staged_validation/new_project_added/staged/vendor/composer/installed.json

@@ -20,9 +20,7 @@
       "description": "This is newly added project but there should be no error because it is not a drupal project",
       "version": "1.3.1",
       "type": "library"
-    }
-  ],
-  "packages-dev": [
+    },
     {
       "name": "drupal/dev-test_module",
       "version": "1.3.0",
@@ -39,5 +37,11 @@
       "version": "1.3.1",
       "type": "library"
     }
+  ],
+  "dev": true,
+  "dev-package-names": [
+    "drupal/dev-test_module",
+    "drupal/dev-test_module2",
+    "other/dev-new_project"
   ]
 }

core/modules/auto_updates/tests/fixtures/project_staged_validation/no_core_requirements/composer.lock

-{
-    "packages": [],
-    "packages-dev": []
-}

core/modules/auto_updates/tests/fixtures/project_staged_validation/no_core_requirements/vendor/composer/installed.json

+{
+    "packages": []
+}

core/modules/auto_updates/tests/fixtures/project_staged_validation/no_errors/active/composer.lock → core/modules/auto_updates/tests/fixtures/project_staged_validation/no_errors/active/vendor/composer/installed.json

@@ -21,9 +21,7 @@
       "description": "This project version is changed but there should be no error because it is not a Drupal project.",
       "version": "1.3.1",
       "type": "library"
-    }
-  ],
-  "packages-dev": [
+    },
     {
       "name": "drupal/dev-test_module",
       "version": "1.3.0",
@@ -41,5 +39,11 @@
       "version": "1.3.1",
       "type": "library"
     }
+  ],
+  "dev": true,
+  "dev-package-names": [
+    "drupal/dev-test_module",
+    "other/dev-removed",
+    "other/dev-changed"
   ]
 }

core/modules/auto_updates/tests/fixtures/project_staged_validation/no_errors/staged/composer.lock → core/modules/auto_updates/tests/fixtures/project_staged_validation/no_errors/staged/vendor/composer/installed.json

@@ -21,9 +21,7 @@
       "description": "This project version is changed but there should be no error because it is not a Drupal project.",
       "version": "1.3.2",
       "type": "library"
-    }
-  ],
-  "packages-dev": [
+    },
     {
       "name": "drupal/dev-test_module",
       "version": "1.3.0",
@@ -41,5 +39,11 @@
       "version": "1.3.2",
       "type": "library"
     }
+  ],
+  "dev": true,
+  "dev-package-names": [
+    "drupal/dev-test_module",
+    "other/dev-new_project",
+    "other/dev-changed"
   ]
 }

core/modules/auto_updates/tests/fixtures/project_staged_validation/project_removed/active/composer.lock → core/modules/auto_updates/tests/fixtures/project_staged_validation/project_removed/active/vendor/composer/installed.json

@@ -20,9 +20,7 @@
       "description": "This project is removed but there should be no error because it is not a Drupal project.",
       "version": "1.3.1",
       "type": "library"
-    }
-  ],
-  "packages-dev": [
+    },
     {
       "name": "drupal/dev-test_theme",
       "version": "1.3.0",
@@ -39,5 +37,11 @@
       "version": "1.3.1",
       "type": "library"
     }
+  ],
+  "dev": true,
+  "dev-package-names": [
+    "drupal/dev-test_theme",
+    "drupal/dev-test_module2",
+    "other/dev-removed"
   ]
 }

core/modules/auto_updates/tests/fixtures/project_staged_validation/project_removed/staged/composer.lock → core/modules/auto_updates/tests/fixtures/project_staged_validation/project_removed/staged/vendor/composer/installed.json

@@ -9,13 +9,15 @@
       "name": "drupal/test_module2",
       "version": "1.3.1",
       "type": "drupal-module"
-    }
-  ],
-  "packages-dev": [
+    },
     {
       "name": "drupal/dev-test_module2",
       "version": "1.3.1",
       "type": "drupal-module"
     }
+  ],
+  "dev": true,
+  "dev-package-names": [
+    "drupal/dev-test_module2"
   ]
 }