Commit e91e2874 authored by alexpott's avatar alexpott

Issue #2955457 by pfrenssen, Chewie, unrealauk, alexpott, Pol: ConfigFactory...

Issue #2955457 by pfrenssen, Chewie, unrealauk, alexpott, Pol: ConfigFactory static cache gets polluted with data from config overrides
parent 1f4544c9
...@@ -335,10 +335,18 @@ public function listAll($prefix = '') { ...@@ -335,10 +335,18 @@ public function listAll($prefix = '') {
* The configuration event. * The configuration event.
*/ */
public function onConfigSave(ConfigCrudEvent $event) { public function onConfigSave(ConfigCrudEvent $event) {
$saved_config = $event->getConfig();
// We are only concerned with config objects that belong to the collection
// that matches the storage we depend on. Skip if the event was fired for a
// config object belonging to a different collection.
if ($saved_config->getStorage()->getCollectionName() !== $this->storage->getCollectionName()) {
return;
}
// Ensure that the static cache contains up to date configuration objects by // Ensure that the static cache contains up to date configuration objects by
// replacing the data on any entries for the configuration object apart // replacing the data on any entries for the configuration object apart
// from the one that references the actual config object being saved. // from the one that references the actual config object being saved.
$saved_config = $event->getConfig();
foreach ($this->getConfigCacheKeys($saved_config->getName()) as $cache_key) { foreach ($this->getConfigCacheKeys($saved_config->getName()) as $cache_key) {
$cached_config = $this->cache[$cache_key]; $cached_config = $this->cache[$cache_key];
if ($cached_config !== $saved_config) { if ($cached_config !== $saved_config) {
...@@ -356,8 +364,17 @@ public function onConfigSave(ConfigCrudEvent $event) { ...@@ -356,8 +364,17 @@ public function onConfigSave(ConfigCrudEvent $event) {
* The configuration event. * The configuration event.
*/ */
public function onConfigDelete(ConfigCrudEvent $event) { public function onConfigDelete(ConfigCrudEvent $event) {
$deleted_config = $event->getConfig();
// We are only concerned with config objects that belong to the collection
// that matches the storage we depend on. Skip if the event was fired for a
// config object belonging to a different collection.
if ($deleted_config->getStorage()->getCollectionName() !== $this->storage->getCollectionName()) {
return;
}
// Ensure that the static cache does not contain deleted configuration. // Ensure that the static cache does not contain deleted configuration.
foreach ($this->getConfigCacheKeys($event->getConfig()->getName()) as $cache_key) { foreach ($this->getConfigCacheKeys($deleted_config->getName()) as $cache_key) {
unset($this->cache[$cache_key]); unset($this->cache[$cache_key]);
} }
} }
......
<?php
namespace Drupal\Tests\language\Kernel;
use Drupal\Core\Config\ConfigImporter;
use Drupal\Core\Config\StorageComparer;
use Drupal\KernelTests\KernelTestBase;
/**
* Tests importing of config with language overrides.
*
* @group language
*/
class OverriddenConfigImportTest extends KernelTestBase {
/**
* Config Importer object used for testing.
*
* @var \Drupal\Core\Config\ConfigImporter
*/
protected $configImporter;
/**
* {@inheritdoc}
*/
protected static $modules = ['system', 'language'];
/**
* {@inheritdoc}
*/
protected function setUp() {
parent::setUp();
$this->installConfig(['system']);
$this->copyConfig($this->container->get('config.storage'), $this->container->get('config.storage.sync'));
// Set up the ConfigImporter object for testing.
$storage_comparer = new StorageComparer(
$this->container->get('config.storage.sync'),
$this->container->get('config.storage'),
$this->container->get('config.manager')
);
$this->configImporter = new ConfigImporter(
$storage_comparer->createChangelist(),
$this->container->get('event_dispatcher'),
$this->container->get('config.manager'),
$this->container->get('lock'),
$this->container->get('config.typed'),
$this->container->get('module_handler'),
$this->container->get('module_installer'),
$this->container->get('theme_handler'),
$this->container->get('string_translation')
);
}
/**
* Tests importing overridden config alongside config in the default language.
*/
public function testConfigImportUpdates() {
$storage = $this->container->get('config.storage');
$sync = $this->container->get('config.storage.sync');
/** @var \Drupal\language\ConfigurableLanguageManagerInterface $language_manager */
$language_manager = $this->container->get('language_manager');
// Make a change to the site configuration in the default collection.
$data = $storage->read('system.site');
$data['name'] = 'English site name';
$sync->write('system.site', $data);
// Also make a change to the same config object, but using a language
// override.
/* @var \Drupal\Core\Config\StorageInterface $overridden_sync */
$overridden_sync = $sync->createCollection('language.fr');
$overridden_sync->write('system.site', ['name' => 'French site name']);
// Before we start the import, the change to the site name should not be
// present. This action also primes the cache in the config factory so that
// we can test whether the cached data is correctly updated.
$config = $this->config('system.site');
$this->assertNotEquals('English site name', $config->getRawData()['name']);
// Before the import is started the site name should not yet be overridden.
$this->assertFalse($config->hasOverrides());
$override = $language_manager->getLanguageConfigOverride('fr', 'system.site');
$this->assertTrue($override->isNew());
// Start the import of the new configuration.
$this->configImporter->reset()->import();
// Verify the new site name in the default language.
$config = $this->config('system.site')->getRawData();
$this->assertEquals('English site name', $config['name']);
// Verify the overridden site name.
$override = $language_manager->getLanguageConfigOverride('fr', 'system.site');
$this->assertEquals('French site name', $override->get('name'));
}
}
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
use Drupal\Component\Utility\Crypt; use Drupal\Component\Utility\Crypt;
use Drupal\Component\Render\FormattableMarkup; use Drupal\Component\Render\FormattableMarkup;
use Drupal\Core\Config\Config;
use Drupal\Core\Config\ConfigNameException; use Drupal\Core\Config\ConfigNameException;
use Drupal\Core\Config\ConfigValueException; use Drupal\Core\Config\ConfigValueException;
use Drupal\Core\Config\InstallStorage; use Drupal\Core\Config\InstallStorage;
...@@ -38,14 +39,19 @@ class ConfigCRUDTest extends KernelTestBase { ...@@ -38,14 +39,19 @@ class ConfigCRUDTest extends KernelTestBase {
* Tests CRUD operations. * Tests CRUD operations.
*/ */
public function testCRUD() { public function testCRUD() {
$event_dispatcher = $this->container->get('event_dispatcher');
$typed_config_manager = $this->container->get('config.typed');
$storage = $this->container->get('config.storage'); $storage = $this->container->get('config.storage');
$collection_storage = $storage->createCollection('test_collection');
$config_factory = $this->container->get('config.factory'); $config_factory = $this->container->get('config.factory');
$name = 'config_test.crud'; $name = 'config_test.crud';
// Create a new configuration object in the default collection.
$config = $this->config($name); $config = $this->config($name);
$this->assertIdentical($config->isNew(), TRUE); $this->assertIdentical($config->isNew(), TRUE);
// Create a new configuration object.
$config->set('value', 'initial'); $config->set('value', 'initial');
$config->save(); $config->save();
$this->assertIdentical($config->isNew(), FALSE); $this->assertIdentical($config->isNew(), FALSE);
...@@ -54,6 +60,25 @@ public function testCRUD() { ...@@ -54,6 +60,25 @@ public function testCRUD() {
$actual_data = $storage->read($name); $actual_data = $storage->read($name);
$this->assertIdentical($actual_data, ['value' => 'initial']); $this->assertIdentical($actual_data, ['value' => 'initial']);
// Verify the config factory contains the saved value.
$actual_data = $config_factory->get($name)->getRawData();
$this->assertIdentical($actual_data, ['value' => 'initial']);
// Create another instance of the config object using a custom collection.
$collection_config = new Config(
$name,
$collection_storage,
$event_dispatcher,
$typed_config_manager
);
$collection_config->set('value', 'overridden');
$collection_config->save();
// Verify that the config factory still returns the right value, from the
// config instance in the default collection.
$actual_data = $config_factory->get($name)->getRawData();
$this->assertIdentical($actual_data, ['value' => 'initial']);
// Update the configuration object instance. // Update the configuration object instance.
$config->set('value', 'instance-update'); $config->set('value', 'instance-update');
$config->save(); $config->save();
...@@ -71,6 +96,14 @@ public function testCRUD() { ...@@ -71,6 +96,14 @@ public function testCRUD() {
// Pollute the config factory static cache. // Pollute the config factory static cache.
$config_factory->getEditable($name); $config_factory->getEditable($name);
// Delete the config object that uses a custom collection. This should not
// affect the instance returned by the config factory which depends on the
// default collection storage.
$collection_config->delete();
$actual_config = $config_factory->get($name);
$this->assertIdentical($actual_config->isNew(), FALSE);
$this->assertIdentical($actual_config->getRawData(), ['value' => 'instance-update']);
// Delete the configuration object. // Delete the configuration object.
$config->delete(); $config->delete();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment