Bug 20595 - Fixed incr()/decr() functions for some caches.

* Preserve TTL in objectcache incr() method for APC, XCache and DBA.
  Do not serialize numeric values, since it breaks the counters.
* Also gave some DBABagOStuff functions explicit visibility.

Change-Id: I13856832f418c843afc8e42a6d77686331f6cb41

diff --git a/includes/objectcache/APCBagOStuff.php b/includes/objectcache/APCBagOStuff.php
index 5a7729b..1a0de21 100644 (file)
--- a/includes/objectcache/APCBagOStuff.php
+++ b/includes/objectcache/APCBagOStuff.php
@@ -27,7 +27,6 @@
  * @ingroup Cache
  */
 class APCBagOStuff extends BagOStuff {
-
        /**
         * @param $key string
         * @return mixed
@@ -36,7 +35,11 @@ class APCBagOStuff extends BagOStuff {
                $val = apc_fetch( $key );
 
                if ( is_string( $val ) ) {
-                       $val = unserialize( $val );
+                       if ( $this->isInteger( $val ) ) {
+                               $val = intval( $val );
+                       } else {
+                               $val = unserialize( $val );
+                       }
                }
 
                return $val;
@@ -49,7 +52,11 @@ class APCBagOStuff extends BagOStuff {
         * @return bool
         */
        public function set( $key, $value, $exptime = 0 ) {
-               apc_store( $key, serialize( $value ), $exptime );
+               if ( !$this->isInteger( $value ) ) {
+                       $value = serialize( $value );
+               }
+
+               apc_store( $key, $value, $exptime );
 
                return true;
        }
@@ -65,6 +72,14 @@ class APCBagOStuff extends BagOStuff {
                return true;
        }
 
+       public function incr( $key, $value = 1 ) {
+               return apc_inc( $key, $value );
+       }
+
+       public function decr( $key, $value = 1 ) {
+               return apc_dec( $key, $value );
+       }
+
        /**
         * @return Array
         */
@@ -80,4 +95,3 @@ class APCBagOStuff extends BagOStuff {
                return $keys;
        }
 }
-

diff --git a/includes/objectcache/BagOStuff.php b/includes/objectcache/BagOStuff.php
index fcc3aa9..57029a8 100644 (file)
--- a/includes/objectcache/BagOStuff.php
+++ b/includes/objectcache/BagOStuff.php
@@ -164,10 +164,11 @@ abstract class BagOStuff {
        }
 
        /**
+        * Increase stored value of $key by $value while preserving its TTL
         * @param $key String: Key to increase
         * @param $value Integer: Value to add to $key (Default 1)
         * @return null if lock is not possible else $key value increased by $value
-        * @return bool success
+        * @return integer
         */
        public function incr( $key, $value = 1 ) {
                if ( !$this->lock( $key ) ) {
@@ -186,9 +187,10 @@ abstract class BagOStuff {
        }
 
        /**
+        * Decrease stored value of $key by $value while preserving its TTL
         * @param $key String
         * @param $value Integer
-        * @return bool success
+        * @return integer
         */
        public function decr( $key, $value = 1 ) {
                return $this->incr( $key, - $value );
@@ -235,4 +237,14 @@ abstract class BagOStuff {
                        return $exptime;
                }
        }
+
+       /**
+        * Check if a value is an integer
+        *
+        * @param $value mixed
+        * @return bool
+        */
+       protected function isInteger( $value ) {
+               return ( is_int( $value ) || ctype_digit( $value ) );
+       }
 }

diff --git a/includes/objectcache/DBABagOStuff.php b/includes/objectcache/DBABagOStuff.php
index 8483d7e..264aed7 100644 (file)
--- a/includes/objectcache/DBABagOStuff.php
+++ b/includes/objectcache/DBABagOStuff.php
@@ -45,8 +45,7 @@ class DBABagOStuff extends BagOStuff {
                        $params['dir'] = wfTempDir();
                }
 
-               $this->mFile = $params['dir']."/mw-cache-" . wfWikiID();
-               $this->mFile .= '.db';
+               $this->mFile = $params['dir'] . '/mw-cache-' . wfWikiID() . '.db';
                wfDebug( __CLASS__ . ": using cache file {$this->mFile}\n" );
                $this->mHandler = $wgDBAhandler;
        }
