| Index: trunk/extensions/InterfaceConcurrency/InterfaceConcurrency.i18n.php |
| — | — | @@ -12,9 +12,9 @@ |
| 13 | 13 | * @author Ian Baker |
| 14 | 14 | */ |
| 15 | 15 | $messages['en'] = array( |
| 16 | | - 'interfaceconcurrency-desc' => 'Provides backend for UI that shows which resources are in use', |
| | 16 | + 'interfaceconcurrency-desc' => 'Provides a backend for the user interface that shows which resources are in use', |
| 17 | 17 | ); |
| 18 | 18 | |
| 19 | 19 | $messages['qqq'] = array( |
| 20 | | - 'interfaceconcurrency-desc' => 'Description of this extension', |
| | 20 | + 'interfaceconcurrency-desc' => '{{desc}}', |
| 21 | 21 | ); |
| Index: trunk/extensions/InterfaceConcurrency/InterfaceConcurrency.php |
| — | — | @@ -24,7 +24,7 @@ |
| 25 | 25 | // Check environment |
| 26 | 26 | if ( !defined( 'MEDIAWIKI' ) ) { |
| 27 | 27 | echo( "This is an extension to MediaWiki and cannot be run standalone.\n" ); |
| 28 | | - die( - 1 ); |
| | 28 | + die( -1 ); |
| 29 | 29 | } |
| 30 | 30 | |
| 31 | 31 | // Credits |
| Index: trunk/extensions/InterfaceConcurrency/tests/phpunit/ApiConcurrencyTest.php |
| — | — | @@ -78,7 +78,7 @@ |
| 79 | 79 | |
| 80 | 80 | $wgUser = self::$users['one']->user; |
| 81 | 81 | /* commenting these out since i need to go home and they're breakin CI. See commit summary for details. |
| 82 | | - |
| | 82 | + |
| 83 | 83 | list( $result, , $session ) = $this->doApiRequestWithToken( array( |
| 84 | 84 | 'action' => 'concurrency', |
| 85 | 85 | 'ccaction' => 'checkout', |
| — | — | @@ -142,7 +142,7 @@ |
| 143 | 143 | 'resourcetype' => 'responding-to-moodbar-feedback'), $sessionArray['two'], self::$users['two']->user ); |
| 144 | 144 | |
| 145 | 145 | $this->assertEquals( "success", $result['concurrency']['result'] ); |
| 146 | | - */ |
| | 146 | + */ |
| 147 | 147 | } |
| 148 | 148 | |
| 149 | 149 | /** |
| — | — | @@ -152,17 +152,17 @@ |
| 153 | 153 | $exception = false; |
| 154 | 154 | try { |
| 155 | 155 | global $wgUser; |
| 156 | | - |
| | 156 | + |
| 157 | 157 | $wgUser = self::$users['one']->user; |
| 158 | | - |
| | 158 | + |
| 159 | 159 | list( $result, , $session ) = $this->doApiRequestWithToken( array( |
| 160 | 160 | 'action' => 'concurrency', |
| 161 | 161 | 'ccaction' => 'checkinX', |
| 162 | 162 | 'record' => 1, |
| 163 | | - 'resourcetype' => 'responding-to-moodbar-feedback'), $sessionArray['one'], self::$users['one']->user ); |
| | 163 | + 'resourcetype' => 'responding-to-moodbar-feedback' ), $sessionArray['one'], self::$users['one']->user ); |
| 164 | 164 | } catch ( UsageException $e ) { |
| 165 | 165 | $exception = true; |
| 166 | | - $this->assertEquals("Unrecognized value for parameter 'ccaction': checkinX", |
| | 166 | + $this->assertEquals( "Unrecognized value for parameter 'ccaction': checkinX", |
| 167 | 167 | $e->getMessage() ); |
| 168 | 168 | } |
| 169 | 169 | $this->assertTrue( $exception, "Got exception" ); |
| Index: trunk/extensions/InterfaceConcurrency/tests/phpunit/ConcurrencyCheckTest.php |
| — | — | @@ -10,7 +10,7 @@ |
| 11 | 11 | |
| 12 | 12 | public function setUp() { |
| 13 | 13 | parent::setUp(); |
| 14 | | - |
| | 14 | + |
| 15 | 15 | self::$users = array( |
| 16 | 16 | 'user1' => new ApiTestUser( |
| 17 | 17 | 'Concurrencychecktestuser1', |
| — | — | @@ -43,38 +43,38 @@ |
| 44 | 44 | } |
| 45 | 45 | |
| 46 | 46 | // Actual tests from here down |
| 47 | | - |
| | 47 | + |
| 48 | 48 | public function testCheckoutCheckin() { |
| 49 | 49 | $first = new ConcurrencyCheck( 'CCUnitTest', self::$users['user1']->user ); |
| 50 | 50 | $second = new ConcurrencyCheck( 'CCUnitTest', self::$users['user2']->user ); |
| 51 | 51 | $testKey = 1337; |
| 52 | 52 | |
| 53 | 53 | // clean up after any previously failed tests |
| 54 | | - $first->checkin($testKey); |
| 55 | | - $second->checkin($testKey); |
| | 54 | + $first->checkin( $testKey ); |
| | 55 | + $second->checkin( $testKey ); |
| 56 | 56 | |
| 57 | 57 | // tests |
| 58 | | - $this->assertTrue( $first->checkout($testKey), "Initial checkout" ); |
| 59 | | - $this->assertTrue( $first->checkout($testKey), "Cache hit" ); |
| 60 | | - $this->assertFalse( $second->checkout($testKey), "Checkout of locked resource fails as different user" ); |
| 61 | | - $this->assertTrue( $first->checkout($testKey), "Checkout of locked resource succeeds as original user" ); |
| 62 | | - $this->assertFalse( $second->checkin($testKey), "Checkin of locked resource fails as different user" ); |
| 63 | | - $this->assertTrue( $first->checkin($testKey), "Checkin of locked resource succeeds as original user" ); |
| 64 | | - $second->setExpirationTime(-5); |
| 65 | | - $this->assertTrue( $second->checkout($testKey), "Checked-in resource is now available to second user" ); |
| | 58 | + $this->assertTrue( $first->checkout( $testKey ), "Initial checkout" ); |
| | 59 | + $this->assertTrue( $first->checkout( $testKey ), "Cache hit" ); |
| | 60 | + $this->assertFalse( $second->checkout( $testKey ), "Checkout of locked resource fails as different user" ); |
| | 61 | + $this->assertTrue( $first->checkout( $testKey ), "Checkout of locked resource succeeds as original user" ); |
| | 62 | + $this->assertFalse( $second->checkin( $testKey ), "Checkin of locked resource fails as different user" ); |
| | 63 | + $this->assertTrue( $first->checkin( $testKey ), "Checkin of locked resource succeeds as original user" ); |
| | 64 | + $second->setExpirationTime( -5 ); |
| | 65 | + $this->assertTrue( $second->checkout( $testKey ), "Checked-in resource is now available to second user" ); |
| 66 | 66 | $second->setExpirationTime(); |
| 67 | | - $this->assertTrue( $first->checkout($testKey), "Checkout of expired resource succeeds as first user"); |
| 68 | | - $this->assertTrue( $second->checkout($testKey, true), "Checkout override" ); |
| 69 | | - $this->assertFalse( $first->checkout($testKey), "Checkout of overriden resource fails as different user" ); |
| | 67 | + $this->assertTrue( $first->checkout( $testKey ), "Checkout of expired resource succeeds as first user" ); |
| | 68 | + $this->assertTrue( $second->checkout( $testKey, true ), "Checkout override" ); |
| | 69 | + $this->assertFalse( $first->checkout( $testKey ), "Checkout of overriden resource fails as different user" ); |
| 70 | 70 | |
| 71 | 71 | // cleanup |
| 72 | | - $this->assertTrue( $second->checkin($testKey), "Checkin of record with changed ownership" ); |
| 73 | | - |
| | 72 | + $this->assertTrue( $second->checkin( $testKey ), "Checkin of record with changed ownership" ); |
| | 73 | + |
| 74 | 74 | } |
| 75 | 75 | |
| 76 | 76 | public function testExpire() { |
| 77 | 77 | $cc = new ConcurrencyCheck( 'CCUnitTest', self::$users['user1']->user ); |
| 78 | | - $cc->setExpirationTime(-1); |
| | 78 | + $cc->setExpirationTime( -1 ); |
| 79 | 79 | $cc->checkout( 1338 ); // these numbers are test record ids. |
| 80 | 80 | $cc->checkout( 1339 ); |
| 81 | 81 | $cc->setExpirationTime(); |
| — | — | @@ -82,14 +82,14 @@ |
| 83 | 83 | |
| 84 | 84 | // tests |
| 85 | 85 | $this->assertEquals( 2, $cc->expire(), "Resource expiration" ); |
| 86 | | - $this->assertTrue( $cc->checkin( 13310 ), "Checkin succeeds after expiration" ); |
| | 86 | + $this->assertTrue( $cc->checkin( 13310 ), "Checkin succeeds after expiration" ); |
| 87 | 87 | } |
| 88 | | - |
| | 88 | + |
| 89 | 89 | public function testStatus() { |
| 90 | 90 | $cc = new ConcurrencyCheck( 'CCUnitTest', self::$users['user1']->user ); |
| 91 | 91 | $cc->checkout( 1337 ); |
| 92 | 92 | $cc->checkout( 1338 ); |
| 93 | | - $cc->setExpirationTime(-5); |
| | 93 | + $cc->setExpirationTime( -5 ); |
| 94 | 94 | $cc->checkout( 1339 ); |
| 95 | 95 | $cc->setExpirationTime(); |
| 96 | 96 | |
| — | — | @@ -97,8 +97,8 @@ |
| 98 | 98 | $output = $cc->status( array( 1337, 1338, 1339, 13310 ) ); |
| 99 | 99 | $this->assertEquals( true, is_array( $output ), "Status returns values" ); |
| 100 | 100 | $this->assertEquals( 4, count( $output ), "Output has the correct number of records" ); |
| 101 | | - $this->assertEquals( 'valid', $output[1337]['status'], "Current checkouts are listed as valid"); |
| 102 | | - $this->assertEquals( 'invalid', $output[1339]['status'], "Expired checkouts are invalid"); |
| 103 | | - $this->assertEquals( 'invalid', $output[13310]['status'], "Missing checkouts are invalid"); |
| | 101 | + $this->assertEquals( 'valid', $output[1337]['status'], "Current checkouts are listed as valid" ); |
| | 102 | + $this->assertEquals( 'invalid', $output[1339]['status'], "Expired checkouts are invalid" ); |
| | 103 | + $this->assertEquals( 'invalid', $output[13310]['status'], "Missing checkouts are invalid" ); |
| 104 | 104 | } |
| 105 | 105 | } |
| Index: trunk/extensions/InterfaceConcurrency/InterfaceConcurrency.hooks.php |
| — | — | @@ -1,18 +1,16 @@ |
| 2 | 2 | <?php |
| 3 | 3 | |
| 4 | 4 | class InterfaceConcurrencyHooks { |
| 5 | | - |
| 6 | 5 | /** |
| 7 | 6 | * Runs InterfaceConcurrency schema updates# |
| 8 | 7 | * |
| 9 | 8 | * @param $updater DatabaseUpdater |
| 10 | 9 | */ |
| 11 | 10 | public static function onLoadExtensionSchemaUpdates( $updater = null ) { |
| 12 | | - $dir = dirname(__FILE__) . '/sql'; |
| | 11 | + $dir = dirname( __FILE__ ) . '/sql'; |
| 13 | 12 | |
| 14 | 13 | $updater->addExtensionTable( 'concurrencycheck', "$dir/concurrencycheck.sql" ); |
| 15 | | - |
| | 14 | + |
| 16 | 15 | return true; |
| 17 | 16 | } |
| 18 | | - |
| 19 | | -} |
| \ No newline at end of file |
| | 17 | +} |
| Index: trunk/extensions/InterfaceConcurrency/includes/ConcurrencyCheck.php |
| — | — | @@ -18,7 +18,6 @@ |
| 19 | 19 | * @author Ian Baker <ian@wikimedia.org> |
| 20 | 20 | */ |
| 21 | 21 | class ConcurrencyCheck { |
| 22 | | - |
| 23 | 22 | protected $expirationTime; |
| 24 | 23 | |
| 25 | 24 | /** |
| — | — | @@ -28,7 +27,7 @@ |
| 29 | 28 | |
| 30 | 29 | /** |
| 31 | 30 | * Constructor |
| 32 | | - * |
| | 31 | + * |
| 33 | 32 | * @var $resourceType String The calling application or type of resource, conceptually like a namespace |
| 34 | 33 | * @var $user User object, the current user |
| 35 | 34 | * @var $expirationTime Integer (optional) How long should a checkout last, in seconds |
| — | — | @@ -62,16 +61,16 @@ |
| 63 | 62 | |
| 64 | 63 | // when operating with a single memcached cluster, it's reasonable to check the cache here. |
| 65 | 64 | global $wgConcurrency; |
| 66 | | - if( $wgConcurrency['TrustMemc'] ) { |
| 67 | | - $cached = $wgMemc->get( $cacheKey ); |
| 68 | | - if( $cached ) { |
| 69 | | - if( ! $override && $cached['userId'] != $userId && $cached['expiration'] > time() ) { |
| | 65 | + if ( $wgConcurrency['TrustMemc'] ) { |
| | 66 | + $cached = $wgMemc->get( $cacheKey ); |
| | 67 | + if ( $cached ) { |
| | 68 | + if ( ! $override && $cached['userId'] != $userId && $cached['expiration'] > time() ) { |
| 70 | 69 | // this is already checked out. |
| 71 | 70 | return false; |
| 72 | 71 | } |
| 73 | 72 | } |
| 74 | 73 | } |
| 75 | | - |
| | 74 | + |
| 76 | 75 | // attempt an insert, check success (this is atomic) |
| 77 | 76 | $insertError = null; |
| 78 | 77 | $res = $dbw->insert( |
| — | — | @@ -87,7 +86,7 @@ |
| 88 | 87 | ); |
| 89 | 88 | |
| 90 | 89 | // if the insert succeeded, checkout is done. |
| 91 | | - if( $dbw->affectedRows() === 1 ) { |
| | 90 | + if ( $dbw->affectedRows() === 1 ) { |
| 92 | 91 | // delete any existing cache key. can't create a new key here |
| 93 | 92 | // since the insert didn't happen inside a transaction. |
| 94 | 93 | $wgMemc->delete( $cacheKey ); |
| — | — | @@ -103,14 +102,14 @@ |
| 104 | 103 | array( |
| 105 | 104 | 'cc_resource_type' => $this->resourceType, |
| 106 | 105 | 'cc_record' => $record, |
| 107 | | - '(cc_user = ' . $userId . ' OR cc_expiration <= ' . $dbw->addQuotes(wfTimestamp( TS_MW )) . ')', // only the owner can perform a checkin |
| | 106 | + '(cc_user = ' . $userId . ' OR cc_expiration <= ' . $dbw->addQuotes( wfTimestamp( TS_MW ) ) . ')', // only the owner can perform a checkin |
| 108 | 107 | ), |
| 109 | 108 | __METHOD__, |
| 110 | 109 | array() |
| 111 | 110 | ); |
| 112 | | - |
| | 111 | + |
| 113 | 112 | // delete failed: not checked out by current user, checkout is unexpired, override is unset |
| 114 | | - if( $dbw->affectedRows() !== 1 && ! $override) { |
| | 113 | + if ( $dbw->affectedRows() !== 1 && ! $override ) { |
| 115 | 114 | // fetch the existing data to cache it |
| 116 | 115 | $row = $dbw->selectRow( |
| 117 | 116 | 'concurrencycheck', |
| — | — | @@ -122,7 +121,7 @@ |
| 123 | 122 | __METHOD__, |
| 124 | 123 | array() |
| 125 | 124 | ); |
| 126 | | - |
| | 125 | + |
| 127 | 126 | // this was a cache miss. populate the cache with data from the db. |
| 128 | 127 | // cache is set to expire at the same time as the checkout, since it'll become invalid then anyway. |
| 129 | 128 | // inside this transaction, a row-level lock is established which ensures cache concurrency |
| — | — | @@ -149,7 +148,7 @@ |
| 150 | 149 | |
| 151 | 150 | // cache the result. |
| 152 | 151 | $wgMemc->set( $cacheKey, array( 'userId' => $userId, 'expiration' => $expiration ), $this->expirationTime ); |
| 153 | | - |
| | 152 | + |
| 154 | 153 | $dbw->commit(); |
| 155 | 154 | return true; |
| 156 | 155 | } |
| — | — | @@ -162,7 +161,7 @@ |
| 163 | 162 | */ |
| 164 | 163 | public function checkin( $record ) { |
| 165 | 164 | global $wgMemc; |
| 166 | | - $this->validateId( $record ); |
| | 165 | + $this->validateId( $record ); |
| 167 | 166 | $dbw = $this->dbw; |
| 168 | 167 | $userId = $this->user->getId(); |
| 169 | 168 | $cacheKey = wfMemcKey( 'concurrencycheck', $this->resourceType, $record ); |
| — | — | @@ -179,7 +178,7 @@ |
| 180 | 179 | ); |
| 181 | 180 | |
| 182 | 181 | // check row count (this is atomic, select would not be) |
| 183 | | - if( $dbw->affectedRows() === 1 ) { |
| | 182 | + if ( $dbw->affectedRows() === 1 ) { |
| 184 | 183 | $wgMemc->delete( $cacheKey ); |
| 185 | 184 | return true; |
| 186 | 185 | } |
| — | — | @@ -220,11 +219,11 @@ |
| 221 | 220 | $toSelect = array(); |
| 222 | 221 | |
| 223 | 222 | // validate keys, attempt to retrieve from cache. |
| 224 | | - foreach( $keys as $key ) { |
| | 223 | + foreach ( $keys as $key ) { |
| 225 | 224 | $this->validateId( $key ); |
| 226 | 225 | |
| 227 | 226 | $cached = $wgMemc->get( wfMemcKey( 'concurrencycheck', $this->resourceType, $key ) ); |
| 228 | | - if( $cached && $cached['expiration'] > $now ) { |
| | 227 | + if ( $cached && $cached['expiration'] > $now ) { |
| 229 | 228 | $checkouts[$key] = array( |
| 230 | 229 | 'status' => 'valid', |
| 231 | 230 | 'cc_resource_type' => $this->resourceType, |
| — | — | @@ -239,11 +238,11 @@ |
| 240 | 239 | } |
| 241 | 240 | |
| 242 | 241 | // if there were cache misses... |
| 243 | | - if( $toSelect ) { |
| | 242 | + if ( $toSelect ) { |
| 244 | 243 | // If it's time to go to the database, go ahead and expire old rows. |
| 245 | 244 | $this->expire(); |
| 246 | | - |
| 247 | | - |
| | 245 | + |
| | 246 | + |
| 248 | 247 | // Why LOCK IN SHARE MODE, you might ask? To avoid a race condition: Otherwise, it's possible for |
| 249 | 248 | // a checkin and/or checkout to occur between this select and the value being stored in cache, which |
| 250 | 249 | // makes for an incorrect cache. This, in turn, could make checkout() above (which uses the cache) |
| — | — | @@ -257,13 +256,13 @@ |
| 258 | 257 | // It appears all the DBMSes we use support LOCK IN SHARE MODE, but if that's not the case, the second |
| 259 | 258 | // solution above could be implemented instead. |
| 260 | 259 | $queryParams = array(); |
| 261 | | - if( $wgDBtype === 'mysql' ) { |
| | 260 | + if ( $wgDBtype === 'mysql' ) { |
| 262 | 261 | $queryParamsp[] = 'LOCK IN SHARE MODE'; |
| 263 | 262 | |
| 264 | 263 | // the transaction seems incongruous, I know, but it's to keep the cache update atomic. |
| 265 | 264 | $dbw->begin(); |
| 266 | 265 | } |
| 267 | | - |
| | 266 | + |
| 268 | 267 | $res = $dbw->select( |
| 269 | 268 | 'concurrencycheck', |
| 270 | 269 | array( '*' ), |
| — | — | @@ -276,13 +275,13 @@ |
| 277 | 276 | $queryParams |
| 278 | 277 | ); |
| 279 | 278 | |
| 280 | | - while( $res && $record = $res->fetchRow() ) { |
| | 279 | + while ( $res && $record = $res->fetchRow() ) { |
| 281 | 280 | $record['status'] = 'valid'; |
| 282 | 281 | $checkouts[ $record['cc_record'] ] = $record; |
| 283 | 282 | |
| 284 | 283 | // TODO: implement strategy #2 above, determine which DBMSes need which method. |
| 285 | 284 | // for now, disable adding to cache here for databases that don't support read locking |
| 286 | | - if( $wgDBtype !== 'mysql' ) { |
| | 285 | + if ( $wgDBtype !== 'mysql' ) { |
| 287 | 286 | // safe to store values since this is inside the transaction |
| 288 | 287 | $wgMemc->set( |
| 289 | 288 | wfMemcKey( 'concurrencycheck', $this->resourceType, $record['cc_record'] ), |
| — | — | @@ -292,7 +291,7 @@ |
| 293 | 292 | } |
| 294 | 293 | } |
| 295 | 294 | |
| 296 | | - if( $wgDBtype === 'mysql' ) { |
| | 295 | + if ( $wgDBtype === 'mysql' ) { |
| 297 | 296 | // end the transaction. |
| 298 | 297 | $dbw->rollback(); |
| 299 | 298 | } |
| — | — | @@ -300,8 +299,8 @@ |
| 301 | 300 | |
| 302 | 301 | // if a key was passed in but has no (unexpired) checkout, include it in the |
| 303 | 302 | // result set to make things easier and more consistent on the client-side. |
| 304 | | - foreach( $keys as $key ) { |
| 305 | | - if( ! array_key_exists( $key, $checkouts ) ) { |
| | 303 | + foreach ( $keys as $key ) { |
| | 304 | + if ( ! array_key_exists( $key, $checkouts ) ) { |
| 306 | 305 | $checkouts[$key]['status'] = 'invalid'; |
| 307 | 306 | } |
| 308 | 307 | } |
| — | — | @@ -322,11 +321,11 @@ |
| 323 | 322 | |
| 324 | 323 | public function setExpirationTime( $expirationTime = null ) { |
| 325 | 324 | global $wgConcurrency; |
| 326 | | - |
| | 325 | + |
| 327 | 326 | // check to make sure the time is a number |
| 328 | 327 | // negative number are allowed, though mostly only used for testing |
| 329 | | - if( $expirationTime && (int) $expirationTime == $expirationTime ) { |
| 330 | | - if( $expirationTime > $wgConcurrency['ExpirationMax'] ) { |
| | 328 | + if ( $expirationTime && (int) $expirationTime == $expirationTime ) { |
| | 329 | + if ( $expirationTime > $wgConcurrency['ExpirationMax'] ) { |
| 331 | 330 | $this->expirationTime = $wgConcurrency['ExpirationMax']; // if the number is too high, limit it to the max value. |
| 332 | 331 | } elseif ( $expirationTime < $wgConcurrency['ExpirationMin'] ) { |
| 333 | 332 | $this->expirationTime = $wgConcurrency['ExpirationMin']; // low limit, default -1 min |
| — | — | @@ -346,7 +345,7 @@ |
| 347 | 346 | * @return boolean |
| 348 | 347 | */ |
| 349 | 348 | private static function validateId ( $record ) { |
| 350 | | - if( (int) $record !== $record || $record <= 0 ) { |
| | 349 | + if ( (int) $record !== $record || $record <= 0 ) { |
| 351 | 350 | throw new ConcurrencyCheckBadRecordIdException( 'Record ID ' . $record . ' must be a positive integer' ); |
| 352 | 351 | } |
| 353 | 352 | |
| Index: trunk/extensions/InterfaceConcurrency/ApiConcurrency.php |
| — | — | @@ -92,7 +92,7 @@ |
| 93 | 93 | public function getVersion() { |
| 94 | 94 | return __CLASS__ . ': $Id: ApiConcurrency.php 108560 2012-01-10 23:12:00Z reedy $'; |
| 95 | 95 | } |
| 96 | | - |
| | 96 | + |
| 97 | 97 | private function checkPermission( $user ) { |
| 98 | 98 | if ( $user->isAnon() ) { |
| 99 | 99 | $this->dieUsage( "You don't have permission to do that", 'permission-denied' ); |
| — | — | @@ -101,5 +101,4 @@ |
| 102 | 102 | $this->dieUsageMsg( array( 'blockedtext' ) ); |
| 103 | 103 | } |
| 104 | 104 | } |
| 105 | | - |
| 106 | 105 | } |