Issue #2925203 by alexpott: LocaleConfigSubscriber can result in data loss during install

(cherry picked from commit 86e9f19a30)
(cherry picked from commit 011c3941af)

core/includes/install.core.inc

@@ -31,6 +31,7 @@ use Drupal\Core\StackMiddleware\ReverseProxyMiddleware;
 use Drupal\Core\Extension\ExtensionDiscovery;
 use Drupal\Core\DependencyInjection\ContainerBuilder;
 use Drupal\Core\Url;
+use Drupal\language\ConfigurableLanguageManagerInterface;
 use Drupal\language\Entity\ConfigurableLanguage;
 use Drupal\Core\Routing\RouteObjectInterface;
 use Symfony\Component\DependencyInjection\Reference;
@@ -1830,6 +1831,12 @@ function install_finish_translations(&$install_state) {
     }
   }
 
+  // If installing from configuration, detect custom translations in the
+  // configuration files.
+  if (!empty($install_state['config_install_path']) && \Drupal::service('module_handler')->moduleExists('locale')) {
+    $batches[] = _install_config_locale_overrides();
+  }
+
   // Creates configuration translations.
   $batches[] = locale_config_batch_update_components([], array_keys($languages));
   return $batches;
@@ -2436,3 +2443,89 @@ function install_config_revert_install_changes() {
     }
   }
 }
+
+/**
+ * Creates a batch to process config translations after installing from config.
+ *
+ * This ensures that the logic from LocaleConfigSubscriber::onConfigSave() is
+ * run on sites after installing from configuration so updating translations
+ * from PO files does not result in overwriting customizations.
+ *
+ * @return array
+ *   The batch definition.
+ *
+ * @see \Drupal\locale\LocaleConfigSubscriber::onConfigSave()
+ */
+function _install_config_locale_overrides() {
+  // @todo https://www.drupal.org/project/drupal/issues/3252244 Somehow the
+  //   config cache gets filled up with junk after installing from
+  //   configuration.
+  \Drupal::service('cache.config')->deleteAll();
+
+  // Get the services we need.
+  $language_manager = \Drupal::languageManager();
+  /** @var \Drupal\locale\LocaleConfigManager $locale_config_manager */
+  $locale_config_manager = \Drupal::service('locale.config_manager');
+
+  $langcodes = array_keys($language_manager->getLanguages());
+  if (count($langcodes) > 1 && !$language_manager instanceof ConfigurableLanguageManagerInterface) {
+    throw new \LogicException('There are multiple languages and the language manager is not an instance of ConfigurableLanguageManagerInterface');
+  }
+
+  $batch_builder = (new BatchBuilder())
+    ->setFile('core/includes/install.core.inc')
+    ->setTitle(t('Updating configuration translations'))
+    ->setInitMessage(t('Starting configuration update'))
+    ->setErrorMessage(t('Error updating configuration translations'));
+  $i = 0;
+  $batch_names = [];
+  foreach ($locale_config_manager->getComponentNames() as $name) {
+    $batch_names[] = $name;
+    $i++;
+    // During installation the caching of configuration objects is disabled so
+    // it is very expensive to initialize the \Drupal::config() object on each
+    // request. We batch a small number of configuration object upgrades
+    // together to improve the overall performance of the process.
+    if ($i % 20 == 0) {
+      $batch_builder->addOperation('_install_config_locale_overrides_process_batch', [$batch_names, $langcodes]);
+      $batch_names = [];
+    }
+  }
+  if (!empty($batch_names)) {
+    $batch_builder->addOperation('_install_config_locale_overrides_process_batch', [$batch_names, $langcodes]);
+  }
+  return $batch_builder->toArray();
+}
+
+/**
+ * Batch operation callback for install_config_locale_overrides().
+ *
+ * @param array $names
+ *   The configuration to process.
+ * @param array $langcodes
+ *   The langcodes available on the site.
+ * @param $context
+ *   The batch context.
+ */
+function _install_config_locale_overrides_process_batch(array $names, array $langcodes, &$context) {
+  // Get the services we need.
+  $language_manager = \Drupal::languageManager();
+  /** @var \Drupal\locale\LocaleConfigManager $locale_config_manager */
+  $locale_config_manager = \Drupal::service('locale.config_manager');
+  /** @var \Drupal\locale\LocaleConfigSubscriber $locale_config_subscriber */
+  $locale_config_subscriber = \Drupal::service('locale.config_subscriber');
+
+  foreach ($names as $name) {
+    $active_langcode = $locale_config_manager->getActiveConfigLangcode($name);
+    foreach ($langcodes as $langcode) {
+      if ($langcode === $active_langcode) {
+        $config = \Drupal::config($name);
+      }
+      else {
+        $config = $language_manager->getLanguageConfigOverride($langcode, $name);
+      }
+      $locale_config_subscriber->updateLocaleStorage($config, $langcode);
+    }
+  }
+  $context['finished'] = 1;
+}

