diff --git a/core/includes/config.inc b/core/includes/config.inc index 8d0eea1..a45153c 100644 --- a/core/includes/config.inc +++ b/core/includes/config.inc @@ -1,7 +1,9 @@ delete(); + } + else { + // Get the active store object, set the new data from file, then save. + $target_config = config($name); + $source_config = new DrupalConfig(new FileStorage($name)); + $target_config->setData($source_config->get()); + $target_config->save(); + } + } + } +} + +/** + * Invokes hook_config_sync_validate() and hook_config_sync() implementations. + * + * @param array $config_changes + * An array of changes to be loaded. + */ +function config_sync_invoke_sync_hooks(array $config_changes) { + $target_storage = new DrupalConfig(new DatabaseStorage(NULL)); + $source_storage = new DrupalConfig(new FileStorage(NULL)); + + // Allow all modules to deny configuration changes. + // Note: module_invoke_all() can only be used as long as it does not allow + // implementations to take $config_changes by reference, since they are not + // supposed to change the configuration that is to be imported. + module_invoke_all('config_sync_validate', $config_changes, $target_storage, $source_storage); + + // Allow modules to take over configuration change operations for + // higher-level configuration data. + // First pass deleted, then new, and lastly changed configuration, in order to + // handle dependencies correctly. + $remaining_changes = $config_changes; + foreach (array('delete', 'create', 'change') as $op) { + foreach ($remaining_changes[$op] as $key => $name) { + // Extract owner from configuration object name. + $module = strtok($name, '.'); + // Check whether the module implements hook_config_sync() and ask it to + // handle the configuration change. + $handled_by_module = FALSE; + if (module_hook($module, 'config_sync')) { + $old_config = new DrupalConfig(new DatabaseStorage($name)); + $new_config = new DrupalConfig(new FileStorage($name)); + // @todo $name is obsolete as soon as there is a proper ConfigObject. + $handled_by_module = module_invoke($module, 'config_sync', $op, $name, $new_config, $old_config); + } + if (!empty($handled_by_module)) { + unset($remaining_changes[$op][$key]); + } + } + } + + return $remaining_changes; +} + +/** + * Invokes hook_config_sync_error() implementations. + * + * During a sync run, modules may make changes that cannot be rolled back. + * This hook allows modules to react to an error that occurs after they have + * made such changes, and make sure that the state of configuration in the + * active store is correct. + * + * @param array $config_changes + * An array of changes to be loaded. + */ +function config_sync_invoke_sync_error_hooks(array $config_changes) { + $target_storage = new DrupalConfig(new DatabaseStorage(NULL)); + $source_storage = new DrupalConfig(new FileStorage(NULL)); + + foreach (module_implements('config_sync_error') as $module) { + $function = $module . '_config_sync_error'; + try { + $function($config_changes, $target_storage, $source_storage); + } + catch (ConfigException $e) { + watchdog_exception('config_sync', $e); + // Just keep going, because we need to allow all modules to react even if + // some of them are behaving badly. + } + } +} + +/** + * Returns a list of differences between FileStorage and DatabaseStorage. + * + * @return array|bool + * The list of files changed on disk compared to the active store, or FALSE if + * there are no differences. + */ +function config_sync_get_changes() { + $disk_config_names = FileStorage::getNamesWithPrefix(); + $active_config_names = DatabaseStorage::getNamesWithPrefix(); + $config_changes = array( + 'create' => array_diff($disk_config_names, $active_config_names), + 'change' => array(), + 'delete' => array_diff($active_config_names, $disk_config_names), + ); + foreach (array_intersect($disk_config_names, $active_config_names) as $name) { + $active_config = config($name); + $file_config = new DrupalConfig(new FileStorage($name)); + if ($active_config->get() != $file_config->get()) { + $config_changes['change'][] = $name; + } + } + + // Do not trigger subsequent synchronization operations if there are no + // changes in either category. + if (empty($config_changes['create']) && empty($config_changes['change']) && empty($config_changes['delete'])) { + return FALSE; + } + + return $config_changes; +} + diff --git a/core/includes/module.inc b/core/includes/module.inc index 928abc9..eb69bf5 100644 --- a/core/includes/module.inc +++ b/core/includes/module.inc @@ -487,7 +487,7 @@ function module_enable($module_list, $enable_dependencies = TRUE) { $versions = drupal_get_schema_versions($module); $version = $versions ? max($versions) : SCHEMA_INSTALLED; - // Copy any default configuration data to the system config directory/ + // Install default configuration of the module. config_install_default_config($module); // If the module has no current updates, but has some that were diff --git a/core/lib/Drupal/Core/Config/DrupalConfig.php b/core/lib/Drupal/Core/Config/DrupalConfig.php index f5a9220..c05417e 100644 --- a/core/lib/Drupal/Core/Config/DrupalConfig.php +++ b/core/lib/Drupal/Core/Config/DrupalConfig.php @@ -18,6 +18,13 @@ class DrupalConfig { protected $storage; /** + * The name of the current configuration object. + * + * @var string + */ + protected $name; + + /** * The data of the configuration object. * * @var array @@ -34,7 +41,24 @@ class DrupalConfig { */ public function __construct(StorageInterface $storage) { $this->storage = $storage; + // Retrieve the configuration object name assigned to the storage + // controller and automatically load it, if any. + $this->name = $this->storage->getName(); + if (isset($this->name)) { + $this->read(); + } + } + + /** + * Loads a configuration object. + * + * @param string $name + * The configuration object name to load. + */ + public function load($name) { + $this->storage->setName($name); $this->read(); + return $this; } /** @@ -201,6 +225,7 @@ class DrupalConfig { else { drupal_array_unset_nested_value($this->data, $parts); } + return $this; } /** @@ -208,6 +233,7 @@ class DrupalConfig { */ public function save() { $this->storage->write($this->data); + return $this; } /** @@ -216,5 +242,6 @@ class DrupalConfig { public function delete() { $this->data = array(); $this->storage->delete(); + return $this; } } diff --git a/core/lib/Drupal/Core/Config/FileStorage.php b/core/lib/Drupal/Core/Config/FileStorage.php index 2a6d448..5446f96 100644 --- a/core/lib/Drupal/Core/Config/FileStorage.php +++ b/core/lib/Drupal/Core/Config/FileStorage.php @@ -2,15 +2,15 @@ namespace Drupal\Core\Config; +use Drupal\Core\Config\StorageInterface; use Symfony\Component\Yaml\Yaml; /** * Represents the file storage controller. * - * @todo Implement StorageInterface after removing DrupalConfig methods. * @todo Consider to extend StorageBase. */ -class FileStorage { +class FileStorage implements StorageInterface { /** * The name of the configuration object. @@ -82,7 +82,7 @@ class FileStorage { * @return bool * TRUE if the configuration file exists, FALSE otherwise. */ - protected function exists() { + public function exists() { return file_exists($this->getFilePath()); } @@ -96,6 +96,7 @@ class FileStorage { if (!file_put_contents($this->getFilePath(), $data)) { throw new FileStorageException('Failed to write configuration file: ' . $this->getFilePath()); } + return $this; } /** @@ -120,8 +121,8 @@ class FileStorage { * Deletes a configuration file. */ public function delete() { - // Needs error handling and etc. - @drupal_unlink($this->getFilePath()); + // @todo Error handling. + return @drupal_unlink($this->getFilePath()); } /** @@ -169,4 +170,41 @@ class FileStorage { }; return array_map($clean_name, $files); } + + /** + * Implements StorageInterface::copyToFile(). + */ + public function copyToFile() { + // @todo Untangle StorageInterface. + } + + /** + * Implements StorageInterface::copyFromFile(). + */ + public function copyFromFile() { + // @todo Untangle StorageInterface. + } + + /** + * Implements StorageInterface::deleteFile(). + */ + public function deleteFile() { + // @todo Untangle StorageInterface. + return $this->delete(); + } + + /** + * Implements StorageInterface::writeToActive(). + */ + public function writeToActive($data) { + // @todo Untangle StorageInterface. + } + + /** + * Implements StorageInterface::writeToFile(). + */ + public function writeToFile($data) { + // @todo Untangle StorageInterface. + return $this->write($data); + } } diff --git a/core/lib/Drupal/Core/Config/StorageBase.php b/core/lib/Drupal/Core/Config/StorageBase.php index b03ff27..f521332 100644 --- a/core/lib/Drupal/Core/Config/StorageBase.php +++ b/core/lib/Drupal/Core/Config/StorageBase.php @@ -76,13 +76,6 @@ abstract class StorageBase implements StorageInterface { } /** - * Implements StorageInterface::isOutOfSync(). - */ - public function isOutOfSync() { - return $this->read() !== $this->readFromFile(); - } - - /** * Implements StorageInterface::write(). */ public function write($data) { diff --git a/core/lib/Drupal/Core/Config/StorageInterface.php b/core/lib/Drupal/Core/Config/StorageInterface.php index 43141a5..103b382 100644 --- a/core/lib/Drupal/Core/Config/StorageInterface.php +++ b/core/lib/Drupal/Core/Config/StorageInterface.php @@ -41,15 +41,6 @@ interface StorageInterface { function deleteFile(); /** - * Checks whether the file and the storage is in sync. - * - * @return - * TRUE if the file and the storage contains the same data, FALSE - * if not. - */ - function isOutOfSync(); - - /** * Writes the configuration data into the active storage and the file. * * @param $data diff --git a/core/modules/config/config.admin.inc b/core/modules/config/config.admin.inc new file mode 100644 index 0000000..2366c5a --- /dev/null +++ b/core/modules/config/config.admin.inc @@ -0,0 +1,62 @@ + t('There are no configuration changes.'), + ); + return $form; + } + + foreach ($config_changes as $config_change_type => $config_files) { + if (empty($config_files)) { + continue; + } + $form[$config_change_type] = array( + '#type' => 'fieldset', + '#title' => $config_change_type . ' (' . count($config_files) . ')', + '#collapsible' => TRUE, + ); + $form[$config_change_type]['config_files'] = array( + '#theme' => 'table', + '#header' => array('Name'), + ); + foreach ($config_files as $config_file) { + $form[$config_change_type]['config_files']['#rows'][] = array($config_file); + } + } + $form['submit'] = array( + '#type' => 'submit', + '#value' => t('Import'), + ); + return $form; +} + +/** + * Form submission handler for config_admin_import_form(). + */ +function config_admin_import_form_submit($form, &$form_state) { + if (config_sync()) { + drupal_set_message(t('The configuration was imported successfully.')); + } + else { + // Another request may be synchronizing configuration already. Wait for it + // to complete before returning the error, so already synchronized changes + // do not appear again. + lock_wait(__FUNCTION__); + drupal_set_message(t('The import failed due to an error. Any errors have been logged.'), 'error'); + } +} + diff --git a/core/modules/config/config.api.php b/core/modules/config/config.api.php new file mode 100644 index 0000000..7e92658 --- /dev/null +++ b/core/modules/config/config.api.php @@ -0,0 +1,112 @@ +load() to load a + * configuration object. + * @param $source_storage + * A configuration class acting on the source storage from which configuration + * differences were read. Use $target_storage->load() to load a configuration + * object. + * + * @throws ConfigException + * In case a configuration change cannot be allowed. + */ +function hook_config_sync_validate($config_changes, $target_storage, $source_storage) { + // Deny changes to our settings. + if (isset($config_changes['change']['mymodule.locked'])) { + throw new ConfigException('MyModule settings cannot be changed.'); + } +} + +/** + * Synchronize configuration changes. + * + * This hook is invoked when configuration is synchronized between storages and + * allows a module to take over the synchronization of configuration data. + * + * Modules should implement this hook if they manage higher-level configuration + * data (such as image styles, node types, or fields), which needs to be + * prepared and passed through module API functions to properly handle a + * configuration change. + * + * @param string $op + * The operation to perform for the configuration data; one of 'create', + * 'delete', or 'change'. + * @param string $name + * The name of the configuration object. + * @param Drupal\Core\Config\DrupalConfig $new_config + * A configuration class acting on the target storage to which the new + * configuration will be written. Use $target_storage->load() to load a + * configuration object. + * @param Drupal\Core\Config\DrupalConfig $old_config + * A configuration class acting on the source storage from which configuration + * differences were read. Use $target_storage->load() to load a configuration + * object. + */ +function hook_config_sync($op, $name, $new_config, $old_config) { + // Only image styles require custom handling. Any other module settings can be + // synchronized directly. + if (strpos($name, 'image.style.') !== 0) { + return FALSE; + } + + if ($op == 'delete') { + $style = $old_config->get(); + return image_style_delete($style); + } + if ($op == 'new') { + $style = $new_config->get(); + return image_style_save($style); + } + if ($op == 'change') { + $style = $new_config->get(); + return image_style_save($style); + } +} + +/** + * Validate configuration changes before they are saved to the active store. + * + * During synchronization of configuration, modules may make changes that cannot + * be rolled back. This hook allows modules to react to an error that occurs + * after they have made such changes, and make sure that the state of + * configuration is as correct as possible. + * + * @param array $config_changes + * An associative array whose keys denote the configuration differences + * ('create', 'change', 'delete') and whose values are arrays of configuration + * object names. + * @param $target_storage + * A configuration class acting on the target storage to which the new + * configuration will be written. Use $target_storage->load() to load a + * configuration object. + * @param $source_storage + * A configuration class acting on the source storage from which configuration + * differences were read. Use $target_storage->load() to load a configuration + * object. + */ +function hook_config_sync_error($config_changes, $target_storage, $source_storage) { + // @todo Feasability and usage of this hook is still unclear, without having a + // backup of $target_storage at hand. +} + diff --git a/core/modules/config/config.module b/core/modules/config/config.module index b3d9bbc..56c3bb8 100644 --- a/core/modules/config/config.module +++ b/core/modules/config/config.module @@ -1 +1,34 @@ t('Import configuration'), + 'restrict access' => TRUE, + ); + return $permissions; +} + +/** + * Implements hook_menu(). + */ +function config_menu() { + $items['admin/config/development/import'] = array( + 'title' => 'Import configuration', + 'description' => 'Import and synchronize configuration changes.', + 'page callback' => 'drupal_get_form', + 'page arguments' => array('config_admin_import_form'), + 'access arguments' => array('import configuration'), + 'file' => 'config.admin.inc', + ); + return $items; +} + diff --git a/core/modules/config/config_test/config/config_test.delete.yml b/core/modules/config/config_test/config/config_test.delete.yml new file mode 100644 index 0000000..b8ccb67 --- /dev/null +++ b/core/modules/config/config_test/config/config_test.delete.yml @@ -0,0 +1 @@ +delete_me: bar diff --git a/core/modules/config/config_test/config/config_test.system.yml b/core/modules/config/config_test/config/config_test.system.yml new file mode 100644 index 0000000..20e9ff3 --- /dev/null +++ b/core/modules/config/config_test/config/config_test.system.yml @@ -0,0 +1 @@ +foo: bar diff --git a/core/modules/config/config_test/config_test.info b/core/modules/config/config_test/config_test.info new file mode 100644 index 0000000..8735450 --- /dev/null +++ b/core/modules/config/config_test/config_test.info @@ -0,0 +1,6 @@ +name = Configuration test module +package = Core +version = VERSION +core = 8.x +dependencies[] = config +hidden = TRUE diff --git a/core/modules/config/config_test/config_test.module b/core/modules/config/config_test/config_test.module new file mode 100644 index 0000000..8af386e --- /dev/null +++ b/core/modules/config/config_test/config_test.module @@ -0,0 +1,36 @@ + 'Import configuration', + 'description' => 'Tests importing configuration from files into active store.', + 'group' => 'Configuration', + ); + } + + function setUp() { + parent::setUp('config_test'); + $this->fileExtension = FileStorage::getFileExtension(); + } + + /** + * Tests deletion of configuration during import. + */ + function testDeleted() { + $name = 'config_test.system'; + + // Verify the default configuration value exists. + $config = config($name); + $this->assertIdentical($config->get('foo'), 'bar'); + + // Delete the configuration object. + $file = new FileStorage($name); + $file->delete(); + + // Import. + config_sync(); + + // Verify the value has disappeared. + $config = config($name); + $this->assertIdentical($config->get('foo'), NULL); + } + + /** + * Tests creation of configuration during import. + */ + function testNew() { + $name = 'config_test.new'; + + // Verify the configuration to create does not exist yet. + $file = new FileStorage($name); + $this->assertIdentical($file->exists(), FALSE, $name . ' not found.'); + + // Create a new configuration object. + $file->write(array( + 'add_me' => 'new value', + )); + $this->assertIdentical($file->exists(), TRUE, $name . ' found.'); + + // Import. + config_sync(); + + // Verify the value has appeared. + $config = config($name); + $this->assertIdentical($config->get('add_me'), 'new value'); + } + + /** + * Tests updating of configuration during import. + */ + function testUpdated() { + $name = 'config_test.system'; + + // Replace the file content of the existing configuration object. + $file = new FileStorage($name); + $this->assertIdentical($file->exists(), TRUE, $name . ' found.'); + $file->write(array( + 'foo' => 'beer', + )); + + // Verify the active store still returns the default value. + $config = config($name); + $this->assertIdentical($config->get('foo'), 'bar'); + + // Import. + config_sync(); + + // Verify the value was updated. + $config = config($name); + $this->assertIdentical($config->get('foo'), 'beer'); + } + + /** + * Tests config_sync() hook invocations. + */ + function testSyncHooks() { + $name = 'config_test.system'; + + // Delete a file so that hook_config_sync() hooks are run. + $file = new FileStorage($name); + $this->assertIdentical($file->exists(), TRUE, $name . ' found.'); + $file->delete(); + + // Make the test implementation throw an error during synchronization, so + // hook_config_sync_error() is also invoked. + $GLOBALS['config_sync_throw_error'] = TRUE; + + // Import. + config_sync(); + + // Verify hook_config_sync() was invoked. + $this->assertIdentical($GLOBALS['hook_config_sync'], 'config_test_config_sync'); + // Verify hook_config_sync_error() was invoked. + $this->assertIdentical($GLOBALS['hook_config_sync_validate'], 'config_test_config_sync_validate'); + // Verify hook_config_sync_error() was invoked. + $this->assertIdentical($GLOBALS['hook_config_sync_error'], 'config_test_config_sync_error'); + } + + /** + * Tests abort of import upon validation error. + */ + function testSyncValidationError() { + $name = 'config_test.system'; + + // Delete a file so that hook_config_sync() hooks are run. + $file = new FileStorage($name); + $this->assertIdentical($file->exists(), TRUE, $name . ' found.'); + $file->delete(); + + // Make the test implementation throw a validation error, aborting the + // synchronization. + $GLOBALS['config_sync_validate_throw_error'] = TRUE; + + // Import. + config_sync(); + + // Verify the active store was not updated. + $config = config($name); + $this->assertIdentical($config->get('foo'), 'bar'); + } +} diff --git a/core/modules/image/image.module b/core/modules/image/image.module index 905e6a7..7825a18 100644 --- a/core/modules/image/image.module +++ b/core/modules/image/image.module @@ -500,6 +500,36 @@ function image_path_flush($path) { } /** + * Implements hook_config_sync(). + */ +function image_config_sync($op, $name, $new_config, $old_config) { + // Only image styles require custom handling. Any other module settings can be + // synchronized directly. + if (strpos($name, 'image.style.') !== 0) { + return FALSE; + } + + 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_sync()? And how can that work? If an 'old_ID' key would be a + // standard, wouldn't this belong into 'changed' instead? + $style = $old_config->get(); + return image_style_delete($style); + } + if ($op == 'new') { + $style = $new_config->get(); + return image_style_save($style); + } + if ($op == 'change') { + $style = $new_config->get(); + return image_style_save($style); + } +} + +/** * Get an array of all styles and their settings. * * @return