From 359d2fd2f93d9b5373d06a1965936a9bb36480a3 Mon Sep 17 00:00:00 2001 From: Vitaliy Filippov Date: Fri, 25 May 2012 14:41:53 +0400 Subject: [PATCH] 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 --- includes/objectcache/APCBagOStuff.php | 22 +++++++-- includes/objectcache/BagOStuff.php | 16 ++++++- includes/objectcache/DBABagOStuff.php | 59 ++++++++++++++++++------ includes/objectcache/XCacheBagOStuff.php | 21 +++++++-- 4 files changed, 96 insertions(+), 22 deletions(-) diff --git a/includes/objectcache/APCBagOStuff.php b/includes/objectcache/APCBagOStuff.php index 5a7729b68d..1a0de21865 100644 --- 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 fcc3aa9d64..57029a89f5 100644 --- 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 8483d7ee22..264aed721e 100644 --- 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 08f52b74bf..6c88289203 100644 --- 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 ); + } +} -- 2.20.1