From 69c34dc784e7c4dea4bc31051f187fb1f18af726 Mon Sep 17 00:00:00 2001 From: phenaproxima <phenaproxima@205645.no-reply.drupal.org> Date: Fri, 11 Feb 2022 19:29:48 +0000 Subject: [PATCH] Issue #3263865 by phenaproxima: Module uninstall form fails because of our shim --- .../AutomaticUpdates93ShimServiceProvider.php | 49 ------ .../src/ModuleInstaller.php | 33 ---- .../src/ProxyClass/ModuleInstaller.php | 104 ----------- .../src/UpdateHookRegistry.php | 164 ------------------ .../src/UpdateHookRegistryFactory.php | 28 --- .../src/Validator/PendingUpdatesValidator.php | 23 ++- src/Controller/UpdateController.php | 53 ++---- .../AutomaticUpdatesTestServiceProvider.php | 24 +++ .../src/Routing/RouteSubscriber.php | 3 - .../TestPendingUpdatesValidator.php} | 14 +- tests/src/Functional/UpdaterFormTest.php | 17 +- 11 files changed, 67 insertions(+), 445 deletions(-) delete mode 100644 automatic_updates_9_3_shim/src/AutomaticUpdates93ShimServiceProvider.php delete mode 100644 automatic_updates_9_3_shim/src/ModuleInstaller.php delete mode 100644 automatic_updates_9_3_shim/src/ProxyClass/ModuleInstaller.php delete mode 100644 automatic_updates_9_3_shim/src/UpdateHookRegistry.php delete mode 100644 automatic_updates_9_3_shim/src/UpdateHookRegistryFactory.php create mode 100644 tests/modules/automatic_updates_test/src/AutomaticUpdatesTestServiceProvider.php rename tests/modules/automatic_updates_test/src/{Controller/TestUpdateController.php => Validator/TestPendingUpdatesValidator.php} (60%) diff --git a/automatic_updates_9_3_shim/src/AutomaticUpdates93ShimServiceProvider.php b/automatic_updates_9_3_shim/src/AutomaticUpdates93ShimServiceProvider.php deleted file mode 100644 index 59c96efa52..0000000000 --- a/automatic_updates_9_3_shim/src/AutomaticUpdates93ShimServiceProvider.php +++ /dev/null @@ -1,49 +0,0 @@ -<?php - -namespace Drupal\automatic_updates_9_3_shim; - -use Drupal\Core\DependencyInjection\ContainerBuilder; -use Drupal\Core\DependencyInjection\ServiceProviderBase; -use Symfony\Component\DependencyInjection\Reference; - -/** - * Modifies container service definitions. - */ -class AutomaticUpdates93ShimServiceProvider extends ServiceProviderBase { - - /** - * {@inheritdoc} - */ - public function alter(ContainerBuilder $container) { - parent::alter($container); - - $service_id = 'update.update_hook_registry_factory'; - if ($container->hasDefinition($service_id)) { - $container->getDefinition($service_id) - ->setClass(UpdateHookRegistryFactory::class); - } - else { - $container->register($service_id, UpdateHookRegistryFactory::class) - ->addMethodCall('setContainer', [ - new Reference('service_container'), - ]); - } - - $service_id = 'update.update_hook_registry'; - if ($container->hasDefinition($service_id)) { - $container->getDefinition($service_id) - ->setClass(UpdateHookRegistry::class); - } - else { - $container->register($service_id, UpdateHookRegistry::class) - ->setFactory([ - new Reference($service_id . '_factory'), - 'create', - ]); - } - - $container->getDefinition('module_installer') - ->setClass(ModuleInstaller::class); - } - -} diff --git a/automatic_updates_9_3_shim/src/ModuleInstaller.php b/automatic_updates_9_3_shim/src/ModuleInstaller.php deleted file mode 100644 index 2731bad51c..0000000000 --- a/automatic_updates_9_3_shim/src/ModuleInstaller.php +++ /dev/null @@ -1,33 +0,0 @@ -<?php - -namespace Drupal\automatic_updates_9_3_shim; - -use Drupal\Core\Database\Connection; -use Drupal\Core\DrupalKernelInterface; -use Drupal\Core\Extension\ModuleHandlerInterface; -use Drupal\Core\Extension\ModuleInstaller as CoreModuleInstaller; - -/** - * A module installer which accepts our update registry shim in its constructor. - */ -class ModuleInstaller extends CoreModuleInstaller { - - /** - * {@inheritdoc} - */ - public function __construct($root, ModuleHandlerInterface $module_handler, DrupalKernelInterface $kernel, Connection $connection = NULL, UpdateHookRegistry $update_registry = NULL) { - $this->root = $root; - $this->moduleHandler = $module_handler; - $this->kernel = $kernel; - if (!$connection) { - @trigger_error('The database connection must be passed to ' . __METHOD__ . '(). Creating ' . __CLASS__ . ' without it is deprecated in drupal:9.2.0 and will be required in drupal:10.0.0. See https://www.drupal.org/node/2970993', E_USER_DEPRECATED); - $connection = \Drupal::service('database'); - } - $this->connection = $connection; - if (!$update_registry) { - $update_registry = \Drupal::service('update.update_hook_registry'); - } - $this->updateRegistry = $update_registry; - } - -} diff --git a/automatic_updates_9_3_shim/src/ProxyClass/ModuleInstaller.php b/automatic_updates_9_3_shim/src/ProxyClass/ModuleInstaller.php deleted file mode 100644 index cb10e26a7d..0000000000 --- a/automatic_updates_9_3_shim/src/ProxyClass/ModuleInstaller.php +++ /dev/null @@ -1,104 +0,0 @@ -<?php -// phpcs:ignoreFile - -/** - * This file was generated via php core/scripts/generate-proxy-class.php 'Drupal\automatic_updates_9_3_shim\ModuleInstaller' "modules/contrib/automatic_updates/automatic_updates_9_3_shim/src". - */ - -namespace Drupal\automatic_updates_9_3_shim\ProxyClass { - - /** - * Provides a proxy class for \Drupal\automatic_updates_9_3_shim\ModuleInstaller. - * - * @see \Drupal\Component\ProxyBuilder - */ - class ModuleInstaller implements \Drupal\Core\Extension\ModuleInstallerInterface - { - - use \Drupal\Core\DependencyInjection\DependencySerializationTrait; - - /** - * The id of the original proxied service. - * - * @var string - */ - protected $drupalProxyOriginalServiceId; - - /** - * The real proxied service, after it was lazy loaded. - * - * @var \Drupal\automatic_updates_9_3_shim\ModuleInstaller - */ - protected $service; - - /** - * The service container. - * - * @var \Symfony\Component\DependencyInjection\ContainerInterface - */ - protected $container; - - /** - * Constructs a ProxyClass Drupal proxy object. - * - * @param \Symfony\Component\DependencyInjection\ContainerInterface $container - * The container. - * @param string $drupal_proxy_original_service_id - * The service ID of the original service. - */ - public function __construct(\Symfony\Component\DependencyInjection\ContainerInterface $container, $drupal_proxy_original_service_id) - { - $this->container = $container; - $this->drupalProxyOriginalServiceId = $drupal_proxy_original_service_id; - } - - /** - * Lazy loads the real service from the container. - * - * @return object - * Returns the constructed real service. - */ - protected function lazyLoadItself() - { - if (!isset($this->service)) { - $this->service = $this->container->get($this->drupalProxyOriginalServiceId); - } - - return $this->service; - } - - /** - * {@inheritdoc} - */ - public function addUninstallValidator(\Drupal\Core\Extension\ModuleUninstallValidatorInterface $uninstall_validator) - { - return $this->lazyLoadItself()->addUninstallValidator($uninstall_validator); - } - - /** - * {@inheritdoc} - */ - public function install(array $module_list, $enable_dependencies = true) - { - return $this->lazyLoadItself()->install($module_list, $enable_dependencies); - } - - /** - * {@inheritdoc} - */ - public function uninstall(array $module_list, $uninstall_dependents = true) - { - return $this->lazyLoadItself()->uninstall($module_list, $uninstall_dependents); - } - - /** - * {@inheritdoc} - */ - public function validateUninstall(array $module_list) - { - return $this->lazyLoadItself()->validateUninstall($module_list); - } - - } - -} diff --git a/automatic_updates_9_3_shim/src/UpdateHookRegistry.php b/automatic_updates_9_3_shim/src/UpdateHookRegistry.php deleted file mode 100644 index 1d88009a27..0000000000 --- a/automatic_updates_9_3_shim/src/UpdateHookRegistry.php +++ /dev/null @@ -1,164 +0,0 @@ -<?php - -namespace Drupal\automatic_updates_9_3_shim; - -use Drupal\Core\KeyValueStore\KeyValueStoreInterface; - -/** - * Provides module updates versions handling. - */ -class UpdateHookRegistry { - - /** - * Indicates that a module has not been installed yet. - */ - public const SCHEMA_UNINSTALLED = -1; - - /** - * A list of enabled modules. - * - * @var string[] - */ - protected $enabledModules; - - /** - * The key value storage. - * - * @var \Drupal\Core\KeyValueStore\KeyValueStoreInterface - */ - protected $keyValue; - - /** - * A static cache of schema currentVersions per module. - * - * Stores schema versions of the modules based on their defined hook_update_N - * implementations. - * Example: - * ``` - * [ - * 'example_module' => [ - * 8000, - * 8001, - * 8002 - * ] - * ] - * ``` - * - * @var int[][] - * @see \Drupal\Core\Update\UpdateHookRegistry::getAvailableUpdates() - */ - protected $allAvailableSchemaVersions = []; - - /** - * Constructs a new UpdateRegistry. - * - * @param string[] $enabled_modules - * A list of enabled modules. - * @param \Drupal\Core\KeyValueStore\KeyValueStoreInterface $key_value - * The key value store. - */ - public function __construct(array $enabled_modules, KeyValueStoreInterface $key_value) { - $this->enabledModules = $enabled_modules; - $this->keyValue = $key_value; - } - - /** - * Returns an array of available schema versions for a module. - * - * @param string $module - * A module name. - * - * @return int[] - * An array of available updates sorted by version. Empty array returned if - * no updates available. - */ - public function getAvailableUpdates(string $module): array { - if (!isset($this->allAvailableSchemaVersions[$module])) { - $this->allAvailableSchemaVersions[$module] = []; - - foreach ($this->enabledModules as $enabled_module) { - $this->allAvailableSchemaVersions[$enabled_module] = []; - } - - // Prepare regular expression to match all possible defined - // hook_update_N(). - $regexp = '/^(?<module>.+)_update_(?<version>\d+)$/'; - $functions = get_defined_functions(); - // Narrow this down to functions ending with an integer, since all - // hook_update_N() functions end this way, and there are other - // possible functions which match '_update_'. We use preg_grep() here - // since looping through all PHP functions can take significant page - // execution time and this function is called on every administrative page - // via system_requirements(). - foreach (preg_grep('/_\d+$/', $functions['user']) as $function) { - // If this function is a module update function, add it to the list of - // module updates. - if (preg_match($regexp, $function, $matches)) { - $this->allAvailableSchemaVersions[$matches['module']][] = (int) $matches['version']; - } - } - // Ensure that updates are applied in numerical order. - array_walk( - $this->allAvailableSchemaVersions, - static function (&$module_updates) { - sort($module_updates, SORT_NUMERIC); - } - ); - } - - return $this->allAvailableSchemaVersions[$module]; - } - - /** - * Returns the currently installed schema version for a module. - * - * @param string $module - * A module name. - * - * @return int - * The currently installed schema version, or self::SCHEMA_UNINSTALLED if the - * module is not installed. - */ - public function getInstalledVersion(string $module): int { - return $this->keyValue->get($module, self::SCHEMA_UNINSTALLED); - } - - /** - * Updates the installed version information for a module. - * - * @param string $module - * A module name. - * @param int $version - * The new schema version. - * - * @return self - * Returns self to support chained method calls. - */ - public function setInstalledVersion(string $module, int $version): self { - $this->keyValue->set($module, $version); - return $this; - } - - /** - * Deletes the installed version information for the module. - * - * @param string $module - * The module name to delete. - */ - public function deleteInstalledVersion(string $module): void { - $this->keyValue->delete($module); - } - - /** - * Returns the currently installed schema version for all modules. - * - * @return int[] - * Array of modules as the keys and values as the currently installed - * schema version of corresponding module, or self::SCHEMA_UNINSTALLED if the - * module is not installed. - */ - public function getAllInstalledVersions(): array { - return $this->keyValue->getAll(); - } - -} diff --git a/automatic_updates_9_3_shim/src/UpdateHookRegistryFactory.php b/automatic_updates_9_3_shim/src/UpdateHookRegistryFactory.php deleted file mode 100644 index 01d332fe68..0000000000 --- a/automatic_updates_9_3_shim/src/UpdateHookRegistryFactory.php +++ /dev/null @@ -1,28 +0,0 @@ -<?php - -namespace Drupal\automatic_updates_9_3_shim; - -use Symfony\Component\DependencyInjection\ContainerAwareInterface; -use Symfony\Component\DependencyInjection\ContainerAwareTrait; - -/** - * Service factory for the versioning update registry. - */ -class UpdateHookRegistryFactory implements ContainerAwareInterface { - - use ContainerAwareTrait; - - /** - * Creates a new UpdateHookRegistry instance. - * - * @return \Drupal\Core\Update\UpdateHookRegistry - * The update registry instance. - */ - public function create() { - return new UpdateHookRegistry( - array_keys($this->container->get('module_handler')->getModuleList()), - $this->container->get('keyvalue')->get('system.schema') - ); - } - -} diff --git a/package_manager/src/Validator/PendingUpdatesValidator.php b/package_manager/src/Validator/PendingUpdatesValidator.php index 16062a98a6..5dd4f300e2 100644 --- a/package_manager/src/Validator/PendingUpdatesValidator.php +++ b/package_manager/src/Validator/PendingUpdatesValidator.php @@ -50,6 +50,22 @@ class PendingUpdatesValidator implements PreOperationStageValidatorInterface { * {@inheritdoc} */ public function validateStagePreOperation(PreOperationStageEvent $event): void { + if ($this->updatesExist()) { + $message = $this->t('Some modules have database schema updates to install. You should run the <a href=":update">database update script</a> immediately.', [ + ':update' => Url::fromRoute('system.db_update')->toString(), + ]); + $event->addError([$message]); + } + } + + /** + * Checks if there are any pending update or post-update hooks. + * + * @return bool + * TRUE if there are any pending update or post-update hooks, FALSE + * otherwise. + */ + public function updatesExist(): bool { require_once $this->appRoot . '/core/includes/install.inc'; require_once $this->appRoot . '/core/includes/update.inc'; @@ -57,12 +73,7 @@ class PendingUpdatesValidator implements PreOperationStageValidatorInterface { $hook_updates = update_get_update_list(); $post_updates = $this->updateRegistry->getPendingUpdateFunctions(); - if ($hook_updates || $post_updates) { - $message = $this->t('Some modules have database schema updates to install. You should run the <a href=":update">database update script</a> immediately.', [ - ':update' => Url::fromRoute('system.db_update')->toString(), - ]); - $event->addError([$message]); - } + return $hook_updates || $post_updates; } /** diff --git a/src/Controller/UpdateController.php b/src/Controller/UpdateController.php index f2feda107f..94754faa6f 100644 --- a/src/Controller/UpdateController.php +++ b/src/Controller/UpdateController.php @@ -2,10 +2,9 @@ namespace Drupal\automatic_updates\Controller; -use Drupal\automatic_updates_9_3_shim\UpdateHookRegistry; use Drupal\Core\Controller\ControllerBase; -use Drupal\Core\Update\UpdateRegistry; use Drupal\Core\Url; +use Drupal\package_manager\Validator\PendingUpdatesValidator; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\HttpFoundation\RedirectResponse; @@ -18,30 +17,20 @@ use Symfony\Component\HttpFoundation\RedirectResponse; class UpdateController extends ControllerBase { /** - * The update hook registry service. + * The pending updates validator. * - * @var \Drupal\automatic_updates_9_3_shim\UpdateHookRegistry + * @var \Drupal\package_manager\Validator\PendingUpdatesValidator */ - protected $updateHookRegistry; - - /** - * The post-update registry service. - * - * @var \Drupal\Core\Update\UpdateRegistry - */ - protected $postUpdateRegistry; + protected $pendingUpdatesValidator; /** * Constructs an UpdateController object. * - * @param \Drupal\automatic_updates_9_3_shim\UpdateHookRegistry $update_hook_registry - * The update hook registry service. - * @param \Drupal\Core\Update\UpdateRegistry $post_update_registry - * The post-update registry service. + * @param \Drupal\package_manager\Validator\PendingUpdatesValidator $pending_updates_validator + * The pending updates validator. */ - public function __construct(UpdateHookRegistry $update_hook_registry, UpdateRegistry $post_update_registry) { - $this->updateHookRegistry = $update_hook_registry; - $this->postUpdateRegistry = $post_update_registry; + public function __construct(PendingUpdatesValidator $pending_updates_validator) { + $this->pendingUpdatesValidator = $pending_updates_validator; } /** @@ -49,8 +38,7 @@ class UpdateController extends ControllerBase { */ public static function create(ContainerInterface $container) { return new static( - $container->get('update.update_hook_registry'), - $container->get('update.post_update_registry') + $container->get('package_manager.validator.pending_updates') ); } @@ -65,7 +53,7 @@ class UpdateController extends ControllerBase { * A redirect to the appropriate destination. */ public function onFinish(): RedirectResponse { - if ($this->pendingUpdatesExist()) { + if ($this->pendingUpdatesValidator->updatesExist()) { $message = $this->t('Please apply database updates to complete the update process.'); $url = Url::fromRoute('system.db_update'); } @@ -77,25 +65,4 @@ class UpdateController extends ControllerBase { 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/tests/modules/automatic_updates_test/src/AutomaticUpdatesTestServiceProvider.php b/tests/modules/automatic_updates_test/src/AutomaticUpdatesTestServiceProvider.php new file mode 100644 index 0000000000..50e1b43149 --- /dev/null +++ b/tests/modules/automatic_updates_test/src/AutomaticUpdatesTestServiceProvider.php @@ -0,0 +1,24 @@ +<?php + +namespace Drupal\automatic_updates_test; + +use Drupal\automatic_updates_test\Validator\TestPendingUpdatesValidator; +use Drupal\Core\DependencyInjection\ContainerBuilder; +use Drupal\Core\DependencyInjection\ServiceProviderBase; + +/** + * Modifies container services for testing purposes. + */ +class AutomaticUpdatesTestServiceProvider extends ServiceProviderBase { + + /** + * {@inheritdoc} + */ + public function alter(ContainerBuilder $container) { + parent::alter($container); + + $container->getDefinition('package_manager.validator.pending_updates') + ->setClass(TestPendingUpdatesValidator::class); + } + +} diff --git a/tests/modules/automatic_updates_test/src/Routing/RouteSubscriber.php b/tests/modules/automatic_updates_test/src/Routing/RouteSubscriber.php index bda1b49843..3edebbeb35 100644 --- a/tests/modules/automatic_updates_test/src/Routing/RouteSubscriber.php +++ b/tests/modules/automatic_updates_test/src/Routing/RouteSubscriber.php @@ -2,7 +2,6 @@ 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; @@ -18,8 +17,6 @@ class RouteSubscriber extends RouteSubscriberBase { 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/modules/automatic_updates_test/src/Controller/TestUpdateController.php b/tests/modules/automatic_updates_test/src/Validator/TestPendingUpdatesValidator.php similarity index 60% rename from tests/modules/automatic_updates_test/src/Controller/TestUpdateController.php rename to tests/modules/automatic_updates_test/src/Validator/TestPendingUpdatesValidator.php index d1cadc7e7a..f12963c7b4 100644 --- a/tests/modules/automatic_updates_test/src/Controller/TestUpdateController.php +++ b/tests/modules/automatic_updates_test/src/Validator/TestPendingUpdatesValidator.php @@ -1,19 +1,19 @@ <?php -namespace Drupal\automatic_updates_test\Controller; +namespace Drupal\automatic_updates_test\Validator; -use Drupal\automatic_updates\Controller\UpdateController; +use Drupal\package_manager\Validator\PendingUpdatesValidator; /** - * A test-only version of the update controller. + * Defines a test-only implementation of the pending updates validator. */ -class TestUpdateController extends UpdateController { +class TestPendingUpdatesValidator extends PendingUpdatesValidator { /** * {@inheritdoc} */ - protected function pendingUpdatesExist(): bool { - $pending_updates = $this->state() + public function updatesExist(): bool { + $pending_updates = \Drupal::state() ->get('automatic_updates_test.staged_database_updates', []); // If the System module should expose a pending update, create one that will @@ -24,7 +24,7 @@ class TestUpdateController extends UpdateController { // @codingStandardsIgnoreLine eval('function system_update_4294967294() {}'); } - return parent::pendingUpdatesExist(); + return parent::updatesExist(); } } diff --git a/tests/src/Functional/UpdaterFormTest.php b/tests/src/Functional/UpdaterFormTest.php index b489505f60..bf5c7f44b6 100644 --- a/tests/src/Functional/UpdaterFormTest.php +++ b/tests/src/Functional/UpdaterFormTest.php @@ -289,14 +289,6 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase { $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', - ], - ]); - // Flag a warning, which will not block the update but should be displayed // on the updater form. $this->createTestValidationResults(); @@ -313,6 +305,15 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase { $this->checkForMetaRefresh(); $this->assertUpdateStagedTimes(1); $this->assertUpdateReady(); + // Simulate a staged database update in the System module. We must do this + // after the update has started, because the pending updates validator + // will prevent an update from starting. + $this->container->get('state') + ->set('automatic_updates_test.staged_database_updates', [ + 'system' => [ + 'name' => 'System', + ], + ]); // The warning from the updater form should be not be repeated, but we // should see a warning about pending database updates, and once the staged // changes have been applied, we should be redirected to update.php, where -- GitLab