From 0142ce1f40c4fe6ba026ab8ecc7a8f2f5f55d0cc Mon Sep 17 00:00:00 2001 From: catch Date: Tue, 30 Oct 2012 11:00:41 +0000 Subject: [PATCH] Issue #1809206 by katbailey, chx, sun, Berdir: Fixed KeyValueFactory hard-codes DatabaseStorage. --- core/includes/bootstrap.inc | 11 ++++- core/lib/Drupal/Core/CoreBundle.php | 4 -- .../Core/KeyValueStore/DatabaseStorage.php | 25 ++++++----- .../DatabaseStorageExpirable.php | 8 ++-- .../KeyValueDatabaseExpirableFactory.php | 33 ++++++++++++++ .../KeyValueStore/KeyValueDatabaseFactory.php | 41 ++++++++++++++++++ .../KeyValueExpirableFactory.php | 43 +++++++++++++++++++ .../Core/KeyValueStore/KeyValueFactory.php | 29 ++++++++++++- .../KeyValueStore/KeyValueMemoryFactory.php | 26 +++++++++++ .../DatabaseStorageExpirableTest.php | 24 ++++++++--- .../KeyValueStore/DatabaseStorageTest.php | 19 +++++--- .../KeyValueStore/GarbageCollectionTest.php | 5 ++- .../Tests/KeyValueStore/MemoryStorageTest.php | 28 ++++++++++-- .../Tests/KeyValueStore/StorageTestBase.php | 39 +++++++++++++---- .../user/lib/Drupal/user/TempStoreFactory.php | 2 +- 15 files changed, 286 insertions(+), 51 deletions(-) create mode 100644 core/lib/Drupal/Core/KeyValueStore/KeyValueDatabaseExpirableFactory.php create mode 100644 core/lib/Drupal/Core/KeyValueStore/KeyValueDatabaseFactory.php create mode 100644 core/lib/Drupal/Core/KeyValueStore/KeyValueExpirableFactory.php create mode 100644 core/lib/Drupal/Core/KeyValueStore/KeyValueMemoryFactory.php diff --git a/core/includes/bootstrap.inc b/core/includes/bootstrap.inc index dfb61e1514f..0dd19d571fe 100644 --- a/core/includes/bootstrap.inc +++ b/core/includes/bootstrap.inc @@ -2465,9 +2465,18 @@ function drupal_container(Container $new_container = NULL, $rebuild = FALSE) { ->register('config.storage.staging', 'Drupal\Core\Config\FileStorage') ->addArgument(config_get_config_directory(CONFIG_STAGING_DIRECTORY)); + // Register the service for the default database connection. + $container->register('database', 'Drupal\Core\Database\Connection') + ->setFactoryClass('Drupal\Core\Database\Database') + ->setFactoryMethod('getConnection') + ->addArgument('default'); // Register the KeyValueStore factory. $container - ->register('keyvalue', 'Drupal\Core\KeyValueStore\KeyValueFactory'); + ->register('keyvalue', 'Drupal\Core\KeyValueStore\KeyValueFactory') + ->addArgument(new Reference('service_container')); + $container + ->register('keyvalue.database', 'Drupal\Core\KeyValueStore\KeyValueDatabaseFactory') + ->addArgument(new Reference('database')); } return $container; } diff --git a/core/lib/Drupal/Core/CoreBundle.php b/core/lib/Drupal/Core/CoreBundle.php index ba6db3c1423..5a07c550d57 100644 --- a/core/lib/Drupal/Core/CoreBundle.php +++ b/core/lib/Drupal/Core/CoreBundle.php @@ -49,10 +49,6 @@ class CoreBundle extends Bundle $container->register('language_manager', 'Drupal\Core\Language\LanguageManager') ->addArgument(new Reference('request')) ->setScope('request'); - $container->register('database', 'Drupal\Core\Database\Connection') - ->setFactoryClass('Drupal\Core\Database\Database') - ->setFactoryMethod('getConnection') - ->addArgument('default'); $container->register('database.slave', 'Drupal\Core\Database\Connection') ->setFactoryClass('Drupal\Core\Database\Database') ->setFactoryMethod('getConnection') diff --git a/core/lib/Drupal/Core/KeyValueStore/DatabaseStorage.php b/core/lib/Drupal/Core/KeyValueStore/DatabaseStorage.php index c837796bd58..3ccda1478a7 100644 --- a/core/lib/Drupal/Core/KeyValueStore/DatabaseStorage.php +++ b/core/lib/Drupal/Core/KeyValueStore/DatabaseStorage.php @@ -8,19 +8,23 @@ namespace Drupal\Core\KeyValueStore; use Drupal\Core\Database\Query\Merge; +use Drupal\Core\Database\Connection; /** * Defines a default key/value store implementation. * * This is Drupal's default key/value store implementation. It uses the database * to store key/value data. - * - * @todo This class still calls db_* functions directly because it's needed - * very early, pre-Container. Once the early bootstrap dependencies are - * sorted out, consider using an injected database connection instead. */ class DatabaseStorage extends StorageBase { + /** + * The database connection. + * + * @var \Drupal\Core\Database\Connection + */ + protected $connection; + /** * The name of the SQL table to use. * @@ -36,8 +40,9 @@ class DatabaseStorage extends StorageBase { * @param string $table * The name of the SQL table to use, defaults to key_value. */ - public function __construct($collection, $table = 'key_value') { + public function __construct($collection, Connection $connection, $table = 'key_value') { parent::__construct($collection); + $this->connection = $connection; $this->table = $table; } @@ -47,7 +52,7 @@ class DatabaseStorage extends StorageBase { public function getMultiple(array $keys) { $values = array(); try { - $result = db_query('SELECT name, value FROM {' . db_escape_table($this->table) . '} WHERE name IN (:keys) AND collection = :collection', array(':keys' => $keys, ':collection' => $this->collection))->fetchAllAssoc('name'); + $result = $this->connection->query('SELECT name, value FROM {' . $this->connection->escapeTable($this->table) . '} WHERE name IN (:keys) AND collection = :collection', array(':keys' => $keys, ':collection' => $this->collection))->fetchAllAssoc('name'); foreach ($keys as $key) { if (isset($result[$key])) { $values[$key] = unserialize($result[$key]->value); @@ -66,7 +71,7 @@ class DatabaseStorage extends StorageBase { * Implements Drupal\Core\KeyValueStore\KeyValueStoreInterface::getAll(). */ public function getAll() { - $result = db_query('SELECT name, value FROM {' . db_escape_table($this->table) . '} WHERE collection = :collection', array(':collection' => $this->collection)); + $result = $this->connection->query('SELECT name, value FROM {' . $this->connection->escapeTable($this->table) . '} WHERE collection = :collection', array(':collection' => $this->collection)); $values = array(); foreach ($result as $item) { @@ -81,7 +86,7 @@ class DatabaseStorage extends StorageBase { * Implements Drupal\Core\KeyValueStore\KeyValueStoreInterface::set(). */ public function set($key, $value) { - db_merge($this->table) + $this->connection->merge($this->table) ->key(array( 'name' => $key, 'collection' => $this->collection, @@ -94,7 +99,7 @@ class DatabaseStorage extends StorageBase { * Implements Drupal\Core\KeyValueStore\KeyValueStoreInterface::setIfNotExists(). */ public function setIfNotExists($key, $value) { - $result = db_merge($this->table) + $result = $this->connection->merge($this->table) ->insertFields(array( 'collection' => $this->collection, 'name' => $key, @@ -112,7 +117,7 @@ class DatabaseStorage extends StorageBase { public function deleteMultiple(array $keys) { // Delete in chunks when a large array is passed. do { - db_delete($this->table) + $this->connection->delete($this->table) ->condition('name', array_splice($keys, 0, 1000)) ->condition('collection', $this->collection) ->execute(); diff --git a/core/lib/Drupal/Core/KeyValueStore/DatabaseStorageExpirable.php b/core/lib/Drupal/Core/KeyValueStore/DatabaseStorageExpirable.php index 0ce79346e9f..e9894703fdb 100644 --- a/core/lib/Drupal/Core/KeyValueStore/DatabaseStorageExpirable.php +++ b/core/lib/Drupal/Core/KeyValueStore/DatabaseStorageExpirable.php @@ -7,8 +7,8 @@ namespace Drupal\Core\KeyValueStore; +use Drupal\Core\Database\Connection; use Drupal\Core\Database\Query\Merge; -use Drupal\Core\Database\Database; /** * Defines a default key/value store implementation for expiring items. @@ -51,10 +51,8 @@ class DatabaseStorageExpirable extends DatabaseStorage implements KeyValueStoreE * - table: (optional) The name of the SQL table to use. Defaults to * key_value_expire. */ - public function __construct($collection, array $options = array()) { - parent::__construct($collection, $options); - $this->connection = isset($options['connection']) ? $options['connection'] : Database::getConnection(); - $this->table = isset($options['table']) ? $options['table'] : 'key_value_expire'; + public function __construct($collection, Connection $connection, $table = 'key_value_expire') { + parent::__construct($collection, $connection, $table); } /** diff --git a/core/lib/Drupal/Core/KeyValueStore/KeyValueDatabaseExpirableFactory.php b/core/lib/Drupal/Core/KeyValueStore/KeyValueDatabaseExpirableFactory.php new file mode 100644 index 00000000000..08e673e7544 --- /dev/null +++ b/core/lib/Drupal/Core/KeyValueStore/KeyValueDatabaseExpirableFactory.php @@ -0,0 +1,33 @@ +connection); + } +} diff --git a/core/lib/Drupal/Core/KeyValueStore/KeyValueDatabaseFactory.php b/core/lib/Drupal/Core/KeyValueStore/KeyValueDatabaseFactory.php new file mode 100644 index 00000000000..ded528e128e --- /dev/null +++ b/core/lib/Drupal/Core/KeyValueStore/KeyValueDatabaseFactory.php @@ -0,0 +1,41 @@ +connection = $connection; + } + + /** + * Constructs a new key/value database storage object for a given collection name. + * + * @param string $collection + * The name of the collection holding key and value pairs. + * @param \Drupal\Core\Database\Connection $connection + * The connection to run against. + * @return \Drupal\Core\KeyValueStore\DatabaseStorage + * A key/value store implementation for the given $collection. + */ + public function get($collection) { + return new DatabaseStorage($collection, $this->connection); + } +} diff --git a/core/lib/Drupal/Core/KeyValueStore/KeyValueExpirableFactory.php b/core/lib/Drupal/Core/KeyValueStore/KeyValueExpirableFactory.php new file mode 100644 index 00000000000..8747e53033a --- /dev/null +++ b/core/lib/Drupal/Core/KeyValueStore/KeyValueExpirableFactory.php @@ -0,0 +1,43 @@ +stores[$collection])) { + if (isset($conf['keyvalue_expirable_service_' . $collection])) { + $service_name = $conf['keyvalue_expirable_service_' . $collection]; + } + elseif (isset($conf['keyvalue_expirable_default'])) { + $service_name = $conf['keyvalue_expirable_default']; + } + else { + $service_name = 'keyvalue.expirable.database'; + } + $this->stores[$collection] = $this->container->get($service_name)->get($collection); + } + return $this->stores[$collection]; + } +} + diff --git a/core/lib/Drupal/Core/KeyValueStore/KeyValueFactory.php b/core/lib/Drupal/Core/KeyValueStore/KeyValueFactory.php index 7edb75404fe..4e3f0a7d87a 100644 --- a/core/lib/Drupal/Core/KeyValueStore/KeyValueFactory.php +++ b/core/lib/Drupal/Core/KeyValueStore/KeyValueFactory.php @@ -6,6 +6,7 @@ */ namespace Drupal\Core\KeyValueStore; +use Symfony\Component\DependencyInjection\ContainerInterface; /** * Defines the key/value store factory. @@ -19,19 +20,43 @@ class KeyValueFactory { */ protected $stores = array(); + /** + * var \Symfony\Component\DependencyInjection\ContainerInterface + */ + protected $container; + + + /** + * @param \Symfony\Component\DependencyInjection\ContainerInterface $container + */ + function __construct(ContainerInterface $container) { + $this->container = $container; + } + /** * Constructs a new key/value store for a given collection name. * * @param string $collection * The name of the collection holding key and value pairs. * - * @return Drupal\Core\KeyValueStore\DatabaseStorage + * @return \Drupal\Core\KeyValueStore\KeyValueStoreInterface * A key/value store implementation for the given $collection. */ public function get($collection) { + global $conf; if (!isset($this->stores[$collection])) { - $this->stores[$collection] = new DatabaseStorage($collection); + if (isset($conf['keyvalue_service_' . $collection])) { + $service_name = $conf['keyvalue_service_' . $collection]; + } + elseif (isset($conf['keyvalue_default'])) { + $service_name = $conf['keyvalue_default']; + } + else { + $service_name = 'keyvalue.database'; + } + $this->stores[$collection] = $this->container->get($service_name)->get($collection); } return $this->stores[$collection]; } } + diff --git a/core/lib/Drupal/Core/KeyValueStore/KeyValueMemoryFactory.php b/core/lib/Drupal/Core/KeyValueStore/KeyValueMemoryFactory.php new file mode 100644 index 00000000000..e5b7fc05f69 --- /dev/null +++ b/core/lib/Drupal/Core/KeyValueStore/KeyValueMemoryFactory.php @@ -0,0 +1,26 @@ + 'Expirable database storage', @@ -29,12 +24,27 @@ class DatabaseStorageExpirableTest extends StorageTestBase { protected function setUp() { parent::setUp(); + $this->factory = 'keyvalue.expirable'; module_load_install('system'); $schema = system_schema(); db_create_table('key_value_expire', $schema['key_value_expire']); + $this->container + ->register('database', 'Drupal\Core\Database\Connection') + ->setFactoryClass('Drupal\Core\Database\Database') + ->setFactoryMethod('getConnection') + ->addArgument('default'); + $this->container + ->register('keyvalue.expirable.database', 'Drupal\Core\KeyValueStore\KeyValueDatabaseExpirableFactory') + ->addArgument(new Reference('database')); + global $conf; + $conf['keyvalue_expirable_default'] = 'keyvalue.expirable.database'; } protected function tearDown() { + // The DatabaseExpirableStorage class has a destructor which deletes rows + // from the key_value_expire table. We need to make sure the destructor + // runs before the table is deleted. + $this->container->set('keyvalue.expirable', NULL); db_drop_table('key_value_expire'); parent::tearDown(); } diff --git a/core/modules/system/lib/Drupal/system/Tests/KeyValueStore/DatabaseStorageTest.php b/core/modules/system/lib/Drupal/system/Tests/KeyValueStore/DatabaseStorageTest.php index 374b6c86044..a76281a915d 100644 --- a/core/modules/system/lib/Drupal/system/Tests/KeyValueStore/DatabaseStorageTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/KeyValueStore/DatabaseStorageTest.php @@ -7,18 +7,13 @@ namespace Drupal\system\Tests\KeyValueStore; +use Symfony\Component\DependencyInjection\Reference; + /** * Tests the key-value database storage. */ class DatabaseStorageTest extends StorageTestBase { - /** - * The name of the class to test. - * - * The tests themselves are in StorageTestBase and use this class. - */ - protected $storageClass = 'Drupal\Core\KeyValueStore\DatabaseStorage'; - public static function getInfo() { return array( 'name' => 'Database storage', @@ -32,6 +27,16 @@ class DatabaseStorageTest extends StorageTestBase { module_load_install('system'); $schema = system_schema(); db_create_table('key_value', $schema['key_value']); + $this->container + ->register('database', 'Drupal\Core\Database\Connection') + ->setFactoryClass('Drupal\Core\Database\Database') + ->setFactoryMethod('getConnection') + ->addArgument('default'); + $this->container + ->register('keyvalue.database', 'Drupal\Core\KeyValueStore\KeyValueDatabaseFactory') + ->addArgument(new Reference('database')); + global $conf; + $conf['keyvalue_default'] = 'keyvalue.database'; } protected function tearDown() { diff --git a/core/modules/system/lib/Drupal/system/Tests/KeyValueStore/GarbageCollectionTest.php b/core/modules/system/lib/Drupal/system/Tests/KeyValueStore/GarbageCollectionTest.php index e5c1c20dc61..6334382ec85 100644 --- a/core/modules/system/lib/Drupal/system/Tests/KeyValueStore/GarbageCollectionTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/KeyValueStore/GarbageCollectionTest.php @@ -7,8 +7,9 @@ namespace Drupal\system\Tests\KeyValueStore; -use Drupal\simpletest\UnitTestBase; +use Drupal\Core\Database\Database; use Drupal\Core\KeyValueStore\DatabaseStorageExpirable; +use Drupal\simpletest\UnitTestBase; /** * Tests garbage collection for DatabaseStorageExpirable. @@ -40,7 +41,7 @@ class GarbageCollectionTest extends UnitTestBase { */ public function testGarbageCollection() { $collection = $this->randomName(); - $store = new DatabaseStorageExpirable($collection); + $store = new DatabaseStorageExpirable($collection, Database::getConnection()); // Insert some items and confirm that they're set. for ($i = 0; $i <= 3; $i++) { diff --git a/core/modules/system/lib/Drupal/system/Tests/KeyValueStore/MemoryStorageTest.php b/core/modules/system/lib/Drupal/system/Tests/KeyValueStore/MemoryStorageTest.php index e3d6b081c17..5f1f0ba5aba 100644 --- a/core/modules/system/lib/Drupal/system/Tests/KeyValueStore/MemoryStorageTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/KeyValueStore/MemoryStorageTest.php @@ -13,11 +13,11 @@ namespace Drupal\system\Tests\KeyValueStore; class MemoryStorageTest extends StorageTestBase { /** - * The name of the class to test. + * Holds the original default key/value service name. * - * The tests themselves are in StorageTestBase and use this class. + * @var String */ - protected $storageClass = 'Drupal\Core\KeyValueStore\MemoryStorage'; + protected $originalKeyValue = NULL; public static function getInfo() { return array( @@ -27,4 +27,26 @@ class MemoryStorageTest extends StorageTestBase { ); } + protected function setUp() { + parent::setUp(); + $this->container + ->register('keyvalue.memory', 'Drupal\Core\KeyValueStore\KeyValueMemoryFactory'); + global $conf; + if (isset($conf['keyvalue_default'])) { + $this->originalKeyValue = $conf['keyvalue_default']; + } + $conf['keyvalue_default'] = 'keyvalue.memory'; + } + + protected function tearDown() { + global $conf; + if (isset($this->originalKeyValue)) { + $conf['keyvalue_default'] = $this->originalKeyValue; + } + else { + unset($conf['keyvalue_default']); + } + parent::tearDown(); + } + } diff --git a/core/modules/system/lib/Drupal/system/Tests/KeyValueStore/StorageTestBase.php b/core/modules/system/lib/Drupal/system/Tests/KeyValueStore/StorageTestBase.php index 1e7b05c9c57..9bd2412f54d 100644 --- a/core/modules/system/lib/Drupal/system/Tests/KeyValueStore/StorageTestBase.php +++ b/core/modules/system/lib/Drupal/system/Tests/KeyValueStore/StorageTestBase.php @@ -8,19 +8,14 @@ namespace Drupal\system\Tests\KeyValueStore; use Drupal\simpletest\UnitTestBase; +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Reference; /** * Base class for testing key-value storages. */ abstract class StorageTestBase extends UnitTestBase { - /** - * The fully qualified class name of the key-value storage to test. - * - * @var string - */ - protected $storageClass; - /** * An array of random stdClass objects. * @@ -35,9 +30,34 @@ abstract class StorageTestBase extends UnitTestBase { */ protected $collections = array(); + /** + * Whether we are using an expirable key/value store. + * + * @var boolean + */ + protected $factory = 'keyvalue'; + + /** + * A container for the services needed in these tests. + * + * @var ContainerBuilder + */ + protected $container; + protected function setUp() { parent::setUp(); + $this->container = new ContainerBuilder(); + $this->container + ->register('service_container', 'Symfony\Component\DependencyInjection\ContainerBuilder') + ->setSynthetic(TRUE); + $this->container->set('service_container', $this->container); + $this->container + ->register('keyvalue', 'Drupal\Core\KeyValueStore\KeyValueFactory') + ->addArgument(new Reference('service_container')); + $this->container + ->register('keyvalue.expirable', 'Drupal\Core\KeyValueStore\KeyValueExpirableFactory') + ->addArgument(new Reference('service_container')); // Define two data collections, $this->collections = array(0 => 'zero', 1 => 'one'); @@ -52,7 +72,6 @@ abstract class StorageTestBase extends UnitTestBase { */ public function testCRUD() { $stores = $this->createStorage(); - // Verify that each store returns its own collection name. $this->assertIdentical($stores[0]->getCollectionName(), $this->collections[0]); $this->assertIdentical($stores[1]->getCollectionName(), $this->collections[1]); @@ -120,12 +139,14 @@ abstract class StorageTestBase extends UnitTestBase { $this->assertFalse($stores[0]->getMultiple(array('foo', 'bar'))); // Verify that the item in the other collection still exists. $this->assertIdenticalObject($this->objects[5], $stores[1]->get('foo')); + } /** * Tests expected behavior for non-existing keys. */ public function testNonExistingKeys() { + $stores = $this->createStorage(); // Verify that a non-existing key returns NULL as value. @@ -187,7 +208,7 @@ abstract class StorageTestBase extends UnitTestBase { protected function createStorage() { $stores = array(); foreach ($this->collections as $i => $collection) { - $stores[$i] = new $this->storageClass($collection); + $stores[$i] = $this->container->get($this->factory)->get($collection); } return $stores; diff --git a/core/modules/user/lib/Drupal/user/TempStoreFactory.php b/core/modules/user/lib/Drupal/user/TempStoreFactory.php index 473d515b2f8..ddd1a752047 100644 --- a/core/modules/user/lib/Drupal/user/TempStoreFactory.php +++ b/core/modules/user/lib/Drupal/user/TempStoreFactory.php @@ -65,7 +65,7 @@ class TempStoreFactory { } // Store the data for this collection in the database. - $storage = new DatabaseStorageExpirable($collection, array('connection' => $this->connection)); + $storage = new DatabaseStorageExpirable($collection, $this->connection); return new TempStore($storage, $this->lockBackend, $owner); }