MDL-41106 Cleaning up expired elements in session cache store

- Always make sure the elements in cache are sorted so we need to remove only elements in the beginning of array
- Remove expired elements from session store to free memory
- Minor bug fixes

diff --git a/cache/stores/session/lib.php b/cache/stores/session/lib.php
index 225b45b..99f6c52 100644 (file)
--- a/cache/stores/session/lib.php
+++ b/cache/stores/session/lib.php
@@ -118,7 +118,7 @@ class cachestore_session extends session_data_store implements cache_is_key_awar
 
     /**
      * The maximum size for the store, or false if there isn't one.
-     * @var bool
+     * @var bool|int
      */
     protected $maxsize = false;
 
@@ -210,6 +210,7 @@ class cachestore_session extends session_data_store implements cache_is_key_awar
             $this->maxsize = abs((int)$maxsize);
             $this->storecount = count($this->store);
         }
+        $this->check_ttl();
     }
 
     /**
@@ -237,10 +238,18 @@ class cachestore_session extends session_data_store implements cache_is_key_awar
      */
     public function get($key) {
         if (isset($this->store[$key])) {
-            if ($this->ttl === 0) {
-                return $this->store[$key][0];
+            if ($this->ttl == 0) {
+                $value = $this->store[$key][0];
+                if ($this->maxsize !== false) {
+                    // Make sure the element is now in the end of array.
+                    $this->set($key, $value);
+                }
+                return $value;
             } else if ($this->store[$key][1] >= (cache::now() - $this->ttl)) {
                 return $this->store[$key][0];
+            } else {
+                // Element is present but has expired.
+                $this->check_ttl();
             }
         }
         return false;
@@ -262,16 +271,27 @@ class cachestore_session extends session_data_store implements cache_is_key_awar
             $maxtime = cache::now() - $this->ttl;
         }
 
+        $hasexpiredelements = false;
         foreach ($keys as $key) {
             $return[$key] = false;
             if (isset($this->store[$key])) {
                 if ($this->ttl == 0) {
                     $return[$key] = $this->store[$key][0];
+                    if ($this->maxsize !== false) {
+                        // Make sure the element is now in the end of array.
+                        $this->set($key, $return[$key], false);
+                    }
                 } else if ($this->store[$key][1] >= $maxtime) {
                     $return[$key] = $this->store[$key][0];
+                } else {
+                    $hasexpiredelements = true;
                 }
             }
         }
+        if ($hasexpiredelements) {
+            // There are some elements that are present but have expired.
+            $this->check_ttl();
+        }
         return $return;
     }
 
