diff --git a/core/includes/config.inc b/core/includes/config.inc index 303c316..e17c788 100644 --- a/core/includes/config.inc +++ b/core/includes/config.inc @@ -4,6 +4,7 @@ use Drupal\Core\Config\Config; use Drupal\Core\Config\ConfigException; use Drupal\Core\Config\DatabaseStorage; use Drupal\Core\Config\FileStorage; +use Drupal\Core\Config\NullStorage; use Drupal\Core\Config\StorageInterface; /** @@ -25,12 +26,22 @@ function config_install_default_config($module) { if (is_dir($module_config_dir)) { $source_storage = new FileStorage(array('directory' => $module_config_dir)); $target_storage = new DatabaseStorage(); - - $config_changes = config_sync_get_changes($source_storage, $target_storage); + $null_storage = new NullStorage(); + + // Upon module installation, only new config objects need to be created. + // config_sync_get_changes() would potentially perform a diff of hundreds or + // even thousands of config objects that happen to be contained in the + // active store. We leverage the NullStorage to avoid that needless + // computation of differences. + $config_changes = config_sync_get_changes($source_storage, $null_storage); if (empty($config_changes)) { return; } - $remaining_changes = config_import_invoke_sync_hooks($config_changes, $source_storage, $target_storage); + // Upon re-installation, potentially stale and pre-existing config objects, + // having the same names as default configuration objects, need to be + // deleted before creating the new default config. + $config_changes['delete'] = $config_changes['create']; + $remaining_changes = config_import_invoke_owner($config_changes, $source_storage, $target_storage); config_sync_changes($remaining_changes, $source_storage, $target_storage); } } @@ -147,7 +158,7 @@ function config_import() { } try { - $remaining_changes = config_import_invoke_sync_hooks($config_changes, $source_storage, $target_storage); + $remaining_changes = config_import_invoke_owner($config_changes, $source_storage, $target_storage); config_sync_changes($remaining_changes, $source_storage, $target_storage); // Flush all caches and reset static variables after a successful import. drupal_flush_all_caches(); @@ -162,12 +173,18 @@ function config_import() { } /** - * Invokes hook_config_import() implementations for configuration changes. + * Invokes MODULE_config_import() callbacks for configuration changes. * * @param array $config_changes * An array of changes to be loaded. + * @param Drupal\Core\Config\StorageInterface $source_storage + * The storage to synchronize configuration from. + * @param Drupal\Core\Config\StorageInterface $target_storage + * The storage to synchronize configuration to. + * + * @todo Add support for other extension types; e.g., themes etc. */ -function config_import_invoke_sync_hooks(array $config_changes, StorageInterface $source_storage, StorageInterface $target_storage) { +function config_import_invoke_owner(array $config_changes, StorageInterface $source_storage, StorageInterface $target_storage) { $storage_dispatcher = drupal_container()->get('config.storage.dispatcher'); // Allow modules to take over configuration change operations for @@ -186,8 +203,14 @@ function config_import_invoke_sync_hooks(array $config_changes, StorageInterface $old_config ->setName($name) ->load(); + if ($op == 'delete' && $old_config->isNew()) { + continue; + } $data = $source_storage->read($name); + if ($op == 'create' && $data === FALSE) { + continue; + } $new_config = new Config($storage_dispatcher); $new_config ->setName($name) diff --git a/core/lib/Drupal/Core/Config/Config.php b/core/lib/Drupal/Core/Config/Config.php index 0748b76..457df57 100644 --- a/core/lib/Drupal/Core/Config/Config.php +++ b/core/lib/Drupal/Core/Config/Config.php @@ -20,6 +20,13 @@ class Config { protected $name; /** + * Whether the configuration object is new or has been saved to the storage. + * + * @var bool + */ + protected $isNew = TRUE; + + /** * The data of the configuration object. * * @var array @@ -60,6 +67,13 @@ class Config { } /** + * Returns whether this configuration object is new. + */ + public function isNew() { + return $this->isNew; + } + + /** * Gets data from this config object. * * @param $key @@ -210,6 +224,7 @@ class Config { public function load() { $this->setData(array()); $data = $this->storageDispatcher->selectStorage('read', $this->name)->read($this->name); + $this->isNew = ($data === FALSE); if ($data !== FALSE) { $this->setData($data); } @@ -222,6 +237,7 @@ class Config { public function save() { $this->sortByKey($this->data); $this->storageDispatcher->selectStorage('write', $this->name)->write($this->name, $this->data); + $this->isNew = FALSE; return $this; } @@ -251,6 +267,7 @@ class Config { public function delete() { $this->data = array(); $this->storageDispatcher->selectStorage('write', $this->name)->delete($this->name); + $this->isNew = TRUE; return $this; } } diff --git a/core/lib/Drupal/Core/Config/DatabaseStorage.php b/core/lib/Drupal/Core/Config/DatabaseStorage.php index 1dd8507..f30011c 100644 --- a/core/lib/Drupal/Core/Config/DatabaseStorage.php +++ b/core/lib/Drupal/Core/Config/DatabaseStorage.php @@ -51,7 +51,7 @@ class DatabaseStorage implements StorageInterface { * Only thrown in case $this->options['throw_exception'] is TRUE. */ public function read($name) { - $data = array(); + $data = FALSE; // There are situations, like in the installer, where we may attempt a // read without actually having the database available. In this case, // catch the exception and just return an empty array so the caller can @@ -114,7 +114,7 @@ class DatabaseStorage implements StorageInterface { */ public static function decode($raw) { $data = @unserialize($raw); - return $data !== FALSE ? $data : array(); + return is_array($data) ? $data : FALSE; } /** diff --git a/core/lib/Drupal/Core/Config/FileStorage.php b/core/lib/Drupal/Core/Config/FileStorage.php index 7827dc0..d9e7a5a 100644 --- a/core/lib/Drupal/Core/Config/FileStorage.php +++ b/core/lib/Drupal/Core/Config/FileStorage.php @@ -70,18 +70,15 @@ class FileStorage implements StorageInterface { */ public function read($name) { if (!$this->exists($name)) { - return array(); + return FALSE; } $data = file_get_contents($this->getFilePath($name)); // @todo Yaml throws a ParseException on invalid data. Is it expected to be // caught or not? $data = $this->decode($data); - if ($data === FALSE) { - return array(); - } // A simple string is valid YAML for any reason. if (!is_array($data)) { - return array(); + return FALSE; } return $data; } @@ -131,6 +128,7 @@ class FileStorage implements StorageInterface { * @throws Symfony\Component\Yaml\Exception\ParseException */ public static function decode($raw) { + // @todo An empty file means bogus config data; return FALSE? if (empty($raw)) { return array(); } diff --git a/core/lib/Drupal/Core/Config/NullStorage.php b/core/lib/Drupal/Core/Config/NullStorage.php new file mode 100644 index 0000000..54554c9 --- /dev/null +++ b/core/lib/Drupal/Core/Config/NullStorage.php @@ -0,0 +1,61 @@ +get(); - return image_style_delete($style); + // @todo image_style_delete() supports the notion of a "replacement style" + // to be used by other modules instead of the deleted style. Essential! + // But that is impossible currently, since the config system only knows + // about deleted and added changes. Introduce an 'old_ID' key within + // config objects as a standard? + $config_test = new ConfigTest($old_config); + $config_test->delete(); } if ($op == 'create') { - $style = $new_config->get(); - return image_style_save($style); + $config_test = new ConfigTest($new_config); + $config_test->save(); } if ($op == 'change') { - $style = $new_config->get(); - return image_style_save($style); + $config_test = new ConfigTest($new_config); + $config_test->setOriginal($old_config); + $config_test->save(); } + return TRUE; } diff --git a/core/modules/config/config_test/config_test.module b/core/modules/config/config_test/config_test.module index 5f229ca..8d4e8ec 100644 --- a/core/modules/config/config_test/config_test.module +++ b/core/modules/config/config_test/config_test.module @@ -19,12 +19,6 @@ function config_test_config_import($op, $name, $new_config, $old_config) { } if ($op == 'delete') { - // @todo image_style_delete() supports the notion of a "replacement style" - // to be used by other modules instead of the deleted style. Good idea. - // But squeezing that into a "delete" operation is the worst idea ever. - // Regardless of Image module insanity, add a 'replaced' stack to - // config_import()? And how can that work? If an 'old_ID' key would be a - // standard, wouldn't this belong into 'changed' instead? $config_test = new ConfigTest($old_config); $config_test->delete(); } diff --git a/core/modules/image/image.module b/core/modules/image/image.module index c95af19..28afd0b 100644 --- a/core/modules/image/image.module +++ b/core/modules/image/image.module @@ -511,11 +511,10 @@ function image_config_import($op, $name, $new_config, $old_config) { if ($op == 'delete') { // @todo image_style_delete() supports the notion of a "replacement style" - // to be used by other modules instead of the deleted style. Good idea. - // But squeezing that into a "delete" operation is the worst idea ever. - // Regardless of Image module insanity, add a 'replaced' stack to - // config_import()? And how can that work? If an 'old_ID' key would be a - // standard, wouldn't this belong into 'changed' instead? + // to be used by other modules instead of the deleted style. Essential! + // But that is impossible currently, since the config system only knows + // about deleted and added changes. Introduce an 'old_ID' key within + // config objects as a standard? $style = $old_config->get(); return image_style_delete($style); }