Issue #1015946 by pillarsdotnet, catch, bfroehle, Jej, jose.guevara, Damien Tournoud: Fixed Eliminate ->cache and {session}.cache in favor of ['cache_expiration()'][].

2 files changed, 68 insertions, 16 deletions

diff --git a/includes/cache.inc b/includes/cache.inc
index eb7f09040..a19d3c38c 100644
--- a/includes/cache.inc
+++ b/includes/cache.inc
@@ -379,11 +379,31 @@ class DrupalDatabaseCache implements DrupalCacheInterface {
    *   The bin being requested.
    */
   protected function garbageCollection() {
-    global $user;
+    $cache_lifetime = variable_get('cache_lifetime', 0);
 
-    // Garbage collection necessary when enforcing a minimum cache lifetime.
+    // Clean-up the per-user cache expiration session data, so that the session
+    // handler can properly clean-up the session data for anonymous users.
+    if (isset($_SESSION['cache_expiration'])) {
+      $expire = REQUEST_TIME - $cache_lifetime;
+      foreach ($_SESSION['cache_expiration'] as $bin => $timestamp) {
+        if ($timestamp < $expire) {
+          unset($_SESSION['cache_expiration'][$bin]);
+        }
+      }
+      if (!$_SESSION['cache_expiration']) {
+        unset($_SESSION['cache_expiration']);
+      }
+    }
+
+    // Garbage collection of temporary items is only necessary when enforcing
+    // a minimum cache lifetime.
+    if (!$cache_lifetime) {
+      return;
+    }
+    // When cache lifetime is in force, avoid running garbage collection too
+    // often since this will remove temporary cache items indiscriminately.
     $cache_flush = variable_get('cache_flush_' . $this->bin, 0);
-    if ($cache_flush && ($cache_flush + variable_get('cache_lifetime', 0) <= REQUEST_TIME)) {
+    if ($cache_flush && ($cache_flush + $cache_lifetime <= REQUEST_TIME)) {
       // Reset the variable immediately to prevent a meltdown in heavy load situations.
       variable_set('cache_flush_' . $this->bin, 0);
       // Time to flush old cache data
@@ -413,17 +433,16 @@ class DrupalDatabaseCache implements DrupalCacheInterface {
     if (!isset($cache->data)) {
       return FALSE;
     }
-    // If enforcing a minimum cache lifetime, validate that the data is
-    // currently valid for this user before we return it by making sure the cache
-    // entry was created before the timestamp in the current session's cache
-    // timer. The cache variable is loaded into the $user object by _drupal_session_read()
-    // in session.inc. If the data is permanent or we're not enforcing a minimum
-    // cache lifetime always return the cached data.
-    if ($cache->expire != CACHE_PERMANENT && variable_get('cache_lifetime', 0) && $user->cache > $cache->created) {
-      // This cache data is too old and thus not valid for us, ignore it.
+    // If the cached data is temporary and subject to a per-user minimum
+    // lifetime, compare the cache entry timestamp with the user session
+    // cache_expiration timestamp. If the cache entry is too old, ignore it.
+    if ($cache->expire != CACHE_PERMANENT && variable_get('cache_lifetime', 0) && isset($_SESSION['cache_expiration'][$this->bin]) && $_SESSION['cache_expiration'][$this->bin] > $cache->created) {
+      // Ignore cache data that is too old and thus not valid for this user.
       return FALSE;
     }
 
+    // If the data is permanent or not subject to a minimum cache lifetime,
+    // unserialize and return the cached data.
     if ($cache->serialized) {
       $cache->data = unserialize($cache->data);
     }
@@ -468,11 +487,10 @@ class DrupalDatabaseCache implements DrupalCacheInterface {
 
     if (empty($cid)) {
       if (variable_get('cache_lifetime', 0)) {
-        // We store the time in the current user's $user->cache variable which
-        // will be saved into the sessions bin by _drupal_session_write(). We then
-        // simulate that the cache was flushed for this user by not returning
-        // cached data that was cached before the timestamp.
-        $user->cache = REQUEST_TIME;
+        // We store the time in the current user's session. We then simulate
+        // that the cache was flushed for this user by not returning cached
+        // data that was cached before the timestamp.
+        $_SESSION['cache_expiration'][$this->bin] = REQUEST_TIME;
 
         $cache_flush = variable_get('cache_flush_' . $this->bin, 0);
         if ($cache_flush == 0) {
diff --git a/modules/simpletest/tests/cache.test b/modules/simpletest/tests/cache.test
index d292fa661..47f2df318 100644
--- a/modules/simpletest/tests/cache.test
+++ b/modules/simpletest/tests/cache.test
@@ -339,6 +339,40 @@ class CacheClearCase extends CacheTestCase {
       $this->assertFalse($this->checkCacheExists($id, $this->default_value, $bin), t('All cache entries removed from @bin.', array('@bin' => $bin)));
     }
   }
+
+  /**
+   * Test minimum cache lifetime.
+   */
+  function testMinimumCacheLifetime() {
+    // Set a minimum/maximum cache lifetime.
+    $this->setupLifetime(300);
+    // Login as a newly-created user.
+    $account = $this->drupalCreateUser(array());
+    $this->drupalLogin($account);
+
+    // Set two cache objects in different bins.
+    $data = $this->randomName(100);
+    cache_set($data, $data, 'cache', CACHE_TEMPORARY);
+    $cached = cache_get($data);
+    $this->assertTrue(isset($cached->data) && $cached->data === $data, 'Cached item retrieved.');
+    cache_set($data, $data, 'cache_page', CACHE_TEMPORARY);
+
+    // Expire temporary items in the 'page' bin.
+    cache_clear_all(NULL, 'cache_page');
+
+    // Since the database cache uses REQUEST_TIME, set the $_SESSION variable
+    // manually to force it to the current time.
+    $_SESSION['cache_expiration']['cache_page'] = time();
+
+    // Items in the default cache bin should not be expired.
+    $cached = cache_get($data);
+    $this->assertTrue(isset($cached->data) && $cached->data == $data, 'Cached item retrieved');
+
+    // Despite the minimum cache lifetime, the item in the 'page' bin should
+    // be invalidated for the current user.
+    $cached = cache_get($data, 'cache_page');
+    $this->assertFalse($cached, 'Cached item was invalidated');
+  }
 }
 
 /**