diff --git a/core/includes/config.inc b/core/includes/config.inc index 0771d60..d1e7183 100644 --- a/core/includes/config.inc +++ b/core/includes/config.inc @@ -26,7 +26,7 @@ function config_install_default_config($module) { $database_storage = new DatabaseStorage(); $module_file_storage = new FileStorage(array('directory' => $module_config_dir)); - foreach ($module_file_storage->getNamesWithPrefix() as $config_name) { + foreach ($module_file_storage->listAll() as $config_name) { $data = $module_file_storage->read($config_name); $database_storage->write($config_name, $data); } @@ -38,7 +38,7 @@ function config_install_default_config($module) { */ function config_get_storage_names_with_prefix($prefix = '') { $storage = new DatabaseStorage(); - return $storage->getNamesWithPrefix($prefix); + return $storage->listAll($prefix); } /** @@ -73,8 +73,8 @@ function config($name) { * storage, or FALSE if there are no differences. */ function config_sync_get_changes(StorageInterface $source_storage, StorageInterface $target_storage) { - $source_names = $source_storage->getNamesWithPrefix(); - $target_names = $target_storage->getNamesWithPrefix(); + $source_names = $source_storage->listAll(); + $target_names = $target_storage->listAll(); $config_changes = array( 'create' => array_diff($source_names, $target_names), 'change' => array(), @@ -176,7 +176,7 @@ function config_import_invoke_sync_hooks(array $config_changes) { // @todo Leverage DI + config.storage.info. $source_storage = new FileStorage(); $target_storage = new DatabaseStorage(); - $storage_manager = drupal_container()->get('config.storage.manager'); + $storage_dispatcher = drupal_container()->get('config.storage.dispatcher'); // Allow modules to take over configuration change operations for // higher-level configuration data. @@ -190,21 +190,17 @@ function config_import_invoke_sync_hooks(array $config_changes) { // handle the configuration change. $handled_by_module = FALSE; if (module_hook($module, 'config_import')) { - $old_config = new Config($storage_manager); - $old_config->setName($name); - try { - $old_config->load(); - } - catch (ConfigException $e) { - } - try { - $data = $source_storage->read($name); - } - catch (ConfigException $e) { - $data = array(); - } - $new_config = new Config($storage_manager); - $new_config->setName($name)->setData($data); + $old_config = new Config($storage_dispatcher); + $old_config + ->setName($name) + ->load(); + + $data = $source_storage->read($name); + $new_config = new Config($storage_dispatcher); + $new_config + ->setName($name) + ->setData($data); + $handled_by_module = module_invoke($module, 'config_import', $op, $name, $new_config, $old_config); } if (!empty($handled_by_module)) { diff --git a/core/includes/install.inc b/core/includes/install.inc index 32c9941..cd67257 100644 --- a/core/includes/install.inc +++ b/core/includes/install.inc @@ -384,7 +384,7 @@ function drupal_uninstall_modules($module_list = array(), $uninstall_dependents drupal_uninstall_schema($module); // Remove all configuration belonging to the module. - $config_names = $storage->getNamesWithPrefix($module . '.'); + $config_names = $storage->listAll($module . '.'); foreach ($config_names as $config_name) { config($config_name)->delete(); } diff --git a/core/includes/update.inc b/core/includes/update.inc index a1bd480..ab373cd 100644 --- a/core/includes/update.inc +++ b/core/includes/update.inc @@ -10,6 +10,7 @@ use Drupal\Component\Graph\Graph; use Drupal\Core\Config\FileStorage; +use Drupal\Core\Config\ConfigException; /** * Minimum schema version of Drupal 7 required for upgrade to Drupal 8. @@ -942,9 +943,10 @@ function update_variables_to_config($config_name, array $variable_map) { $module = strtok($config_name, '.'); // Load and set default configuration values. - // Throws a StorageException if there is no default configuration file, which - // is required to exist. $file = new FileStorage(array('directory' => drupal_get_path('module', $module) . '/config')); + if (!$file->exists($config_name)) { + throw new ConfigException("Default configuration file $config_name for $module extension not found but is required to exist."); + } $default_data = $file->read($config_name); // Merge any possibly existing original data into default values. diff --git a/core/lib/Drupal/Core/Config/Config.php b/core/lib/Drupal/Core/Config/Config.php index 7eb2d6c..0748b76 100644 --- a/core/lib/Drupal/Core/Config/Config.php +++ b/core/lib/Drupal/Core/Config/Config.php @@ -1,5 +1,10 @@ options['throw_exception'] is TRUE. */ public function read($name) { + $data = array(); // 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 // handle it if need be. - $data = array(); + // @todo Remove this and use appropriate StorageDispatcher configuration in + // the installer instead. try { $raw = $this->getConnection()->query('SELECT data FROM {config} WHERE name = :name', array(':name' => $name), $this->options)->fetchField(); if ($raw !== FALSE) { @@ -58,43 +68,57 @@ class DatabaseStorage implements StorageInterface { } /** - * Implements StorageInterface::write(). + * Implements Drupal\Core\Config\StorageInterface::write(). + * + * @throws PDOException */ public function write($name, array $data) { $data = $this->encode($data); - return $this->getConnection()->merge('config', $this->options) + $options = array('return' => Database::RETURN_AFFECTED) + $this->options; + return (bool) $this->getConnection()->merge('config', $options) ->key(array('name' => $name)) ->fields(array('data' => $data)) ->execute(); } /** - * Implements StorageInterface::delete(). + * Implements Drupal\Core\Config\StorageInterface::delete(). + * + * @throws PDOException */ public function delete($name) { - $this->getConnection()->delete('config', $this->options) + $options = array('return' => Database::RETURN_AFFECTED) + $this->options; + return (bool) $this->getConnection()->delete('config', $options) ->condition('name', $name) ->execute(); } /** - * Implements StorageInterface::encode(). + * Implements Drupal\Core\Config\StorageInterface::encode(). */ public static function encode($data) { return serialize($data); } /** - * Implements StorageInterface::decode(). + * Implements Drupal\Core\Config\StorageInterface::decode(). + * + * @throws ErrorException + * unserialize() triggers E_NOTICE if the string cannot be unserialized. */ public static function decode($raw) { - return unserialize($raw); + $data = @unserialize($raw); + return $data !== FALSE ? $data : array(); } /** - * Implements StorageInterface::getNamesWithPrefix(). + * Implements Drupal\Core\Config\StorageInterface::listAll(). + * + * @throws PDOException + * @throws Drupal\Core\Database\DatabaseExceptionWrapper + * Only thrown in case $this->options['throw_exception'] is TRUE. */ - public function getNamesWithPrefix($prefix = '') { + public function listAll($prefix = '') { return $this->getConnection()->query('SELECT name FROM {config} WHERE name LIKE :name', array( ':name' => db_like($prefix) . '%', ), $this->options)->fetchCol(); diff --git a/core/lib/Drupal/Core/Config/FileStorage.php b/core/lib/Drupal/Core/Config/FileStorage.php index bafd099..cee29ae 100644 --- a/core/lib/Drupal/Core/Config/FileStorage.php +++ b/core/lib/Drupal/Core/Config/FileStorage.php @@ -1,8 +1,12 @@ encode($data); - if (!file_put_contents($this->getFilePath($name), $data)) { - throw new StorageException('Failed to write configuration file: ' . $this->getFilePath($name)); - } - return $this; - } - - /** - * Implements StorageInterface::read(). + * Implements Drupal\Core\Config\StorageInterface::read(). * - * @throws StorageException + * @throws Symfony\Component\Yaml\Exception\ParseException */ public function read($name) { - // @todo DatabaseStorage does not throw an exception if $name does not exist. if (!$this->exists($name)) { - throw new StorageException("Configuration file '$name' does not exist."); + return array(); } - $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) { - throw new StorageException("Failed to decode configuration file '$name'."); + return array(); + } + // A simple string is valid YAML for any reason. + if (!is_array($data)) { + return array(); } return $data; } /** - * Deletes a configuration file. + * Implements Drupal\Core\Config\StorageInterface::write(). + * + * @throws Symfony\Component\Yaml\Exception\DumpException + * @throws Drupal\Core\Config\StorageException + */ + public function write($name, array $data) { + $data = $this->encode($data); + $status = file_put_contents($this->getFilePath($name), $data); + if ($status === FALSE) { + throw new StorageException('Failed to write configuration file: ' . $this->getFilePath($name)); + } + return TRUE; + } + + /** + * Implements Drupal\Core\Config\StorageInterface::delete(). */ public function delete($name) { - // @todo Error handling. - return @drupal_unlink($this->getFilePath($name)); + if (!$this->exists($name)) { + return FALSE; + } + return drupal_unlink($this->getFilePath($name)); } /** - * Implements StorageInterface::encode(). + * Implements Drupal\Core\Config\StorageInterface::encode(). + * + * @throws Symfony\Component\Yaml\Exception\DumpException */ public static function encode($data) { // The level where you switch to inline YAML is set to PHP_INT_MAX to ensure @@ -109,7 +123,9 @@ class FileStorage implements StorageInterface { } /** - * Implements StorageInterface::decode(). + * Implements Drupal\Core\Config\StorageInterface::decode(). + * + * @throws Symfony\Component\Yaml\Exception\ParseException */ public static function decode($raw) { if (empty($raw)) { @@ -119,11 +135,14 @@ class FileStorage implements StorageInterface { } /** - * Implements StorageInterface::getNamesWithPrefix(). + * Implements Drupal\Core\Config\StorageInterface::listAll(). */ - public function getNamesWithPrefix($prefix = '') { + public function listAll($prefix = '') { $extension = '.' . self::getFileExtension(); $files = glob($this->options['directory'] . '/' . $prefix . '*' . $extension); + if ($files === FALSE) { + return array(); + } $clean_name = function ($value) use ($extension) { return basename($value, $extension); }; diff --git a/core/lib/Drupal/Core/Config/StorageDispatcher.php b/core/lib/Drupal/Core/Config/StorageDispatcher.php index 9e14c73..ebf7049 100644 --- a/core/lib/Drupal/Core/Config/StorageDispatcher.php +++ b/core/lib/Drupal/Core/Config/StorageDispatcher.php @@ -76,6 +76,11 @@ class StorageDispatcher { * @param string $name * The name of the configuration object that is operated on. * + * @return Drupal\Core\Config\StorageInterface + * The storage controller instance that can handle the requested operation. + * + * @throws Drupal\Core\Config\ConfigException + * * @todo Allow write operations to write to multiple storages. */ public function selectStorage($access_operation, $name) { diff --git a/core/lib/Drupal/Core/Config/StorageException.php b/core/lib/Drupal/Core/Config/StorageException.php index 8fddd17..a956fea 100644 --- a/core/lib/Drupal/Core/Config/StorageException.php +++ b/core/lib/Drupal/Core/Config/StorageException.php @@ -10,6 +10,6 @@ namespace Drupal\Core\Config; use Drupal\Core\Config\ConfigException; /** - * An exception to be thrown in case of storage controller operation errors. + * An exception thrown in case of storage controller operation errors. */ class StorageException extends ConfigException {} diff --git a/core/lib/Drupal/Core/Config/StorageInterface.php b/core/lib/Drupal/Core/Config/StorageInterface.php index 6ad1c11..907e884 100644 --- a/core/lib/Drupal/Core/Config/StorageInterface.php +++ b/core/lib/Drupal/Core/Config/StorageInterface.php @@ -1,9 +1,14 @@ getNamesWithPrefix('foo'); + $files = $database_storage->listAll('foo'); $this->assertEqual(count($files), 2, 'Two files listed with the prefix \'foo\'.'); // Get file listing for all files starting with 'biff'. Should return // one element. - $files = $database_storage->getNamesWithPrefix('biff'); + $files = $database_storage->listAll('biff'); $this->assertEqual(count($files), 1, 'One file listed with the prefix \'biff\'.'); // Get file listing for all files starting with 'foo.bar'. Should return // one element. - $files = $database_storage->getNamesWithPrefix('foo.bar'); + $files = $database_storage->listAll('foo.bar'); $this->assertEqual(count($files), 1, 'One file listed with the prefix \'foo.bar\'.'); // Get file listing for all files starting with 'bar'. Should return // an empty array. - $files = $database_storage->getNamesWithPrefix('bar'); + $files = $database_storage->listAll('bar'); $this->assertEqual($files, array(), 'No files listed with the prefix \'bar\'.'); // Delete the configuration. diff --git a/core/modules/config/lib/Drupal/config/Tests/ConfigUpgradeTest.php b/core/modules/config/lib/Drupal/config/Tests/ConfigUpgradeTest.php index b2286da..0b43de4 100644 --- a/core/modules/config/lib/Drupal/config/Tests/ConfigUpgradeTest.php +++ b/core/modules/config/lib/Drupal/config/Tests/ConfigUpgradeTest.php @@ -7,6 +7,7 @@ namespace Drupal\config\Tests; +use Drupal\Core\Config\ConfigException; use Drupal\simpletest\WebTestBase; /** @@ -80,5 +81,14 @@ class ConfigUpgradeTest extends WebTestBase { // Verify that variables have been deleted. $variables = db_query('SELECT name FROM {variable} WHERE name IN (:names)', array(':names' => array('config_upgrade_additional')))->fetchCol(); $this->assertFalse($variables); + + // Verify that a default module configuration file is required to exist. + try { + update_variables_to_config('config_upgrade.missing.default.config', array()); + $this->fail('Exception was not thrown on missing default module configuration file.'); + } + catch (ConfigException $e) { + $this->pass('Exception was thrown on missing default module configuration file.'); + } } } diff --git a/core/modules/config/lib/Drupal/config/Tests/Storage/ConfigStorageTestBase.php b/core/modules/config/lib/Drupal/config/Tests/Storage/ConfigStorageTestBase.php new file mode 100644 index 0000000..15ccb90 --- /dev/null +++ b/core/modules/config/lib/Drupal/config/Tests/Storage/ConfigStorageTestBase.php @@ -0,0 +1,118 @@ +storage->read($name); + $this->assertIdentical($data, array()); + + // Reading a name containing non-decodeable data returns an empty array. + $this->insert($name, ''); + $data = $this->storage->read($name); + $this->assertIdentical($data, array()); + + $this->update($name, 'foo'); + $data = $this->storage->read($name); + $this->assertIdentical($data, array()); + + $this->delete($name); + + // Writing data returns TRUE and the data has been written. + $data = array('foo' => 'bar'); + $result = $this->storage->write($name, $data); + $this->assertIdentical($result, TRUE); + $raw_data = $this->read($name); + $this->assertIdentical($raw_data, $data); + + // Writing the identical data again still returns TRUE. + $result = $this->storage->write($name, $data); + $this->assertIdentical($result, TRUE); + + // Listing all names returns all. + $names = $this->storage->listAll(); + $this->assertTrue(in_array('system.performance', $names)); + $this->assertTrue(in_array($name, $names)); + + // Listing all names with prefix returns names with that prefix only. + $names = $this->storage->listAll('config_test.'); + $this->assertFalse(in_array('system.performance', $names)); + $this->assertTrue(in_array($name, $names)); + + // Deleting an existing name returns TRUE. + $result = $this->storage->delete($name); + $this->assertIdentical($result, TRUE); + + // Deleting a non-existing name returns FALSE. + $result = $this->storage->delete($name); + $this->assertIdentical($result, FALSE); + + // Reading from a non-existing storage bin returns an empty data array. + $data = $this->invalidStorage->read($name); + $this->assertIdentical($data, array()); + + // Writing to a non-existing storage bin throws an exception. + try { + $this->invalidStorage->write($name, array('foo' => 'bar')); + $this->fail('Exception not thrown upon writing to a non-existing storage bin.'); + } + catch (\Exception $e) { + $this->pass('Exception thrown upon writing to a non-existing storage bin.'); + } + + // Deleting from a non-existing storage bin throws an exception. + try { + $this->invalidStorage->delete($name); + $this->fail('Exception not thrown upon deleting from a non-existing storage bin.'); + } + catch (\Exception $e) { + $this->pass('Exception thrown upon deleting from a non-existing storage bin.'); + } + + // Listing on a non-existing storage bin throws an exception. + try { + $this->invalidStorage->listAll(); + $this->fail('Exception not thrown upon listing from a non-existing storage bin.'); + } + catch (\Exception $e) { + $this->pass('Exception thrown upon listing from a non-existing storage bin.'); + } + } + + abstract protected function read($name); + + abstract protected function insert($name, $data); + + abstract protected function update($name, $data); + + abstract protected function delete($name); +} diff --git a/core/modules/config/lib/Drupal/config/Tests/Storage/DatabaseStorageTest.php b/core/modules/config/lib/Drupal/config/Tests/Storage/DatabaseStorageTest.php new file mode 100644 index 0000000..6fa1094 --- /dev/null +++ b/core/modules/config/lib/Drupal/config/Tests/Storage/DatabaseStorageTest.php @@ -0,0 +1,46 @@ + 'DatabaseStorage controller operations', + 'description' => 'Tests DatabaseStorage controller operations.', + 'group' => 'Configuration', + ); + } + + function setUp() { + parent::setUp(); + $this->storage = new DatabaseStorage(); + $this->invalidStorage = new DatabaseStorage(array('target' => 'invalid')); + } + + protected function read($name) { + $data = db_query('SELECT data FROM {config} WHERE name = :name', array(':name' => $name))->fetchField(); + return unserialize($data); + } + + protected function insert($name, $data) { + db_insert('config')->fields(array('name' => $name, 'data' => $data))->execute(); + } + + protected function update($name, $data) { + db_update('config')->fields(array('data' => $data))->condition('name', $name)->execute(); + } + + protected function delete($name) { + db_delete('config')->condition('name', $name)->execute(); + } +} diff --git a/core/modules/config/lib/Drupal/config/Tests/Storage/FileStorageTest.php b/core/modules/config/lib/Drupal/config/Tests/Storage/FileStorageTest.php new file mode 100644 index 0000000..092687f --- /dev/null +++ b/core/modules/config/lib/Drupal/config/Tests/Storage/FileStorageTest.php @@ -0,0 +1,50 @@ + 'FileStorage controller operations', + 'description' => 'Tests FileStorage controller operations.', + 'group' => 'Configuration', + ); + } + + function setUp() { + parent::setUp(); + $this->storage = new FileStorage(); + $this->invalidStorage = new FileStorage(array('directory' => $this->configFileDirectory . '/nonexisting')); + + // FileStorage::listAll() requires other configuration data to exist. + $this->storage->write('system.performance', config('system.performance')->get()); + } + + protected function read($name) { + $data = file_get_contents($this->storage->getFilePath($name)); + return Yaml::parse($data); + } + + protected function insert($name, $data) { + file_put_contents($this->storage->getFilePath($name), $data); + } + + protected function update($name, $data) { + file_put_contents($this->storage->getFilePath($name), $data); + } + + protected function delete($name) { + unlink($this->storage->getFilePath($name)); + } +} diff --git a/core/modules/system/lib/Drupal/system/Tests/Module/ModuleTestBase.php b/core/modules/system/lib/Drupal/system/Tests/Module/ModuleTestBase.php index 2c0712f..8f372ca 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Module/ModuleTestBase.php +++ b/core/modules/system/lib/Drupal/system/Tests/Module/ModuleTestBase.php @@ -92,7 +92,7 @@ class ModuleTestBase extends WebTestBase { return; } $module_file_storage = new FileStorage(array('directory' => $module_config_dir)); - $names = $module_file_storage->getNamesWithPrefix(); + $names = $module_file_storage->listAll(); // Verify that the config directory is not empty. $this->assertTrue($names);