From 4ff3848262488cfd22d916af942bb5b34ba98d69 Mon Sep 17 00:00:00 2001
From: Alex Pott <alex.a.pott@googlemail.com>
Date: Tue, 10 Mar 2020 16:10:29 +0000
Subject: [PATCH] Issue #3066801 by catch, WidgetsBurritos, pobster, jungle,
tim.plunkett, xjm, dww, alexpott, webchick, benjifisher, longwave,
worldlinemine, lauriii, Berdir: Add hook_removed_post_updates()
---
.../Drupal/Core/Extension/ModuleInstaller.php | 7 +-
core/lib/Drupal/Core/Extension/module.api.php | 25 ++++
.../Update/RemovedPostUpdateNameException.php | 12 ++
.../lib/Drupal/Core/Update/UpdateRegistry.php | 25 +++-
core/modules/system/system.install | 30 ++++
.../update_test_postupdate.post_update.php | 12 ++
.../Module/InstallUninstallTest.php | 11 +-
.../UpdateSystem/UpdatePostUpdateTest.php | 13 ++
.../UpdateRemovedPostUpdateTest.php | 133 ++++++++++++++++++
.../Tests/Core/Update/UpdateRegistryTest.php | 66 +++++++++
10 files changed, 329 insertions(+), 5 deletions(-)
create mode 100644 core/lib/Drupal/Core/Update/RemovedPostUpdateNameException.php
create mode 100644 core/modules/system/tests/src/Functional/UpdateSystem/UpdateRemovedPostUpdateTest.php
diff --git a/core/lib/Drupal/Core/Extension/ModuleInstaller.php b/core/lib/Drupal/Core/Extension/ModuleInstaller.php
index 70157580d4cc..17bb70030623 100644
--- a/core/lib/Drupal/Core/Extension/ModuleInstaller.php
+++ b/core/lib/Drupal/Core/Extension/ModuleInstaller.php
@@ -295,10 +295,13 @@ public function install(array $module_list, $enable_dependencies = TRUE) {
}
drupal_set_installed_schema_version($module, $version);
- // Ensure that all post_update functions are registered already.
+ // Ensure that all post_update functions are registered already. This
+ // should include existing post-updates, as well as any specified as
+ // having been previously removed, to ensure that newly installed and
+ // updated sites have the same entries in the registry.
/** @var \Drupal\Core\Update\UpdateRegistry $post_update_registry */
$post_update_registry = \Drupal::service('update.post_update_registry');
- $post_update_registry->registerInvokedUpdates($post_update_registry->getModuleUpdateFunctions($module));
+ $post_update_registry->registerInvokedUpdates(array_merge($post_update_registry->getModuleUpdateFunctions($module), array_keys($post_update_registry->getRemovedPostUpdates($module))));
// Record the fact that it was installed.
$modules_installed[] = $module;
diff --git a/core/lib/Drupal/Core/Extension/module.api.php b/core/lib/Drupal/Core/Extension/module.api.php
index e55804d2c264..7c02338233e1 100644
--- a/core/lib/Drupal/Core/Extension/module.api.php
+++ b/core/lib/Drupal/Core/Extension/module.api.php
@@ -745,6 +745,7 @@ function hook_update_N(&$sandbox) {
* @ingroup update_api
*
* @see hook_update_N()
+ * @see hook_removed_post_updates()
*/
function hook_post_update_NAME(&$sandbox) {
// Example of updating some content.
@@ -763,6 +764,30 @@ function hook_post_update_NAME(&$sandbox) {
return $result;
}
+/**
+ * Return an array of removed hook_post_update_NAME() function names.
+ *
+ * This should be used to indicate post-update functions that have existed in
+ * some previous version of the module, but are no longer available.
+ *
+ * This implementation has to be placed in a MODULE.post_update.php file.
+ *
+ * @return string[]
+ * An array where the keys are removed post-update function names, and the
+ * values are the first stable version in which the update was removed.
+ *
+ * @ingroup update_api
+ *
+ * @see hook_post_update_NAME()
+ */
+function hook_removed_post_updates() {
+ return [
+ 'mymodule_post_update_foo' => '8.x-2.0',
+ 'mymodule_post_update_bar' => '8.x-3.0',
+ 'mymodule_post_update_baz' => '8.x-3.0',
+ ];
+}
+
/**
* Return an array of information about module update dependencies.
*
diff --git a/core/lib/Drupal/Core/Update/RemovedPostUpdateNameException.php b/core/lib/Drupal/Core/Update/RemovedPostUpdateNameException.php
new file mode 100644
index 000000000000..33fe314ac98f
--- /dev/null
+++ b/core/lib/Drupal/Core/Update/RemovedPostUpdateNameException.php
@@ -0,0 +1,12 @@
+<?php
+
+namespace Drupal\Core\Update;
+
+/**
+ * An exception thrown for removed post-update functions.
+ *
+ * Occurs when a module defines hook_post_update_NAME() implementations
+ * that are listed as removed in hook_removed_post_updates().
+ */
+class RemovedPostUpdateNameException extends \LogicException {
+}
diff --git a/core/lib/Drupal/Core/Update/UpdateRegistry.php b/core/lib/Drupal/Core/Update/UpdateRegistry.php
index 76ac81cb8ad7..8b3f03ea24ef 100644
--- a/core/lib/Drupal/Core/Update/UpdateRegistry.php
+++ b/core/lib/Drupal/Core/Update/UpdateRegistry.php
@@ -86,6 +86,21 @@ public function __construct($root, $site_path, array $enabled_modules, KeyValueS
$this->includeTests = $include_tests;
}
+ /**
+ * Gets removed hook_post_update_NAME() implementations for a module.
+ *
+ * @return string[]
+ * A list of post-update functions that have been removed.
+ */
+ public function getRemovedPostUpdates($module) {
+ $this->scanExtensionsAndLoadUpdateFiles();
+ $function = "{$module}_removed_post_updates";
+ if (function_exists($function)) {
+ return $function();
+ }
+ return [];
+ }
+
/**
* Gets all available update functions.
*
@@ -102,11 +117,17 @@ protected function getAvailableUpdateFunctions() {
// module updates.
if (preg_match($regexp, $function, $matches)) {
if (in_array($matches['module'], $this->enabledModules)) {
- $updates[] = $matches['module'] . '_' . $this->updateType . '_' . $matches['name'];
+ $function_name = $matches['module'] . '_' . $this->updateType . '_' . $matches['name'];
+ if ($this->updateType === 'post_update') {
+ $removed = array_keys($this->getRemovedPostUpdates($matches['module']));
+ if (array_search($function_name, $removed) !== FALSE) {
+ throw new RemovedPostUpdateNameException(sprintf('The following update is specified as removed in hook_removed_post_updates() but still exists in the code base: %s', $function_name));
+ }
+ }
+ $updates[] = $function_name;
}
}
}
-
// Ensure that the update order is deterministic.
sort($updates);
return $updates;
diff --git a/core/modules/system/system.install b/core/modules/system/system.install
index 140de3b1aecb..8db3d323679e 100644
--- a/core/modules/system/system.install
+++ b/core/modules/system/system.install
@@ -19,6 +19,7 @@
use Drupal\Core\Site\Settings;
use Drupal\Core\StreamWrapper\PrivateStream;
use Drupal\Core\StreamWrapper\PublicStream;
+use Drupal\Core\StringTranslation\PluralTranslatableMarkup;
use Drupal\Core\StringTranslation\TranslatableMarkup;
use Drupal\Core\Url;
use Symfony\Component\HttpFoundation\Request;
@@ -1135,6 +1136,35 @@ function system_requirements($phase) {
];
}
}
+ // Also check post-updates. Only do this if we're not already showing an
+ // error for hook_update_N().
+ if (empty($module_list)) {
+ $existing_updates = \Drupal::service('keyvalue')->get('post_update')->get('existing_updates', []);
+ $post_update_registry = \Drupal::service('update.post_update_registry');
+ $modules = \Drupal::moduleHandler()->getModuleList();
+ $module_extension_list = \Drupal::service('extension.list.module');
+ foreach ($modules as $module => $extension) {
+ $module_info = $module_extension_list->get($module);
+ $removed_post_updates = $post_update_registry->getRemovedPostUpdates($module);
+ if ($missing_updates = array_diff(array_keys($removed_post_updates), $existing_updates)) {
+ $versions = array_unique(array_intersect_key($removed_post_updates, array_flip($missing_updates)));
+ $description = new PluralTranslatableMarkup(count($versions),
+ 'The installed version of the %module module is too old to update. Update to a version prior to @versions first (missing updates: @missing_updates).',
+ 'The installed version of the %module module is too old to update. Update first to a version prior to all of the following: @versions (missing updates: @missing_updates).',
+ [
+ '%module' => $module_info->info['name'],
+ '@missing_updates' => implode(', ', $missing_updates),
+ '@versions' => implode(', ', $versions),
+ ]
+ );
+ $requirements[$module . '_post_update_removed'] = [
+ 'title' => t('Missing updates for: @module', ['@module' => $module_info->info['name']]),
+ 'description' => $description,
+ 'severity' => REQUIREMENT_ERROR,
+ ];
+ }
+ }
+ }
}
return $requirements;
diff --git a/core/modules/system/tests/modules/update_test_postupdate/update_test_postupdate.post_update.php b/core/modules/system/tests/modules/update_test_postupdate/update_test_postupdate.post_update.php
index 54e1ddf61891..db3cf193e497 100644
--- a/core/modules/system/tests/modules/update_test_postupdate/update_test_postupdate.post_update.php
+++ b/core/modules/system/tests/modules/update_test_postupdate/update_test_postupdate.post_update.php
@@ -67,3 +67,15 @@ function update_test_postupdate_post_update_test_batch(&$sandbox = NULL) {
$sandbox['#finished'] = $sandbox['current_step'] / $sandbox['steps'];
return 'Test post update batches';
}
+
+/**
+ * Implements hook_removed_post_updates().
+ */
+function update_test_postupdate_removed_post_updates() {
+ return [
+ 'update_test_postupdate_post_update_foo' => '8.x-1.0',
+ 'update_test_postupdate_post_update_bar' => '8.x-2.0',
+ 'update_test_postupdate_post_update_pub' => '3.0.0',
+ 'update_test_postupdate_post_update_baz' => '3.0.0',
+ ];
+}
diff --git a/core/modules/system/tests/src/Functional/Module/InstallUninstallTest.php b/core/modules/system/tests/src/Functional/Module/InstallUninstallTest.php
index 6e878b03982d..355612099e94 100644
--- a/core/modules/system/tests/src/Functional/Module/InstallUninstallTest.php
+++ b/core/modules/system/tests/src/Functional/Module/InstallUninstallTest.php
@@ -300,7 +300,16 @@ protected function assertInstallModuleUpdates($module) {
$existing_updates = \Drupal::keyValue('post_update')->get('existing_updates', []);
switch ($module) {
case 'update_test_postupdate':
- $this->assertEmpty(array_diff(['update_test_postupdate_post_update_first', 'update_test_postupdate_post_update_second', 'update_test_postupdate_post_update_test1', 'update_test_postupdate_post_update_test0'], $existing_updates));
+ $expected = [
+ 'update_test_postupdate_post_update_first',
+ 'update_test_postupdate_post_update_second',
+ 'update_test_postupdate_post_update_test1',
+ 'update_test_postupdate_post_update_test0',
+ 'update_test_postupdate_post_update_foo',
+ 'update_test_postupdate_post_update_bar',
+ 'update_test_postupdate_post_update_baz',
+ ];
+ $this->assertSame($expected, $existing_updates);
break;
}
}
diff --git a/core/modules/system/tests/src/Functional/UpdateSystem/UpdatePostUpdateTest.php b/core/modules/system/tests/src/Functional/UpdateSystem/UpdatePostUpdateTest.php
index 46f5ea37c87b..d7502ba05824 100644
--- a/core/modules/system/tests/src/Functional/UpdateSystem/UpdatePostUpdateTest.php
+++ b/core/modules/system/tests/src/Functional/UpdateSystem/UpdatePostUpdateTest.php
@@ -54,6 +54,19 @@ protected function setUp() {
->condition('collection', '')
->condition('name', 'core.extension')
->execute();
+
+ // Mimic the behaviour of ModuleInstaller::install() for removed post
+ // updates. Don't include the actual post updates because we want them to
+ // run.
+ $key_value = \Drupal::service('keyvalue');
+ $existing_updates = $key_value->get('post_update')->get('existing_updates', []);
+ $post_updates = [
+ 'update_test_postupdate_post_update_foo',
+ 'update_test_postupdate_post_update_bar',
+ 'update_test_postupdate_post_update_pub',
+ 'update_test_postupdate_post_update_baz',
+ ];
+ $key_value->get('post_update')->set('existing_updates', array_merge($existing_updates, $post_updates));
}
/**
diff --git a/core/modules/system/tests/src/Functional/UpdateSystem/UpdateRemovedPostUpdateTest.php b/core/modules/system/tests/src/Functional/UpdateSystem/UpdateRemovedPostUpdateTest.php
new file mode 100644
index 000000000000..096b6473c85b
--- /dev/null
+++ b/core/modules/system/tests/src/Functional/UpdateSystem/UpdateRemovedPostUpdateTest.php
@@ -0,0 +1,133 @@
+<?php
+
+namespace Drupal\Tests\system\Functional\UpdateSystem;
+
+use Drupal\Core\Database\Database;
+use Drupal\Core\Url;
+use Drupal\Tests\BrowserTestBase;
+use Drupal\Tests\UpdatePathTestTrait;
+
+/**
+ * Tests hook_removed_post_updates().
+ *
+ * @group Update
+ */
+class UpdateRemovedPostUpdateTest extends BrowserTestBase {
+ use UpdatePathTestTrait;
+
+ /**
+ * {@inheritdoc}
+ */
+ protected $defaultTheme = 'stark';
+
+ /**
+ * {@inheritdoc}
+ */
+ protected function setUp() {
+ parent::setUp();
+ $connection = Database::getConnection();
+
+ // Set the schema version.
+ $connection->merge('key_value')
+ ->condition('collection', 'system.schema')
+ ->condition('name', 'update_test_postupdate')
+ ->fields([
+ 'collection' => 'system.schema',
+ 'name' => 'update_test_postupdate',
+ 'value' => 'i:8000;',
+ ])
+ ->execute();
+
+ // Update core.extension.
+ $extensions = $connection->select('config')
+ ->fields('config', ['data'])
+ ->condition('collection', '')
+ ->condition('name', 'core.extension')
+ ->execute()
+ ->fetchField();
+ $extensions = unserialize($extensions);
+ $extensions['module']['update_test_postupdate'] = 8000;
+ $connection->update('config')
+ ->fields([
+ 'data' => serialize($extensions),
+ ])
+ ->condition('collection', '')
+ ->condition('name', 'core.extension')
+ ->execute();
+
+ $this->updateUrl = Url::fromRoute('system.db_update');
+ $this->updateUser = $this->drupalCreateUser(['administer software updates']);
+ }
+
+ /**
+ * Tests hook_post_update_NAME().
+ */
+ public function testRemovedPostUpdate() {
+ // Mimic the behaviour of ModuleInstaller::install().
+ $key_value = \Drupal::service('keyvalue');
+ $existing_updates = $key_value->get('post_update')->get('existing_updates', []);
+
+ // Excludes 'update_test_postupdate_post_update_baz',
+ // 'update_test_postupdate_post_update_bar', and
+ // 'update_test_postupdate_pub' to simulate a module updating from
+ // a version prior to the post-updates being added, to a version
+ // after they were removed.
+ $post_updates = [
+ 'update_test_postupdate_post_update_first',
+ 'update_test_postupdate_post_update_second',
+ 'update_test_postupdate_post_update_test1',
+ 'update_test_postupdate_post_update_test0',
+ 'update_test_postupdate_post_update_foo',
+ ];
+ $key_value->get('post_update')->set('existing_updates', array_merge($existing_updates, $post_updates));
+
+ // The message should inform us we've skipped two major versions.
+ $this->drupalLogin($this->updateUser);
+ $this->drupalGet($this->updateUrl);
+ $assert_session = $this->assertSession();
+ $assert_session->pageTextContains('Requirements problem');
+ $assert_session->pageTextContains('The installed version of the Update test after module is too old to update. Update first to a version prior to all of the following: 8.x-2.0, 3.0.0');
+ $assert_session->pageTextContains('update_test_postupdate_post_update_baz');
+ $assert_session->pageTextContains('update_test_postupdate_post_update_bar');
+ $assert_session->pageTextContains('update_test_postupdate_post_update_pub');
+
+ // Excludes 'update_test_postupdate_post_update_baz' and
+ // 'update_test_post_update_pub' to simulate two updates being
+ // removed from a single version.
+ $post_updates = [
+ 'update_test_postupdate_post_update_first',
+ 'update_test_postupdate_post_update_second',
+ 'update_test_postupdate_post_update_test1',
+ 'update_test_postupdate_post_update_test0',
+ 'update_test_postupdate_post_update_foo',
+ 'update_test_postupdate_post_update_bar',
+ ];
+ $key_value->get('post_update')->set('existing_updates', array_merge($existing_updates, $post_updates));
+ // Now the message should inform us we've skipped one version.
+ $this->drupalGet($this->updateUrl);
+ $assert_session = $this->assertSession();
+ $assert_session->pageTextContains('Requirements problem');
+ $assert_session->pageTextContains('The installed version of the Update test after module is too old to update. Update to a version prior to 3.0.0');
+ $assert_session->pageTextContains('update_test_postupdate_post_update_baz');
+ $assert_session->pageTextContains('update_test_postupdate_post_update_pub');
+
+ // Excludes 'update_test_postupdate_post_update_baz' to simulate
+ // updating when only a single update has been skipped.
+ $post_updates = [
+ 'update_test_postupdate_post_update_first',
+ 'update_test_postupdate_post_update_second',
+ 'update_test_postupdate_post_update_test1',
+ 'update_test_postupdate_post_update_test0',
+ 'update_test_postupdate_post_update_foo',
+ 'update_test_postupdate_post_update_bar',
+ 'update_test_postupdate_post_update_pub',
+ ];
+ $key_value->get('post_update')->set('existing_updates', array_merge($existing_updates, $post_updates));
+ $this->drupalGet($this->updateUrl);
+ $assert_session = $this->assertSession();
+ $assert_session->pageTextContains('Requirements problem');
+ $assert_session->pageTextContains('The installed version of the Update test after module is too old to update. Update to a version prior to 3.0.0');
+ $assert_session->pageTextContains('update_test_postupdate_post_update_baz');
+ }
+
+}
diff --git a/core/tests/Drupal/Tests/Core/Update/UpdateRegistryTest.php b/core/tests/Drupal/Tests/Core/Update/UpdateRegistryTest.php
index 64a6bad790a3..9527de870e3c 100644
--- a/core/tests/Drupal/Tests/Core/Update/UpdateRegistryTest.php
+++ b/core/tests/Drupal/Tests/Core/Update/UpdateRegistryTest.php
@@ -4,6 +4,7 @@
use Drupal\Core\KeyValueStore\KeyValueStoreInterface;
use Drupal\Core\Site\Settings;
+use Drupal\Core\Update\RemovedPostUpdateNameException;
use Drupal\Core\Update\UpdateRegistry;
use Drupal\Tests\UnitTestCase;
use org\bovigo\vfs\vfsStream;
@@ -44,6 +45,12 @@ protected function setupBasicModules() {
type: module
name: Module B
core_version_requirement: '*'
+EOS;
+
+ $info_c = <<<'EOS'
+type: module
+name: Module C
+core_version_requirement: '*'
EOS;
$module_a = <<<'EOS'
@@ -71,6 +78,43 @@ function module_a_post_update_a() {
function module_b_post_update_a() {
}
+/**
+ * Implements hook_removed_post_updates().
+ */
+function module_b_removed_post_updates() {
+ return [
+ 'module_b_post_update_b' => '8.9.0',
+ 'module_b_post_update_c' => '8.9.0',
+ ];
+}
+
+EOS;
+
+ $module_c = <<<'EOS'
+<?php
+
+/**
+ * Module C update A.
+ */
+function module_c_post_update_a() {
+}
+
+/**
+ * Module C update B.
+ */
+function module_c_post_update_b() {
+}
+
+/**
+ * Implements hook_removed_post_updates().
+ */
+function module_c_removed_post_updates() {
+ return [
+ 'module_c_post_update_b' => '8.9.0',
+ 'module_c_post_update_c' => '8.9.0',
+ ];
+}
+
EOS;
vfsStream::setup('drupal');
vfsStream::create([
@@ -85,6 +129,10 @@ function module_b_post_update_a() {
'module_b.post_update.php' => $module_b,
'module_b.info.yml' => $info_b,
],
+ 'module_c' => [
+ 'module_c.post_update.php' => $module_c,
+ 'module_c.info.yml' => $info_c,
+ ],
],
],
],
@@ -209,6 +257,24 @@ public function testGetPendingUpdateInformationWithExistingUpdates() {
$this->assertEquals($expected, $update_registry->getPendingUpdateInformation());
}
+ /**
+ * @covers ::getPendingUpdateInformation
+ */
+ public function testGetPendingUpdateInformationWithRemovedUpdates() {
+ $this->setupBasicModules();
+
+ $key_value = $this->prophesize(KeyValueStoreInterface::class);
+ $key_value->get('existing_updates', [])->willReturn(['module_a_post_update_a']);
+ $key_value = $key_value->reveal();
+
+ $update_registry = new UpdateRegistry('vfs://drupal', 'sites/default', [
+ 'module_c',
+ ], $key_value, FALSE);
+
+ $this->expectException(RemovedPostUpdateNameException::class);
+ $update_registry->getPendingUpdateInformation();
+ }
+
/**
* @covers ::getModuleUpdateFunctions
*/
--
GitLab