Merge "Rm unused $config from SpecialRecentChanges::getDefaultOptions"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Tue, 1 Mar 2016 16:49:49 +0000 (16:49 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Tue, 1 Mar 2016 16:49:49 +0000 (16:49 +0000)
includes/api/ApiLogout.php
includes/jobqueue/Job.php
includes/jobqueue/JobRunner.php
includes/jobqueue/jobs/AssembleUploadChunksJob.php
includes/jobqueue/jobs/PublishStashedFileJob.php
includes/session/SessionManager.php
tests/phpunit/includes/session/SessionManagerTest.php

index 2c38208..6a26e2e 100644 (file)
@@ -24,6 +24,8 @@
  * @file
  */
 
+use MediaWiki\Session\BotPasswordSessionProvider;
+
 /**
  * API module to allow users to log out of the wiki. API equivalent of
  * Special:Userlogout.
 class ApiLogout extends ApiBase {
 
        public function execute() {
-               // Make sure it's possible to log out
                $session = MediaWiki\Session\SessionManager::getGlobalSession();
+
+               // Handle bot password logout specially
+               if ( $session->getProvider() instanceof BotPasswordSessionProvider ) {
+                       $session->unpersist();
+                       return;
+               }
+
+               // Make sure it's possible to log out
                if ( !$session->canSetUser() ) {
                        $this->dieUsage(
                                'Cannot log out when using ' .
index 37e3d0f..48d38d9 100644 (file)
@@ -47,6 +47,9 @@ abstract class Job implements IJobSpecification {
        /** @var string Text for error that occurred last */
        protected $error;
 
+       /** @var callable[] */
+       protected $teardownCallbacks = [];
+
        /**
         * Run the job
         * @return bool Success
@@ -279,6 +282,25 @@ abstract class Job implements IJobSpecification {
                return $this->hasRootJobParams() && !empty( $this->params['rootJobIsSelf'] );
        }
 
+       /**
+        * @param callable $callback
+        * @since 1.27
+        */
+       protected function addTeardownCallback( $callback ) {
+               $this->teardownCallbacks[] = $callback;
+       }
+
+       /**
+        * Do any final cleanup after run(), deferred updates, and all DB commits happen
+        *
+        * @since 1.27
+        */
+       public function teardown() {
+               foreach ( $this->teardownCallbacks as $callback ) {
+                       call_user_func( $callback );
+               }
+       }
+
        /**
         * Insert a single job into the queue.
         * @return bool True on success
index c542d97..ef3d61d 100644 (file)
@@ -266,6 +266,7 @@ class JobRunner implements LoggerAwareInterface {
 
                        DeferredUpdates::doUpdates();
                        $this->commitMasterChanges( $job );
+                       $job->teardown();
                } catch ( Exception $e ) {
                        MWExceptionHandler::rollbackMasterChangesAndLog( $e );
                        $status = false;
index 395da22..bc2f7c4 100644 (file)
@@ -35,6 +35,10 @@ class AssembleUploadChunksJob extends Job {
        public function run() {
                /** @noinspection PhpUnusedLocalVariableInspection */
                $scope = RequestContext::importScopedSession( $this->params['session'] );
+               $this->addTeardownCallback( function () use ( &$scope ) {
+                       ScopedCallback::consume( $scope ); // T126450
+               } );
+
                $context = RequestContext::getMain();
                $user = $context->getUser();
                try {
index 34ce4fe..a6d2f70 100644 (file)
@@ -37,6 +37,10 @@ class PublishStashedFileJob extends Job {
        public function run() {
                /** @noinspection PhpUnusedLocalVariableInspection */
                $scope = RequestContext::importScopedSession( $this->params['session'] );
+               $this->addTeardownCallback( function () use ( &$scope ) {
+                       ScopedCallback::consume( $scope ); // T126450
+               } );
+
                $context = RequestContext::getMain();
                $user = $context->getUser();
                try {
index 6a8b8a3..58f995f 100644 (file)
@@ -663,8 +663,14 @@ final class SessionManager implements SessionManagerInterface {
                                                // This is going to error out below, but we want to
                                                // provide a complete list.
                                                $retInfos[] = $info;
+                                       } else {
+                                               // Session load failed, so unpersist it from this request
+                                               $info->getProvider()->unpersistSession( $request );
                                        }
                                }
+                       } else {
+                               // Session load failed, so unpersist it from this request
+                               $info->getProvider()->unpersistSession( $request );
                        }
                }
 
index fe2c3b7..a1b9bb4 100644 (file)
@@ -131,6 +131,8 @@ class SessionManagerTest extends MediaWikiTestCase {
        public function testGetSessionForRequest() {
                $manager = $this->getManager();
                $request = new \FauxRequest();
+               $request->unpersist1 = false;
+               $request->unpersist2 = false;
 
                $id1 = '';
                $id2 = '';
@@ -138,7 +140,7 @@ class SessionManagerTest extends MediaWikiTestCase {
 
                $providerBuilder = $this->getMockBuilder( 'DummySessionProvider' )
                        ->setMethods(
-                               [ 'provideSessionInfo', 'newSessionInfo', '__toString', 'describe' ]
+                               [ 'provideSessionInfo', 'newSessionInfo', '__toString', 'describe', 'unpersistSession' ]
                        );
 
                $provider1 = $providerBuilder->getMock();
@@ -160,6 +162,10 @@ class SessionManagerTest extends MediaWikiTestCase {
                        ->will( $this->returnValue( 'Provider1' ) );
                $provider1->expects( $this->any() )->method( 'describe' )
                        ->will( $this->returnValue( '#1 sessions' ) );
+               $provider1->expects( $this->any() )->method( 'unpersistSession' )
+                       ->will( $this->returnCallback( function ( $request ) {
+                               $request->unpersist1 = true;
+                       } ) );
 
                $provider2 = $providerBuilder->getMock();
                $provider2->expects( $this->any() )->method( 'provideSessionInfo' )
@@ -171,6 +177,10 @@ class SessionManagerTest extends MediaWikiTestCase {
                        ->will( $this->returnValue( 'Provider2' ) );
                $provider2->expects( $this->any() )->method( 'describe' )
                        ->will( $this->returnValue( '#2 sessions' ) );
+               $provider2->expects( $this->any() )->method( 'unpersistSession' )
+                       ->will( $this->returnCallback( function ( $request ) {
+                               $request->unpersist2 = true;
+                       } ) );
 
                $this->config->set( 'SessionProviders', [
                        $this->objectCacheDef( $provider1 ),
@@ -183,6 +193,8 @@ class SessionManagerTest extends MediaWikiTestCase {
                $session = $manager->getSessionForRequest( $request );
                $this->assertInstanceOf( 'MediaWiki\\Session\\Session', $session );
                $this->assertSame( $idEmpty, $session->getId() );
+               $this->assertFalse( $request->unpersist1 );
+               $this->assertFalse( $request->unpersist2 );
 
                // Both providers return info, picks best one
                $request->info1 = new SessionInfo( SessionInfo::MIN_PRIORITY + 1, [
@@ -200,6 +212,8 @@ class SessionManagerTest extends MediaWikiTestCase {
                $session = $manager->getSessionForRequest( $request );
                $this->assertInstanceOf( 'MediaWiki\\Session\\Session', $session );
                $this->assertSame( $id2, $session->getId() );
+               $this->assertFalse( $request->unpersist1 );
+               $this->assertFalse( $request->unpersist2 );
 
                $request->info1 = new SessionInfo( SessionInfo::MIN_PRIORITY + 2, [
                        'provider' => $provider1,
@@ -216,6 +230,8 @@ class SessionManagerTest extends MediaWikiTestCase {
                $session = $manager->getSessionForRequest( $request );
                $this->assertInstanceOf( 'MediaWiki\\Session\\Session', $session );
                $this->assertSame( $id1, $session->getId() );
+               $this->assertFalse( $request->unpersist1 );
+               $this->assertFalse( $request->unpersist2 );
 
                // Tied priorities
                $request->info1 = new SessionInfo( SessionInfo::MAX_PRIORITY, [
@@ -244,6 +260,8 @@ class SessionManagerTest extends MediaWikiTestCase {
                        $this->assertContains( $request->info1, $ex->sessionInfos );
                        $this->assertContains( $request->info2, $ex->sessionInfos );
                }
+               $this->assertFalse( $request->unpersist1 );
+               $this->assertFalse( $request->unpersist2 );
 
                // Bad provider
                $request->info1 = new SessionInfo( SessionInfo::MAX_PRIORITY, [
@@ -262,6 +280,8 @@ class SessionManagerTest extends MediaWikiTestCase {
                                $ex->getMessage()
                        );
                }
+               $this->assertFalse( $request->unpersist1 );
+               $this->assertFalse( $request->unpersist2 );
 
                // Unusable session info
                $this->logger->setCollect( true );
@@ -282,6 +302,31 @@ class SessionManagerTest extends MediaWikiTestCase {
                $this->assertInstanceOf( 'MediaWiki\\Session\\Session', $session );
                $this->assertSame( $id2, $session->getId() );
                $this->logger->setCollect( false );
+               $this->assertTrue( $request->unpersist1 );
+               $this->assertFalse( $request->unpersist2 );
+               $request->unpersist1 = false;
+
+               $this->logger->setCollect( true );
+               $request->info1 = new SessionInfo( SessionInfo::MAX_PRIORITY, [
+                       'provider' => $provider1,
+                       'id' => ( $id1 = $manager->generateSessionId() ),
+                       'persisted' => true,
+                       'idIsSafe' => true,
+               ] );
+               $request->info2 = new SessionInfo( SessionInfo::MAX_PRIORITY, [
+                       'provider' => $provider2,
+                       'id' => ( $id2 = $manager->generateSessionId() ),
+                       'persisted' => true,
+                       'userInfo' => UserInfo::newFromName( 'UTSysop', false ),
+                       'idIsSafe' => true,
+               ] );
+               $session = $manager->getSessionForRequest( $request );
+               $this->assertInstanceOf( 'MediaWiki\\Session\\Session', $session );
+               $this->assertSame( $id1, $session->getId() );
+               $this->logger->setCollect( false );
+               $this->assertFalse( $request->unpersist1 );
+               $this->assertTrue( $request->unpersist2 );
+               $request->unpersist2 = false;
 
                // Unpersisted session ID
                $request->info1 = new SessionInfo( SessionInfo::MAX_PRIORITY, [
@@ -295,6 +340,8 @@ class SessionManagerTest extends MediaWikiTestCase {
                $session = $manager->getSessionForRequest( $request );
                $this->assertInstanceOf( 'MediaWiki\\Session\\Session', $session );
                $this->assertSame( $id1, $session->getId() );
+               $this->assertTrue( $request->unpersist1 ); // The saving of the session does it
+               $this->assertFalse( $request->unpersist2 );
                $session->persist();
                $this->assertTrue( $session->isPersistent(), 'sanity check' );
        }