Verified Commit ba827d46 authored by Alex Pott's avatar Alex Pott
Browse files

Issue #3103962 by godotislate, smustgrave, alexpott, larowlan:...

Issue #3103962 by godotislate, smustgrave, alexpott, larowlan: ConfigInstaller::isSyncing() is not reset before module preinstall
parent 8e5acc67
Loading
Loading
Loading
Loading
Loading
+18 −12
Changes for core/lib/Drupal/Core/Extension/ModuleInstaller.php: 18 added lines, 12 removed lines.
Original line number Diff line number Diff line
@@ -179,9 +179,6 @@ public function install(array $module_list, $enable_dependencies = TRUE) {
    /** @var \Drupal\Core\Config\ConfigInstaller $config_installer */
    $config_installer = \Drupal::service('config.installer');
    $sync_status = $config_installer->isSyncing();
    if ($sync_status) {
      $source_storage = $config_installer->getSourceStorage();
    }
    $modules_installed = [];
    foreach ($module_list as $module) {
      $enabled = $extension_config->get("module.$module") !== NULL;
@@ -268,7 +265,7 @@ public function install(array $module_list, $enable_dependencies = TRUE) {
        }

        // Allow modules to react prior to the installation of a module.
        $this->moduleHandler->invokeAll('module_preinstall', [$module]);
        $this->moduleHandler->invokeAll('module_preinstall', [$module, $sync_status]);

        // Now install the module's schema if necessary.
        $this->installSchema($module);
@@ -326,12 +323,7 @@ public function install(array $module_list, $enable_dependencies = TRUE) {

        // Install default configuration of the module.
        $config_installer = \Drupal::service('config.installer');
        if ($sync_status) {
          $config_installer
            ->setSyncing(TRUE)
            ->setSourceStorage($source_storage);
        }
        \Drupal::service('config.installer')->installDefaultConfig('module', $module);
        $config_installer->installDefaultConfig('module', $module);

        // If the module has no current updates, but has some that were
        // previously removed, set the version to the value of
@@ -467,7 +459,7 @@ public function uninstall(array $module_list, $uninstall_dependents = TRUE) {
      }

      // Allow modules to react prior to the uninstallation of a module.
      $this->moduleHandler->invokeAll('module_preuninstall', [$module]);
      $this->moduleHandler->invokeAll('module_preuninstall', [$module, $sync_status]);

      // Uninstall the module.
      $this->moduleHandler->loadInclude($module, 'install');
@@ -604,10 +596,17 @@ function ($definition) {
  /**
   * Updates the kernel module list.
   *
   * @param string $module_filenames
   * @param string[] $module_filenames
   *   The list of installed modules.
   */
  protected function updateKernel($module_filenames) {
    // Save current state of config installer, so it can be restored after the
    // container is rebuilt.
    /** @var \Drupal\Core\Config\ConfigInstallerInterface $config_installer */
    $config_installer = $this->kernel->getContainer()->get('config.installer');
    $sync_status = $config_installer->isSyncing();
    $source_storage = $config_installer->getSourceStorage();

    // This reboots the kernel to register the module's bundle and its services
    // in the service container. The $module_filenames argument is taken over as
    // %container.modules% parameter, which is passed to a fresh ModuleHandler
@@ -619,6 +618,13 @@ protected function updateKernel($module_filenames) {
    $this->moduleHandler = $container->get('module_handler');
    $this->connection = $container->get('database');
    $this->updateRegistry = $container->get('update.update_hook_registry');

    // Restore state of config installer.
    if ($sync_status) {
      $container->get('config.installer')
        ->setSyncing(TRUE)
        ->setSourceStorage($source_storage);
    }
  }

  /**
+15 −2
Changes for core/lib/Drupal/Core/Extension/module.api.php: 15 added lines, 2 removed lines.
Original line number Diff line number Diff line
@@ -161,8 +161,15 @@ function hook_system_info_alter(array &$info, \Drupal\Core\Extension\Extension $
 *
 * @param string $module
 *   The name of the module about to be installed.
 * @param bool $is_syncing
 *   TRUE if the module is being installed as part of a configuration import. In
 *   these cases, your hook implementation needs to carefully consider what
 *   changes, if any, it should make. For example, it should not make any
 *   changes to configuration objects or configuration entities. Those changes
 *   should be made earlier and exported so during import there's no need to
 *   do them again.
 */
function hook_module_preinstall($module) {
function hook_module_preinstall($module, bool $is_syncing) {
  mymodule_cache_clear();
}

@@ -252,8 +259,14 @@ function hook_install($is_syncing) {
 *
 * @param string $module
 *   The name of the module about to be uninstalled.
 * @param bool $is_syncing
 *   TRUE if the module is being uninstalled as part of a configuration import.
 *   In these cases, your hook implementation needs to carefully consider what
 *   changes to configuration objects or configuration entities. Those changes
 *   should be made earlier and exported so during import there's no need to
 *   do them again.
 */
function hook_module_preuninstall($module) {
function hook_module_preuninstall($module, bool $is_syncing) {
  mymodule_cache_clear();
}

+27 −3
Changes for core/modules/system/tests/modules/system_test/system_test.module: 27 added lines, 3 removed lines.
Original line number Diff line number Diff line
@@ -35,12 +35,20 @@ function system_test_modules_installed($modules) {
/**
 * Implements hook_modules_uninstalled().
 */
function system_test_modules_uninstalled($modules) {
function system_test_modules_uninstalled($modules, $is_syncing) {
  if (\Drupal::state()->get('system_test.verbose_module_hooks')) {
    foreach ($modules as $module) {
      \Drupal::messenger()->addStatus(t('hook_modules_uninstalled fired for @module', ['@module' => $module]));
    }
  }

  // Save the config.installer isSyncing() value to state to check that it is
  // correctly set when installing module during config import.
  \Drupal::state()->set('system_test_modules_uninstalled_config_installer_syncing', \Drupal::service('config.installer')->isSyncing());

  // Save the $is_syncing parameter value to state to check that it is correctly
  // set when installing module during config import.
  \Drupal::state()->set('system_test_modules_uninstalled_syncing_param', $is_syncing);
}

/**
@@ -132,14 +140,30 @@ function system_test_filetransfer_info() {
/**
 * Implements hook_module_preinstall().
 */
function system_test_module_preinstall($module) {
function system_test_module_preinstall($module, bool $is_syncing) {
  \Drupal::messenger()->addStatus('system_test_preinstall_module called');
  \Drupal::state()->set('system_test_preinstall_module', $module);

  // Save the config.installer isSyncing() value to state to check that it is
  // correctly set when installing module during config import.
  \Drupal::state()->set('system_test_preinstall_module_config_installer_syncing', \Drupal::service('config.installer')->isSyncing());

  // Save the $is_syncing parameter value to state to check that it is correctly
  // set when installing module during config import.
  \Drupal::state()->set('system_test_preinstall_module_syncing_param', $is_syncing);
}

/**
 * Implements hook_module_preuninstall().
 */
function system_test_module_preuninstall($module) {
function system_test_module_preuninstall($module, bool $is_syncing) {
  \Drupal::state()->set('system_test_preuninstall_module', $module);

  // Save the config.installer isSyncing() value to state to check that it is
  // correctly set when uninstalling module during config import.
  \Drupal::state()->set('system_test_preuninstall_module_config_installer_syncing', \Drupal::service('config.installer')->isSyncing());

  // Save the $is_syncing parameter value to state to check that it is correctly
  // set when installing module during config import.
  \Drupal::state()->set('system_test_preuninstall_module_syncing_param', $is_syncing);
}
+44 −0
Changes for core/tests/Drupal/KernelTests/Core/Config/ConfigImporterTest.php: 44 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -829,6 +829,50 @@ public function testIsSyncingInHooks() {
    $this->assertTrue($state['entity_state::predelete'], 'ConfigEntity::isSyncing() returns TRUE');
    $this->assertTrue($state['global_state::delete'], '\Drupal::isConfigSyncing() returns TRUE');
    $this->assertTrue($state['entity_state::delete'], 'ConfigEntity::isSyncing() returns TRUE');

    // Test that isSyncing is TRUE in hook_module_preinstall() when installing
    // module via config import.
    $extensions = $sync->read('core.extension');
    // First, install system_test so that its hook_module_preinstall() will run
    // when module_test is installed.
    $this->container->get('module_installer')->install(['system_test']);
    // Add module_test and system_test to the enabled modules to be imported,
    // so that module_test gets installed on import and system_test does not get
    // uninstalled.
    $extensions['module']['module_test'] = 0;
    $extensions['module']['system_test'] = 0;
    $extensions['module'] = module_config_sort($extensions['module']);
    $sync->write('core.extension', $extensions);
    $this->configImporter->reset()->import();

    // Syncing values stored in state by hook_module_preinstall should be TRUE
    // when module is installed via config import.
    $this->assertTrue(\Drupal::state()->get('system_test_preinstall_module_config_installer_syncing'), '\Drupal::isConfigSyncing() in system_test_module_preinstall() returns TRUE');
    $this->assertTrue(\Drupal::state()->get('system_test_preinstall_module_syncing_param'), 'system_test_module_preinstall() $is_syncing value is TRUE');

    // Syncing value stored in state by uninstall hooks should be FALSE
    // when uninstalling outside of config import.
    $this->container->get('module_installer')->uninstall(['module_test']);
    $this->assertFalse(\Drupal::state()->get('system_test_preuninstall_module_config_installer_syncing'), '\Drupal::isConfigSyncing() in system_test_module_preuninstall() returns FALSE');
    $this->assertFalse(\Drupal::state()->get('system_test_preuninstall_module_syncing_param'), 'system_test_module_preuninstall() $is_syncing value is FALSE');
    $this->assertFalse(\Drupal::state()->get('system_test_modules_uninstalled_config_installer_syncing'), '\Drupal::isConfigSyncing() in system_test_modules_uninstalled returns FALSE');
    $this->assertFalse(\Drupal::state()->get('system_test_modules_uninstalled_syncing_param'), 'system_test_modules_uninstalled $is_syncing value is FALSE');

    // Syncing value stored in state by hook_module_preinstall should be FALSE
    // when installing outside of config import.
    $this->container->get('module_installer')->install(['module_test']);
    $this->assertFalse(\Drupal::state()->get('system_test_preinstall_module_config_installer_syncing'), '\Drupal::isConfigSyncing() in system_test_module_preinstall() returns TRUE');
    $this->assertFalse(\Drupal::state()->get('system_test_preinstall_module_syncing_param'), 'system_test_module_preinstall() $is_syncing value is TRUE');

    // Uninstall module_test via config import. Syncing value stored in state
    // by uninstall hooks should be TRUE.
    unset($extensions['module']['module_test']);
    $sync->write('core.extension', $extensions);
    $this->configImporter->reset()->import();
    $this->assertTrue(\Drupal::state()->get('system_test_preuninstall_module_config_installer_syncing'), '\Drupal::isConfigSyncing() in system_test_module_preuninstall() returns TRUE');
    $this->assertTrue(\Drupal::state()->get('system_test_preuninstall_module_syncing_param'), 'system_test_module_preuninstall() $is_syncing value is TRUE');
    $this->assertTrue(\Drupal::state()->get('system_test_modules_uninstalled_config_installer_syncing'), '\Drupal::isConfigSyncing() in system_test_modules_uninstalled returns TRUE');
    $this->assertTrue(\Drupal::state()->get('system_test_modules_uninstalled_syncing_param'), 'system_test_modules_uninstalled $is_syncing value is TRUE');
  }

  /**