@@ -285,17 +305,21 @@ class cachestore_session extends session_data_store implements cache_is_key_awar
      */
     public function set($key, $data, $testmaxsize = true) {
         $testmaxsize = ($testmaxsize && $this->maxsize !== false);
-        $increment = ($testmaxsize && !isset($this->store[$key]));
+        $increment = $this->maxsize !== false && !isset($this->store[$key]);
+        if (($this->maxsize !== false && !$increment) || $this->ttl != 0) {
+            // Make sure the element is added to the end of $this->store array.
+            unset($this->store[$key]);
+        }
         if ($this->ttl === 0) {
             $this->store[$key] = array($data, 0);
         } else {
             $this->store[$key] = array($data, cache::now());
         }
-        if ($testmaxsize && $increment) {
+        if ($increment) {
             $this->storecount++;
-            if ($this->storecount > $this->maxsize) {
-                $this->reduce_for_maxsize();
-            }
+        }
+        if ($testmaxsize && $this->storecount > $this->maxsize) {
+            $this->reduce_for_maxsize();
         }
         return true;
     }
@@ -315,7 +339,11 @@ class cachestore_session extends session_data_store implements cache_is_key_awar
             $key = $pair['key'];
             $data = $pair['value'];
             $count++;
-            if (!isset($this->store[$key])) {
+            if ($this->maxsize !== false || $this->ttl !== 0) {
+                // Make sure the element is added to the end of $this->store array.
+                $this->delete($key);
+                $increment++;
+            } else if (!isset($this->store[$key])) {
                 $increment++;
             }
             if ($this->ttl === 0) {
@@ -400,12 +428,14 @@ class cachestore_session extends session_data_store implements cache_is_key_awar
      * @return bool Returns true if the operation was a success, false otherwise.
      */
     public function delete($key) {
-        $result = isset($this->store[$key]);
+        if (!isset($this->store[$key])) {
+            return false;
+        }
         unset($this->store[$key]);
         if ($this->maxsize !== false) {
             $this->storecount--;
         }
-        return $result;
+        return true;
     }
 
     /**
@@ -500,12 +530,40 @@ class cachestore_session extends session_data_store implements cache_is_key_awar
         return $this->name;
     }
 
+    /**
+     * Removes expired elements.
+     * @return int number of removed elements
+     */
+    protected function check_ttl() {
+        if ($this->ttl == 0) {
+            return 0;
+        }
+        $maxtime = cache::now() - $this->ttl;
+        $c = 0;
+        for ($value = reset($this->store); $value !== false; $value = next($this->store)) {
+            if ($value[1] >= $maxtime) {
+                // We know that elements are sorted by ttl so no need to continue;
+                break;
+            }
+            $c++;
+        }
+        if ($c) {
+            // Remove first $c elements as they are expired.
+            $this->store = array_slice($this->store, $c, null, true);
+            if ($this->maxsize !== false) {
+                $this->storecount -= $c;
+            }
+        }
+        return $c;
+    }
+
     /**
      * Finds all of the keys being stored in the cache store instance.
      *
      * @return array
      */
     public function find_all() {
+        $this->check_ttl();
         return array_keys($this->store);
     }
 

diff --git a/cache/stores/session/tests/session_test.php b/cache/stores/session/tests/session_test.php
index 9a8b15f..ebf7059 100644 (file)
--- a/cache/stores/session/tests/session_test.php
+++ b/cache/stores/session/tests/session_test.php
@@ -153,5 +153,47 @@ class cachestore_session_test extends cachestore_tests {
             'key1', 'key2', 'key3'
         )));
 
+        // Test that that cache deletes element that was least recently accessed.
+        $this->assertEquals('valueA', $cacheone->get('keyA'));
+        $cacheone->set('keyD', 'valueD');
+        $this->assertEquals('valueA', $cacheone->get('keyA'));
+        $this->assertFalse($cacheone->get('keyB'));
+        $this->assertEquals(array('keyD' => 'valueD', 'keyC' => 'valueC'), $cacheone->get_many(array('keyD', 'keyC')));
+        $cacheone->set('keyE', 'valueE');
+        $this->assertFalse($cacheone->get('keyB'));
+        $this->assertFalse($cacheone->get('keyA'));
+        $this->assertEquals(array('keyA' => false, 'keyE' => 'valueE', 'keyD' => 'valueD', 'keyC' => 'valueC'),
+                $cacheone->get_many(array('keyA', 'keyE', 'keyD', 'keyC')));
+        // Overwrite keyE (moves it to the end of array), and set keyF.
+        $cacheone->set_many(array('keyE' => 'valueE', 'keyF' => 'valueF'));
+        $this->assertEquals(array('keyC' => 'valueC', 'keyE' => 'valueE', 'keyD' => false, 'keyF' => 'valueF'),
+                $cacheone->get_many(array('keyC', 'keyE', 'keyD', 'keyF')));
+    }
+
+    public function test_ttl() {
+        $config = cache_config_phpunittest::instance();
+        $config->phpunit_add_definition('phpunit/three', array(
+            'mode' => cache_store::MODE_SESSION,
+            'component' => 'phpunit',
+            'area' => 'three',
+            'maxsize' => 3,
+            'ttl' => 3
+        ));
+
+        $cachethree = cache::make('phpunit', 'three');
+
+        // Make sure that when cache with ttl is full the elements that were added first are deleted first regardless of access time.
+        $cachethree->set('key1', 'value1');
+        $cachethree->set('key2', 'value2');
+        $cachethree->set('key3', 'value3');
+        $cachethree->set('key4', 'value4');
+        $this->assertFalse($cachethree->get('key1'));
+        $this->assertEquals('value4', $cachethree->get('key4'));
+        $cachethree->set('key5', 'value5');
+        $this->assertFalse($cachethree->get('key2'));
+        $this->assertEquals('value4', $cachethree->get('key4'));
+        $cachethree->set_many(array('key6' => 'value6', 'key7' => 'value7'));
+        $this->assertEquals(array('key3' => false, 'key4' => false, 'key5' => 'value5', 'key6' => 'value6', 'key7' => 'value7'),
+                $cachethree->get_many(array('key3', 'key4', 'key5', 'key6', 'key7')));
     }
 }
\ No newline at end of file