From edb9ffa33e6d41a555ebb231f78b3c4cdd9dd846 Mon Sep 17 00:00:00 2001
From: webchick <webchick@24967.no-reply.drupal.org>
Date: Tue, 9 Oct 2012 21:05:28 -0700
Subject: [PATCH] Issue #1805444 by xjm: GarbageCollection followups to
 TempStore.

---
 .../DatabaseStorageExpirable.php              |  34 ++++-
 .../DatabaseStorageExpirableTest.php          |  93 ++++++-------
 .../KeyValueStore/GarbageCollectionTest.php   |  79 +++++++++++
 .../Tests/KeyValueStore/StorageTestBase.php   | 125 +++++++++++-------
 .../user/Tests/TempStoreDatabaseTest.php      |  83 ++++++------
 5 files changed, 270 insertions(+), 144 deletions(-)
 create mode 100644 core/modules/system/lib/Drupal/system/Tests/KeyValueStore/GarbageCollectionTest.php

diff --git a/core/lib/Drupal/Core/KeyValueStore/DatabaseStorageExpirable.php b/core/lib/Drupal/Core/KeyValueStore/DatabaseStorageExpirable.php
index eb738787d147..0ce79346e9f0 100644
--- a/core/lib/Drupal/Core/KeyValueStore/DatabaseStorageExpirable.php
+++ b/core/lib/Drupal/Core/KeyValueStore/DatabaseStorageExpirable.php
@@ -25,6 +25,19 @@ class DatabaseStorageExpirable extends DatabaseStorage implements KeyValueStoreE
    */
   protected $connection;
 
+  /**
+   * Flag indicating whether garbage collection should be performed.
+   *
+   * When this flag is TRUE, garbage collection happens at the end of the
+   * request when the object is destructed. The flag is set during set and
+   * delete operations for expirable data, when a write to the table is already
+   * being performed. This eliminates the need for an external system to remove
+   * stale data.
+   *
+   * @var bool
+   */
+  protected $needsGarbageCollection = FALSE;
+
   /**
    * Overrides Drupal\Core\KeyValueStore\StorageBase::__construct().
    *
@@ -44,6 +57,15 @@ public function __construct($collection, array $options = array()) {
     $this->table = isset($options['table']) ? $options['table'] : 'key_value_expire';
   }
 
+  /**
+   * Performs garbage collection as needed when destructing the storage object.
+   */
+  public function __destruct() {
+    if ($this->needsGarbageCollection) {
+      $this->garbageCollection();
+    }
+  }
+
   /**
    * Implements Drupal\Core\KeyValueStore\KeyValueStoreInterface::getMultiple().
    */
