objectcache: defined some missing methods in MultiWriteBagOStuff

Also, reorder some method definitions to match the base class.
This makes it easier to compare the two. In addition, refactor
the doWrite() method to make the calls clearer.

Change-Id: I795b395b9c54645f62962461a28067ae38b291ed

diff --git a/includes/libs/objectcache/MultiWriteBagOStuff.php b/includes/libs/objectcache/MultiWriteBagOStuff.php
index 2d3eed5..fb99717 100644 (file)
--- a/includes/libs/objectcache/MultiWriteBagOStuff.php
+++ b/includes/libs/objectcache/MultiWriteBagOStuff.php
@@ -27,6 +27,9 @@ use Wikimedia\ObjectFactory;
  * are implemented by reading from the caches in the order they are given in
  * the configuration until a cache gives a positive result.
  *
+ * Note that cache key construction will use the first cache backend in the list,
+ * so make sure that the other backends can handle such keys (e.g. via encoding).
+ *
  * @ingroup Cache
  */
 class MultiWriteBagOStuff extends BagOStuff {
@@ -124,67 +127,72 @@ class MultiWriteBagOStuff extends BagOStuff {
                ) {
                        // Backfill the value to the higher (and often faster/smaller) cache tiers
                        $this->doWrite(
-                               $missIndexes, $this->asyncWrites, 'set', $key, $value, self::UPGRADE_TTL
+                               $missIndexes,
+                               $this->asyncWrites,
+                               'set',
+                               [ $key, $value, self::UPGRADE_TTL ]
                        );
                }
 
                return $value;
        }
-
        public function set( $key, $value, $exptime = 0, $flags = 0 ) {
-               $asyncWrites = ( ( $flags & self::WRITE_SYNC ) == self::WRITE_SYNC )
-                       ? false
-                       : $this->asyncWrites;
-
-               return $this->doWrite( $this->cacheIndexes, $asyncWrites, 'set', $key, $value, $exptime );
+               return $this->doWrite(
+                       $this->cacheIndexes,
+                       $this->usesAsyncWritesGivenFlags( $flags ),
+                       __FUNCTION__,
+                       func_get_args()
+               );
        }
 
        public function delete( $key, $flags = 0 ) {
-               return $this->doWrite( $this->cacheIndexes, $this->asyncWrites, 'delete', $key );
+               return $this->doWrite(
+                       $this->cacheIndexes,
+                       $this->usesAsyncWritesGivenFlags( $flags ),
+                       __FUNCTION__,
+                       func_get_args()
+               );
        }
 
        public function add( $key, $value, $exptime = 0, $flags = 0 ) {
                // Try the write to the top-tier cache
-               $ok = $this->doWrite( [ 0 ], $this->asyncWrites, 'add', $key, $value, $exptime );
+               $ok = $this->doWrite(
+                       [ 0 ],
+                       $this->usesAsyncWritesGivenFlags( $flags ),
+                       __FUNCTION__,
+                       func_get_args()
+               );
+
                if ( $ok ) {
                        // Relay the add() using set() if it succeeded. This is meant to handle certain
                        // migration scenarios where the same store might get written to twice for certain
                        // keys. In that case, it does not make sense to return false due to "self-conflicts".
                        return $this->doWrite(
                                array_slice( $this->cacheIndexes, 1 ),
-                               $this->asyncWrites,
+                               $this->usesAsyncWritesGivenFlags( $flags ),
                                'set',
-                               $key,
-                               $value,
-                               $exptime
+                               [ $key, $value, $exptime, $flags ]
                        );
                }
 
                return false;
        }
 
-       public function incr( $key, $value = 1 ) {
-               return $this->doWrite( $this->cacheIndexes, $this->asyncWrites, 'incr', $key, $value );
-       }
-
-       public function decr( $key, $value = 1 ) {
-               return $this->doWrite( $this->cacheIndexes, $this->asyncWrites, 'decr', $key, $value );
-       }
-
        public function merge( $key, callable $callback, $exptime = 0, $attempts = 10, $flags = 0 ) {
-               $asyncWrites = ( ( $flags & self::WRITE_SYNC ) == self::WRITE_SYNC )
-                       ? false
-                       : $this->asyncWrites;
+               return $this->doWrite(
+                       $this->cacheIndexes,
+                       $this->usesAsyncWritesGivenFlags( $flags ),
+                       __FUNCTION__,
+                       func_get_args()
+               );
+       }
 
+       public function changeTTL( $key, $exptime = 0, $flags = 0 ) {
                return $this->doWrite(
                        $this->cacheIndexes,
-                       $asyncWrites,
-                       'merge',
-                       $key,
-                       $callback,
-                       $exptime,
-                       $attempts,
-                       $flags
+                       $this->usesAsyncWritesGivenFlags( $flags ),
+                       __FUNCTION__,
+                       func_get_args()
                );
        }
 
@@ -197,6 +205,82 @@ class MultiWriteBagOStuff extends BagOStuff {
                // Only the first cache is locked
                return $this->caches[0]->unlock( $key );
        }
