Improve SessionManager unit test coverage, and fix two namespacing bugs

Change-Id: Ie0bcba77625e04ca3e89eb400626f63024c6e1a1

diff --git a/includes/session/BotPasswordSessionProvider.php b/includes/session/BotPasswordSessionProvider.php
index 44199bd..70c771d 100644 (file)
--- a/includes/session/BotPasswordSessionProvider.php
+++ b/includes/session/BotPasswordSessionProvider.php
@@ -165,16 +165,19 @@ class BotPasswordSessionProvider extends ImmutableSessionProviderWithCookie {
                return true;
        }
 
+       /**
+        * @codeCoverageIgnore
+        */
        public function preventSessionsForUser( $username ) {
                BotPassword::removeAllPasswordsForUser( $username );
        }
 
        public function getAllowedUserRights( SessionBackend $backend ) {
                if ( $backend->getProvider() !== $this ) {
-                       throw new InvalidArgumentException( 'Backend\'s provider isn\'t $this' );
+                       throw new \InvalidArgumentException( 'Backend\'s provider isn\'t $this' );
                }
                $data = $backend->getProviderMetadata();
-               if ( $data ) {
+               if ( $data && isset( $data['rights'] ) && is_array( $data['rights'] ) ) {
                        return $data['rights'];
                }
 

diff --git a/includes/session/MetadataMergeException.php b/includes/session/MetadataMergeException.php
index 9f42c27..882084d 100644 (file)
--- a/includes/session/MetadataMergeException.php
+++ b/includes/session/MetadataMergeException.php
@@ -22,6 +22,7 @@
 
 namespace MediaWiki\Session;
 
+use Exception;
 use UnexpectedValueException;
 
 /**

diff --git a/includes/session/PHPSessionHandler.php b/includes/session/PHPSessionHandler.php
index 8630809..695ce5a 100644 (file)
--- a/includes/session/PHPSessionHandler.php
+++ b/includes/session/PHPSessionHandler.php
@@ -111,9 +111,11 @@ class PHPSessionHandler implements \SessionHandlerInterface {
                        return;
                }
 
+               // @codeCoverageIgnoreStart
                if ( defined( 'MW_NO_SESSION_HANDLER' ) ) {
                        throw new \BadMethodCallException( 'MW_NO_SESSION_HANDLER is defined' );
                }
+               // @codeCoverageIgnoreEnd
 
                self::$instance = new self( $manager );
 

diff --git a/includes/session/SessionManager.php b/includes/session/SessionManager.php
index b1d5d77..6a8b8a3 100644 (file)
--- a/includes/session/SessionManager.php
+++ b/includes/session/SessionManager.php
@@ -296,9 +296,11 @@ final class SessionManager implements SessionManagerInterface {
        }
 
        public function getVaryHeaders() {
+               // @codeCoverageIgnoreStart
                if ( defined( 'MW_NO_SESSION' ) && MW_NO_SESSION !== 'warn' ) {
                        return [];
                }
+               // @codeCoverageIgnoreEnd
                if ( $this->varyHeaders === null ) {
                        $headers = [];
                        foreach ( $this->getProviders() as $provider ) {
@@ -317,9 +319,11 @@ final class SessionManager implements SessionManagerInterface {
        }
 
        public function getVaryCookies() {
+               // @codeCoverageIgnoreStart
                if ( defined( 'MW_NO_SESSION' ) && MW_NO_SESSION !== 'warn' ) {
                        return [];
                }
+               // @codeCoverageIgnoreEnd
                if ( $this->varyCookies === null ) {
                        $cookies = [];
                        foreach ( $this->getProviders() as $provider ) {
@@ -513,12 +517,14 @@ final class SessionManager implements SessionManagerInterface {
                }
 
                # Notify AuthPlugin
+               // @codeCoverageIgnoreStart
                $tmpUser = $user;
                $wgAuth->initUser( $tmpUser, true );
                if ( $tmpUser !== $user ) {
                        $logger->warning( __METHOD__ . ': ' .
                                get_class( $wgAuth ) . '::initUser() replaced the user object' );
                }
+               // @codeCoverageIgnoreEnd
 
                # Notify hooks (e.g. Newuserlog)
                \Hooks::run( 'AuthPluginAutoCreate', [ $user ] );
@@ -957,6 +963,7 @@ final class SessionManager implements SessionManagerInterface {
         * @return Session
         */
        public function getSessionFromInfo( SessionInfo $info, WebRequest $request ) {
+               // @codeCoverageIgnoreStart
                if ( defined( 'MW_NO_SESSION' ) ) {
                        if ( MW_NO_SESSION === 'warn' ) {
                                // Undocumented safety case for converting existing entry points
@@ -965,6 +972,7 @@ final class SessionManager implements SessionManagerInterface {
                                throw new \BadMethodCallException( 'Sessions are disabled for this entry point' );
                        }
                }
+               // @codeCoverageIgnoreEnd
 
                $id = $info->getId();
 

diff --git a/tests/phpunit/includes/session/BotPasswordSessionProviderTest.php b/tests/phpunit/includes/session/BotPasswordSessionProviderTest.php
index b40a05c..590f287 100644 (file)
--- a/tests/phpunit/includes/session/BotPasswordSessionProviderTest.php
+++ b/tests/phpunit/includes/session/BotPasswordSessionProviderTest.php
@@ -281,4 +281,58 @@ class BotPasswordSessionProviderTest extends MediaWikiTestCase {
                $this->assertSame( [], $logger->getBuffer() );
                $this->assertEquals( $dataMD + [ 'rights' => [ 'read' ] ], $metadata );
        }
+
+       public function testGetAllowedUserRights() {
+               $logger = new \TestLogger( true );
+               $provider = $this->getProvider();
+               $provider->setLogger( $logger );
+
+               $backend = TestUtils::getDummySessionBackend();
+               $backendPriv = \TestingAccessWrapper::newFromObject( $backend );
+
+               try {
+                       $provider->getAllowedUserRights( $backend );
+                       $this->fail( 'Expected exception not thrown' );
+               } catch ( \InvalidArgumentException $ex ) {
+                       $this->assertSame( 'Backend\'s provider isn\'t $this', $ex->getMessage() );
+               }
+
+               $backendPriv->provider = $provider;
+               $backendPriv->providerMetadata = [ 'rights' => [ 'foo', 'bar', 'baz' ] ];
+               $this->assertSame( [ 'foo', 'bar', 'baz' ], $provider->getAllowedUserRights( $backend ) );
+               $this->assertSame( [], $logger->getBuffer() );
+
+               $backendPriv->providerMetadata = [ 'foo' => 'bar' ];
+               $this->assertSame( [], $provider->getAllowedUserRights( $backend ) );
+               $this->assertSame( [
+                       [
+                               LogLevel::DEBUG,
+                               'MediaWiki\\Session\\BotPasswordSessionProvider::getAllowedUserRights: ' .
+                                       'No provider metadata, returning no rights allowed'
+                       ]
+               ], $logger->getBuffer() );
+               $logger->clearBuffer();
+
+               $backendPriv->providerMetadata = [ 'rights' => 'bar' ];
+               $this->assertSame( [], $provider->getAllowedUserRights( $backend ) );
+               $this->assertSame( [
+                       [
+                               LogLevel::DEBUG,
+                               'MediaWiki\\Session\\BotPasswordSessionProvider::getAllowedUserRights: ' .
+                                       'No provider metadata, returning no rights allowed'
+                       ]
+               ], $logger->getBuffer() );
+               $logger->clearBuffer();
+
+               $backendPriv->providerMetadata = null;
+               $this->assertSame( [], $provider->getAllowedUserRights( $backend ) );
+               $this->assertSame( [
+                       [
+                               LogLevel::DEBUG,
+                               'MediaWiki\\Session\\BotPasswordSessionProvider::getAllowedUserRights: ' .
+                                       'No provider metadata, returning no rights allowed'
+                       ]
+               ], $logger->getBuffer() );
+               $logger->clearBuffer();
+       }
 }

diff --git a/tests/phpunit/includes/session/CookieSessionProviderTest.php b/tests/phpunit/includes/session/CookieSessionProviderTest.php
index d376159..a52aa4b 100644 (file)
--- a/tests/phpunit/includes/session/CookieSessionProviderTest.php
+++ b/tests/phpunit/includes/session/CookieSessionProviderTest.php
@@ -762,4 +762,25 @@ class CookieSessionProviderTest extends MediaWikiTestCase {
        public function onUserSetCookies( $user, &$sessionData, &$cookies ) {
        }
 
+       public function testGetCookie() {
+               $provider = new CookieSessionProvider( [
+                       'priority' => 1,
+                       'sessionName' => 'MySessionName',
+                       'cookieOptions' => [ 'prefix' => 'x' ],
+               ] );
+               $provider->setLogger( new \Psr\Log\NullLogger() );
+               $provider->setConfig( $this->getConfig() );
+               $provider->setManager( SessionManager::singleton() );
+               $provider = \TestingAccessWrapper::newFromObject( $provider );
+
+               $request = new \FauxRequest();
+               $request->setCookies( [
+                       'xFoo' => 'foo!',
+                       'xBar' => 'deleted',
+               ], '' );
+               $this->assertSame( 'foo!', $provider->getCookie( $request, 'Foo', 'x' ) );
+               $this->assertNull( $provider->getCookie( $request, 'Bar', 'x' ) );
+               $this->assertNull( $provider->getCookie( $request, 'Baz', 'x' ) );
+       }
+
 }

diff --git a/tests/phpunit/includes/session/MetadataMergeExceptionTest.php b/tests/phpunit/includes/session/MetadataMergeExceptionTest.php
new file mode 100644 (file)
index 0000000..0981f02
--- /dev/null
+++ b/tests/phpunit/includes/session/MetadataMergeExceptionTest.php
@@ -0,0 +1,30 @@
+<?php
+
+namespace MediaWiki\Session;
+
+use MediaWikiTestCase;
+
+/**
+ * @group Session
+ * @covers MediaWiki\Session\MetadataMergeException
+ */
+class MetadataMergeExceptionTest extends MediaWikiTestCase {
+
+       public function testBasics() {
+               $data = [ 'foo' => 'bar' ];
+
+               $ex = new MetadataMergeException();
+               $this->assertInstanceOf( 'UnexpectedValueException', $ex );
+               $this->assertSame( [], $ex->getContext() );
+
+               $ex2 = new MetadataMergeException( 'Message', 42, $ex, $data );
+               $this->assertSame( 'Message', $ex2->getMessage() );
+               $this->assertSame( 42, $ex2->getCode() );
+               $this->assertSame( $ex, $ex2->getPrevious() );
+               $this->assertSame( $data, $ex2->getContext() );
+
+               $ex->setContext( $data );
+               $this->assertSame( $data, $ex->getContext() );
+       }
+
+}

diff --git a/tests/phpunit/includes/session/SessionBackendTest.php b/tests/phpunit/includes/session/SessionBackendTest.php
index 5824ce1..54ad0f4 100644 (file)
--- a/tests/phpunit/includes/session/SessionBackendTest.php
+++ b/tests/phpunit/includes/session/SessionBackendTest.php
@@ -216,6 +216,42 @@ class SessionBackendTest extends MediaWikiTestCase {
                $this->assertArrayHasKey( $backend->getId(), $manager->allSessionIds );
        }
 
+       public function testSetProviderMetadata() {
+               $backend = $this->getBackend();
+               $priv = \TestingAccessWrapper::newFromObject( $backend );
+               $priv->providerMetadata = [ 'dummy' ];
+
+               try {
+                       $backend->setProviderMetadata( 'foo' );
+                       $this->fail( 'Expected exception not thrown' );
+               } catch ( \InvalidArgumentException $ex ) {
+                       $this->assertSame( '$metadata must be an array or null', $ex->getMessage() );
+               }
+
+               try {
+                       $backend->setProviderMetadata( (object)[] );
+                       $this->fail( 'Expected exception not thrown' );
+               } catch ( \InvalidArgumentException $ex ) {
+                       $this->assertSame( '$metadata must be an array or null', $ex->getMessage() );
+               }
+
+               $this->assertFalse( $this->store->getSession( self::SESSIONID ), 'sanity check' );
+               $backend->setProviderMetadata( [ 'dummy' ] );
+               $this->assertFalse( $this->store->getSession( self::SESSIONID ) );
+
+               $this->assertFalse( $this->store->getSession( self::SESSIONID ), 'sanity check' );
+               $backend->setProviderMetadata( [ 'test' ] );
+               $this->assertNotFalse( $this->store->getSession( self::SESSIONID ) );
+               $this->assertSame( [ 'test' ], $backend->getProviderMetadata() );
+               $this->store->deleteSession( self::SESSIONID );
+
+               $this->assertFalse( $this->store->getSession( self::SESSIONID ), 'sanity check' );
+               $backend->setProviderMetadata( null );
+               $this->assertNotFalse( $this->store->getSession( self::SESSIONID ) );
+               $this->assertSame( null, $backend->getProviderMetadata() );
+               $this->store->deleteSession( self::SESSIONID );
+       }
+
        public function testResetId() {
                $id = session_id();
 

diff --git a/tests/phpunit/includes/session/SessionProviderTest.php b/tests/phpunit/includes/session/SessionProviderTest.php
index 24b9716..e92eb09 100644 (file)
--- a/tests/phpunit/includes/session/SessionProviderTest.php
+++ b/tests/phpunit/includes/session/SessionProviderTest.php
@@ -128,6 +128,11 @@ class SessionProviderTest extends MediaWikiTestCase {
                        $provider->preventSessionsForUser( 'Foo' );
                        $this->fail( 'Expected exception not thrown' );
                } catch ( \BadMethodCallException $ex ) {
+                       $this->assertSame(
+                               'MediaWiki\\Session\\SessionProvider::preventSessionsForUser must be implmented ' .
+                                       'when canChangeUser() is false',
+                               $ex->getMessage()
+                       );
                }
 
        }