Clean up after Ie161e0f
authorBrad Jorsch <bjorsch@wikimedia.org>
Sat, 30 Jan 2016 01:09:57 +0000 (20:09 -0500)
committerAnomie <bjorsch@wikimedia.org>
Wed, 3 Feb 2016 21:45:18 +0000 (21:45 +0000)
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

12 files changed:
autoload.php
includes/libs/objectcache/BagOStuff.php
includes/libs/objectcache/CachedBagOStuff.php [new file with mode: 0644]
includes/session/SessionBackend.php
includes/session/SessionManager.php
tests/phpunit/includes/libs/objectcache/CachedBagOStuffTest.php [new file with mode: 0644]
tests/phpunit/includes/session/CookieSessionProviderTest.php
tests/phpunit/includes/session/ImmutableSessionProviderWithCookieTest.php
tests/phpunit/includes/session/PHPSessionHandlerTest.php
tests/phpunit/includes/session/SessionBackendTest.php
tests/phpunit/includes/session/SessionManagerTest.php
tests/phpunit/includes/session/TestBagOStuff.php

index 9073005..c9e3b60 100644 (file)
@@ -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',
index b9be43d..3736103 100644 (file)
@@ -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 (file)
index 0000000..fc15618
--- /dev/null
@@ -0,0 +1,114 @@
+<?php
+/**
+ * Wrapper around a BagOStuff that caches data in memory
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ * @ingroup Cache
+ */
+
+use Psr\Log\LoggerInterface;
+
+/**
+ * Wrapper around a BagOStuff that caches data in memory
+ *
+ * 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).
+ *
+ * @ingroup Cache
+ */
+class CachedBagOStuff extends HashBagOStuff {
+       /** @var BagOStuff */
+       protected $backend;
+
+       /**
+        * @param BagOStuff $backend Permanent backend to use
+        * @param array $params Parameters for HashBagOStuff
+        */
+       function __construct( BagOStuff $backend, $params = array() ) {
+               $this->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 );
+       }
+
+}
index 2a13ed2..488f6e7 100644 (file)
@@ -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;
index 4b38a5c..57d5664 100644 (file)
@@ -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 (file)
index 0000000..3b19c9a
--- /dev/null
@@ -0,0 +1,52 @@
+<?php
+
+/**
+ * @group BagOStuff
+ */
+class CachedBagOStuffTest extends PHPUnit_Framework_TestCase {
+
+       public function testGetFromBackend() {
+               $backend = new HashBagOStuff;
+               $cache = new CachedBagOStuff( $backend );
+
+               $backend->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
+       }
+}
index e5df458..9ba67df 100644 (file)
@@ -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
                );
index 46f23f3..95f8e01 100644 (file)
@@ -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
                );
index 1c54a20..3044aa7 100644 (file)
@@ -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();
index 85fa9bd..f08a07d 100644 (file)
@@ -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;
index 4fde341..d8d5b4b 100644 (file)
@@ -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' );
index e674e7b..8d1b544 100644 (file)
@@ -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
         */