+       /**
+        * Delete objects expiring before a certain date.
+        *
+        * Succeed if any of the child caches succeed.
+        * @param string $date
+        * @param bool|callable $progressCallback
+        * @return bool
+        */
+       public function deleteObjectsExpiringBefore( $date, $progressCallback = false ) {
+               $ret = false;
+               foreach ( $this->caches as $cache ) {
+                       if ( $cache->deleteObjectsExpiringBefore( $date, $progressCallback ) ) {
+                               $ret = true;
+                       }
+               }
+
+               return $ret;
+       }
+
+       public function getMulti( array $keys, $flags = 0 ) {
+               // Just iterate over each key in order to handle all the backfill logic
+               $res = [];
+               foreach ( $keys as $key ) {
+                       $val = $this->get( $key, $flags );
+                       if ( $val !== false ) {
+                               $res[$key] = $val;
+                       }
+               }
+
+               return $res;
+       }
+
+       public function setMulti( array $data, $exptime = 0, $flags = 0 ) {
+               return $this->doWrite(
+                       $this->cacheIndexes,
+                       $this->usesAsyncWritesGivenFlags( $flags ),
+                       __FUNCTION__,
+                       func_get_args()
+               );
+       }
+
+       public function deleteMulti( array $data, $flags = 0 ) {
+               return $this->doWrite(
+                       $this->cacheIndexes,
+                       $this->usesAsyncWritesGivenFlags( $flags ),
+                       __FUNCTION__,
+                       func_get_args()
+               );
+       }
+
+       public function incr( $key, $value = 1 ) {
+               return $this->doWrite(
+                       $this->cacheIndexes,
+                       $this->asyncWrites,
+                       __FUNCTION__,
+                       func_get_args()
+               );
+       }
+
+       public function decr( $key, $value = 1 ) {
+               return $this->doWrite(
+                       $this->cacheIndexes,
+                       $this->asyncWrites,
+                       __FUNCTION__,
+                       func_get_args()
+               );
+       }
+
+       public function incrWithInit( $key, $ttl, $value = 1, $init = 1 ) {
+               return $this->doWrite(
+                       $this->cacheIndexes,
+                       $this->asyncWrites,
+                       __FUNCTION__,
+                       func_get_args()
+               );
+       }
 
        public function getLastError() {
                return $this->caches[0]->getLastError();
@@ -205,19 +289,17 @@ class MultiWriteBagOStuff extends BagOStuff {
        public function clearLastError() {
                $this->caches[0]->clearLastError();
        }
-
        /**
         * Apply a write method to the backing caches specified by $indexes (in order)
         *
         * @param int[] $indexes List of backing cache indexes
         * @param bool $asyncWrites
-        * @param string $method
-        * @param mixed $args,...
+        * @param string $method Method name of backing caches
+        * @param array[] $args Arguments to the method of backing caches
         * @return bool
         */
-       protected function doWrite( $indexes, $asyncWrites, $method /*, ... */ ) {
+       protected function doWrite( $indexes, $asyncWrites, $method, array $args ) {
                $ret = true;
-               $args = array_slice( func_get_args(), 3 );
 
                if ( array_diff( $indexes, [ 0 ] ) && $asyncWrites && $method !== 'merge' ) {
                        // Deep-clone $args to prevent misbehavior when something writes an
@@ -249,22 +331,11 @@ class MultiWriteBagOStuff extends BagOStuff {
        }
 
        /**
-        * Delete objects expiring before a certain date.
-        *
-        * Succeed if any of the child caches succeed.
-        * @param string $date
-        * @param bool|callable $progressCallback
+        * @param int $flags
         * @return bool
         */
-       public function deleteObjectsExpiringBefore( $date, $progressCallback = false ) {
-               $ret = false;
-               foreach ( $this->caches as $cache ) {
-                       if ( $cache->deleteObjectsExpiringBefore( $date, $progressCallback ) ) {
-                               $ret = true;
-                       }
-               }
-
-               return $ret;
+       protected function usesAsyncWritesGivenFlags( $flags ) {
+               return ( ( $flags & self::WRITE_SYNC ) == self::WRITE_SYNC ) ? false : $this->asyncWrites;
        }
 
        public function makeKeyInternal( $keyspace, $args ) {

diff --git a/tests/phpunit/includes/libs/objectcache/BagOStuffTest.php b/tests/phpunit/includes/libs/objectcache/BagOStuffTest.php
index 3d8c9cb..b68f150 100644 (file)
--- a/tests/phpunit/includes/libs/objectcache/BagOStuffTest.php
+++ b/tests/phpunit/includes/libs/objectcache/BagOStuffTest.php
@@ -98,7 +98,13 @@ class BagOStuffTest extends MediaWikiTestCase {
                        $this->cache->merge( $key, $callback, 5, 1 ),
                        'Non-blocking merge (CAS)'
                );
-               $this->assertEquals( 1, $calls );
+               if ( $this->cache instanceof MultiWriteBagOStuff ) {
+                       $wrapper = \Wikimedia\TestingAccessWrapper::newFromObject( $this->cache );
+                       $n = count( $wrapper->caches );
+               } else {
+                       $n = 1;
+               }
+               $this->assertEquals( $n, $calls );
        }
 
        /**