From 25dbd91513f1e55ee2ae9f01ef6abce648376a32 Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Fri, 29 Jan 2016 20:09:57 -0500 Subject: [PATCH] Clean up after Ie161e0f Ie161e0f was done in a hurry, and so didn't do things in the best ways. This introduces a new "CachedBagOStuff" that transparently handles all the logic that had been copy-pasted all over in Ie161e0f. The differences between CachedBagOStuff and MultiWriteBagOStuff are: * CachedBagOStuff supports only one "backend". * There's a flag for writes to only go to the in-memory cache. * The in-memory cache is always updated. * Locks go to the backend cache (with MultiWriteBagOStuff, it would wind up going to the HashBagOStuff used for the in-memory cache). Change-Id: Iea494729bd2e8c6c5ab8facf4c241232e31e8215 --- autoload.php | 1 + includes/libs/objectcache/BagOStuff.php | 1 + includes/libs/objectcache/CachedBagOStuff.php | 114 ++++++++++++++++++ includes/session/SessionBackend.php | 44 ++----- includes/session/SessionManager.php | 63 +++------- .../libs/objectcache/CachedBagOStuffTest.php | 52 ++++++++ .../session/CookieSessionProviderTest.php | 9 +- ...ImmutableSessionProviderWithCookieTest.php | 3 +- .../session/PHPSessionHandlerTest.php | 12 +- .../includes/session/SessionBackendTest.php | 22 +++- .../includes/session/SessionManagerTest.php | 13 +- .../includes/session/TestBagOStuff.php | 14 ++- 12 files changed, 242 insertions(+), 106 deletions(-) create mode 100644 includes/libs/objectcache/CachedBagOStuff.php create mode 100644 tests/phpunit/includes/libs/objectcache/CachedBagOStuffTest.php diff --git a/autoload.php b/autoload.php index 90730051ae..c9e3b60ced 100644 --- a/autoload.php +++ b/autoload.php @@ -190,6 +190,7 @@ $wgAutoloadLocalClasses = array( 'CacheHelper' => __DIR__ . '/includes/cache/CacheHelper.php', 'CacheTime' => __DIR__ . '/includes/parser/CacheTime.php', 'CachedAction' => __DIR__ . '/includes/actions/CachedAction.php', + 'CachedBagOStuff' => __DIR__ . '/includes/libs/objectcache/CachedBagOStuff.php', 'CachingSiteStore' => __DIR__ . '/includes/site/CachingSiteStore.php', 'CapsCleanup' => __DIR__ . '/maintenance/cleanupCaps.php', 'Category' => __DIR__ . '/includes/Category.php', diff --git a/includes/libs/objectcache/BagOStuff.php b/includes/libs/objectcache/BagOStuff.php index b9be43df8a..3736103351 100644 --- a/includes/libs/objectcache/BagOStuff.php +++ b/includes/libs/objectcache/BagOStuff.php @@ -69,6 +69,7 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface { const READ_VERIFIED = 2; // promise that caller can tell when keys are stale /** Bitfield constants for set()/merge() */ const WRITE_SYNC = 1; // synchronously write to all locations for replicated stores + const WRITE_CACHE_ONLY = 2; // Only change state of the in-memory cache public function __construct( array $params = array() ) { if ( isset( $params['logger'] ) ) { diff --git a/includes/libs/objectcache/CachedBagOStuff.php b/includes/libs/objectcache/CachedBagOStuff.php new file mode 100644 index 0000000000..fc156187fe --- /dev/null +++ b/includes/libs/objectcache/CachedBagOStuff.php @@ -0,0 +1,114 @@ +backend = $backend; + parent::__construct( $params ); + } + + protected function doGet( $key, $flags = 0 ) { + $ret = parent::doGet( $key, $flags ); + if ( $ret === false ) { + $ret = $this->backend->doGet( $key, $flags ); + if ( $ret !== false ) { + $this->set( $key, $ret, 0, self::WRITE_CACHE_ONLY ); + } + } + return $ret; + } + + public function set( $key, $value, $exptime = 0, $flags = 0 ) { + parent::set( $key, $value, $exptime, $flags ); + if ( !( $flags & self::WRITE_CACHE_ONLY ) ) { + $this->backend->set( $key, $value, $exptime, $flags & ~self::WRITE_CACHE_ONLY ); + } + return true; + } + + public function delete( $key, $flags = 0 ) { + unset( $this->bag[$key] ); + if ( !( $flags & self::WRITE_CACHE_ONLY ) ) { + $this->backend->delete( $key ); + } + + return true; + } + + public function setLogger( LoggerInterface $logger ) { + parent::setLogger( $logger ); + $this->backend->setLogger( $logger ); + } + + public function setDebug( $bool ) { + parent::setDebug( $bool ); + $this->backend->setDebug( $bool ); + } + + public function lock( $key, $timeout = 6, $expiry = 6, $rclass = '' ) { + return $this->backend->lock( $key, $timeout, $expiry, $rclass ); + } + + public function unlock( $key ) { + return $this->backend->unlock( $key ); + } + + public function deleteObjectsExpiringBefore( $date, $progressCallback = false ) { + parent::deleteObjectsExpiringBefore( $date, $progressCallback ); + return $this->backend->deleteObjectsExpiringBefore( $date, $progressCallback ); + } + + public function getLastError() { + return $this->backend->getLastError(); + } + + public function clearLastError() { + $this->backend->clearLastError(); + } + + public function modifySimpleRelayEvent( array $event ) { + return $this->backend->modifySimpleRelayEvent( $event ); + } + +} diff --git a/includes/session/SessionBackend.php b/includes/session/SessionBackend.php index 2a13ed20ef..488f6e7684 100644 --- a/includes/session/SessionBackend.php +++ b/includes/session/SessionBackend.php @@ -23,7 +23,7 @@ namespace MediaWiki\Session; -use BagOStuff; +use CachedBagOStuff; use Psr\Log\LoggerInterface; use User; use WebRequest; @@ -64,10 +64,8 @@ final class SessionBackend { /** @var string Used to detect subarray modifications */ private $dataHash = null; - /** @var BagOStuff */ - private $tempStore; - /** @var BagOStuff */ - private $permStore; + /** @var CachedBagOStuff */ + private $store; /** @var LoggerInterface */ private $logger; @@ -99,14 +97,12 @@ final class SessionBackend { /** * @param SessionId $id Session ID object * @param SessionInfo $info Session info to populate from - * @param BagOStuff $tempStore In-process data store - * @param BagOStuff $permstore Backend data store for persisted sessions + * @param CachedBagOStuff $store Backend data store * @param LoggerInterface $logger * @param int $lifetime Session data lifetime in seconds */ public function __construct( - SessionId $id, SessionInfo $info, BagOStuff $tempStore, BagOStuff $permStore, - LoggerInterface $logger, $lifetime + SessionId $id, SessionInfo $info, CachedBagOStuff $store, LoggerInterface $logger, $lifetime ) { $phpSessionHandling = \RequestContext::getMain()->getConfig()->get( 'PHPSessionHandling' ); $this->usePhpSessionHandling = $phpSessionHandling !== 'disable'; @@ -125,8 +121,7 @@ final class SessionBackend { $this->id = $id; $this->user = $info->getUserInfo() ? $info->getUserInfo()->getUser() : new User; - $this->tempStore = $tempStore; - $this->permStore = $permStore; + $this->store = $store; $this->logger = $logger; $this->lifetime = $lifetime; $this->provider = $info->getProvider(); @@ -135,14 +130,7 @@ final class SessionBackend { $this->forceHTTPS = $info->forceHTTPS(); $this->providerMetadata = $info->getProviderMetadata(); - $key = wfMemcKey( 'MWSession', (string)$this->id ); - $blob = $tempStore->get( $key ); - if ( $blob === false ) { - $blob = $permStore->get( $key ); - if ( $blob !== false ) { - $tempStore->set( $key, $blob ); - } - } + $blob = $store->get( wfMemcKey( 'MWSession', (string)$this->id ) ); if ( !is_array( $blob ) || !isset( $blob['metadata'] ) || !is_array( $blob['metadata'] ) || !isset( $blob['data'] ) || !is_array( $blob['data'] ) @@ -241,8 +229,7 @@ final class SessionBackend { $this->autosave(); // Delete the data for the old session ID now - $this->tempStore->delete( wfMemcKey( 'MWSession', $oldId ) ); - $this->permStore->delete( wfMemcKey( 'MWSession', $oldId ) ); + $this->store->delete( wfMemcKey( 'MWSession', $oldId ) ); } } @@ -626,24 +613,15 @@ final class SessionBackend { } } - $this->tempStore->set( + $this->store->set( wfMemcKey( 'MWSession', (string)$this->id ), array( 'data' => $this->data, 'metadata' => $metadata, ), - $metadata['expires'] + $metadata['expires'], + $this->persist ? 0 : CachedBagOStuff::WRITE_CACHE_ONLY ); - if ( $this->persist ) { - $this->permStore->set( - wfMemcKey( 'MWSession', (string)$this->id ), - array( - 'data' => $this->data, - 'metadata' => $metadata, - ), - $metadata['expires'] - ); - } $this->metaDirty = false; $this->dataDirty = false; diff --git a/includes/session/SessionManager.php b/includes/session/SessionManager.php index 4b38a5c318..57d56646b4 100644 --- a/includes/session/SessionManager.php +++ b/includes/session/SessionManager.php @@ -25,6 +25,7 @@ namespace MediaWiki\Session; use Psr\Log\LoggerInterface; use BagOStuff; +use CachedBagOStuff; use Config; use FauxRequest; use Language; @@ -54,11 +55,8 @@ final class SessionManager implements SessionManagerInterface { /** @var Config */ private $config; - /** @var BagOStuff|null */ - private $tempStore; - - /** @var BagOStuff|null */ - private $permStore; + /** @var CachedBagOStuff|null */ + private $store; /** @var SessionProvider[] */ private $sessionProviders = null; @@ -162,18 +160,18 @@ final class SessionManager implements SessionManagerInterface { $this->setLogger( \MediaWiki\Logger\LoggerFactory::getInstance( 'session' ) ); } - $this->tempStore = new \HashBagOStuff; if ( isset( $options['store'] ) ) { if ( !$options['store'] instanceof BagOStuff ) { throw new \InvalidArgumentException( '$options[\'store\'] must be an instance of BagOStuff' ); } - $this->permStore = $options['store']; + $store = $options['store']; } else { - $this->permStore = \ObjectCache::getInstance( $this->config->get( 'SessionCacheType' ) ); - $this->permStore->setLogger( $this->logger ); + $store = \ObjectCache::getInstance( $this->config->get( 'SessionCacheType' ) ); + $store->setLogger( $this->logger ); } + $this->store = $store instanceof CachedBagOStuff ? $store : new CachedBagOStuff( $store ); register_shutdown_function( array( $this, 'shutdown' ) ); } @@ -206,14 +204,7 @@ final class SessionManager implements SessionManagerInterface { // Test this here to provide a better log message for the common case // of "no such ID" $key = wfMemcKey( 'MWSession', $id ); - $existing = $this->tempStore->get( $key ); - if ( $existing === false ) { - $existing = $this->permStore->get( $key ); - if ( $existing !== false ) { - $this->tempStore->set( $key, $existing ); - } - } - if ( is_array( $existing ) ) { + if ( is_array( $this->store->get( $key ) ) ) { $info = new SessionInfo( SessionInfo::MIN_PRIORITY, array( 'id' => $id, 'idIsSafe' => true ) ); if ( $this->loadSessionInfoFromStore( $info, $request ) ) { $session = $this->getSessionFromInfo( $info, $request ); @@ -251,14 +242,7 @@ final class SessionManager implements SessionManagerInterface { } $key = wfMemcKey( 'MWSession', $id ); - $existing = $this->tempStore->get( $key ); - if ( $existing === false ) { - $existing = $this->permStore->get( $key ); - if ( $existing !== false ) { - $this->tempStore->set( $key, $existing ); - } - } - if ( is_array( $existing ) ) { + if ( is_array( $this->store->get( $key ) ) ) { throw new \InvalidArgumentException( 'Session ID already exists' ); } } @@ -678,13 +662,7 @@ final class SessionManager implements SessionManagerInterface { */ private function loadSessionInfoFromStore( SessionInfo &$info, WebRequest $request ) { $key = wfMemcKey( 'MWSession', $info->getId() ); - $blob = $this->tempStore->get( $key ); - if ( $blob === false ) { - $blob = $this->permStore->get( $key ); - if ( $blob !== false ) { - $this->tempStore->set( $key, $blob ); - } - } + $blob = $this->store->get( $key ); $newParams = array(); @@ -692,8 +670,7 @@ final class SessionManager implements SessionManagerInterface { // Sanity check: blob must be an array, if it's saved at all if ( !is_array( $blob ) ) { $this->logger->warning( "Session $info: Bad data" ); - $this->tempStore->delete( $key ); - $this->permStore->delete( $key ); + $this->store->delete( $key ); return false; } @@ -702,8 +679,7 @@ final class SessionManager implements SessionManagerInterface { !isset( $blob['metadata'] ) || !is_array( $blob['metadata'] ) ) { $this->logger->warning( "Session $info: Bad data structure" ); - $this->tempStore->delete( $key ); - $this->permStore->delete( $key ); + $this->store->delete( $key ); return false; } @@ -718,8 +694,7 @@ final class SessionManager implements SessionManagerInterface { !array_key_exists( 'provider', $metadata ) ) { $this->logger->warning( "Session $info: Bad metadata" ); - $this->tempStore->delete( $key ); - $this->permStore->delete( $key ); + $this->store->delete( $key ); return false; } @@ -729,8 +704,7 @@ final class SessionManager implements SessionManagerInterface { $newParams['provider'] = $provider = $this->getProvider( $metadata['provider'] ); if ( !$provider ) { $this->logger->warning( "Session $info: Unknown provider, " . $metadata['provider'] ); - $this->tempStore->delete( $key ); - $this->permStore->delete( $key ); + $this->store->delete( $key ); return false; } } elseif ( $metadata['provider'] !== (string)$provider ) { @@ -921,8 +895,7 @@ final class SessionManager implements SessionManagerInterface { $backend = new SessionBackend( $this->allSessionIds[$id], $info, - $this->tempStore, - $this->permStore, + $this->store, $this->logger, $this->config->get( 'ObjectCacheSessionExpiry' ) ); @@ -999,9 +972,7 @@ final class SessionManager implements SessionManagerInterface { do { $id = wfBaseConvert( \MWCryptRand::generateHex( 40 ), 16, 32, 32 ); $key = wfMemcKey( 'MWSession', $id ); - } while ( isset( $this->allSessionIds[$id] ) || - is_array( $this->tempStore->get( $key ) ) || is_array( $this->permStore->get( $key ) ) - ); + } while ( isset( $this->allSessionIds[$id] ) || is_array( $this->store->get( $key ) ) ); return $id; } @@ -1011,7 +982,7 @@ final class SessionManager implements SessionManagerInterface { * @param PHPSessionHandler $handler */ public function setupPHPSessionHandler( PHPSessionHandler $handler ) { - $handler->setManager( $this, $this->permStore, $this->logger ); + $handler->setManager( $this, $this->store, $this->logger ); } /** diff --git a/tests/phpunit/includes/libs/objectcache/CachedBagOStuffTest.php b/tests/phpunit/includes/libs/objectcache/CachedBagOStuffTest.php new file mode 100644 index 0000000000..3b19c9a88e --- /dev/null +++ b/tests/phpunit/includes/libs/objectcache/CachedBagOStuffTest.php @@ -0,0 +1,52 @@ +set( 'foo', 'bar' ); + $this->assertEquals( 'bar', $cache->get( 'foo' ) ); + + $backend->set( 'foo', 'baz' ); + $this->assertEquals( 'bar', $cache->get( 'foo' ), 'cached' ); + } + + public function testSetAndDelete() { + $backend = new HashBagOStuff; + $cache = new CachedBagOStuff( $backend ); + + for ( $i = 0; $i < 10; $i++ ) { + $cache->set( "key$i", 1 ); + $this->assertEquals( 1, $cache->get( "key$i" ) ); + $this->assertEquals( 1, $backend->get( "key$i" ) ); + $cache->delete( "key$i" ); + $this->assertEquals( false, $cache->get( "key$i" ) ); + $this->assertEquals( false, $backend->get( "key$i" ) ); + } + } + + public function testWriteCacheOnly() { + $backend = new HashBagOStuff; + $cache = new CachedBagOStuff( $backend ); + + $cache->set( 'foo', 'bar', 0, CachedBagOStuff::WRITE_CACHE_ONLY ); + $this->assertEquals( 'bar', $cache->get( 'foo' ) ); + $this->assertFalse( $backend->get( 'foo' ) ); + + $cache->set( 'foo', 'old' ); + $this->assertEquals( 'old', $cache->get( 'foo' ) ); + $this->assertEquals( 'old', $backend->get( 'foo' ) ); + + $cache->set( 'foo', 'new', 0, CachedBagOStuff::WRITE_CACHE_ONLY ); + $this->assertEquals( 'new', $cache->get( 'foo' ) ); + $this->assertEquals( 'old', $backend->get( 'foo' ) ); + + $cache->delete( 'foo', CachedBagOStuff::WRITE_CACHE_ONLY ); + $this->assertEquals( 'old', $cache->get( 'foo' ) ); // Reloaded from backend + } +} diff --git a/tests/phpunit/includes/session/CookieSessionProviderTest.php b/tests/phpunit/includes/session/CookieSessionProviderTest.php index e5df458802..9ba67df244 100644 --- a/tests/phpunit/includes/session/CookieSessionProviderTest.php +++ b/tests/phpunit/includes/session/CookieSessionProviderTest.php @@ -352,7 +352,7 @@ class CookieSessionProviderTest extends MediaWikiTestCase { $provider->setManager( SessionManager::singleton() ); $sessionId = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'; - $store = new \HashBagOStuff(); + $store = new TestBagOStuff(); $user = User::newFromName( 'UTSysop' ); $anon = new User; @@ -365,7 +365,6 @@ class CookieSessionProviderTest extends MediaWikiTestCase { 'idIsSafe' => true, ) ), $store, - $store, new \Psr\Log\NullLogger(), 10 ); @@ -451,8 +450,7 @@ class CookieSessionProviderTest extends MediaWikiTestCase { 'persisted' => true, 'idIsSafe' => true, ) ), - new \EmptyBagOStuff(), - new \EmptyBagOStuff(), + new TestBagOStuff(), new \Psr\Log\NullLogger(), 10 ); @@ -544,7 +542,7 @@ class CookieSessionProviderTest extends MediaWikiTestCase { $provider->setManager( SessionManager::singleton() ); $sessionId = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'; - $store = new \HashBagOStuff(); + $store = new TestBagOStuff(); $user = User::newFromName( 'UTSysop' ); $anon = new User; @@ -557,7 +555,6 @@ class CookieSessionProviderTest extends MediaWikiTestCase { 'idIsSafe' => true, ) ), $store, - $store, new \Psr\Log\NullLogger(), 10 ); diff --git a/tests/phpunit/includes/session/ImmutableSessionProviderWithCookieTest.php b/tests/phpunit/includes/session/ImmutableSessionProviderWithCookieTest.php index 46f23f3841..95f8e0127f 100644 --- a/tests/phpunit/includes/session/ImmutableSessionProviderWithCookieTest.php +++ b/tests/phpunit/includes/session/ImmutableSessionProviderWithCookieTest.php @@ -204,8 +204,7 @@ class ImmutableSessionProviderWithCookieTest extends MediaWikiTestCase { 'userInfo' => UserInfo::newFromUser( $user, true ), 'idIsSafe' => true, ) ), - new \EmptyBagOStuff(), - new \EmptyBagOStuff(), + new TestBagOStuff(), new \Psr\Log\NullLogger(), 10 ); diff --git a/tests/phpunit/includes/session/PHPSessionHandlerTest.php b/tests/phpunit/includes/session/PHPSessionHandlerTest.php index 1c54a20af3..3044aa7a61 100644 --- a/tests/phpunit/includes/session/PHPSessionHandlerTest.php +++ b/tests/phpunit/includes/session/PHPSessionHandlerTest.php @@ -78,7 +78,7 @@ class PHPSessionHandlerTest extends MediaWikiTestCase { ini_set( 'session.use_cookies', 1 ); ini_set( 'session.use_trans_sid', 1 ); - $store = new \HashBagOStuff(); + $store = new TestBagOStuff(); $logger = new \TestLogger(); $manager = new SessionManager( array( 'store' => $store, @@ -112,7 +112,7 @@ class PHPSessionHandlerTest extends MediaWikiTestCase { 'wgObjectCacheSessionExpiry' => 2, ) ); - $store = new \HashBagOStuff(); + $store = new TestBagOStuff(); $logger = new \TestLogger( true, function ( $m ) { return preg_match( '/^SessionBackend a{32} /', $m ) ? null : $m; } ); @@ -172,6 +172,14 @@ class PHPSessionHandlerTest extends MediaWikiTestCase { $this->assertSame( $expect, $_SESSION ); } + // Test expiry + session_write_close(); + ini_set( 'session.gc_divisor', 1 ); + ini_set( 'session.gc_probability', 1 ); + sleep( 3 ); + session_start(); + $this->assertSame( array(), $_SESSION ); + // Re-fill the session, then test that session_destroy() works. $_SESSION['AuthenticationSessionTest'] = $rand; session_write_close(); diff --git a/tests/phpunit/includes/session/SessionBackendTest.php b/tests/phpunit/includes/session/SessionBackendTest.php index 85fa9bdca0..f08a07d7d4 100644 --- a/tests/phpunit/includes/session/SessionBackendTest.php +++ b/tests/phpunit/includes/session/SessionBackendTest.php @@ -59,7 +59,7 @@ class SessionBackendTest extends MediaWikiTestCase { ) ); $id = new SessionId( $info->getId() ); - $backend = new SessionBackend( $id, $info, $this->store, $this->store, $logger, 10 ); + $backend = new SessionBackend( $id, $info, $this->store, $logger, 10 ); $priv = \TestingAccessWrapper::newFromObject( $backend ); $priv->persist = false; $priv->requests = array( 100 => new \FauxRequest() ); @@ -87,7 +87,7 @@ class SessionBackendTest extends MediaWikiTestCase { $id = new SessionId( $info->getId() ); $logger = new \Psr\Log\NullLogger(); try { - new SessionBackend( $id, $info, $this->store, $this->store, $logger, 10 ); + new SessionBackend( $id, $info, $this->store, $logger, 10 ); $this->fail( 'Expected exception not thrown' ); } catch ( \InvalidArgumentException $ex ) { $this->assertSame( @@ -103,7 +103,7 @@ class SessionBackendTest extends MediaWikiTestCase { ) ); $id = new SessionId( $info->getId() ); try { - new SessionBackend( $id, $info, $this->store, $this->store, $logger, 10 ); + new SessionBackend( $id, $info, $this->store, $logger, 10 ); $this->fail( 'Expected exception not thrown' ); } catch ( \InvalidArgumentException $ex ) { $this->assertSame( 'Cannot create session without a provider', $ex->getMessage() ); @@ -118,7 +118,7 @@ class SessionBackendTest extends MediaWikiTestCase { ) ); $id = new SessionId( '!' . $info->getId() ); try { - new SessionBackend( $id, $info, $this->store, $this->store, $logger, 10 ); + new SessionBackend( $id, $info, $this->store, $logger, 10 ); $this->fail( 'Expected exception not thrown' ); } catch ( \InvalidArgumentException $ex ) { $this->assertSame( @@ -135,7 +135,7 @@ class SessionBackendTest extends MediaWikiTestCase { 'idIsSafe' => true, ) ); $id = new SessionId( $info->getId() ); - $backend = new SessionBackend( $id, $info, $this->store, $this->store, $logger, 10 ); + $backend = new SessionBackend( $id, $info, $this->store, $logger, 10 ); $this->assertSame( self::SESSIONID, $backend->getId() ); $this->assertSame( $id, $backend->getSessionId() ); $this->assertSame( $this->provider, $backend->getProvider() ); @@ -157,7 +157,7 @@ class SessionBackendTest extends MediaWikiTestCase { 'idIsSafe' => true, ) ); $id = new SessionId( $info->getId() ); - $backend = new SessionBackend( $id, $info, $this->store, $this->store, $logger, 10 ); + $backend = new SessionBackend( $id, $info, $this->store, $logger, 10 ); $this->assertSame( self::SESSIONID, $backend->getId() ); $this->assertSame( $id, $backend->getSessionId() ); $this->assertSame( $this->provider, $backend->getProvider() ); @@ -468,6 +468,8 @@ class SessionBackendTest extends MediaWikiTestCase { $this->assertInternalType( 'array', $metadata ); $this->assertArrayHasKey( '???', $metadata ); $this->assertSame( '!!!', $metadata['???'] ); + $this->assertFalse( $this->store->getSessionFromBackend( self::SESSIONID ), + 'making sure it didn\'t save to backend' ); // Persistent, not dirty $this->provider = $neverProvider; @@ -516,6 +518,8 @@ class SessionBackendTest extends MediaWikiTestCase { $this->assertInternalType( 'array', $metadata ); $this->assertArrayHasKey( '???', $metadata ); $this->assertSame( '!!!', $metadata['???'] ); + $this->assertNotSame( false, $this->store->getSessionFromBackend( self::SESSIONID ), + 'making sure it did save to backend' ); $this->provider = $this->getMock( 'DummySessionProvider', array( 'persistSession' ) ); $this->provider->expects( $this->atLeastOnce() )->method( 'persistSession' ); @@ -538,6 +542,8 @@ class SessionBackendTest extends MediaWikiTestCase { $this->assertInternalType( 'array', $metadata ); $this->assertArrayHasKey( '???', $metadata ); $this->assertSame( '!!!', $metadata['???'] ); + $this->assertNotSame( false, $this->store->getSessionFromBackend( self::SESSIONID ), + 'making sure it did save to backend' ); $this->provider = $this->getMock( 'DummySessionProvider', array( 'persistSession' ) ); $this->provider->expects( $this->atLeastOnce() )->method( 'persistSession' ); @@ -559,6 +565,8 @@ class SessionBackendTest extends MediaWikiTestCase { $this->assertInternalType( 'array', $metadata ); $this->assertArrayHasKey( '???', $metadata ); $this->assertSame( '!!!', $metadata['???'] ); + $this->assertNotSame( false, $this->store->getSessionFromBackend( self::SESSIONID ), + 'making sure it did save to backend' ); // Not marked dirty, but dirty data $this->provider = $neverProvider; @@ -581,6 +589,8 @@ class SessionBackendTest extends MediaWikiTestCase { $this->assertInternalType( 'array', $metadata ); $this->assertArrayHasKey( '???', $metadata ); $this->assertSame( '!!!', $metadata['???'] ); + $this->assertNotSame( false, $this->store->getSessionFromBackend( self::SESSIONID ), + 'making sure it did save to backend' ); // Bad hook $this->provider = null; diff --git a/tests/phpunit/includes/session/SessionManagerTest.php b/tests/phpunit/includes/session/SessionManagerTest.php index 4fde3410f9..d8d5b4b5fb 100644 --- a/tests/phpunit/includes/session/SessionManagerTest.php +++ b/tests/phpunit/includes/session/SessionManagerTest.php @@ -103,7 +103,7 @@ class SessionManagerTest extends MediaWikiTestCase { $manager = \TestingAccessWrapper::newFromObject( $this->getManager() ); $this->assertSame( $this->config, $manager->config ); $this->assertSame( $this->logger, $manager->logger ); - $this->assertSame( $this->store, $manager->permStore ); + $this->assertSame( $this->store, $manager->store ); $manager = \TestingAccessWrapper::newFromObject( new SessionManager() ); $this->assertSame( \RequestContext::getMain()->getConfig(), $manager->config ); @@ -111,7 +111,7 @@ class SessionManagerTest extends MediaWikiTestCase { $manager = \TestingAccessWrapper::newFromObject( new SessionManager( array( 'config' => $this->config, ) ) ); - $this->assertSame( \ObjectCache::$instances['testSessionStore'], $manager->permStore ); + $this->assertSame( \ObjectCache::$instances['testSessionStore'], $manager->store ); foreach ( array( 'config' => '$options[\'config\'] must be an instance of Config', @@ -300,10 +300,6 @@ class SessionManagerTest extends MediaWikiTestCase { public function testGetSessionById() { $manager = $this->getManager(); - - // Disable the in-process cache so our $this->store->setSession() takes effect. - \TestingAccessWrapper::newFromObject( $manager )->tempStore = new \EmptyBagOStuff; - try { $manager->getSessionById( 'bad' ); $this->fail( 'Expected exception not thrown' ); @@ -766,7 +762,7 @@ class SessionManagerTest extends MediaWikiTestCase { $that = $this; - \ObjectCache::$instances[__METHOD__] = new \HashBagOStuff(); + \ObjectCache::$instances[__METHOD__] = new TestBagOStuff(); $this->setMwGlobals( array( 'wgMainCacheType' => __METHOD__ ) ); $this->stashMwGlobals( array( 'wgGroupPermissions' ) ); @@ -1086,9 +1082,6 @@ class SessionManagerTest extends MediaWikiTestCase { $manager->setLogger( $logger ); $request = new \FauxRequest(); - // Disable the in-process cache so our $this->store->setSession() takes effect. - \TestingAccessWrapper::newFromObject( $manager )->tempStore = new \EmptyBagOStuff; - // TestingAccessWrapper can't handle methods with reference arguments, sigh. $rClass = new \ReflectionClass( $manager ); $rMethod = $rClass->getMethod( 'loadSessionInfoFromStore' ); diff --git a/tests/phpunit/includes/session/TestBagOStuff.php b/tests/phpunit/includes/session/TestBagOStuff.php index e674e7bce0..8d1b5443a1 100644 --- a/tests/phpunit/includes/session/TestBagOStuff.php +++ b/tests/phpunit/includes/session/TestBagOStuff.php @@ -5,7 +5,11 @@ namespace MediaWiki\Session; /** * BagOStuff with utility functions for MediaWiki\\Session\\* testing */ -class TestBagOStuff extends \HashBagOStuff { +class TestBagOStuff extends \CachedBagOStuff { + + public function __construct() { + parent::__construct( new \HashBagOStuff ); + } /** * @param string $id Session ID @@ -68,6 +72,14 @@ class TestBagOStuff extends \HashBagOStuff { return $this->get( wfMemcKey( 'MWSession', $id ) ); } + /** + * @param string $id Session ID + * @return mixed + */ + public function getSessionFromBackend( $id ) { + return $this->backend->get( wfMemcKey( 'MWSession', $id ) ); + } + /** * @param string $id Session ID */ -- 2.20.1