core/modules/locale/src/LocaleConfigSubscriber.php

@@ -47,6 +47,13 @@ class LocaleConfigSubscriber implements EventSubscriberInterface {
    */
   protected $localeConfigManager;
 
+  /**
+   * The language manager.
+   *
+   * @var \Drupal\Core\Language\LanguageManagerInterface
+   */
+  protected $languageManager;
+
   /**
    * Constructs a LocaleConfigSubscriber.
    *
@@ -115,7 +122,7 @@ class LocaleConfigSubscriber implements EventSubscriberInterface {
    *   override. This allows us to update locale keys for data not in the
    *   override but still in the active configuration.
    */
-  protected function updateLocaleStorage(StorableConfigBase $config, $langcode, array $reference_config = []) {
+  public function updateLocaleStorage(StorableConfigBase $config, $langcode, array $reference_config = []) {
     $name = $config->getName();
     if ($this->localeConfigManager->isSupported($name) && locale_is_translatable($langcode)) {
       $translatables = $this->localeConfigManager->getTranslatableDefaultConfig($name);
@@ -209,6 +216,11 @@ class LocaleConfigSubscriber implements EventSubscriberInterface {
   protected function saveCustomizedTranslation($name, $source, $context, $new_translation, $langcode) {
     $locale_translation = $this->localeConfigManager->getStringTranslation($name, $langcode, $source, $context);
     if (!empty($locale_translation)) {
+      // If this code is triggered during installation never set the translation
+      // to the source string.
+      if (InstallerKernel::installationAttempted() && $source === $new_translation) {
+        return;
+      }
       // Save this translation as custom if it was a new translation and not the
       // same as the source. (The interface prefills translation values with the
       // source). Or if there was an existing (non-empty) translation and the

core/modules/locale/tests/src/Functional/LocaleConfigTranslationImportTest.php

@@ -84,6 +84,36 @@ class LocaleConfigTranslationImportTest extends BrowserTestBase {
     $override = \Drupal::languageManager()->getLanguageConfigOverride('af', 'system.maintenance');
     // cSpell:disable-next-line
     $this->assertEquals('Ons is tans besig met onderhoud op @site. Wees asseblief geduldig, ons sal binnekort weer terug wees.', $override->get('message'));
+
+    // Ensure that \Drupal\locale\LocaleConfigSubscriber::onConfigSave() works
+    // as expected during a configuration install that installs locale.
+    /** @var \Drupal\Core\Config\FileStorage $sync */
+    $sync = $this->container->get('config.storage.sync');
+    $this->copyConfig($this->container->get('config.storage'), $sync);
+
+    // Add our own translation to the config that will be imported.
+    $af_sync = $sync->createCollection('language.af');
+    $data = $af_sync->read('system.maintenance');
+    $data['message'] = 'Test af message';
+    $af_sync->write('system.maintenance', $data);
+
+    // Uninstall locale module.
+    $this->container->get('module_installer')->uninstall(['locale_test_translate']);
+    $this->container->get('module_installer')->uninstall(['locale']);
+    $this->resetAll();
+
+    $this->configImporter()->import();
+
+    $this->drupalGet('admin/reports/translations/check');
+    $status = locale_translation_get_status();
+    $status['drupal']['af']->type = 'current';
+    \Drupal::state()->set('locale.translation_status', $status);
+    $this->drupalGet('admin/reports/translations');
+    $this->submitForm([], 'Update translations');
+
+    // Check if configuration translations have been imported.
+    $override = \Drupal::languageManager()->getLanguageConfigOverride('af', 'system.maintenance');
+    $this->assertEquals('Test af message', $override->get('message'));
   }
 
   /**

core/tests/Drupal/FunctionalTests/Installer/InstallerExistingConfigSyncDirectoryMultilingualTest.php

@@ -2,6 +2,10 @@
 
 namespace Drupal\FunctionalTests\Installer;
 
+use Drupal\Component\Serialization\Yaml;
+
+// cSpell:ignore Anónimo Aplicar
+
 /**
  * Verifies that installing from existing configuration works.
  *
@@ -41,14 +45,166 @@ class InstallerExistingConfigSyncDirectoryMultilingualTest extends InstallerExis
     return __DIR__ . '/../../../fixtures/config_install/multilingual.tar.gz';
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  protected function prepareEnvironment() {
+    parent::prepareEnvironment();
+    // Place custom local translations in the translations directory and fix up
+    // configuration.
+    mkdir($this->publicFilesDirectory . '/translations', 0777, TRUE);
+    file_put_contents($this->publicFilesDirectory . '/translations/drupal-8.0.0.es.po', $this->getPo('es'));
+    $locale_settings = Yaml::decode(file_get_contents($this->siteDirectory . '/config/sync/locale.settings.yml'));
+    $locale_settings['translation']['use_source'] = 'local';
+    $locale_settings['translation']['path'] = $this->publicFilesDirectory . '/translations';
+    file_put_contents($this->siteDirectory . '/config/sync/locale.settings.yml', Yaml::encode($locale_settings));
+  }
+
   /**
    * Confirms that the installation installed the configuration correctly.
    */
   public function testConfigSync() {
-    parent::testConfigSync();
+    $comparer = $this->configImporter()->getStorageComparer();
+    $expected_changelist_default_collection = [
+      'create' => [],
+      // The system.mail is changed configuration because the test system
+      // changes it to ensure that mails are not sent.
+      'update' => ['system.mail'],
+      'delete' => [],
+      'rename' => [],
+    ];
+    $this->assertEquals($expected_changelist_default_collection, $comparer->getChangelist());
+    $expected_changelist_spanish_collection = [
+      'create' => [],
+      // The view was untranslated but the translation exists so the installer
+      // performs the translation.
+      'update' => ['views.view.who_s_new'],
+      'delete' => [],
+      'rename' => [],
+    ];
+    $this->assertEquals($expected_changelist_spanish_collection, $comparer->getChangelist(NULL, 'language.es'));
+
     // Ensure that menu blocks have been created correctly.
     $this->assertSession()->responseNotContains('This block is broken or missing.');
     $this->assertSession()->linkExists('Add content');
+
+    // Ensure that the Spanish translation of anonymous is the one from
+    // configuration and not the PO file.
+    // cspell:disable-next-line
+    $this->assertSame('Anónimo', \Drupal::languageManager()->getLanguageConfigOverride('es', 'user.settings')->get('anonymous'));
+
+    /** @var \Drupal\locale\StringStorageInterface $locale_storage */
+    $locale_storage = \Drupal::service('locale.storage');
+    // If configuration contains a translation that is not in the po file then
+    // _install_config_locale_overrides_process_batch() will create a customized
+    // translation.
+    $translation = $locale_storage->findTranslation(['source' => 'Anonymous', 'language' => 'es']);
+    $this->assertSame('Anónimo', $translation->getString());
+    $this->assertTrue((bool) $translation->customized, "A customized translation has been created for Anonymous");
+
+    // If configuration contains a translation that is in the po file then
+    // _install_config_locale_overrides_process_batch() will not create a
+    // customized translation.
+    $translation = $locale_storage->findTranslation(['source' => 'Apply', 'language' => 'es']);
+    $this->assertSame('Aplicar', $translation->getString());
+    $this->assertFalse((bool) $translation->customized, "A non-customized translation has been created for Apply");
+
+    /** @var \Drupal\language\Config\LanguageConfigOverride $view_config */
+    // Ensure that views are translated as expected.
+    $view_config = \Drupal::languageManager()->getLanguageConfigOverride('es', 'views.view.who_s_new');
+    $this->assertSame('Aplicar', $view_config->get('display.default.display_options.exposed_form.options.submit_button'));
+    $view_config = \Drupal::languageManager()->getLanguageConfigOverride('es', 'views.view.archive');
+    $this->assertSame('Aplicar', $view_config->get('display.default.display_options.exposed_form.options.submit_button'));
+
+    // Manually update the translation status so can re-run the import.
+    $status = locale_translation_get_status();
+    $status['drupal']['es']->type = 'local';
+    $status['drupal']['es']->files['local']->timestamp = time();
+    \Drupal::keyValue('locale.translation_status')->set('drupal', $status['drupal']);
+    // Run the translation import.
+    $this->drupalGet('admin/reports/translations');
+    $this->submitForm([], 'Update translations');
+
+    // Ensure that only the config we expected to have changed has.
+    $comparer = $this->configImporter()->getStorageComparer();
+    $expected_changelist_spanish_collection = [
+      'create' => [],
+      // The view was untranslated but the translation exists so the installer
+      // performs the translation.
+      'update' => ['views.view.who_s_new'],
+      'delete' => [],
+      'rename' => [],
+    ];
+    $this->assertEquals($expected_changelist_spanish_collection, $comparer->getChangelist(NULL, 'language.es'));
+
+    // Change a translation and ensure configuration is updated.
+    $po = <<<ENDPO
+msgid ""
+msgstr ""
+
+msgid "Anonymous"
+msgstr "Anonymous es"
+
+msgid "Apply"
+msgstr "Aplicar New"
+
+ENDPO;
+    file_put_contents($this->publicFilesDirectory . '/translations/drupal-8.0.0.es.po', $po);
+
+    // Manually update the translation status so can re-run the import.
+    $status = locale_translation_get_status();
+    $status['drupal']['es']->type = 'local';
+    $status['drupal']['es']->files['local']->timestamp = time();
+    \Drupal::keyValue('locale.translation_status')->set('drupal', $status['drupal']);
+    // Run the translation import.
+    $this->drupalGet('admin/reports/translations');
+    $this->submitForm([], 'Update translations');
+
+    $translation = \Drupal::service('locale.storage')->findTranslation(['source' => 'Apply', 'language' => 'es']);
+    $this->assertSame('Aplicar New', $translation->getString());
+    $this->assertFalse((bool) $translation->customized, "A non-customized translation has been created for Apply");
+
+    // Ensure that only the config we expected to have changed has.
+    $comparer = $this->configImporter()->getStorageComparer();
+    $expected_changelist_spanish_collection = [
+      'create' => [],
+      // All views with 'Aplicar' will have been changed to use the new
+      // translation.
+      'update' => [
+        'views.view.archive',
+        'views.view.content_recent',
+        'views.view.frontpage',
+        'views.view.glossary',
+        'views.view.who_s_new',
+        'views.view.who_s_online',
+      ],
+      'delete' => [],
+      'rename' => [],
+    ];
+    $this->assertEquals($expected_changelist_spanish_collection, $comparer->getChangelist(NULL, 'language.es'));
+  }
+
+  /**
+   * Returns the string for the test .po file.
+   *
+   * @param string $langcode
+   *   The language code.
+   *
+   * @return string
+   *   Contents for the test .po file.
+   */
+  protected function getPo($langcode) {
+    return <<<ENDPO
+msgid ""
+msgstr ""
+
+msgid "Anonymous"
+msgstr "Anonymous $langcode"
+
+msgid "Apply"
+msgstr "Aplicar"
+
+ENDPO;
   }
 
 }