diff --git a/automatic_updates.routing.yml b/automatic_updates.routing.yml index a3a37dab4ac08a617683fa507ea2e765c5c83ac6..c07665524061beb98db4e21dea06cebb6b821b28 100644 --- a/automatic_updates.routing.yml +++ b/automatic_updates.routing.yml @@ -13,6 +13,12 @@ automatic_updates.confirmation_page: requirements: _permission: 'administer software updates' _access_update_manager: 'TRUE' +automatic_updates.finish: + path: '/automatic-update/finish' + defaults: + _controller: '\Drupal\automatic_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. automatic_updates.report_update: diff --git a/src/BatchProcessor.php b/src/BatchProcessor.php index 418df2d480c858652f4ec126cfd99e61b3ba1459..4304aeabc494b6910771bcd3ce009591bd56e73f 100644 --- a/src/BatchProcessor.php +++ b/src/BatchProcessor.php @@ -162,12 +162,11 @@ class BatchProcessor { * 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('automatic_updates.finish') + ->setAbsolute() + ->toString(); + return new RedirectResponse($url); } static::handleBatchError($results); return NULL; diff --git a/src/Controller/UpdateController.php b/src/Controller/UpdateController.php new file mode 100644 index 0000000000000000000000000000000000000000..8f024d2b6d105825c76c18a4dcb730a772051493 --- /dev/null +++ b/src/Controller/UpdateController.php @@ -0,0 +1,101 @@ +<?php + +namespace Drupal\automatic_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; + } + +} diff --git a/src/Form/UpdateReady.php b/src/Form/UpdateReady.php index 114786f5f0a976209c336fc6862f2cbe6a387bd1..48954ad4e8f35c2c89df4484acfe37ae19cab79d 100644 --- a/src/Form/UpdateReady.php +++ b/src/Form/UpdateReady.php @@ -4,7 +4,9 @@ namespace Drupal\automatic_updates\Form; use Drupal\automatic_updates\BatchProcessor; use Drupal\automatic_updates\Updater; +use Drupal\automatic_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\automatic_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\automatic_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 @@ class UpdateReady extends FormBase { return new static( $container->get('automatic_updates.updater'), $container->get('messenger'), - $container->get('state') + $container->get('state'), + $container->get('extension.list.module'), + $container->get('automatic_updates.validator.staged_database_updates') ); } @@ -80,6 +104,23 @@ class UpdateReady extends FormBase { 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 @@ class UpdateReady extends FormBase { 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} */ diff --git a/src/Validator/StagedDatabaseUpdateValidator.php b/src/Validator/StagedDatabaseUpdateValidator.php index 061a00b35a13ae864bca8d0ef2937c7f1fe295ff..45e55b07c2030cc02022318543f921ae47d2607e 100644 --- a/src/Validator/StagedDatabaseUpdateValidator.php +++ b/src/Validator/StagedDatabaseUpdateValidator.php @@ -3,6 +3,7 @@ namespace Drupal\automatic_updates\Validator; use Drupal\automatic_updates\CronUpdater; +use Drupal\automatic_updates\Updater; use Drupal\Core\Extension\Extension; use Drupal\Core\Extension\ModuleExtensionList; use Drupal\Core\StringTranslation\StringTranslationTrait; @@ -60,22 +61,13 @@ class StagedDatabaseUpdateValidator implements EventSubscriberInterface { 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\automatic_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 @@ class StagedDatabaseUpdateValidator implements EventSubscriberInterface { /** * 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\automatic_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 @@ class StagedDatabaseUpdateValidator implements EventSubscriberInterface { * * @see https://www.drupal.org/project/automatic_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); diff --git a/tests/modules/automatic_updates_test/automatic_updates_test.services.yml b/tests/modules/automatic_updates_test/automatic_updates_test.services.yml index 8ded4f9233c2f7c05e9add1d343a14ce1ef9a15e..be6497f649dc11dfc060d971dd1fc348848998c8 100644 --- a/tests/modules/automatic_updates_test/automatic_updates_test.services.yml +++ b/tests/modules/automatic_updates_test/automatic_updates_test.services.yml @@ -1,4 +1,8 @@ services: + automatic_updates_test.route_subscriber: + class: \Drupal\automatic_updates_test\Routing\RouteSubscriber + tags: + - { name: event_subscriber } automatic_updates_test.checker: class: Drupal\automatic_updates_test\ReadinessChecker\TestChecker1 tags: diff --git a/tests/modules/automatic_updates_test/src/Controller/TestUpdateController.php b/tests/modules/automatic_updates_test/src/Controller/TestUpdateController.php new file mode 100644 index 0000000000000000000000000000000000000000..d1cadc7e7ad725ecd6080bf3e07c286821f349c5 --- /dev/null +++ b/tests/modules/automatic_updates_test/src/Controller/TestUpdateController.php @@ -0,0 +1,30 @@ +<?php + +namespace Drupal\automatic_updates_test\Controller; + +use Drupal\automatic_updates\Controller\UpdateController; + +/** + * A test-only version of the update controller. + */ +class TestUpdateController extends UpdateController { + + /** + * {@inheritdoc} + */ + protected function pendingUpdatesExist(): bool { + $pending_updates = $this->state() + ->get('automatic_updates_test.staged_database_updates', []); + + // If the System module should expose a pending update, create one that will + // be detected by the update hook registry. We only do this for System so + // that there is NO way we could possibly evaluate any user input (i.e., + // if malicious code were somehow injected into state). + if (array_key_exists('system', $pending_updates)) { + // @codingStandardsIgnoreLine + eval('function system_update_4294967294() {}'); + } + return parent::pendingUpdatesExist(); + } + +} diff --git a/tests/modules/automatic_updates_test/src/Form/TestUpdateReady.php b/tests/modules/automatic_updates_test/src/Form/TestUpdateReady.php new file mode 100644 index 0000000000000000000000000000000000000000..fb749d88bbdffc8ea76356ea5a74d7ca910de6ad --- /dev/null +++ b/tests/modules/automatic_updates_test/src/Form/TestUpdateReady.php @@ -0,0 +1,19 @@ +<?php + +namespace Drupal\automatic_updates_test\Form; + +use Drupal\automatic_updates\Form\UpdateReady; + +/** + * A test-only version of the form displayed before applying an update. + */ +class TestUpdateReady extends UpdateReady { + + /** + * {@inheritdoc} + */ + protected function getModulesWithStagedDatabaseUpdates(): array { + return $this->state->get('automatic_updates_test.staged_database_updates', parent::getModulesWithStagedDatabaseUpdates()); + } + +} diff --git a/tests/modules/automatic_updates_test/src/Routing/RouteSubscriber.php b/tests/modules/automatic_updates_test/src/Routing/RouteSubscriber.php new file mode 100644 index 0000000000000000000000000000000000000000..bda1b49843bafc97a108f9b65fbeb43f8b39f615 --- /dev/null +++ b/tests/modules/automatic_updates_test/src/Routing/RouteSubscriber.php @@ -0,0 +1,25 @@ +<?php + +namespace Drupal\automatic_updates_test\Routing; + +use Drupal\automatic_updates_test\Controller\TestUpdateController; +use Drupal\automatic_updates_test\Form\TestUpdateReady; +use Drupal\Core\Routing\RouteSubscriberBase; +use Symfony\Component\Routing\RouteCollection; + +/** + * Alters route definitions for testing purposes. + */ +class RouteSubscriber extends RouteSubscriberBase { + + /** + * {@inheritdoc} + */ + protected function alterRoutes(RouteCollection $collection) { + $collection->get('automatic_updates.confirmation_page') + ->setDefault('_form', TestUpdateReady::class); + $collection->get('automatic_updates.finish') + ->setDefault('_controller', TestUpdateController::class . '::onFinish'); + } + +} diff --git a/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php b/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php index f6e07aaa11d3e7577c15d5b46b1e0a951e2dcf3f..4abd7e8c32841d1ddc6f3b78ae7f40ef3cf0a02c 100644 --- a/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php +++ b/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php @@ -24,21 +24,31 @@ abstract class AutomaticUpdatesFunctionalTestBase extends BrowserTestBase { protected function prepareSettings() { parent::prepareSettings(); - // Disable the filesystem permissions validator, since we cannot guarantee - // that the current code base will be writable in all testing situations. We - // test this validator in our build tests, since those do give us control - // over the filesystem permissions. - // @see \Drupal\Tests\automatic_updates\Build\CoreUpdateTest::assertReadOnlyFileSystemError() $settings['settings']['automatic_updates_disable_validators'] = (object) [ - 'value' => [ - 'automatic_updates.validator.file_system_permissions', - 'package_manager.validator.file_system', - ], + 'value' => $this->disableValidators(), 'required' => TRUE, ]; $this->writeSettings($settings); } + /** + * Returns the service IDs of any validators to disable. + * + * @return string[] + * The service IDs of the validators to disable. + */ + protected function disableValidators(): array { + // Disable the filesystem permissions validators, since we cannot guarantee + // that the current code base will be writable in all testing situations. We + // test these validators in our build tests, since those do give us control + // over the filesystem permissions. + // @see \Drupal\Tests\automatic_updates\Build\CoreUpdateTest::assertReadOnlyFileSystemError() + return [ + 'automatic_updates.validator.file_system_permissions', + 'package_manager.validator.file_system', + ]; + } + /** * Sets the current (running) version of core, as known to the Update module. * diff --git a/tests/src/Functional/UpdaterFormTest.php b/tests/src/Functional/UpdaterFormTest.php index 28362db1b56c8625eeee4a776c18705d3eaca854..82eef1933dd0702a2dbcddeb04c341387ffceb7b 100644 --- a/tests/src/Functional/UpdaterFormTest.php +++ b/tests/src/Functional/UpdaterFormTest.php @@ -45,6 +45,20 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase { $this->checkForUpdates(); } + /** + * {@inheritdoc} + */ + protected function disableValidators(): array { + $disabled_validators = parent::disableValidators(); + + // In this test class, all actual staging operations are bypassed by + // package_manager_bypass, which means this validator will complain because + // there is no actual Composer data for it to inspect. + $disabled_validators[] = 'automatic_updates.staged_projects_validator'; + + return $disabled_validators; + } + /** * Data provider for URLs to the update form. * @@ -270,4 +284,63 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase { $assert_session->buttonExists('Continue'); } + /** + * Tests the update form when staged modules have database updates. + */ + public function testStagedDatabaseUpdates(): void { + $this->setCoreVersion('9.8.0'); + $this->checkForUpdates(); + + // Simulate a staged database update in the System module. + $this->container->get('state') + ->set('automatic_updates_test.staged_database_updates', [ + 'system' => [ + 'name' => 'System', + ], + ]); + + $page = $this->getSession()->getPage(); + $this->drupalGet('/admin/modules/automatic-update'); + $page->pressButton('Update'); + $this->checkForMetaRefresh(); + $this->assertUpdateStagedTimes(1); + $this->assertUpdateReady(); + // We should see a warning about pending database updates, and once the + // staged changes have been applied, we should be redirected to update.php. + $assert_session = $this->assertSession(); + $possible_update_message = '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.'; + $assert_session->pageTextContains($possible_update_message); + $assert_session->pageTextContains('System'); + $page->pressButton('Continue'); + $this->checkForMetaRefresh(); + $assert_session->addressEquals('/update.php'); + $assert_session->pageTextNotContains($possible_update_message); + $assert_session->pageTextContainsOnce('Please apply database updates to complete the update process.'); + } + + /** + * Tests an update that has no errors or special conditions. + * + * @param string $update_form_url + * The URL of the update form to visit. + * + * @dataProvider providerUpdateFormReferringUrl + */ + public function testSuccessfulUpdate(string $update_form_url): void { + $this->setCoreVersion('9.8.0'); + $this->checkForUpdates(); + + $page = $this->getSession()->getPage(); + $this->drupalGet($update_form_url); + $page->pressButton('Update'); + $this->checkForMetaRefresh(); + $this->assertUpdateStagedTimes(1); + $this->assertUpdateReady(); + $assert_session = $this->assertSession(); + $page->pressButton('Continue'); + $this->checkForMetaRefresh(); + $assert_session->addressEquals('/admin/reports/updates'); + $assert_session->pageTextContainsOnce('Update complete!'); + } + }