uuid: Simplify GlobalIdGenerator by removing QUICK_VOLATILE flag

AaronSchulz · Krinkle · commit 07b8154766de · 2021-04-14T21:40:35.000Z
Change-Id: I4f504be5d805281eeb9400cd4fd783c50b561b87
diff --git a/includes/libs/uuid/GlobalIdGenerator.php b/includes/libs/uuid/GlobalIdGenerator.php
@@ -20,8 +20,6 @@
 
 namespace Wikimedia\UUID;
 
-use BagOStuff;
-use EmptyBagOStuff;
 use InvalidArgumentException;
 use RuntimeException;
 use Wikimedia\Assert\Assert;
@@ -34,8 +32,6 @@
  * @since 1.35
  */
 class GlobalIdGenerator {
-	/** @var BagOStuff Server-local persistent cache instance */
-	protected $srvCache;
 	/** @var callable Callback for running shell commands */
 	protected $shellCallback;
 
@@ -60,7 +56,8 @@ class GlobalIdGenerator {
 	/** @var array Cached file handles */
 	protected $fileHandles = []; // cached file handles
 
-	public const QUICK_VOLATILE = 1; // use an APC like in-memory counter if available
+	/** @var int B/C constant (deprecated since 1.36) */
+	public const QUICK_VOLATILE = 1;
 
 	/**
 	 * Avoid using __CLASS__ so namespace separators aren't interpreted
@@ -81,10 +78,16 @@ class GlobalIdGenerator {
 
 	/**
 	 * @param string $tempDirectory A writable temporary directory
-	 * @param BagOStuff $srvCache A server-local APC-like cache instance
 	 * @param callback $shellCallback A callback that takes a shell command and returns the output
 	 */
-	public function __construct( $tempDirectory, BagOStuff $srvCache, callable $shellCallback ) {
+	public function __construct( $tempDirectory, $shellCallback ) {
+		if ( func_num_args() >= 3 && !is_callable( $shellCallback ) ) {
+			trigger_error(
+				__CLASS__ . ' with a BagOStuff instance was deprecated in MediaWiki 1.37.',
+				E_USER_DEPRECATED
+			);
+			$shellCallback = func_get_arg( 2 );
+		}
 		if ( !strlen( $tempDirectory ) ) {
 			throw new InvalidArgumentException( "No temp directory provided" );
 		}
@@ -96,7 +99,6 @@ public function __construct( $tempDirectory, BagOStuff $srvCache, callable $shel
 		$this->lockFile128 = $tempDirectory . '/' . self::FILE_PREFIX . '-UID-128';
 		$this->lockFileUUID = $tempDirectory . '/' . self::FILE_PREFIX . '-UUID-128';
 
-		$this->srvCache = $srvCache;
 		$this->shellCallback = $shellCallback;
 	}
 
@@ -373,55 +375,39 @@ protected function getSequentialPerNodeIDs( $bucket, $bits, $count, $flags ) {
 			throw new RuntimeException( "Requested bit size ($bits) is out of range." );
 		}
 
-		$counter = null; // post-increment persistent counter value
-
-		// Use APC/etc if requested, available, and not in CLI mode;
-		// Counter values would not survive across script instances in CLI mode.
-		if (
-			( $flags & self::QUICK_VOLATILE ) &&
-			!( $this->srvCache instanceof EmptyBagOStuff )
-		) {
-			$cache = $this->srvCache;
-			$counter = $cache->incrWithInit( $bucket, $cache::TTL_INDEFINITE, $count, $count );
-			if ( $counter === false ) {
-				throw new RuntimeException( 'Unable to set value to ' . get_class( $cache ) );
-			}
+		$path = $this->tmpDir . '/' . self::FILE_PREFIX . '-' . rawurlencode( $bucket ) . '-48';
+		// Get the UID lock file handle
+		if ( isset( $this->fileHandles[$path] ) ) {
+			$handle = $this->fileHandles[$path];
+		} else {
+			$handle = fopen( $path, 'cb+' );
+			$this->fileHandles[$path] = $handle ?: null; // cache
 		}
-
-		// Note: use of fmod() avoids "division by zero" on 32 bit machines
-		if ( $counter === null ) {
-			$path = $this->tmpDir . '/' . self::FILE_PREFIX . '-' . rawurlencode( $bucket ) . '-48';
-			// Get the UID lock file handle
-			if ( isset( $this->fileHandles[$path] ) ) {
-				$handle = $this->fileHandles[$path];
-			} else {
-				$handle = fopen( $path, 'cb+' );
-				$this->fileHandles[$path] = $handle ?: null; // cache
-			}
-			// Acquire the UID lock file
-			if ( $handle === false ) {
-				throw new RuntimeException( "Could not open '{$path}'." );
-			}
-			if ( !flock( $handle, LOCK_EX ) ) {
-				fclose( $handle );
-				throw new RuntimeException( "Could not acquire '{$path}'." );
-			}
-			// Fetch the counter value and increment it...
-			rewind( $handle );
-			$counter = floor( (float)trim( fgets( $handle ) ) ) + $count; // fetch as float
-			// Write back the new counter value
-			ftruncate( $handle, 0 );
-			rewind( $handle );
-			fwrite( $handle, fmod( $counter, 2 ** 48 ) ); // warp-around as needed
-			fflush( $handle );
-			// Release the UID lock file
-			flock( $handle, LOCK_UN );
+		// Acquire the UID lock file
+		if ( $handle === false ) {
+			throw new RuntimeException( "Could not open '{$path}'." );
 		}
+		if ( !flock( $handle, LOCK_EX ) ) {
+			fclose( $handle );
+			throw new RuntimeException( "Could not acquire '{$path}'." );
+		}
+		// Fetch the counter value and increment it...
+		rewind( $handle );
+		$counter = floor( (float)trim( fgets( $handle ) ) ) + $count; // fetch as float
+		// Write back the new counter value
+		ftruncate( $handle, 0 );
+		rewind( $handle );
+		// Use fmod() to avoid "division by zero" on 32 bit machines
+		fwrite( $handle, fmod( $counter, 2 ** 48 ) ); // warp-around as needed
+		fflush( $handle );
+		// Release the UID lock file
+		flock( $handle, LOCK_UN );
 
 		$ids = [];
 		$divisor = 2 ** $bits;
 		$currentId = floor( $counter - $count ); // pre-increment counter value
 		for ( $i = 0; $i < $count; ++$i ) {
+			// Use fmod() to avoid "division by zero" on 32 bit machines
 			$ids[] = fmod( ++$currentId, $divisor );
 		}
 
diff --git a/tests/phpunit/unit/includes/libs/uuid/GlobalIdGeneratorTest.php b/tests/phpunit/unit/includes/libs/uuid/GlobalIdGeneratorTest.php
@@ -218,7 +218,6 @@ public function testGetTimestampFromUUIDv1InvalidUUIDv1( string $uuid ) {
 	protected function setUp() : void {
 		$this->globalIdGenerator = new GlobalIdGenerator(
 			wfTempDir(),
-			new HashBagOStuff( [] ),
 			static function ( $command ) {
 				return wfShellExec( $command );
 			}

Original file line number	Diff line number	Diff line change
`@@ -218,7 +218,6 @@ public function testGetTimestampFromUUIDv1InvalidUUIDv1( string $uuid ) {`
`218`	`218`	`protected function setUp() : void {`
`219`	`219`	`$this->globalIdGenerator = new GlobalIdGenerator(`
`220`	`220`	`wfTempDir(),`
`221`		`- new HashBagOStuff( [] ),`
`222`	`221`	`static function ( $command ) {`
`223`	`222`	`return wfShellExec( $command );`
`224`	`223`	`}`