diff --git a/core/lib/Drupal/Core/KeyValueStore/DatabaseStorageExpirable.php b/core/lib/Drupal/Core/KeyValueStore/DatabaseStorageExpirable.php index 1bcbe47..eb73878 100644 --- a/core/lib/Drupal/Core/KeyValueStore/DatabaseStorageExpirable.php +++ b/core/lib/Drupal/Core/KeyValueStore/DatabaseStorageExpirable.php @@ -13,8 +13,8 @@ /** * Defines a default key/value store implementation for expiring items. * - * This is Drupal's default key/value store implementation. It uses the database - * to store key/value data with an expire date. + * This key/value store implementation uses the database to store key/value + * data with an expire date. */ class DatabaseStorageExpirable extends DatabaseStorage implements KeyValueStoreExpirableInterface { @@ -75,7 +75,6 @@ public function getAll() { * Implements Drupal\Core\KeyValueStore\KeyValueStoreExpireInterface::setWithExpire(). */ function setWithExpire($key, $value, $expire) { - $this->garbageCollection(); $this->connection->merge($this->table) ->key(array( 'name' => $key, @@ -92,7 +91,6 @@ function setWithExpire($key, $value, $expire) { * Implements Drupal\Core\KeyValueStore\KeyValueStoreExpirableInterface::setWithExpireIfNotExists(). */ function setWithExpireIfNotExists($key, $value, $expire) { - $this->garbageCollection(); $result = $this->connection->merge($this->table) ->insertFields(array( 'collection' => $this->collection, @@ -111,7 +109,7 @@ function setWithExpireIfNotExists($key, $value, $expire) { */ function setMultipleWithExpire(array $data, $expire) { foreach ($data as $key => $value) { - $this->set($key, $value, $expire); + $this->setWithExpire($key, $value, $expire); } } @@ -126,7 +124,7 @@ public function deleteMultiple(array $keys) { /** * Deletes expired items. */ - protected function garbageCollection() { + public 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 db85d35..7b592be 100644 --- a/core/modules/system/lib/Drupal/system/Tests/KeyValueStore/DatabaseStorageExpirableTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/KeyValueStore/DatabaseStorageExpirableTest.php @@ -39,4 +39,130 @@ protected function tearDown() { parent::tearDown(); } + /** + * Tests CRUD functionality with expiration. + */ + public function testCRUDWithExpiration() { + // Verify that an item can be stored with setWithExpire(). + $expire = 604800; + $this->verbose($expire); + $this->store1->setWithExpire('foo', $this->objects[0], $expire); + $this->assertIdenticalObject($this->objects[0], $this->store1->get('foo')); + // Verify that the other collection is not affected. + $this->assertFalse($this->store2->get('foo')); + + // Verify that an item can be updated with setWithExpire(). + $this->store1->setWithExpire('foo', $this->objects[1], $expire); + $this->assertIdenticalObject($this->objects[1], $this->store1->get('foo')); + // Verify that the other collection is still not affected. + $this->assertFalse($this->store2->get('foo')); + + // Verify that the expirable data key is unique. + $this->store2->setWithExpire('foo', $this->objects[2], $expire); + $this->assertIdenticalObject($this->objects[1], $this->store1->get('foo')); + $this->assertIdenticalObject($this->objects[2], $this->store2->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, $expire); + $result = $this->store1->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')); + + // 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(); + // 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(); + $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'))); + // Verify that the item in the other collection still exists. + $this->assertIdenticalObject($this->objects[5], $this->store2->get('foo')); + + // Test that setWithExpireIfNotExists() succeeds only the first time. + $key = $this->randomName(); + 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->setWithExpireIfNotExists($key, $this->objects[$i], $expire)); + $this->assertIdenticalObject($this->objects[0], $this->store1->get($key)); + // Verify that the other collection is not affected. + $this->assertFalse($this->store2->get($key)); + } + + // Remove the item and try to set it again. + $this->store1->delete($key); + $this->store1->setIfNotExists($key, $this->objects[1]); + // This time it should succeed. + $this->assertIdenticalObject($this->objects[1], $this->store1->get($key)); + // Verify that the other collection is still not affected. + $this->assertFalse($this->store2->get($key)); + + } + + /** + * Tests data expiration and garbage collection. + */ + public function testExpiration() { + $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'); + + // 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); + + // 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( + 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); + + // Ensure only non-expired items are retrived. + $all = $this->store1->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/StorageTestBase.php b/core/modules/system/lib/Drupal/system/Tests/KeyValueStore/StorageTestBase.php index 2fa841c..41d216c 100644 --- a/core/modules/system/lib/Drupal/system/Tests/KeyValueStore/StorageTestBase.php +++ b/core/modules/system/lib/Drupal/system/Tests/KeyValueStore/StorageTestBase.php @@ -142,11 +142,11 @@ public function testNonExistingKeys() { * Tests the setIfNotExists() method. */ public function testSetIfNotExists() { - $key = $this->randomName(); // Test that setIfNotExists() succeeds only the first time. for ($i = 0; $i <= 1; $i++) { - // setIfNotExists() should fail the second time ($i = 1). + // 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)); // Verify that the other collection is not affected. @@ -160,7 +160,6 @@ public function testSetIfNotExists() { $this->assertIdenticalObject($this->objects[1], $this->store1->get($key)); // Verify that the other collection is still not affected. $this->assertFalse($this->store2->get($key)); - } } diff --git a/core/modules/system/system.install b/core/modules/system/system.install index 3382fdf..28d95da 100644 --- a/core/modules/system/system.install +++ b/core/modules/system/system.install @@ -858,7 +858,7 @@ function system_schema() { 'translatable' => TRUE, ), 'expire' => array( - 'description' => 'The time since Unix epoch in seconds when this item expires. Defaults to January 18 2038 (the end of the Unix epoch).', + 'description' => 'The time since Unix epoch in seconds when this item expires. Defaults to the maximum possible time.', 'type' => 'int', 'not null' => TRUE, 'default' => 2147483647, @@ -2169,7 +2169,7 @@ function system_update_8023() { 'translatable' => TRUE, ), 'expire' => array( - 'description' => 'The time since Unix epoch in seconds when this item expires.', + 'description' => 'The time since Unix epoch in seconds when this item expires. Defaults to the maximum possible time.', 'type' => 'int', 'not null' => TRUE, 'default' => 2147483647, diff --git a/core/modules/user/lib/Drupal/user/TempStore.php b/core/modules/user/lib/Drupal/user/TempStore.php index 3ee8732..9ebde36 100644 --- a/core/modules/user/lib/Drupal/user/TempStore.php +++ b/core/modules/user/lib/Drupal/user/TempStore.php @@ -14,15 +14,29 @@ * Stores and retrieves temporary data for a given owner. * * A TempStore can be used to make temporary, non-cache data available across - * requests. Each TempStore belongs to a particular owner (e.g. a user, - * session, or process).TempStore data expires automatically after a given - * timeframe. + * requests. The data for the TempStore is stored in one key/value collection. + * TempStore data expires automatically after a given timeframe. * * The TempStore is different from a cache, because the data in it is not yet * saved permanently and so it cannot be rebuilt. Typically, the TempStore * might be used to store work in progress that is later saved permanently * elsewhere, e.g. autosave data, multistep forms, or in-progress changes - * to configuration that are not ready to be saved. + * to complex configuration that are not ready to be saved. + * + * Each TempStore belongs belongs to a particular owner (e.g. a user, session, + * or process). Multiple owners may use the same key/value collection, and the + * owner is stored along with the key/value pair. + * + * Every key is unique within the collection, so the TempStore can check + * whether a particular key is already set by a different owner. This is + * useful for informing one owner that the data is already in use by another; + * for example, to let one user know that another user is in the process of + * editing certain data, or even to restrict other users from editing it at + * the same time. It is the responsibility of the implementation to decide + * when and whether one owner can use or update another owner's data. + * + * @todo We could add getIfOwner() or setIfOwner() methods to make this more + * explicit. */ class TempStore { @@ -141,7 +155,7 @@ function set($key, $value) { } /** - * Gets the metadata associated with a particular key/value pair. + * Returns the metadata associated with a particular key/value pair. * * @param string $key * The key of the data to store. @@ -151,8 +165,10 @@ function set($key, $value) { * NULL otherwise. */ function getMetadata($key) { + // Fetch the key/value pair and its metadata. $object = $this->storage->get($key); if ($object) { + // Don't keep the data itself in memory. unset($object->data); return $object; } diff --git a/core/modules/user/lib/Drupal/user/Tests/TempStoreDatabaseTest.php b/core/modules/user/lib/Drupal/user/Tests/TempStoreDatabaseTest.php index ae779b7..b0ef393 100644 --- a/core/modules/user/lib/Drupal/user/Tests/TempStoreDatabaseTest.php +++ b/core/modules/user/lib/Drupal/user/Tests/TempStoreDatabaseTest.php @@ -81,6 +81,12 @@ protected function setUp() { } + protected function tearDown() { + db_drop_table('key_value_expire'); + db_drop_table('semaphore'); + parent::tearDown(); + } + /** * Tests the UserTempStore API. */