Issue #1809206 by katbailey, chx, sun, Berdir: Fixed KeyValueFactory hard-codes DatabaseStorage.

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;
 }

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')

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();

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()) {
+  public function __construct($collection, Connection $connection, $table = 'key_value_expire') {
-    parent::__construct($collection, $options);
+    parent::__construct($collection, $connection, $table);
-    $this->connection = isset($options['connection']) ? $options['connection'] : Database::getConnection();
-    $this->table = isset($options['table']) ? $options['table'] : 'key_value_expire';
   }
 
   /**

core/lib/Drupal/Core/KeyValueStore/KeyValueDatabaseExpirableFactory.php

@@ -0,0 +1,33 @@
+<?php
+
+/**
+ * @file
+ * Contains Drupal\Core\KeyValueStore\KeyValueDatabaseExpirableFactory.
+ */
+
+namespace Drupal\Core\KeyValueStore;
+
+use Drupal\Core\Database\Connection;
+use Drupal\Core\Database\Database;
+use Drupal\Core\KeyValueStore\KeyValueDatabaseFactory;
+
+/**
+ * Defines the key/value store factory for the database backend.
+ */
+class KeyValueDatabaseExpirableFactory extends KeyValueDatabaseFactory {
+
+  /**
+   * Constructs a new key/value expirable 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\DatabaseStorageExpirable
+   *   A key/value store implementation for the given $collection.
+   */
+  public function get($collection) {
+    return new DatabaseStorageExpirable($collection, $this->connection);
+  }
+}

core/lib/Drupal/Core/KeyValueStore/KeyValueDatabaseFactory.php

@@ -0,0 +1,41 @@
+<?php
+
+/**
+ * @file
+ * Contains Drupal\Core\KeyValueStore\KeyValueDatabaseFactory.
+ */
+
+namespace Drupal\Core\KeyValueStore;
+use Drupal\Core\Database\Connection;
+use Drupal\Core\Database\Database;
+
+/**
+ * Defines the key/value store factory for the database backend.
+ */
+class KeyValueDatabaseFactory {
+
+  /**
+   * Constructs this factory object.
+   *
+   *
+   * @param \Drupal\Core\Database\Connection $connection
+   *   The Connection object containing the key-value tables.
+   */
+  function __construct(Connection $connection) {
+    $this->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);
+  }
+}

core/lib/Drupal/Core/KeyValueStore/KeyValueExpirableFactory.php

@@ -0,0 +1,43 @@
+<?php
+
+/**
+ * @file
+ * Contains Drupal\Core\KeyValueStore\KeyValueExpirableFactory.
+ */
+
+namespace Drupal\Core\KeyValueStore;
+
+use Symfony\Component\DependencyInjection\ContainerInterface;
+
+/**
+ * Defines the key/value store factory.
+ */
+class KeyValueExpirableFactory extends KeyValueFactory {
+
+  /**
+   * Constructs a new expirable 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\KeyValueStoreExpirableInterface
+   *   An expirable key/value store implementation for the given $collection.
+   */
+  public function get($collection) {
+    global $conf;
+    if (!isset($this->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];
+  }
+}
+

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];
   }
 }
+

core/lib/Drupal/Core/KeyValueStore/KeyValueMemoryFactory.php

@@ -0,0 +1,26 @@
+<?php
+
+/**
+ * @file
+ * Contains Drupal\Core\KeyValueStore\KeyValueMemoryFactory.
+ */
+
+namespace Drupal\Core\KeyValueStore;
+
+/**
+ * Defines the key/value store factory for the database backend.
+ */
+class KeyValueMemoryFactory {
+
+  /**
+   * Constructs a new key/value memory storage object for a given collection name.
+   *
+   * @param string $collection
+   *   The name of the collection holding key and value pairs.
+   * @return \Drupal\Core\KeyValueStore\MemoryStorage
+   *   A key/value store implementation for the given $collection.
+   */
+  public function get($collection) {
+    return new MemoryStorage($collection);
+  }
+}

core/modules/system/lib/Drupal/system/Tests/KeyValueStore/DatabaseStorageExpirableTest.php

@@ -7,18 +7,13 @@
 
 namespace Drupal\system\Tests\KeyValueStore;
 
+use Symfony\Component\DependencyInjection\Reference;
+
 /**
  * Tests the key-value database storage.
  */
 class DatabaseStorageExpirableTest extends StorageTestBase {
 
-  /**
-   * The name of the class to test.
-   *
-   * The tests themselves are in StorageTestBase and use this class.
-   */
-  protected $storageClass = 'Drupal\Core\KeyValueStore\DatabaseStorageExpirable';
-
   public static function getInfo() {
     return array(
       'name' => '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();
   }

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() {

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++) {

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();
+  }
+
 }

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;

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);
   }