@@ -75,6 +97,9 @@ public function getAll() {
    * Implements Drupal\Core\KeyValueStore\KeyValueStoreExpireInterface::setWithExpire().
    */
   function setWithExpire($key, $value, $expire) {
+    // We are already writing to the table, so perform garbage collection at
+    // the end of this request.
+    $this->needsGarbageCollection = TRUE;
     $this->connection->merge($this->table)
       ->key(array(
         'name' => $key,
@@ -91,6 +116,9 @@ function setWithExpire($key, $value, $expire) {
    * Implements Drupal\Core\KeyValueStore\KeyValueStoreExpirableInterface::setWithExpireIfNotExists().
    */
   function setWithExpireIfNotExists($key, $value, $expire) {
+    // We are already writing to the table, so perform garbage collection at
+    // the end of this request.
+    $this->needsGarbageCollection = TRUE;
     $result = $this->connection->merge($this->table)
       ->insertFields(array(
         'collection' => $this->collection,
@@ -117,14 +145,16 @@ function setMultipleWithExpire(array $data, $expire) {
    * Implements Drupal\Core\KeyValueStore\KeyValueStoreInterface::deleteMultiple().
    */
   public function deleteMultiple(array $keys) {
-    $this->garbageCollection();
+    // We are already writing to the table, so perform garbage collection at
+    // the end of this request.
+    $this->needsGarbageCollection = TRUE;
     parent::deleteMultiple($keys);
   }
 
   /**
    * Deletes expired items.
    */
-  public function garbageCollection() {
+  protected function garbageCollection() {
     $this->connection->delete($this->table)
       ->condition('expire', REQUEST_TIME, '<')
       ->execute();
diff --git a/core/modules/system/lib/Drupal/system/Tests/KeyValueStore/DatabaseStorageExpirableTest.php b/core/modules/system/lib/Drupal/system/Tests/KeyValueStore/DatabaseStorageExpirableTest.php
index b173445bf5ae..2a4c7a7d803f 100644
--- a/core/modules/system/lib/Drupal/system/Tests/KeyValueStore/DatabaseStorageExpirableTest.php
+++ b/core/modules/system/lib/Drupal/system/Tests/KeyValueStore/DatabaseStorageExpirableTest.php
@@ -43,125 +43,118 @@ protected function tearDown() {
    * Tests CRUD functionality with expiration.
    */
   public function testCRUDWithExpiration() {
+    $stores = $this->createStorage();
+
     // Verify that an item can be stored with setWithExpire().
     // Use a random expiration in each test.
-    $this->store1->setWithExpire('foo', $this->objects[0], rand(500, 299792458));
-    $this->assertIdenticalObject($this->objects[0], $this->store1->get('foo'));
+    $stores[0]->setWithExpire('foo', $this->objects[0], rand(500, 299792458));
+    $this->assertIdenticalObject($this->objects[0], $stores[0]->get('foo'));
     // Verify that the other collection is not affected.
-    $this->assertFalse($this->store2->get('foo'));
+    $this->assertFalse($stores[1]->get('foo'));
 
     // Verify that an item can be updated with setWithExpire().
-    $this->store1->setWithExpire('foo', $this->objects[1], rand(500, 299792458));
-    $this->assertIdenticalObject($this->objects[1], $this->store1->get('foo'));
+    $stores[0]->setWithExpire('foo', $this->objects[1], rand(500, 299792458));
+    $this->assertIdenticalObject($this->objects[1], $stores[0]->get('foo'));
     // Verify that the other collection is still not affected.
-    $this->assertFalse($this->store2->get('foo'));
+    $this->assertFalse($stores[1]->get('foo'));
 
     // Verify that the expirable data key is unique.
-    $this->store2->setWithExpire('foo', $this->objects[2], rand(500, 299792458));
-    $this->assertIdenticalObject($this->objects[1], $this->store1->get('foo'));
-    $this->assertIdenticalObject($this->objects[2], $this->store2->get('foo'));
+    $stores[1]->setWithExpire('foo', $this->objects[2], rand(500, 299792458));
+    $this->assertIdenticalObject($this->objects[1], $stores[0]->get('foo'));
+    $this->assertIdenticalObject($this->objects[2], $stores[1]->get('foo'));
 
     // Verify that multiple items can be stored with setMultipleWithExpire().
     $values = array(
       'foo' => $this->objects[3],
       'bar' => $this->objects[4],
     );
-    $this->store1->setMultipleWithExpire($values, rand(500, 299792458));
-    $result = $this->store1->getMultiple(array('foo', 'bar'));
+    $stores[0]->setMultipleWithExpire($values, rand(500, 299792458));
+    $result = $stores[0]->getMultiple(array('foo', 'bar'));
     foreach ($values as $j => $value) {
       $this->assertIdenticalObject($value, $result[$j]);
     }
 
     // Verify that the other collection was not affected.
-    $this->assertIdenticalObject($this->store2->get('foo'), $this->objects[2]);
-    $this->assertFalse($this->store2->get('bar'));
+    $this->assertIdenticalObject($stores[1]->get('foo'), $this->objects[2]);
+    $this->assertFalse($stores[1]->get('bar'));
 
     // Verify that all items in a collection can be retrieved.
     // Ensure that an item with the same name exists in the other collection.
-    $this->store2->set('foo', $this->objects[5]);
-    $result = $this->store1->getAll();
+    $stores[1]->set('foo', $this->objects[5]);
+    $result = $stores[0]->getAll();
     // Not using assertIdentical(), since the order is not defined for getAll().
     $this->assertEqual(count($result), count($values));
     foreach ($result as $key => $value) {
       $this->assertEqual($values[$key], $value);
     }
     // Verify that all items in the other collection are different.
-    $result = $this->store2->getAll();
+    $result = $stores[1]->getAll();
     $this->assertEqual($result, array('foo' => $this->objects[5]));
 
     // Verify that multiple items can be deleted.
-    $this->store1->deleteMultiple(array_keys($values));
-    $this->assertFalse($this->store1->get('foo'));
-    $this->assertFalse($this->store1->get('bar'));
-    $this->assertFalse($this->store1->getMultiple(array('foo', 'bar')));
+    $stores[0]->deleteMultiple(array_keys($values));
+    $this->assertFalse($stores[0]->get('foo'));
+    $this->assertFalse($stores[0]->get('bar'));
+    $this->assertFalse($stores[0]->getMultiple(array('foo', 'bar')));
     // Verify that the item in the other collection still exists.
-    $this->assertIdenticalObject($this->objects[5], $this->store2->get('foo'));
+    $this->assertIdenticalObject($this->objects[5], $stores[1]->get('foo'));
 
     // Test that setWithExpireIfNotExists() succeeds only the first time.
     $key = $this->randomName();
     for ($i = 0; $i <= 1; $i++) {
       // setWithExpireIfNotExists() should be TRUE the first time (when $i is
       // 0) and FALSE the second time (when $i is 1).
-      $this->assertEqual(!$i, $this->store1->setWithExpireIfNotExists($key, $this->objects[$i], rand(500, 299792458)));
-      $this->assertIdenticalObject($this->objects[0], $this->store1->get($key));
+      $this->assertEqual(!$i, $stores[0]->setWithExpireIfNotExists($key, $this->objects[$i], rand(500, 299792458)));
+      $this->assertIdenticalObject($this->objects[0], $stores[0]->get($key));
       // Verify that the other collection is not affected.
-      $this->assertFalse($this->store2->get($key));
+      $this->assertFalse($stores[1]->get($key));
     }
 
     // Remove the item and try to set it again.
-    $this->store1->delete($key);
-    $this->store1->setWithExpireIfNotExists($key, $this->objects[1], rand(500, 299792458));
+    $stores[0]->delete($key);
+    $stores[0]->setWithExpireIfNotExists($key, $this->objects[1], rand(500, 299792458));
     // This time it should succeed.
-    $this->assertIdenticalObject($this->objects[1], $this->store1->get($key));
+    $this->assertIdenticalObject($this->objects[1], $stores[0]->get($key));
     // Verify that the other collection is still not affected.
-    $this->assertFalse($this->store2->get($key));
+    $this->assertFalse($stores[1]->get($key));
 
   }
 
   /**
-   * Tests data expiration and garbage collection.
+   * Tests data expiration.
    */
   public function testExpiration() {
+    $stores = $this->createStorage();
     $day = 604800;
 
     // Set an item to expire in the past and another without an expiration.
-    $this->store1->setWithExpire('yesterday', 'all my troubles seemed so far away', -1 * $day);
-    $this->store1->set('troubles', 'here to stay');
+    $stores[0]->setWithExpire('yesterday', 'all my troubles seemed so far away', -1 * $day);
+    $stores[0]->set('troubles', 'here to stay');
 
     // Only the non-expired item should be returned.
-    $this->assertFalse($this->store1->get('yesterday'));
-    $this->assertIdentical($this->store1->get('troubles'), 'here to stay');
-    $this->assertIdentical(count($this->store1->getMultiple(array('yesterday', 'troubles'))), 1);
+    $this->assertFalse($stores[0]->get('yesterday'));
+    $this->assertIdentical($stores[0]->get('troubles'), 'here to stay');
+    $this->assertIdentical(count($stores[0]->getMultiple(array('yesterday', 'troubles'))), 1);
 
     // Store items set to expire in the past in various ways.
-    $this->store1->setWithExpire($this->randomName(), $this->objects[0], -7 * $day);
-    $this->store1->setWithExpireIfNotExists($this->randomName(), $this->objects[1], -5 * $day);
-    $this->store1->setMultipleWithExpire(
+    $stores[0]->setWithExpire($this->randomName(), $this->objects[0], -7 * $day);
+    $stores[0]->setWithExpireIfNotExists($this->randomName(), $this->objects[1], -5 * $day);
+    $stores[0]->setMultipleWithExpire(
       array(
         $this->randomName() => $this->objects[2],
         $this->randomName() => $this->objects[3],
       ),
       -3 * $day
     );
-    $this->store1->setWithExpireIfNotExists('yesterday', "you'd forgiven me", -1 * $day);
-    $this->store1->setWithExpire('still', "'til we say we're sorry", 2 * $day);
+    $stores[0]->setWithExpireIfNotExists('yesterday', "you'd forgiven me", -1 * $day);
+    $stores[0]->setWithExpire('still', "'til we say we're sorry", 2 * $day);
 
     // Ensure only non-expired items are retrived.
-    $all = $this->store1->getAll();
+    $all = $stores[0]->getAll();
     $this->assertIdentical(count($all), 2);
     foreach (array('troubles', 'still') as $key) {
       $this->assertTrue(!empty($all[$key]));
     }
-
-    // Perform garbage collection and confirm that the expired items are
-    // deleted from the database.
-    $this->store1->garbageCollection();
-    $result = db_query(
-      'SELECT name, value FROM {key_value_expire} WHERE collection = :collection',
-      array(
-        ':collection' => $this->collection1,
-      ))->fetchAll();
-    $this->assertIdentical(sizeof($result), 2);
   }
 
 }
diff --git a/core/modules/system/lib/Drupal/system/Tests/KeyValueStore/GarbageCollectionTest.php b/core/modules/system/lib/Drupal/system/Tests/KeyValueStore/GarbageCollectionTest.php
new file mode 100644
index 000000000000..e5c1c20dc610
--- /dev/null
+++ b/core/modules/system/lib/Drupal/system/Tests/KeyValueStore/GarbageCollectionTest.php
@@ -0,0 +1,79 @@
+<?php
+
+/**
+ * @file
+ * Contains Drupal\system\Tests\KeyValueStore\GarbageCollectionTest.
+ */
+
+namespace Drupal\system\Tests\KeyValueStore;
+
+use Drupal\simpletest\UnitTestBase;
+use Drupal\Core\KeyValueStore\DatabaseStorageExpirable;
+
+/**
+ * Tests garbage collection for DatabaseStorageExpirable.
+ */
+class GarbageCollectionTest extends UnitTestBase {
+
+  public static function getInfo() {
+    return array(
+      'name' => 'Garbage collection',
+      'description' => 'Tests garbage collection for the the expirable key-value database storage.',
+      'group' => 'Key-value store',
+    );
+  }
+
+  protected function setUp() {
+    parent::setUp();
+    module_load_install('system');
+    $schema = system_schema();
+    db_create_table('key_value_expire', $schema['key_value_expire']);
+  }
+
+  protected function tearDown() {
+    db_drop_table('key_value_expire');
+    parent::tearDown();
+  }
+
+  /**
+   * Tests garbage collection.
+   */
+  public function testGarbageCollection() {
+    $collection = $this->randomName();
+    $store = new DatabaseStorageExpirable($collection);
+
+    // Insert some items and confirm that they're set.
+    for ($i = 0; $i <= 3; $i++) {
+      $store->setWithExpire('key_' . $i, $this->randomObject(), rand(500, 100000));
+    }
+    $this->assertIdentical(sizeof($store->getAll()), 4, 'Four items were written to the storage.');
+
+    // Manually expire the data.
+    for ($i = 0; $i <= 3; $i++) {
+      db_merge('key_value_expire')
+        ->key(array(
+            'name' => 'key_' . $i,
+            'collection' => $collection,
+          ))
+        ->fields(array(
+            'expire' => REQUEST_TIME - 1,
+          ))
+        ->execute();
+    }
+
+    // Perform a new set operation and then manually unset the object to
+    // trigger garbage collection.
+    $store->setWithExpire('autumn', 'winter', rand(500, 1000000));
+    unset($store);
+
+    // Query the database and confirm that the stale records were deleted.
+    $result = db_query(
+      'SELECT name, value FROM {key_value_expire} WHERE collection = :collection',
+      array(
+        ':collection' => $collection,
+      ))->fetchAll();
+    $this->assertIdentical(sizeof($result), 1, 'Only one item remains after garbage collection');
+
+  }
+
+}
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 41d216c8bb69..1e7b05c9c57f 100644
--- a/core/modules/system/lib/Drupal/system/Tests/KeyValueStore/StorageTestBase.php
+++ b/core/modules/system/lib/Drupal/system/Tests/KeyValueStore/StorageTestBase.php
@@ -28,14 +28,18 @@ abstract class StorageTestBase extends UnitTestBase {
    */
   protected $objects = array();
 
+  /**
+   * An array of data collection labels.
+   *
+   * @var array
+   */
+  protected $collections = array();
+
   protected function setUp() {
     parent::setUp();
 
-    $this->collection1 = 'first';
-    $this->collection2 = 'second';
-
-    $this->store1 = new $this->storageClass($this->collection1);
-    $this->store2 = new $this->storageClass($this->collection2);
+    // Define two data collections,
+    $this->collections = array(0 => 'zero', 1 => 'one');
 
     // Create several objects for testing.
     for ($i = 0; $i <= 5; $i++) {
@@ -47,93 +51,97 @@ protected function setUp() {
    * Tests CRUD operations.
    */
   public function testCRUD() {
+    $stores = $this->createStorage();
+
     // Verify that each store returns its own collection name.
-    $this->assertIdentical($this->store1->getCollectionName(), $this->collection1);
-    $this->assertIdentical($this->store2->getCollectionName(), $this->collection2);
+    $this->assertIdentical($stores[0]->getCollectionName(), $this->collections[0]);
+    $this->assertIdentical($stores[1]->getCollectionName(), $this->collections[1]);
 
     // Verify that an item can be stored.
-    $this->store1->set('foo', $this->objects[0]);
-    $this->assertIdenticalObject($this->objects[0], $this->store1->get('foo'));
+    $stores[0]->set('foo', $this->objects[0]);
+    $this->assertIdenticalObject($this->objects[0], $stores[0]->get('foo'));
     // Verify that the other collection is not affected.
-    $this->assertFalse($this->store2->get('foo'));
+    $this->assertFalse($stores[1]->get('foo'));
 
     // Verify that an item can be updated.
-    $this->store1->set('foo', $this->objects[1]);
-    $this->assertIdenticalObject($this->objects[1], $this->store1->get('foo'));
+    $stores[0]->set('foo', $this->objects[1]);
+    $this->assertIdenticalObject($this->objects[1], $stores[0]->get('foo'));
     // Verify that the other collection is still not affected.
-    $this->assertFalse($this->store2->get('foo'));
+    $this->assertFalse($stores[1]->get('foo'));
 
     // Verify that a collection/name pair is unique.
-    $this->store2->set('foo', $this->objects[2]);
-    $this->assertIdenticalObject($this->objects[1], $this->store1->get('foo'));
-    $this->assertIdenticalObject($this->objects[2], $this->store2->get('foo'));
+    $stores[1]->set('foo', $this->objects[2]);
+    $this->assertIdenticalObject($this->objects[1], $stores[0]->get('foo'));
+    $this->assertIdenticalObject($this->objects[2], $stores[1]->get('foo'));
 
     // Verify that an item can be deleted.
-    $this->store1->delete('foo');
-    $this->assertFalse($this->store1->get('foo'));
+    $stores[0]->delete('foo');
+    $this->assertFalse($stores[0]->get('foo'));
 
     // Verify that the other collection is not affected.
-    $this->assertIdenticalObject($this->objects[2], $this->store2->get('foo'));
-    $this->store2->delete('foo');
-    $this->assertFalse($this->store2->get('foo'));
+    $this->assertIdenticalObject($this->objects[2], $stores[1]->get('foo'));
+    $stores[1]->delete('foo');
+    $this->assertFalse($stores[1]->get('foo'));
 
     // Verify that multiple items can be stored.
     $values = array(
       'foo' => $this->objects[3],
       'bar' => $this->objects[4],
     );
-    $this->store1->setMultiple($values);
+    $stores[0]->setMultiple($values);
 
     // Verify that multiple items can be retrieved.
-    $result = $this->store1->getMultiple(array('foo', 'bar'));
+    $result = $stores[0]->getMultiple(array('foo', 'bar'));
     foreach ($values as $j => $value) {
       $this->assertIdenticalObject($value, $result[$j]);
     }
 
     // Verify that the other collection was not affected.
-    $this->assertFalse($this->store2->get('foo'));
-    $this->assertFalse($this->store2->get('bar'));
+    $this->assertFalse($stores[1]->get('foo'));
+    $this->assertFalse($stores[1]->get('bar'));
 
     // Verify that all items in a collection can be retrieved.
     // Ensure that an item with the same name exists in the other collection.
-    $this->store2->set('foo', $this->objects[5]);
-    $result = $this->store1->getAll();
+    $stores[1]->set('foo', $this->objects[5]);
+    $result = $stores[0]->getAll();
     // Not using assertIdentical(), since the order is not defined for getAll().
     $this->assertEqual(count($result), count($values));
     foreach ($result as $key => $value) {
       $this->assertEqual($values[$key], $value);
     }
     // Verify that all items in the other collection are different.
-    $result = $this->store2->getAll();
+    $result = $stores[1]->getAll();
     $this->assertEqual($result, array('foo' => $this->objects[5]));
 
     // Verify that multiple items can be deleted.
-    $this->store1->deleteMultiple(array_keys($values));
-    $this->assertFalse($this->store1->get('foo'));
-    $this->assertFalse($this->store1->get('bar'));
-    $this->assertFalse($this->store1->getMultiple(array('foo', 'bar')));
+    $stores[0]->deleteMultiple(array_keys($values));
+    $this->assertFalse($stores[0]->get('foo'));
+    $this->assertFalse($stores[0]->get('bar'));
+    $this->assertFalse($stores[0]->getMultiple(array('foo', 'bar')));
     // Verify that the item in the other collection still exists.
-    $this->assertIdenticalObject($this->objects[5], $this->store2->get('foo'));
+    $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.
-    $this->assertNull($this->store1->get('foo'));
+    $this->assertNull($stores[0]->get('foo'));
 
     // Verify that a FALSE value can be stored.
-    $this->store1->set('foo', FALSE);
-    $this->assertIdentical($this->store1->get('foo'), FALSE);
+    $stores[0]->set('foo', FALSE);
+    $this->assertIdentical($stores[0]->get('foo'), FALSE);
 
     // Verify that a deleted key returns NULL as value.
-    $this->store1->delete('foo');
-    $this->assertNull($this->store1->get('foo'));
+    $stores[0]->delete('foo');
+    $this->assertNull($stores[0]->get('foo'));
 
     // Verify that a non-existing key is not returned when getting multiple keys.
-    $this->store1->set('bar', 'baz');
-    $values = $this->store1->getMultiple(array('foo', 'bar'));
+    $stores[0]->set('bar', 'baz');
+    $values = $stores[0]->getMultiple(array('foo', 'bar'));
     $this->assertFalse(isset($values['foo']), "Key 'foo' not found.");
     $this->assertIdentical($values['bar'], 'baz');
   }
@@ -142,24 +150,47 @@ public function testNonExistingKeys() {
    * Tests the setIfNotExists() method.
    */
   public function testSetIfNotExists() {
+    $stores = $this->createStorage();
+
     $key = $this->randomName();
     // Test that setIfNotExists() succeeds only the first time.
     for ($i = 0; $i <= 1; $i++) {
       // setIfNotExists() should be TRUE the first time (when $i is 0) and
       // FALSE the second time (when $i is 1).
-      $this->assertEqual(!$i, $this->store1->setIfNotExists($key, $this->objects[$i]));
-      $this->assertIdenticalObject($this->objects[0], $this->store1->get($key));
+      $this->assertEqual(!$i, $stores[0]->setIfNotExists($key, $this->objects[$i]));
+      $this->assertIdenticalObject($this->objects[0], $stores[0]->get($key));
       // Verify that the other collection is not affected.
-      $this->assertFalse($this->store2->get($key));
+      $this->assertFalse($stores[1]->get($key));
     }
 
     // Remove the item and try to set it again.
-    $this->store1->delete($key);
-    $this->store1->setIfNotExists($key, $this->objects[1]);
+    $stores[0]->delete($key);
+    $stores[0]->setIfNotExists($key, $this->objects[1]);
     // This time it should succeed.
-    $this->assertIdenticalObject($this->objects[1], $this->store1->get($key));
+    $this->assertIdenticalObject($this->objects[1], $stores[0]->get($key));
     // Verify that the other collection is still not affected.
-    $this->assertFalse($this->store2->get($key));
+    $this->assertFalse($stores[1]->get($key));
+  }
+
+  /**
+   * Creates storage objects for each collection defined for this class.
+   *
+   * Storing the storage objects in a class member variable causes a fatal
+   * exception in DatabaseStorageExpirableTest, because in that situation
+   * garbage collection is not triggered until the test class itself is
+   * destructed, after tearDown() has deleted the database tables. Instead,
+   * create the storage objects locally in each test using this method.
+   *
+   * @see Drupal\system\Tests\KeyValueStore\DatabaseStorageExpirable
+   * @see Drupal\Core\KeyValueStore\DatabaseStorageExpirable::garbageCollection()
+   */
+  protected function createStorage() {
+    $stores = array();
+    foreach ($this->collections as $i => $collection) {
+      $stores[$i] = new $this->storageClass($collection);
+    }
+
+    return $stores;
   }
 
 }
diff --git a/core/modules/user/lib/Drupal/user/Tests/TempStoreDatabaseTest.php b/core/modules/user/lib/Drupal/user/Tests/TempStoreDatabaseTest.php
index 362e7396c4b5..5fb54e31e73d 100644
--- a/core/modules/user/lib/Drupal/user/Tests/TempStoreDatabaseTest.php
+++ b/core/modules/user/lib/Drupal/user/Tests/TempStoreDatabaseTest.php
@@ -65,19 +65,10 @@ protected function setUp() {
     db_create_table('semaphore', $schema['semaphore']);
     db_create_table('key_value_expire', $schema['key_value_expire']);
 
-    // Create a key/value collection.
-    $this->storeFactory = new TempStoreFactory(Database::getConnection(), new DatabaseLockBackend());
-    $this->collection = $this->randomName();
-
     // Create several objects for testing.
     for ($i = 0; $i <= 3; $i++) {
       $this->objects[$i] = $this->randomObject();
     }
-    // Create two mock users for testing.
-    for ($i = 0; $i <= 1; $i++) {
-      $this->users[$i] = mt_rand(500, 5000000);
-      $this->stores[$i] = $this->getStorePerUID($this->users[$i]);
-    }
 
   }
 
@@ -91,65 +82,67 @@ protected function tearDown() {
    * Tests the UserTempStore API.
    */
   public function testUserTempStore() {
+    // Create a key/value collection.
+    $factory = new TempStoreFactory(Database::getConnection(), new DatabaseLockBackend());
+    $collection = $this->randomName();
+
+    // Create two mock users.
+    for ($i = 0; $i <= 1; $i++) {
+      $users[$i] = mt_rand(500, 5000000);
+
+      // Storing the TempStore objects in a class member variable causes a
+      // fatal exception, because in that situation garbage collection is not
+      // triggered until the test class itself is destructed, after tearDown()
+      // has deleted the database tables. Store the objects locally instead.
+      $stores[$i] = $factory->get($collection, $users[$i]);
+    }
+
     $key = $this->randomName();
     // Test that setIfNotExists() succeeds only the first time.
     for ($i = 0; $i <= 1; $i++) {
       // setIfNotExists() should be TRUE the first time (when $i is 0) and
       // FALSE the second time (when $i is 1).
-      $this->assertEqual(!$i, $this->stores[0]->setIfNotExists($key, $this->objects[$i]));
-      $metadata = $this->stores[0]->getMetadata($key);
-      $this->assertEqual($this->users[0], $metadata->owner);
-      $this->assertIdenticalObject($this->objects[0], $this->stores[0]->get($key));
+      $this->assertEqual(!$i, $stores[0]->setIfNotExists($key, $this->objects[$i]));
+      $metadata = $stores[0]->getMetadata($key);
+      $this->assertEqual($users[0], $metadata->owner);
+      $this->assertIdenticalObject($this->objects[0], $stores[0]->get($key));
       // Another user should get the same result.
-      $metadata = $this->stores[1]->getMetadata($key);
-      $this->assertEqual($this->users[0], $metadata->owner);
-      $this->assertIdenticalObject($this->objects[0], $this->stores[1]->get($key));
+      $metadata = $stores[1]->getMetadata($key);
+      $this->assertEqual($users[0], $metadata->owner);
+      $this->assertIdenticalObject($this->objects[0], $stores[1]->get($key));
     }
 
     // Remove the item and try to set it again.
-    $this->stores[0]->delete($key);
-    $this->stores[0]->setIfNotExists($key, $this->objects[1]);
+    $stores[0]->delete($key);
+    $stores[0]->setIfNotExists($key, $this->objects[1]);
     // This time it should succeed.
-    $this->assertIdenticalObject($this->objects[1], $this->stores[0]->get($key));
+    $this->assertIdenticalObject($this->objects[1], $stores[0]->get($key));
 
     // This user can update the object.
-    $this->stores[0]->set($key, $this->objects[2]);
-    $this->assertIdenticalObject($this->objects[2], $this->stores[0]->get($key));
+    $stores[0]->set($key, $this->objects[2]);
+    $this->assertIdenticalObject($this->objects[2], $stores[0]->get($key));
     // The object is the same when another user loads it.
-    $this->assertIdenticalObject($this->objects[2], $this->stores[1]->get($key));
+    $this->assertIdenticalObject($this->objects[2], $stores[1]->get($key));
     // Another user can update the object and become the owner.
-    $this->stores[1]->set($key, $this->objects[3]);
-    $this->assertIdenticalObject($this->objects[3], $this->stores[0]->get($key));
-    $this->assertIdenticalObject($this->objects[3], $this->stores[1]->get($key));
-    $metadata = $this->stores[1]->getMetadata($key);
-    $this->assertEqual($this->users[1], $metadata->owner);
+    $stores[1]->set($key, $this->objects[3]);
+    $this->assertIdenticalObject($this->objects[3], $stores[0]->get($key));
+    $this->assertIdenticalObject($this->objects[3], $stores[1]->get($key));
+    $metadata = $stores[1]->getMetadata($key);
+    $this->assertEqual($users[1], $metadata->owner);
 
     // The first user should be informed that the second now owns the data.
-    $metadata = $this->stores[0]->getMetadata($key);
-    $this->assertEqual($this->users[1], $metadata->owner);
+    $metadata = $stores[0]->getMetadata($key);
+    $this->assertEqual($users[1], $metadata->owner);
 
     // Now manually expire the item (this is not exposed by the API) and then
     // assert it is no longer accessible.
     db_update('key_value_expire')
       ->fields(array('expire' => REQUEST_TIME - 1))
-      ->condition('collection', $this->collection)
+      ->condition('collection', $collection)
       ->condition('name', $key)
       ->execute();
-    $this->assertFalse($this->stores[0]->get($key));
-    $this->assertFalse($this->stores[1]->get($key));
-  }
-
-  /**
-   * Returns a TempStore for this collection belonging to the given user.
-   *
-   * @param int $uid
-   *   A user ID.
-   *
-   * @return Drupal\user\TempStore
-   *   The key/value store object.
-   */
-  protected function getStorePerUID($uid) {
-    return $this->storeFactory->get($this->collection, $uid);
+    $this->assertFalse($stores[0]->get($key));
+    $this->assertFalse($stores[1]->get($key));
   }
 
 }
-- 
GitLab