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

Change-Id: If3c3fbf21207b0c74cad8a29fa5bbabe0af896e3

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

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

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 );
 	}

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] );
 

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 = [

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 ) {