@@ -58,7 +57,7 @@ class DBABagOStuff extends BagOStuff {
         *
         * @return string
         */
-       function encode( $value, $expiry ) {
+       protected function encode( $value, $expiry ) {
                # Convert to absolute time
                $expiry = $this->convertExpiry( $expiry );
 
@@ -69,7 +68,7 @@ class DBABagOStuff extends BagOStuff {
         * @param $blob string
         * @return array list containing value first and expiry second
         */
-       function decode( $blob ) {
+       protected function decode( $blob ) {
                if ( !is_string( $blob ) ) {
                        return array( null, 0 );
                } else {
@@ -83,7 +82,7 @@ class DBABagOStuff extends BagOStuff {
        /**
         * @return resource
         */
-       function getReader() {
+       protected function getReader() {
                if ( file_exists( $this->mFile ) ) {
                        $handle = dba_open( $this->mFile, 'rl', $this->mHandler );
                } else {
@@ -100,7 +99,7 @@ class DBABagOStuff extends BagOStuff {
        /**
         * @return resource
         */
-       function getWriter() {
+       protected function getWriter() {
                $handle = dba_open( $this->mFile, 'cl', $this->mHandler );
 
                if ( !$handle ) {
@@ -114,7 +113,7 @@ class DBABagOStuff extends BagOStuff {
         * @param $key string
         * @return mixed|null|string
         */
-       function get( $key ) {
+       public function get( $key ) {
                wfProfileIn( __METHOD__ );
                wfDebug( __METHOD__ . "($key)\n" );
 
@@ -149,7 +148,7 @@ class DBABagOStuff extends BagOStuff {
         * @param $exptime int
         * @return bool
         */
-       function set( $key, $value, $exptime = 0 ) {
+       public function set( $key, $value, $exptime = 0 ) {
                wfProfileIn( __METHOD__ );
                wfDebug( __METHOD__ . "($key)\n" );
 
@@ -173,7 +172,7 @@ class DBABagOStuff extends BagOStuff {
         * @param $time int
         * @return bool
         */
-       function delete( $key, $time = 0 ) {
+       public function delete( $key, $time = 0 ) {
                wfProfileIn( __METHOD__ );
                wfDebug( __METHOD__ . "($key)\n" );
 
@@ -196,7 +195,7 @@ class DBABagOStuff extends BagOStuff {
         * @param $exptime int
         * @return bool
         */
-       function add( $key, $value, $exptime = 0 ) {
+       public function add( $key, $value, $exptime = 0 ) {
                wfProfileIn( __METHOD__ );
 
                $blob = $this->encode( $value, $exptime );
@@ -214,7 +213,7 @@ class DBABagOStuff extends BagOStuff {
                if ( !$ret ) {
                        list( $value, $expiry ) = $this->decode( dba_fetch( $key, $handle ) );
 
-                       if ( $expiry < time() ) {
+                       if ( $expiry && $expiry < time() ) {
                                # Yes expired, delete and try again
                                dba_delete( $key, $handle );
                                $ret = dba_insert( $key, $blob, $handle );
@@ -229,8 +228,43 @@ class DBABagOStuff extends BagOStuff {
        }
 
        /**
-        * @return Array
+        * @param $key string
+        * @param $step integer
+        * @return integer|bool
         */
+       public function incr( $key, $step = 1 ) {
+               wfProfileIn( __METHOD__ );
+
+               $handle = $this->getWriter();
+
+               if ( !$handle ) {
+                       wfProfileOut( __METHOD__ );
+                       return false;
+               }
+
+               list( $value, $expiry ) = $this->decode( dba_fetch( $key, $handle ) );
+               if ( !is_null( $value ) ) {
+                       if ( $expiry && $expiry < time() ) {
+                               # Key is expired, delete it
+                               dba_delete( $key, $handle );
+                               wfDebug( __METHOD__ . ": $key expired\n" );
+                               $value = null;
+                       } else {
+                               $value += $step;
+                               $blob = $this->encode( $value, $expiry );
+
+                               $ret = dba_replace( $key, $blob, $handle );
+                               $value = $ret ? $value : null;
+                       }
+               }
+
+               dba_close( $handle );
+
+               wfProfileOut( __METHOD__ );
+
+               return is_null( $value ) ? false : (int)$value;
+       }
+
        function keys() {
                $reader = $this->getReader();
                $k1 = dba_firstkey( $reader );
@@ -250,4 +284,3 @@ class DBABagOStuff extends BagOStuff {
                return $result;
        }
 }
-

diff --git a/includes/objectcache/XCacheBagOStuff.php b/includes/objectcache/XCacheBagOStuff.php
index 08f52b7..6c88289 100644 (file)
--- a/includes/objectcache/XCacheBagOStuff.php
+++ b/includes/objectcache/XCacheBagOStuff.php
@@ -38,7 +38,11 @@ class XCacheBagOStuff extends BagOStuff {
                $val = xcache_get( $key );
 
                if ( is_string( $val ) ) {
-                       $val = unserialize( $val );
+                       if ( $this->isInteger( $val ) ) {
+                               $val = intval( $val );
+                       } else {
+                               $val = unserialize( $val );
+                       }
                }
 
                return $val;
@@ -53,7 +57,11 @@ class XCacheBagOStuff extends BagOStuff {
         * @return bool
         */
        public function set( $key, $value, $expire = 0 ) {
-               xcache_set( $key, serialize( $value ), $expire );
+               if ( !$this->isInteger( $value ) ) {
+                       $value = serialize( $value );
+               }
+
+               xcache_set( $key, $value, $expire );
                return true;
        }
 
@@ -68,5 +76,12 @@ class XCacheBagOStuff extends BagOStuff {
                xcache_unset( $key );
                return true;
        }
-}
 
+       public function incr( $key, $value = 1 ) {
+               return xcache_inc( $key, $value );
+       }
+
+       public function decr( $key, $value = 1 ) {
+               return xcache_dec( $key, $value );
+       }
+}