From 02846154850b24bf089337c60e7cace10d500088 Mon Sep 17 00:00:00 2001 From: Sam Reed Date: Tue, 10 Jan 2012 23:12:00 +0000 Subject: [PATCH] Add svn:keywords Id Trim trailing whitespace Add explicit member variables --- includes/ConcurrencyCheck.php | 59 +++++++++++-------- includes/api/ApiConcurrency.php | 12 ++-- .../phpunit/includes/ConcurrencyCheckTest.php | 20 +++---- .../includes/api/ApiConcurrencyTest.php | 3 +- 4 files changed, 51 insertions(+), 43 deletions(-) diff --git a/includes/ConcurrencyCheck.php b/includes/ConcurrencyCheck.php index d14a8e3dcf..7c20a21aee 100644 --- a/includes/ConcurrencyCheck.php +++ b/includes/ConcurrencyCheck.php @@ -18,6 +18,14 @@ * @author Ian Baker */ class ConcurrencyCheck { + + protected $expirationTime; + + /** + * @var User + */ + protected $user; + /** * Constructor * @@ -75,7 +83,7 @@ class ConcurrencyCheck { 'cc_expiration' => time() + $this->expirationTime, ), __METHOD__, - array('IGNORE') + array( 'IGNORE' ) ); // if the insert succeeded, checkout is done. @@ -143,7 +151,7 @@ class ConcurrencyCheck { $dbw = $this->dbw; $userId = $this->user->getId(); $cacheKey = wfMemcKey( $this->resourceType, $record ); - + $dbw->delete( 'concurrencycheck', array( @@ -154,14 +162,14 @@ class ConcurrencyCheck { __METHOD__, array() ); - + // check row count (this is atomic, select would not be) if( $dbw->affectedRows() === 1 ) { $memc->delete( $cacheKey ); return true; } - - return false; + + return false; } /** @@ -171,9 +179,9 @@ class ConcurrencyCheck { */ public function expire() { $memc = wfGetMainCache(); - $dbw = $this->dbw; + $dbw = $this->dbw; $now = time(); - + // get the rows to remove from cache. $res = $dbw->select( 'concurrencycheck', @@ -184,13 +192,13 @@ class ConcurrencyCheck { __METHOD__, array() ); - + // build a list of rows to delete. $toExpire = array(); while( $res && $record = $res->fetchRow() ) { $toExpire[] = $record['cc_record']; } - + // remove the rows from the db $dbw->delete( 'concurrencycheck', @@ -200,7 +208,7 @@ class ConcurrencyCheck { __METHOD__, array() ); - + // delete all those rows from cache // outside a transaction because deletes don't require atomicity. foreach( $toExpire as $expire ) { @@ -210,7 +218,7 @@ class ConcurrencyCheck { // return the number of rows removed. return $dbw->affectedRows(); } - + public function status( $keys ) { $memc = wfGetMainCache(); $dbw = $this->dbw; @@ -218,11 +226,11 @@ class ConcurrencyCheck { $checkouts = array(); $toSelect = array(); - + // validate keys, attempt to retrieve from cache. foreach( $keys as $key ) { $this->validateId( $key ); - + $cached = $memc->get( wfMemcKey( $this->resourceType, $key ) ); if( $cached && $cached['expiration'] > $now ) { $checkouts[$key] = array( @@ -256,11 +264,11 @@ class ConcurrencyCheck { __METHOD__, array() ); - + while( $res && $record = $res->fetchRow() ) { $record['status'] = 'valid'; $checkouts[ $record['cc_record'] ] = $record; - + // safe to store values since this is inside the transaction $memc->set( wfMemcKey( $this->resourceType, $record['cc_record'] ), @@ -272,7 +280,7 @@ class ConcurrencyCheck { // end the transaction. $dbw->rollback(); } - + // if a key was passed in but has no (unexpired) checkout, include it in the // result set to make things easier and more consistent on the client-side. foreach( $keys as $key ) { @@ -280,19 +288,22 @@ class ConcurrencyCheck { $checkouts[$key]['status'] = 'invalid'; } } - + return $checkouts; } - + public function listCheckouts() { // TODO: fill in the function that lets you get the complete set of checkouts for a given application. } - - public function setUser ( $user ) { + + /** + * @param $user user + */ + public function setUser( $user ) { $this->user = $user; } - - public function setExpirationTime ( $expirationTime = null ) { + + public function setExpirationTime( $expirationTime = null ) { global $wgConcurrencyExpirationDefault, $wgConcurrencyExpirationMax, $wgConcurrencyExpirationMin; // check to make sure the time is digits only, so it can be used in queries @@ -321,10 +332,10 @@ class ConcurrencyCheck { if( ! preg_match('/^\d+$/', $record) ) { throw new ConcurrencyCheckBadRecordIdException( 'Record ID ' . $record . ' must be a positive integer' ); } - + // TODO: add a hook here for client-side validation. return true; } } -class ConcurrencyCheckBadRecordIdException extends MWException {}; +class ConcurrencyCheckBadRecordIdException extends MWException {} diff --git a/includes/api/ApiConcurrency.php b/includes/api/ApiConcurrency.php index 1c2d5a793a..4eeaeab214 100644 --- a/includes/api/ApiConcurrency.php +++ b/includes/api/ApiConcurrency.php @@ -4,7 +4,6 @@ * API module that handles cooperative locking of web resources */ class ApiConcurrency extends ApiBase { - public function __construct( $main, $action ) { parent::__construct( $main, $action ); } @@ -24,10 +23,9 @@ class ApiConcurrency extends ApiBase { case 'checkout': case 'checkin': if ( $concurrencyCheck->$params['ccaction']( $params['record'] ) ) { - $res['result'] = 'success'; - } - else { - $res['result'] = 'failure'; + $res['result'] = 'success'; + } else { + $res['result'] = 'failure'; } break; @@ -92,7 +90,7 @@ class ApiConcurrency extends ApiBase { } public function getVersion() { - return __CLASS__ . ': $Id: ApiConcurrency.php $'; + return __CLASS__ . ': $Id$'; } private function checkPermission( $user ) { @@ -104,4 +102,4 @@ class ApiConcurrency extends ApiBase { } } -} \ No newline at end of file +} diff --git a/tests/phpunit/includes/ConcurrencyCheckTest.php b/tests/phpunit/includes/ConcurrencyCheckTest.php index fe0f77072a..9d80932844 100644 --- a/tests/phpunit/includes/ConcurrencyCheckTest.php +++ b/tests/phpunit/includes/ConcurrencyCheckTest.php @@ -5,9 +5,9 @@ class ConcurrencyCheckTest extends MediaWikiTestCase { * @var Array of test users */ public static $users; - + // Prepare test environment - + public function setUp() { parent::setUp(); @@ -25,22 +25,22 @@ class ConcurrencyCheckTest extends MediaWikiTestCase { array() ), ); - + // turn on memcached for this test. // if no memcached is present, this still works fine. global $wgMainCacheType; $this->oldcache = $wgMainCacheType; $wgMainCacheType = CACHE_MEMCACHED; } - + public function tearDown() { // turn off caching again. global $wgMainCacheType; $wgMainCacheType = $this->oldcache; - - parent::tearDown(); + + parent::tearDown(); } - + // Actual tests from here down public function testCheckoutCheckin() { @@ -51,7 +51,7 @@ class ConcurrencyCheckTest extends MediaWikiTestCase { // clean up after any previously failed tests $first->checkin($testKey); $second->checkin($testKey); - + // tests $this->assertTrue( $first->checkout($testKey), "Initial checkout" ); $this->assertTrue( $first->checkout($testKey), "Cache hit" ); @@ -77,7 +77,7 @@ class ConcurrencyCheckTest extends MediaWikiTestCase { $cc->checkout( 1339 ); $cc->setExpirationTime(); $cc->checkout( 13310 ); - + // tests $this->assertEquals( 2, $cc->expire(), "Resource expiration" ); $this->assertTrue( $cc->checkin( 13310 ), "Checkin succeeds after expiration" ); @@ -99,4 +99,4 @@ class ConcurrencyCheckTest extends MediaWikiTestCase { $this->assertEquals( 'invalid', $output[1339]['status'], "Expired checkouts are invalid"); $this->assertEquals( 'invalid', $output[13310]['status'], "Missing checkouts are invalid"); } -} \ No newline at end of file +} diff --git a/tests/phpunit/includes/api/ApiConcurrencyTest.php b/tests/phpunit/includes/api/ApiConcurrencyTest.php index cf00992e09..03c596b59f 100644 --- a/tests/phpunit/includes/api/ApiConcurrencyTest.php +++ b/tests/phpunit/includes/api/ApiConcurrencyTest.php @@ -106,7 +106,6 @@ class ApiConcurrencyTest extends ApiTestCase { } - /** * @depends testLogin */ @@ -168,4 +167,4 @@ class ApiConcurrencyTest extends ApiTestCase { } -} \ No newline at end of file +} -- 2.20.1