From 25a1651aadad734c5124a52225c972b00e92de96 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Sat, 17 Sep 2016 21:42:56 -0700 Subject: [PATCH] Make LockManager use StatusValue and move classes to /libs Change-Id: Ifa41fc2939f3515d4a056746b0fcbff79786d25b --- autoload.php | 8 ++-- includes/filebackend/FileBackend.php | 4 +- .../filebackend/lockmanager/DBLockManager.php | 4 +- .../lockmanager/MemcLockManager.php | 10 ++--- .../lockmanager/MySqlLockManager.php | 4 +- .../lockmanager/PostgreSqlLockManager.php | 4 +- .../lockmanager/RedisLockManager.php | 6 +-- includes/libs/StatusValue.php | 4 +- .../lockmanager/FSLockManager.php | 12 +++--- .../lockmanager/LockManager.php | 18 +-------- includes/libs/lockmanager/NullLockManager.php | 37 +++++++++++++++++++ .../lockmanager/QuorumLockManager.php | 8 ++-- 12 files changed, 71 insertions(+), 48 deletions(-) rename includes/{filebackend => libs}/lockmanager/FSLockManager.php (96%) rename includes/{filebackend => libs}/lockmanager/LockManager.php (95%) create mode 100644 includes/libs/lockmanager/NullLockManager.php rename includes/{filebackend => libs}/lockmanager/QuorumLockManager.php (98%) diff --git a/autoload.php b/autoload.php index a07df966cc..ff7d488205 100644 --- a/autoload.php +++ b/autoload.php @@ -438,7 +438,7 @@ $wgAutoloadLocalClasses = [ 'FSFileBackendFileList' => __DIR__ . '/includes/filebackend/FSFileBackend.php', 'FSFileBackendList' => __DIR__ . '/includes/filebackend/FSFileBackend.php', 'FSFileOpHandle' => __DIR__ . '/includes/filebackend/FSFileBackend.php', - 'FSLockManager' => __DIR__ . '/includes/filebackend/lockmanager/FSLockManager.php', + 'FSLockManager' => __DIR__ . '/includes/libs/lockmanager/FSLockManager.php', 'FSRepo' => __DIR__ . '/includes/filerepo/FSRepo.php', 'FakeAuthTemplate' => __DIR__ . '/includes/specialpage/LoginSignupSpecialPage.php', 'FakeConverter' => __DIR__ . '/languages/FakeConverter.php', @@ -747,7 +747,7 @@ $wgAutoloadLocalClasses = [ 'LocalSettingsGenerator' => __DIR__ . '/includes/installer/LocalSettingsGenerator.php', 'LocalisationCache' => __DIR__ . '/includes/cache/localisation/LocalisationCache.php', 'LocalisationCacheBulkLoad' => __DIR__ . '/includes/cache/localisation/LocalisationCacheBulkLoad.php', - 'LockManager' => __DIR__ . '/includes/filebackend/lockmanager/LockManager.php', + 'LockManager' => __DIR__ . '/includes/libs/lockmanager/LockManager.php', 'LockManagerGroup' => __DIR__ . '/includes/filebackend/lockmanager/LockManagerGroup.php', 'LogEntry' => __DIR__ . '/includes/logging/LogEntry.php', 'LogEntryBase' => __DIR__ . '/includes/logging/LogEntry.php', @@ -979,7 +979,7 @@ $wgAutoloadLocalClasses = [ 'NullFileOp' => __DIR__ . '/includes/filebackend/FileOp.php', 'NullIndexField' => __DIR__ . '/includes/search/NullIndexField.php', 'NullJob' => __DIR__ . '/includes/jobqueue/jobs/NullJob.php', - 'NullLockManager' => __DIR__ . '/includes/filebackend/lockmanager/LockManager.php', + 'NullLockManager' => __DIR__ . '/includes/libs/lockmanager/NullLockManager.php', 'NullRepo' => __DIR__ . '/includes/filerepo/NullRepo.php', 'NullStatsdDataFactory' => __DIR__ . '/includes/libs/stats/NullStatsdDataFactory.php', 'NumericUppercaseCollation' => __DIR__ . '/includes/collation/NumericUppercaseCollation.php', @@ -1110,7 +1110,7 @@ $wgAutoloadLocalClasses = [ 'PurgeParserCache' => __DIR__ . '/maintenance/purgeParserCache.php', 'QueryPage' => __DIR__ . '/includes/specialpage/QueryPage.php', 'QuickTemplate' => __DIR__ . '/includes/skins/QuickTemplate.php', - 'QuorumLockManager' => __DIR__ . '/includes/filebackend/lockmanager/QuorumLockManager.php', + 'QuorumLockManager' => __DIR__ . '/includes/libs/lockmanager/QuorumLockManager.php', 'RCCacheEntry' => __DIR__ . '/includes/changes/RCCacheEntry.php', 'RCCacheEntryFactory' => __DIR__ . '/includes/changes/RCCacheEntryFactory.php', 'RCDatabaseLogEntry' => __DIR__ . '/includes/logging/LogEntry.php', diff --git a/includes/filebackend/FileBackend.php b/includes/filebackend/FileBackend.php index 1f91b3f13a..ed2bdcc140 100644 --- a/includes/filebackend/FileBackend.php +++ b/includes/filebackend/FileBackend.php @@ -1260,7 +1260,7 @@ abstract class FileBackend { final public function lockFiles( array $paths, $type, $timeout = 0 ) { $paths = array_map( 'FileBackend::normalizeStoragePath', $paths ); - return $this->lockManager->lock( $paths, $type, $timeout ); + return $this->wrapStatus( $this->lockManager->lock( $paths, $type, $timeout ) ); } /** @@ -1273,7 +1273,7 @@ abstract class FileBackend { final public function unlockFiles( array $paths, $type ) { $paths = array_map( 'FileBackend::normalizeStoragePath', $paths ); - return $this->lockManager->unlock( $paths, $type ); + return $this->wrapStatus( $this->lockManager->unlock( $paths, $type ) ); } /** diff --git a/includes/filebackend/lockmanager/DBLockManager.php b/includes/filebackend/lockmanager/DBLockManager.php index cccf71a929..4667dde450 100644 --- a/includes/filebackend/lockmanager/DBLockManager.php +++ b/includes/filebackend/lockmanager/DBLockManager.php @@ -104,7 +104,7 @@ abstract class DBLockManager extends QuorumLockManager { // @todo change this code to work in one batch protected function getLocksOnServer( $lockSrv, array $pathsByType ) { - $status = Status::newGood(); + $status = StatusValue::newGood(); foreach ( $pathsByType as $type => $paths ) { $status->merge( $this->doGetLocksOnServer( $lockSrv, $paths, $type ) ); } @@ -115,7 +115,7 @@ abstract class DBLockManager extends QuorumLockManager { abstract protected function doGetLocksOnServer( $lockSrv, array $paths, $type ); protected function freeLocksOnServer( $lockSrv, array $pathsByType ) { - return Status::newGood(); + return StatusValue::newGood(); } /** diff --git a/includes/filebackend/lockmanager/MemcLockManager.php b/includes/filebackend/lockmanager/MemcLockManager.php index 2e2d0a3533..81ce424b50 100644 --- a/includes/filebackend/lockmanager/MemcLockManager.php +++ b/includes/filebackend/lockmanager/MemcLockManager.php @@ -90,7 +90,7 @@ class MemcLockManager extends QuorumLockManager { // @todo Change this code to work in one batch protected function getLocksOnServer( $lockSrv, array $pathsByType ) { - $status = Status::newGood(); + $status = StatusValue::newGood(); $lockedPaths = []; foreach ( $pathsByType as $type => $paths ) { @@ -112,7 +112,7 @@ class MemcLockManager extends QuorumLockManager { // @todo Change this code to work in one batch protected function freeLocksOnServer( $lockSrv, array $pathsByType ) { - $status = Status::newGood(); + $status = StatusValue::newGood(); foreach ( $pathsByType as $type => $paths ) { $status->merge( $this->doFreeLocksOnServer( $lockSrv, $paths, $type ) ); @@ -129,7 +129,7 @@ class MemcLockManager extends QuorumLockManager { * @return StatusValue */ protected function doGetLocksOnServer( $lockSrv, array $paths, $type ) { - $status = Status::newGood(); + $status = StatusValue::newGood(); $memc = $this->getCache( $lockSrv ); $keys = array_map( [ $this, 'recordKeyForPath' ], $paths ); // lock records @@ -205,7 +205,7 @@ class MemcLockManager extends QuorumLockManager { * @return StatusValue */ protected function doFreeLocksOnServer( $lockSrv, array $paths, $type ) { - $status = Status::newGood(); + $status = StatusValue::newGood(); $memc = $this->getCache( $lockSrv ); $keys = array_map( [ $this, 'recordKeyForPath' ], $paths ); // lock records @@ -257,7 +257,7 @@ class MemcLockManager extends QuorumLockManager { * @return StatusValue */ protected function releaseAllLocks() { - return Status::newGood(); // not supported + return StatusValue::newGood(); // not supported } /** diff --git a/includes/filebackend/lockmanager/MySqlLockManager.php b/includes/filebackend/lockmanager/MySqlLockManager.php index 896e0ffd64..124d41038a 100644 --- a/includes/filebackend/lockmanager/MySqlLockManager.php +++ b/includes/filebackend/lockmanager/MySqlLockManager.php @@ -38,7 +38,7 @@ class MySqlLockManager extends DBLockManager { * @return StatusValue */ protected function doGetLocksOnServer( $lockSrv, array $paths, $type ) { - $status = Status::newGood(); + $status = StatusValue::newGood(); $db = $this->getConnection( $lockSrv ); // checked in isServerUp() @@ -108,7 +108,7 @@ class MySqlLockManager extends DBLockManager { * @return StatusValue */ protected function releaseAllLocks() { - $status = Status::newGood(); + $status = StatusValue::newGood(); foreach ( $this->conns as $lockDb => $db ) { if ( $db->trxLevel() ) { // in transaction diff --git a/includes/filebackend/lockmanager/PostgreSqlLockManager.php b/includes/filebackend/lockmanager/PostgreSqlLockManager.php index 307c16447e..d6b1ce822d 100644 --- a/includes/filebackend/lockmanager/PostgreSqlLockManager.php +++ b/includes/filebackend/lockmanager/PostgreSqlLockManager.php @@ -14,7 +14,7 @@ class PostgreSqlLockManager extends DBLockManager { ]; protected function doGetLocksOnServer( $lockSrv, array $paths, $type ) { - $status = Status::newGood(); + $status = StatusValue::newGood(); if ( !count( $paths ) ) { return $status; // nothing to lock } @@ -64,7 +64,7 @@ class PostgreSqlLockManager extends DBLockManager { * @return StatusValue */ protected function releaseAllLocks() { - $status = Status::newGood(); + $status = StatusValue::newGood(); foreach ( $this->conns as $lockDb => $db ) { try { diff --git a/includes/filebackend/lockmanager/RedisLockManager.php b/includes/filebackend/lockmanager/RedisLockManager.php index 4121ecb29d..6fd819d637 100644 --- a/includes/filebackend/lockmanager/RedisLockManager.php +++ b/includes/filebackend/lockmanager/RedisLockManager.php @@ -79,7 +79,7 @@ class RedisLockManager extends QuorumLockManager { } protected function getLocksOnServer( $lockSrv, array $pathsByType ) { - $status = Status::newGood(); + $status = StatusValue::newGood(); $pathList = call_user_func_array( 'array_merge', array_values( $pathsByType ) ); @@ -172,7 +172,7 @@ LUA; } protected function freeLocksOnServer( $lockSrv, array $pathsByType ) { - $status = Status::newGood(); + $status = StatusValue::newGood(); $pathList = call_user_func_array( 'array_merge', array_values( $pathsByType ) ); @@ -242,7 +242,7 @@ LUA; } protected function releaseAllLocks() { - return Status::newGood(); // not supported + return StatusValue::newGood(); // not supported } protected function isServerUp( $lockSrv ) { diff --git a/includes/libs/StatusValue.php b/includes/libs/StatusValue.php index 45185c5130..bff9abd61f 100644 --- a/includes/libs/StatusValue.php +++ b/includes/libs/StatusValue.php @@ -58,7 +58,7 @@ class StatusValue { * Factory function for fatal errors * * @param string|MessageSpecifier $message Message key or object - * @return StatusValue + * @return static */ public static function newFatal( $message /*, parameters...*/ ) { $params = func_get_args(); @@ -71,7 +71,7 @@ class StatusValue { * Factory function for good results * * @param mixed $value - * @return StatusValue + * @return static */ public static function newGood( $value = null ) { $result = new static(); diff --git a/includes/filebackend/lockmanager/FSLockManager.php b/includes/libs/lockmanager/FSLockManager.php similarity index 96% rename from includes/filebackend/lockmanager/FSLockManager.php rename to includes/libs/lockmanager/FSLockManager.php index b6629aa846..7f33a0abdf 100644 --- a/includes/filebackend/lockmanager/FSLockManager.php +++ b/includes/libs/lockmanager/FSLockManager.php @@ -70,7 +70,7 @@ class FSLockManager extends LockManager { * @return StatusValue */ protected function doLock( array $paths, $type ) { - $status = Status::newGood(); + $status = StatusValue::newGood(); $lockedPaths = []; // files locked in this attempt foreach ( $paths as $path ) { @@ -95,7 +95,7 @@ class FSLockManager extends LockManager { * @return StatusValue */ protected function doUnlock( array $paths, $type ) { - $status = Status::newGood(); + $status = StatusValue::newGood(); foreach ( $paths as $path ) { $status->merge( $this->doSingleUnlock( $path, $type ) ); @@ -112,7 +112,7 @@ class FSLockManager extends LockManager { * @return StatusValue */ protected function doSingleLock( $path, $type ) { - $status = Status::newGood(); + $status = StatusValue::newGood(); if ( isset( $this->locksHeld[$path][$type] ) ) { ++$this->locksHeld[$path][$type]; @@ -157,7 +157,7 @@ class FSLockManager extends LockManager { * @return StatusValue */ protected function doSingleUnlock( $path, $type ) { - $status = Status::newGood(); + $status = StatusValue::newGood(); if ( !isset( $this->locksHeld[$path] ) ) { $status->warning( 'lockmanager-notlocked', $path ); @@ -200,7 +200,7 @@ class FSLockManager extends LockManager { * @return StatusValue */ private function closeLockHandles( $path, array $handlesToClose ) { - $status = Status::newGood(); + $status = StatusValue::newGood(); foreach ( $handlesToClose as $handle ) { if ( !flock( $handle, LOCK_UN ) ) { $status->fatal( 'lockmanager-fail-releaselock', $path ); @@ -218,7 +218,7 @@ class FSLockManager extends LockManager { * @return StatusValue */ private function pruneKeyLockFiles( $path ) { - $status = Status::newGood(); + $status = StatusValue::newGood(); if ( !isset( $this->locksHeld[$path] ) ) { # No locks are held for the lock file anymore if ( !unlink( $this->getLockPath( $path ) ) ) { diff --git a/includes/filebackend/lockmanager/LockManager.php b/includes/libs/lockmanager/LockManager.php similarity index 95% rename from includes/filebackend/lockmanager/LockManager.php rename to includes/libs/lockmanager/LockManager.php index e7f37ed981..80add5b8b7 100644 --- a/includes/filebackend/lockmanager/LockManager.php +++ b/includes/libs/lockmanager/LockManager.php @@ -191,7 +191,7 @@ abstract class LockManager { * @since 1.22 */ protected function doLockByType( array $pathsByType ) { - $status = Status::newGood(); + $status = StatusValue::newGood(); $lockedByType = []; // map of (type => paths) foreach ( $pathsByType as $type => $paths ) { $status->merge( $this->doLock( $paths, $type ) ); @@ -225,7 +225,7 @@ abstract class LockManager { * @since 1.22 */ protected function doUnlockByType( array $pathsByType ) { - $status = Status::newGood(); + $status = StatusValue::newGood(); foreach ( $pathsByType as $type => $paths ) { $status->merge( $this->doUnlock( $paths, $type ) ); } @@ -242,17 +242,3 @@ abstract class LockManager { */ abstract protected function doUnlock( array $paths, $type ); } - -/** - * Simple version of LockManager that does nothing - * @since 1.19 - */ -class NullLockManager extends LockManager { - protected function doLock( array $paths, $type ) { - return Status::newGood(); - } - - protected function doUnlock( array $paths, $type ) { - return Status::newGood(); - } -} diff --git a/includes/libs/lockmanager/NullLockManager.php b/includes/libs/lockmanager/NullLockManager.php new file mode 100644 index 0000000000..5ad558fa74 --- /dev/null +++ b/includes/libs/lockmanager/NullLockManager.php @@ -0,0 +1,37 @@ + type => paths) // Get locks that need to be acquired (buckets => locks)... @@ -83,7 +83,7 @@ abstract class QuorumLockManager extends LockManager { } protected function doUnlockByType( array $pathsByType ) { - $status = Status::newGood(); + $status = StatusValue::newGood(); $pathsToUnlock = []; // (bucket => type => paths) foreach ( $pathsByType as $type => $paths ) { @@ -127,7 +127,7 @@ abstract class QuorumLockManager extends LockManager { * @return StatusValue */ final protected function doLockingRequestBucket( $bucket, array $pathsByType ) { - $status = Status::newGood(); + $status = StatusValue::newGood(); $yesVotes = 0; // locks made on trustable servers $votesLeft = count( $this->srvsByBucket[$bucket] ); // remaining peers @@ -169,7 +169,7 @@ abstract class QuorumLockManager extends LockManager { * @return StatusValue */ final protected function doUnlockingRequestBucket( $bucket, array $pathsByType ) { - $status = Status::newGood(); + $status = StatusValue::newGood(); $yesVotes = 0; // locks freed on trustable servers $votesLeft = count( $this->srvsByBucket[$bucket] ); // remaining peers -- 2.20.1