diff --git a/core/includes/bootstrap.inc b/core/includes/bootstrap.inc index fa1335b..4c28ced 100644 --- a/core/includes/bootstrap.inc +++ b/core/includes/bootstrap.inc @@ -288,8 +288,6 @@ const REGISTRY_WRITE_LOOKUP_CACHE = 2; */ const DRUPAL_PHP_FUNCTION_PATTERN = '[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*'; -require_once DRUPAL_ROOT . '/core/includes/config.inc'; - /** * Starts the timer with the specified name. * @@ -480,6 +478,25 @@ function find_conf_path($http_host, $script_name, $require_settings = TRUE) { } /** + * Returns the path of the configuration directory. + * + * @return string + * The configuration directory path. + */ +function config_get_config_directory() { + global $config_directory_name; + + if ($test_prefix = drupal_valid_test_ua()) { + // @see Drupal\simpletest\WebTestBase::setUp() + $path = conf_path() . '/files/simpletest/' . substr($test_prefix, 10) . '/config'; + } + else { + $path = conf_path() . '/files/' . $config_directory_name; + } + return $path; +} + +/** * Sets appropriate server variables needed for command line scripts to work. * * This function can be called by command line scripts before bootstrapping @@ -2265,6 +2282,9 @@ function _drupal_bootstrap_configuration() { // Activate the class loader. drupal_classloader(); + + // Load the procedural configuration system helper functions. + require_once DRUPAL_ROOT . '/core/includes/config.inc'; } /** diff --git a/core/includes/config.inc b/core/includes/config.inc index 8649256..c6435b3 100644 --- a/core/includes/config.inc +++ b/core/includes/config.inc @@ -2,7 +2,6 @@ use Drupal\Core\Config\DatabaseStorage; use Drupal\Core\Config\FileStorage; -use Drupal\Core\Config\ConfigFactory; /** * @file @@ -10,25 +9,6 @@ use Drupal\Core\Config\ConfigFactory; */ /** - * Gets the randomly generated config directory name. - * - * @return - * The directory name. - */ -function config_get_config_directory() { - global $config_directory_name; - - if ($test_prefix = drupal_valid_test_ua()) { - // @see Drupal\simpletest\WebTestBase::setUp() - $path = conf_path() . '/files/simpletest/' . substr($test_prefix, 10) . '/config'; - } - else { - $path = conf_path() . '/files/' . $config_directory_name; - } - return $path; -} - -/** * Installs the default configuration of a given module. * * @param @@ -42,28 +22,20 @@ function config_install_default_config($module) { if (is_dir($module_config_dir)) { $database_storage = new DatabaseStorage(); $module_file_storage = new FileStorage(array('directory' => $module_config_dir)); - $file_storage = new FileStorage(); - - $files = glob($module_config_dir . '/*.' . FileStorage::getFileExtension()); - foreach ($files as $key => $file) { - // Load config data into the active store and write it out to the - // file system in the drupal config directory. Note the config name - // needs to be the same as the file name WITHOUT the extension. - $config_name = basename($file, '.' . FileStorage::getFileExtension()); + foreach ($module_file_storage->listAll() as $config_name) { $data = $module_file_storage->read($config_name); $database_storage->write($config_name, $data); - // @todo Only explicit exports are supposed to write to FileStorage. - $file_storage->write($config_name, $data); } } } /** - * @todo http://drupal.org/node/1552396 renames this into config_load_all(). + * @todo Modules need a way to access the active store, whatever it is. */ function config_get_storage_names_with_prefix($prefix = '') { - return DatabaseStorage::getNamesWithPrefix($prefix); + $storage = new DatabaseStorage(); + return $storage->listAll($prefix); } /** @@ -77,22 +49,11 @@ function config_get_storage_names_with_prefix($prefix = '') { * The name of the configuration object to retrieve. The name corresponds to * a configuration file. For @code config(book.admin) @endcode, the config * object returned will contain the contents of book.admin configuration file. - * @param string $class - * (optional) The name of the class to use for the configuration object. - * Defaults to Drupal\Core\Config\ConfigObject. * - * @return Drupal\Core\Config\ConfigObject - * An instance of the class specified in the $class parameter. + * @return Drupal\Core\Config\Config + * A configuration object. */ -function config($name, $class = 'Drupal\Core\Config\ConfigObject') { - // @todo Factory needs to be instantiated to prevent statics from being shared - // with parent environment in tests. Replace with DI container. - $factory = &drupal_static(__FUNCTION__); - - if (!isset($factory)) { - $factory = new ConfigFactory(); - } - // @todo Do not reload an already loaded config object. - // @todo Make it possible to skip load for known to be new config objects? - return $factory->get($name, $class)->load(); +function config($name) { + return drupal_container()->get('config.factory')->get($name)->load(); } + diff --git a/core/includes/install.inc b/core/includes/install.inc index d0a776a..cd67257 100644 --- a/core/includes/install.inc +++ b/core/includes/install.inc @@ -1,7 +1,7 @@ delete($config_name); - } + // Remove all configuration belonging to the module. + $config_names = $storage->listAll($module . '.'); + foreach ($config_names as $config_name) { + config($config_name)->delete(); } watchdog('system', '%module module uninstalled.', array('%module' => $module), WATCHDOG_INFO); diff --git a/core/includes/module.inc b/core/includes/module.inc index a71cb18..e15cd95 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/includes/update.inc b/core/includes/update.inc index 3bdacd2..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 FileStorageReadException 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/ConfigObject.php b/core/lib/Drupal/Core/Config/Config.php similarity index 76% rename from core/lib/Drupal/Core/Config/ConfigObject.php rename to core/lib/Drupal/Core/Config/Config.php index 3b6569a..0748b76 100644 --- a/core/lib/Drupal/Core/Config/ConfigObject.php +++ b/core/lib/Drupal/Core/Config/Config.php @@ -1,11 +1,16 @@ setName($name); - $this->storageManager = $storageManager; + public function __construct(StorageDispatcher $storageDispatcher) { + $this->storageDispatcher = $storageDispatcher; } /** @@ -205,8 +208,11 @@ class ConfigObject { * Loads configuration data into this object. */ public function load() { - $data = $this->storageManager->selectStorage('read', $this->name)->read($this->name); - $this->setData($data !== FALSE ? $data : array()); + $this->setData(array()); + $data = $this->storageDispatcher->selectStorage('read', $this->name)->read($this->name); + if ($data !== FALSE) { + $this->setData($data); + } return $this; } @@ -214,16 +220,37 @@ class ConfigObject { * Saves the configuration object. */ public function save() { - $this->storageManager->selectStorage('write', $this->name)->write($this->name, $this->data); + $this->sortByKey($this->data); + $this->storageDispatcher->selectStorage('write', $this->name)->write($this->name, $this->data); return $this; } /** + * Sorts all keys in configuration data. + * + * Ensures that re-inserted keys appear in the same location as before, in + * order to ensure an identical order regardless of storage controller. + * A consistent order is important for any storage that allows any kind of + * diff operation. + * + * @param array $data + * An associative array to sort recursively by key name. + */ + public function sortByKey(array &$data) { + ksort($data); + foreach ($data as &$value) { + if (is_array($value)) { + $this->sortByKey($value); + } + } + } + + /** * Deletes the configuration object. */ public function delete() { $this->data = array(); - $this->storageManager->selectStorage('write', $this->name)->delete($this->name); + $this->storageDispatcher->selectStorage('write', $this->name)->delete($this->name); return $this; } } diff --git a/core/lib/Drupal/Core/Config/ConfigException.php b/core/lib/Drupal/Core/Config/ConfigException.php index c60a449..5e7daed 100644 --- a/core/lib/Drupal/Core/Config/ConfigException.php +++ b/core/lib/Drupal/Core/Config/ConfigException.php @@ -1,8 +1,15 @@ configClass = $config_class; + $this->storageDispatcher = $storage_dispatcher; + } /** * Returns a configuration object for a given name. * - * Configuration objects are instantiated once only. - * * @param string $name * The name of the configuration object to construct. - * @param string $class - * (optional) The name of the class to use for the configuration object. - * Defaults to Drupal\Core\Config\ConfigObject. - * @param string $manager_class - * (optional) The name of the storage manager class to use. Defaults to - * Drupal\Core\Config\StorageManager. * - * @return Drupal\Core\Config\ConfigObject + * @return Drupal\Core\Config\Config * A configuration object with the given $name. */ - public function get($name, $class = 'Drupal\Core\Config\ConfigObject', $manager_class = 'Drupal\Core\Config\StorageManager') { - // Instantiate the storage manager. - if (!isset($this->storageManagers[$manager_class])) { - $this->storageManagers[$manager_class] = new $manager_class(); - } - // Instantiate the configuration object, specific to the storage manager. - if (!isset($this->configObjects[$manager_class][$name])) { - $this->configObjects[$manager_class][$name] = new $class($name, $this->storageManagers[$manager_class]); - } - return $this->configObjects[$manager_class][$name]; + public function get($name) { + // @todo Caching the instantiated objects per name might cut off a fair + // amount of CPU time and memory. Only the data within the configuration + // object changes, so the additional cost of instantiating duplicate + // objects could possibly be avoided. It is not uncommon for a + // configuration object to be retrieved many times during a single + // request; e.g., 'system.performance' alone is retrieved around 10-20 + // times within a single page request. Sub-requests via HttpKernel will + // most likely only increase these counts. + // @todo Benchmarks were performed with a script that essentially retained + // all instantiated configuration objects in memory until script execution + // ended. A variant of that script called config() within a helper + // function only, which inherently meant that PHP destroyed all + // configuration objects after leaving the function. Consequently, + // benchmark results looked entirely different. Profiling should probably + // redone under more realistic conditions; e.g., actual HTTP requests. + // @todo The decrease of CPU time is interesting, since that means that + // ContainerBuilder involves plenty of function calls (which are known to + // be slow in PHP). + $config = new $this->configClass($this->storageDispatcher); + return $config->setName($name); } } diff --git a/core/lib/Drupal/Core/Config/DatabaseStorage.php b/core/lib/Drupal/Core/Config/DatabaseStorage.php index 27f5814..1dd8507 100644 --- a/core/lib/Drupal/Core/Config/DatabaseStorage.php +++ b/core/lib/Drupal/Core/Config/DatabaseStorage.php @@ -1,8 +1,12 @@ 'default', 'target' => 'default', ); - $this->options = $info; + $this->options = $options; } /** * Returns the database connection to use. */ protected function getConnection() { - return Database::getConnection($this->options['target']); + return Database::getConnection($this->options['target'], $this->options['connection']); } /** - * Implements StorageInterface::read(). + * Implements Drupal\Core\Config\StorageInterface::read(). + * + * @throws PDOException + * @throws Drupal\Core\Database\DatabaseExceptionWrapper + * Only thrown in case $this->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 +70,63 @@ class DatabaseStorage implements StorageInterface { } /** - * Implements StorageInterface::write(). + * Implements Drupal\Core\Config\StorageInterface::write(). + * + * @throws PDOException + * + * @todo Ignore slave targets for data manipulation operations. */ 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 + * + * @todo Ignore slave targets for data manipulation operations. */ 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. */ - static public function getNamesWithPrefix($prefix = '') { - return db_query('SELECT name FROM {config} WHERE name LIKE :name', array(':name' => db_like($prefix) . '%'))->fetchCol(); + 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 9c1fc07..7827dc0 100644 --- a/core/lib/Drupal/Core/Config/FileStorage.php +++ b/core/lib/Drupal/Core/Config/FileStorage.php @@ -1,8 +1,12 @@ info = $info; + $this->options = $options; } /** @@ -36,7 +40,7 @@ class FileStorage implements StorageInterface { * The path to the configuration file. */ public function getFilePath($name) { - return $this->info['directory'] . '/' . $name . '.' . self::getFileExtension(); + return $this->options['directory'] . '/' . $name . '.' . self::getFileExtension(); } /** @@ -60,45 +64,60 @@ class FileStorage implements StorageInterface { } /** - * Implements StorageInterface::write(). + * Implements Drupal\Core\Config\StorageInterface::read(). * - * @throws FileStorageException - */ - public function write($name, array $data) { - $data = $this->encode($data); - if (!file_put_contents($this->getFilePath($name), $data)) { - throw new FileStorageException('Failed to write configuration file: ' . $this->getFilePath($name)); - } - } - - /** - * Implements StorageInterface::read(). - * - * @throws FileStorageReadException + * @throws Symfony\Component\Yaml\Exception\ParseException */ public function read($name) { if (!$this->exists($name)) { - throw new FileStorageReadException("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 FileStorageReadException("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) { - // Needs error handling and etc. - @drupal_unlink($this->getFilePath($name)); + if (!$this->exists($name)) { + if (!file_exists($this->options['directory'])) { + throw new StorageException($this->options['directory'] . '/ not found.'); + } + 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 @@ -107,7 +126,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)) { @@ -117,14 +138,18 @@ class FileStorage implements StorageInterface { } /** - * Implements StorageInterface::getNamesWithPrefix(). - * - * @todo Allow to search for files in custom paths. + * Implements Drupal\Core\Config\StorageInterface::listAll(). */ - public static function getNamesWithPrefix($prefix = '') { - $files = glob(config_get_config_directory() . '/' . $prefix . '*.' . FileStorage::getFileExtension()); - $clean_name = function ($value) { - return basename($value, '.' . FileStorage::getFileExtension()); + public function listAll($prefix = '') { + // glob() silently ignores the error of a non-existing search directory, + // even with the GLOB_ERR flag. + if (!file_exists($this->options['directory'])) { + throw new StorageException($this->options['directory'] . '/ not found.'); + } + $extension = '.' . self::getFileExtension(); + $files = glob($this->options['directory'] . '/' . $prefix . '*' . $extension); + $clean_name = function ($value) use ($extension) { + return basename($value, $extension); }; return array_map($clean_name, $files); } diff --git a/core/lib/Drupal/Core/Config/FileStorageException.php b/core/lib/Drupal/Core/Config/FileStorageException.php deleted file mode 100644 index bf3ae5f..0000000 --- a/core/lib/Drupal/Core/Config/FileStorageException.php +++ /dev/null @@ -1,10 +0,0 @@ - array( @@ -59,37 +59,29 @@ class StorageManager { * ) * @endcode */ - public function __construct(array $storage_info = NULL) { - // @todo This is the wrong place to supply defaults. - if (!isset($storage_info)) { - $storage_info = array( - 'Drupal\Core\Config\DatabaseStorage' => array( - 'target' => 'default', - 'read' => TRUE, - 'write' => TRUE, - ), - 'Drupal\Core\Config\FileStorage' => array( - 'directory' => config_get_config_directory(), - 'read' => TRUE, - 'write' => TRUE, - ), - ); - } + public function __construct(array $storage_info) { $this->storageInfo = $storage_info; } /** * Returns a storage controller to use for a given operation. * - * Handles the core functionality of the configuration manager by determining - * which storage can handle a particular configuration object, depending on - * the operation being performed. + * Handles the core functionality of the storage dispatcher by determining + * which storage can handle a particular storage access operation and + * configuration object. * * @param string $access_operation * The operation access level; either 'read' or 'write'. Use 'write' both * for saving and deleting configuration. * @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) { // Determine the appropriate storage controller to use. diff --git a/core/lib/Drupal/Core/Config/StorageException.php b/core/lib/Drupal/Core/Config/StorageException.php new file mode 100644 index 0000000..b1e99a1 --- /dev/null +++ b/core/lib/Drupal/Core/Config/StorageException.php @@ -0,0 +1,13 @@ +register(LANGUAGE_TYPE_CONTENT, 'Drupal\\Core\\Language\\Language'); + + // Register configuration storage dispatcher. + $this->setParameter('config.storage.info', array( + 'Drupal\Core\Config\DatabaseStorage' => array( + 'connection' => 'default', + 'target' => 'default', + 'read' => TRUE, + 'write' => TRUE, + ), + 'Drupal\Core\Config\FileStorage' => array( + 'directory' => config_get_config_directory(), + 'read' => TRUE, + 'write' => FALSE, + ), + )); + $this->register('config.storage.dispatcher', 'Drupal\Core\Config\StorageDispatcher') + ->addArgument('%config.storage.info%'); + + // Register configuration object factory. + $this->register('config.factory', 'Drupal\Core\Config\ConfigFactory') + ->addArgument('Drupal\Core\Config\Config') + ->addArgument(new Reference('config.storage.dispatcher')); } } diff --git a/core/modules/config/lib/Drupal/config/Tests/ConfigCRUDTest.php b/core/modules/config/lib/Drupal/config/Tests/ConfigCRUDTest.php index 5e5b926..aeca024 100644 --- a/core/modules/config/lib/Drupal/config/Tests/ConfigCRUDTest.php +++ b/core/modules/config/lib/Drupal/config/Tests/ConfigCRUDTest.php @@ -2,7 +2,7 @@ /** * @file - * Definition of Drupal\config\Tests\ConfigUpgradeTest. + * Definition of Drupal\config\Tests\ConfigCRUDTest. */ namespace Drupal\config\Tests; @@ -50,9 +50,6 @@ class ConfigCRUDTest extends WebTestBase { $new_config = config($name); $this->assertIdentical($new_config->get(), $config->get()); - // Verify config() returned the existing config instance. - $this->assertIdentical($new_config, $config); - // Delete the configuration object. $config->delete(); @@ -67,9 +64,6 @@ class ConfigCRUDTest extends WebTestBase { $new_config = config($name); $this->assertIdentical($new_config->get(), $config->get()); - // Verify config() returned the existing config instance. - $this->assertIdentical($new_config, $config); - // Re-create the configuration object. $config->set('value', 're-created'); $config->save(); @@ -81,8 +75,32 @@ class ConfigCRUDTest extends WebTestBase { // Verify a call to config() immediately returns the updated value. $new_config = config($name); $this->assertIdentical($new_config->get(), $config->get()); + } - // Verify config() returned the existing config instance. - $this->assertIdentical($new_config, $config); + /** + * Tests Drupal\Core\Config\Config::sortByKey(). + */ + function testDataKeySort() { + $config = config('config_test.keysort'); + $config->set('new', 'Value to be replaced'); + $config->set('static', 'static'); + $config->save(); + // Clone this Config, so this test does not rely on any particular + // architecture. + $config = clone $config; + + // Load the configuration data into a new object. + $new_config = config('config_test.keysort'); + // Clear the 'new' key that came first. + $new_config->clear('new'); + // Add a new 'new' key and save. + $new_config->set('new', 'Value to be replaced'); + $new_config->save(); + + // Verify that the data of both objects is in the identical order. + // assertIdentical() is the required essence of this test; it performs a + // strict comparison, which means that keys and values must be identical and + // their order must be identical. + $this->assertIdentical($new_config->get(), $config->get()); } } diff --git a/core/modules/config/lib/Drupal/config/Tests/ConfigFileContentTest.php b/core/modules/config/lib/Drupal/config/Tests/ConfigFileContentTest.php index 36ab251..3add6d8 100644 --- a/core/modules/config/lib/Drupal/config/Tests/ConfigFileContentTest.php +++ b/core/modules/config/lib/Drupal/config/Tests/ConfigFileContentTest.php @@ -15,8 +15,6 @@ use Drupal\simpletest\WebTestBase; * Tests reading and writing file contents. */ class ConfigFileContentTest extends WebTestBase { - protected $fileExtension; - public static function getInfo() { return array( 'name' => 'File content', @@ -27,14 +25,14 @@ class ConfigFileContentTest extends WebTestBase { function setUp() { parent::setUp(); - - $this->fileExtension = FileStorage::getFileExtension(); } /** * Tests setting, writing, and reading of a configuration setting. */ function testReadWriteConfig() { + $database_storage = new DatabaseStorage(); + $name = 'foo.bar'; $key = 'foo'; $value = 'bar'; @@ -62,15 +60,15 @@ class ConfigFileContentTest extends WebTestBase { $config = config($name); // Verify an configuration object is returned. -// $this->assertEqual($config->name, $name); + $this->assertEqual($config->getName(), $name); $this->assertTrue($config, t('Config object created.')); // Verify the configuration object is empty. $this->assertEqual($config->get(), array(), t('New config object is empty.')); // Verify nothing was saved. - $db_config = db_query('SELECT * FROM {config} WHERE name = :name', array(':name' => $name))->fetch(); - $this->assertIdentical($db_config, FALSE, t('Active store does not have a record for %name', array('%name' => $name))); + $db_data = $database_storage->read($name); + $this->assertIdentical($db_data, array()); // Add a top level value $config = config($name); @@ -96,12 +94,12 @@ class ConfigFileContentTest extends WebTestBase { $config->save(); // Verify the database entry exists. - $db_config = db_query('SELECT * FROM {config} WHERE name = :name', array(':name' => $name))->fetch(); - $this->assertEqual($db_config->name, $name, t('After saving configuration, active store has a record for %name', array('%name' => $name))); + $db_data = $database_storage->read($name); + $this->assertTrue($db_data); // Read top level value $config = config($name); -// $this->assertEqual($config->name, $name); + $this->assertEqual($config->getName(), $name); $this->assertTrue($config, 'Config object created.'); $this->assertEqual($config->get($key), 'bar', t('Top level configuration value found.')); @@ -154,27 +152,27 @@ class ConfigFileContentTest extends WebTestBase { $config->set($key, $value)->save(); // Verify the database entry exists from a chained save. - $db_config = db_query('SELECT * FROM {config} WHERE name = :name', array(':name' => $chained_name))->fetch(); - $this->assertEqual($db_config->name, $chained_name, t('After saving configuration by chaining through set(), active store has a record for %name', array('%name' => $chained_name))); + $db_data = $database_storage->read($chained_name); + $this->assertEqual($db_data, $config->get()); // Get file listing for all files starting with 'foo'. Should return // two elements. - $files = DatabaseStorage::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 = DatabaseStorage::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 = DatabaseStorage::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 = DatabaseStorage::getNamesWithPrefix('bar'); + $files = $database_storage->listAll('bar'); $this->assertEqual($files, array(), 'No files listed with the prefix \'bar\'.'); // Delete the configuration. @@ -182,14 +180,14 @@ class ConfigFileContentTest extends WebTestBase { $config->delete(); // Verify the database entry no longer exists. - $db_config = db_query('SELECT * FROM {config} WHERE name = :name', array(':name' => $name))->fetch(); - $this->assertIdentical($db_config, FALSE); + $db_data = $database_storage->read($name); + $this->assertIdentical($db_data, array()); } /** * Tests serialization of configuration to file. */ - function testConfigSerialization() { + function testSerialization() { $name = $this->randomName(10) . '.' . $this->randomName(10); $config_data = array( // Indexed arrays; the order of elements is essential. diff --git a/core/modules/config/lib/Drupal/config/Tests/ConfigFileSecurityTest.php b/core/modules/config/lib/Drupal/config/Tests/ConfigFileSecurityTest.php deleted file mode 100644 index ae80fb2..0000000 --- a/core/modules/config/lib/Drupal/config/Tests/ConfigFileSecurityTest.php +++ /dev/null @@ -1,48 +0,0 @@ - 'Good morning, Denver!'); - - public static function getInfo() { - return array( - 'name' => 'File security', - 'description' => 'Tests security of saved configuration files.', - 'group' => 'Configuration', - ); - } - - /** - * Tests that a file written by this system can be successfully read back. - */ - function testFilePersist() { - $file = new FileStorage(); - $file->write($this->filename, $this->testContent); - - // Reading should throw an exception in case of bad validation. - // Note that if any other exception is thrown, we let the test system - // handle catching and reporting it. - try { - $saved_content = $file->read($this->filename); - - $this->assertEqual($saved_content, $this->testContent); - } - catch (Exception $e) { - $this->fail('File failed verification when being read.'); - } - } -} 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..3e6b254 --- /dev/null +++ b/core/modules/config/lib/Drupal/config/Tests/Storage/ConfigStorageTestBase.php @@ -0,0 +1,121 @@ +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) { + $class = get_class($e); + $this->pass($class . ' 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) { + $class = get_class($e); + $this->pass($class . ' 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) { + $class = get_class($e); + $this->pass($class . ' 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..2e1599f --- /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('connection' => '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/EnableDisableTest.php b/core/modules/system/lib/Drupal/system/Tests/Module/EnableDisableTest.php index 6319572..8afe33b 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Module/EnableDisableTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Module/EnableDisableTest.php @@ -106,7 +106,7 @@ class EnableDisableTest extends ModuleTestBase { $this->assertText(t('hook_modules_enabled fired for @module', array('@module' => $module_to_enable))); $this->assertModules(array($module_to_enable), TRUE); $this->assertModuleTablesExist($module_to_enable); - $this->assertModuleConfigFilesExist($module_to_enable); + $this->assertModuleConfig($module_to_enable); $this->assertLogMessage('system', "%module module installed.", array('%module' => $module_to_enable), WATCHDOG_INFO); $this->assertLogMessage('system', "%module module enabled.", array('%module' => $module_to_enable), WATCHDOG_INFO); } @@ -187,7 +187,7 @@ class EnableDisableTest extends ModuleTestBase { // Check that the module's database tables still exist. $this->assertModuleTablesExist($module); // Check that the module's config files still exist. - $this->assertModuleConfigFilesExist($module); + $this->assertModuleConfig($module); // Uninstall the module. $edit = array(); @@ -209,6 +209,6 @@ class EnableDisableTest extends ModuleTestBase { // Check that the module's database tables no longer exist. $this->assertModuleTablesDoNotExist($module); // Check that the module's config files no longer exist. - $this->assertModuleConfigFilesDoNotExist($module); + $this->assertNoModuleConfig($module); } } 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 3b2ed5c..8f372ca 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Module/ModuleTestBase.php +++ b/core/modules/system/lib/Drupal/system/Tests/Module/ModuleTestBase.php @@ -8,6 +8,7 @@ namespace Drupal\system\Tests\Module; use Drupal\Core\Database\Database; +use Drupal\Core\Config\DatabaseStorage; use Drupal\Core\Config\FileStorage; use Drupal\simpletest\WebTestBase; @@ -77,65 +78,49 @@ class ModuleTestBase extends WebTestBase { } /** - * Assert that a module's config files have been loaded. + * Asserts that the default configuration of a module has been installed. * * @param string $module * The name of the module. * * @return bool - * TRUE if the module's config files exist, FALSE otherwise. + * TRUE if configuration has been installed, FALSE otherwise. */ - function assertModuleConfigFilesExist($module) { - // Define test variable. - $files_exist = TRUE; - // Get the path to the module's config dir. + function assertModuleConfig($module) { $module_config_dir = drupal_get_path('module', $module) . '/config'; - if (is_dir($module_config_dir)) { - $files = glob($module_config_dir . '/*.' . FileStorage::getFileExtension()); - $this->assertTrue($files); - $config_dir = config_get_config_directory(); - // Get the filename of each config file. - foreach ($files as $file) { - $parts = explode('/', $file); - $filename = array_pop($parts); - if (!file_exists($config_dir . '/' . $filename)) { - $files_exist = FALSE; - } - } + if (!is_dir($module_config_dir)) { + return; } + $module_file_storage = new FileStorage(array('directory' => $module_config_dir)); + $names = $module_file_storage->listAll(); + + // Verify that the config directory is not empty. + $this->assertTrue($names); - return $this->assertTrue($files_exist, t('All config files defined by the @module module have been copied to the live config directory.', array('@module' => $module))); + // Look up each default configuration object name in the active store, and + // if it exists, remove it from the stack. + foreach ($names as $key => $name) { + if (config($name)->get()) { + unset($names[$key]); + } + } + // Verify that all configuration has been installed (which means that $names + // is empty). + return $this->assertFalse($names, format_string('All default configuration of @module module found.', array('@module' => $module))); } /** - * Assert that none of a module's default config files are loaded. + * Asserts that no configuration exists for a given module. * * @param string $module * The name of the module. * * @return bool - * TRUE if the module's config files do not exist, FALSE otherwise. + * TRUE if no configuration was found, FALSE otherwise. */ - function assertModuleConfigFilesDoNotExist($module) { - // Define test variable. - $files_exist = FALSE; - // Get the path to the module's config dir. - $module_config_dir = drupal_get_path('module', $module) . '/config'; - if (is_dir($module_config_dir)) { - $files = glob($module_config_dir . '/*.' . FileStorage::getFileExtension()); - $this->assertTrue($files); - $config_dir = config_get_config_directory(); - // Get the filename of each config file. - foreach ($files as $file) { - $parts = explode('/', $file); - $filename = array_pop($parts); - if (file_exists($config_dir . '/' . $filename)) { - $files_exist = TRUE; - } - } - } - - return $this->assertFalse($files_exist, t('All config files defined by the @module module have been deleted from the live config directory.', array('@module' => $module))); + function assertNoModuleConfig($module) { + $names = config_get_storage_names_with_prefix($module . '.'); + return $this->assertFalse($names, format_string('No configuration found for @module module.', array('@module' => $module))); } /** diff --git a/core/modules/system/system.module b/core/modules/system/system.module index b9558e5..32cfb75 100644 --- a/core/modules/system/system.module +++ b/core/modules/system/system.module @@ -379,7 +379,8 @@ function system_element_info() { $types['email'] = array( '#input' => TRUE, '#size' => 60, - '#maxlength' => EMAIL_MAX_LENGTH, + // user.module is not loaded in case of early bootstrap errors. + '#maxlength' => defined('EMAIL_MAX_LENGTH') ? EMAIL_MAX_LENGTH : 255, '#autocomplete_path' => FALSE, '#process' => array('form_process_autocomplete', 'ajax_process_form', 'form_process_pattern'), '#element_validate' => array('form_validate_email'),