objectcache: make BagOStuff::add() abstract to discourage non-atomic versions

Change-Id: If3c3fbf21207b0c74cad8a29fa5bbabe0af896e3

diff --git a/RELEASE-NOTES-1.33 b/RELEASE-NOTES-1.33
index f8a4db1..cfda64e 100644 (file)
--- a/RELEASE-NOTES-1.33
+++ b/RELEASE-NOTES-1.33
@@ -352,6 +352,7 @@ because of Phabricator reports.
 * The implementation of buildStringCast() in Wikimedia\Rdbms\Database has
   changed to explicitly cast. Subclasses relying on the base-class
   implementation should check whether they need to override it now.
+* BagOStuff::add is now abstract and must explicitly be defined in subclasses.
 
 == Compatibility ==
 MediaWiki 1.33 requires PHP 7.0.13 or later. Although HHVM 3.18.5 or later is

diff --git a/includes/libs/objectcache/BagOStuff.php b/includes/libs/objectcache/BagOStuff.php
index 9c75a2f..224f8cf 100644 (file)
--- a/includes/libs/objectcache/BagOStuff.php
+++ b/includes/libs/objectcache/BagOStuff.php
@@ -630,14 +630,7 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface {
         * @param int $flags Bitfield of BagOStuff::WRITE_* constants (since 1.33)
         * @return bool Success
         */
-       public function add( $key, $value, $exptime = 0, $flags = 0 ) {
-               // @note: avoid lock() here since that method uses *this* method by default
-               if ( $this->get( $key ) === false ) {
-                       return $this->set( $key, $value, $exptime, $flags );
-               }
-
-               return false; // key already set
-       }
+       abstract public function add( $key, $value, $exptime = 0, $flags = 0 );
 
        /**
         * Increase stored value of $key by $value while preserving its TTL

diff --git a/includes/libs/objectcache/CachedBagOStuff.php b/includes/libs/objectcache/CachedBagOStuff.php
index 25fcdb0..95b12b4 100644 (file)
--- a/includes/libs/objectcache/CachedBagOStuff.php
+++ b/includes/libs/objectcache/CachedBagOStuff.php
@@ -101,6 +101,14 @@ class CachedBagOStuff extends HashBagOStuff {
        // These just call the backend (tested elsewhere)
        // @codeCoverageIgnoreStart
 
+       public function add( $key, $value, $exptime = 0, $flags = 0 ) {
+               if ( $this->get( $key ) === false ) {
+                       return $this->set( $key, $value, $exptime, $flags );
+               }
+
+               return false; // key already set
+       }
+
        public function lock( $key, $timeout = 6, $expiry = 6, $rclass = '' ) {
                return $this->backend->lock( $key, $timeout, $expiry, $rclass );
        }

diff --git a/includes/libs/objectcache/HashBagOStuff.php b/includes/libs/objectcache/HashBagOStuff.php
index 7d074fa..f88f567 100644 (file)
--- a/includes/libs/objectcache/HashBagOStuff.php
+++ b/includes/libs/objectcache/HashBagOStuff.php
@@ -106,6 +106,14 @@ class HashBagOStuff extends BagOStuff {
                return true;
        }
 
+       public function add( $key, $value, $exptime = 0, $flags = 0 ) {
+               if ( $this->get( $key ) === false ) {
+                       return $this->set( $key, $value, $exptime, $flags );
+               }
+
+               return false; // key already set
+       }
+
        public function delete( $key, $flags = 0 ) {
                unset( $this->bag[$key] );
 

diff --git a/includes/libs/objectcache/RESTBagOStuff.php b/includes/libs/objectcache/RESTBagOStuff.php
index b0b82d8..135556a 100644 (file)
--- a/includes/libs/objectcache/RESTBagOStuff.php
+++ b/includes/libs/objectcache/RESTBagOStuff.php
@@ -138,6 +138,14 @@ class RESTBagOStuff extends BagOStuff {
                return $this->handleError( "Failed to store $key", $rcode, $rerr );
        }
 
+       public function add( $key, $value, $exptime = 0, $flags = 0 ) {
+               if ( $this->get( $key ) === false ) {
+                       return $this->set( $key, $value, $exptime, $flags );
+               }
+
+               return false; // key already set
+       }
+
        public function delete( $key, $flags = 0 ) {
                // @TODO: respect WRITE_SYNC (e.g. EACH_QUORUM)
                $req = [

diff --git a/includes/libs/objectcache/ReplicatedBagOStuff.php b/includes/libs/objectcache/ReplicatedBagOStuff.php
index ea380a6..14e2fef 100644 (file)
--- a/includes/libs/objectcache/ReplicatedBagOStuff.php
+++ b/includes/libs/objectcache/ReplicatedBagOStuff.php
@@ -103,7 +103,7 @@ class ReplicatedBagOStuff extends BagOStuff {
        }
 
        public function add( $key, $value, $exptime = 0, $flags = 0 ) {
-               return $this->writeStore->add( $key, $value, $exptime );
+               return $this->writeStore->add( $key, $value, $exptime, $flags );
        }
 
        public function incr( $key, $value = 1 ) {