From fb82e1af0e8503e94e27752f9f2dc07d7f778e01 Mon Sep 17 00:00:00 2001 From: Bob Vincent Date: Fri, 23 Mar 2012 06:37:56 -0400 Subject: [PATCH] Issue #1015946 by pillarsdotnet, catch, bfroehle, Jej, jose.guevara, Damien Tournoud: Fixed Eliminate $user->cache and {session}.cache in favor of $_SESSION['cache_expiration()'][]. --- includes/bootstrap.inc | 1 - includes/cache.inc | 49 ++++++++++++++++++++++++---------- includes/session.inc | 1 - modules/simpletest/tests/cache.test | 34 ++++++++++++++++++++++++ 4 files changed, 68 insertions(+), 17 deletions(-) diff --git a/includes/bootstrap.inc b/includes/bootstrap.inc index 5556f38cc164a80ea088c6be0b74fc5f8b166add..121e9d83c90fc3d92c17b4ad87d3d99747dddfed 100644 --- a/includes/bootstrap.inc +++ b/includes/bootstrap.inc @@ -2081,7 +2081,6 @@ function drupal_anonymous_user() { $user->hostname = ip_address(); $user->roles = array(); $user->roles[DRUPAL_ANONYMOUS_RID] = 'anonymous user'; - $user->cache = 0; return $user; } diff --git a/includes/cache.inc b/includes/cache.inc index eb7f0904057b02b46ec1ea595e95a1dbdbfcbe40..55e275b8883f3845addac0f38a7df9bf179787e7 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,10 +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. + // 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; $user->cache = REQUEST_TIME; $cache_flush = variable_get('cache_flush_' . $this->bin, 0); diff --git a/includes/session.inc b/includes/session.inc index 8f1bcafc446cbc84ab878852f37825c3e3889e59..fd739e29ebb3cb559e4d406b9129dc6d6bfa4600 100644 --- a/includes/session.inc +++ b/includes/session.inc @@ -176,7 +176,6 @@ function _drupal_session_write($sid, $value) { // Either ssid or sid or both will be added from $key below. $fields = array( 'uid' => $user->uid, - 'cache' => isset($user->cache) ? $user->cache : 0, 'hostname' => ip_address(), 'session' => $value, 'timestamp' => REQUEST_TIME, diff --git a/modules/simpletest/tests/cache.test b/modules/simpletest/tests/cache.test index d292fa661ea43a6fa177318c969b4e988dfd03f8..9bbe1da669858217a7139167cb464702ce0dc084 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_TEMPORARY); + $cached = cache()->get($data); + $this->assertTrue(isset($cached->data) && $cached->data === $data, 'Cached item retrieved.'); + cache('page')->set($data, $data, CACHE_TEMPORARY); + + // Expire temporary items in the 'page' bin. + cache('page')->expire(); + + // 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('page')->get($data); + $this->assertFalse($cached, 'Cached item was invalidated'); + } } /** -- 1.7.5.4