Phuedx has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/311103

Change subject: Revert "Revert "Add MobileFrontend.Context service""
......................................................................

Revert "Revert "Add MobileFrontend.Context service""

This reverts commit 725abdcce59c0c8517d9df71e1296b76f64aed1d.

Bug: T143875
Change-Id: I2f4f31f7ee675029cf5c9849b85fe7a4c46d0be8
---
M includes/MobileContext.php
M includes/ServiceWiring.php
M tests/phpunit/MobileContextTest.php
M tests/phpunit/MobileFrontend.hooksTest.php
M tests/phpunit/specials/SpecialMobileDiffTest.php
5 files changed, 85 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/MobileFrontend 
refs/changes/03/311103/1

diff --git a/includes/MobileContext.php b/includes/MobileContext.php
index d88da4b..a848cb7 100644
--- a/includes/MobileContext.php
+++ b/includes/MobileContext.php
@@ -99,7 +99,7 @@
        /**
         * @var Config MobileFrontend's config object
         */
-       private $configObj;
+       private $config;
        /**
         * @var String Domain to use for the stopMobileRedirect cookie
         */
@@ -110,31 +110,57 @@
        private $mobileUrlTemplate = false;
 
        /**
-        * Returns the actual MobileContext Instance or create a new if no 
exists
-        * @return MobileContext
+        * Gets the singleton instance.
+        *
+        * @deprecated Removing this method is one of the goals
+        *  [T143189][https://phabricator.wikimedia.org/T143875].
+        * @deprecated If you're creating a new class, then it should accept an 
instance of
+        * `MobileContext` at construction time.
+        * @deprecated If you're adding code to a pre-existing class, then 
first consider if the class can
+        *  be split apart so that you can do the above. If you're confident 
that this can't be done,
+        *  then you can use this method.
+        *
+        * @return {MobileContext}
         */
        public static function singleton() {
                if ( !self::$instance ) {
-                       self::$instance = new MobileContext( 
RequestContext::getMain() );
+                       self::$instance =
+                               MediaWikiServices::getInstance()->getService( 
'MobileFrontend.MobileContext' );
                }
+
                return self::$instance;
        }
 
        /**
-        * Set $this->instance to the given instance of MobileContext or null
-        * @param MobileContext|null $instance MobileContext instance or null 
to set
-        * @return MobileContext|null
+        * Overrides the singleton instance.
+        *
+        * @deprecated See `MobileContext::singleton`.
+        *
+        * @warning This method should only accept instances of 
`MobileContext`. The `BogusMobileContext`
+        *  class is used in the `MobileContextTest` test suite and will be 
removed.
+        *
+        * @param {MobileContext} $instance
         */
-       public static function setInstance( /* MobileContext|null */ $instance 
) {
+       public static function setInstanceForTesting( $instance ) {
                self::$instance = $instance;
        }
 
        /**
-        * Set the IontextSource Object
-        * @param IContextSource $context The IContextSource Object has to set
+        * Resets the `MobileFrontend.MobileContext` service and singleton 
instance.
+        *
+        * @deprecated See `MobileContext::singleton`.
+        *
+        * @throws MWException If called outside the MediaWiki PHPUnit test 
suite
+        *  (see `MediaWikiServices#resetServiceForTesting`).
         */
-       protected function __construct( IContextSource $context ) {
+       public static function resetServiceForTesting() {
+               self::$instance = null;
+               MediaWikiServices::getInstance()->resetServiceForTesting( 
'MobileFrontend.MobileContext' );
+       }
+
+       public function __construct( IContextSource $context, Config $config ) {
                $this->setContext( $context );
+               $this->config = $config;
        }
 
        /**
@@ -142,7 +168,7 @@
         * @return Config
         */
        public function getMFConfig() {
-               return MediaWikiServices::getInstance()->getService( 
'MobileFrontend.Config' );
+               return $this->config;
        }
 
        /**
diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php
index efc21e1..8aad19d 100644
--- a/includes/ServiceWiring.php
+++ b/includes/ServiceWiring.php
@@ -6,5 +6,10 @@
        'MobileFrontend.Config' => function ( MediaWikiServices $services ) {
                return $services->getService( 'ConfigFactory' )
                        ->makeConfig( 'mobilefrontend' );
+       },
+       'MobileFrontend.MobileContext' => function ( MediaWikiServices 
$services ) {
+               $config = $services->getService( 'MobileFrontend.Config' );
+
+               return new MobileContext( RequestContext::getMain(), $config );
        }
 ];
diff --git a/tests/phpunit/MobileContextTest.php 
b/tests/phpunit/MobileContextTest.php
index 5f6a657..b98b6cd 100644
--- a/tests/phpunit/MobileContextTest.php
+++ b/tests/phpunit/MobileContextTest.php
@@ -1,9 +1,22 @@
 <?php
 
+use MediaWiki\MediaWikiServices;
+
 /**
  * @group MobileFrontend
  */
 class MobileContextTest extends MediaWikiTestCase {
+
+       /**
+        * @var Config
+        */
+       private $config;
+
+       /**
+        * @var IContextSource
+        */
+       private $baseContext;
+
        /**
         * PHP 5.3.2 introduces the ReflectionMethod::setAccessible() method to 
allow the invocation of
         * protected and private methods directly through the Reflection API
@@ -22,12 +35,16 @@
        protected function setUp() {
                parent::setUp();
                // Permit no access to the singleton
-               MobileContext::setInstance( new BogusMobileContext() );
+               MobileContext::setInstanceForTesting( new BogusMobileContext() 
);
+
+               $this->config = new GlobalVarConfig();
+               $this->baseContext = RequestContext::getMain();
        }
 
        protected function tearDown() {
-               MobileContext::setInstance( null ); // refresh it
                parent::tearDown();
+
+               MobileContext::resetServiceForTesting();
        }
 
        /**
@@ -48,11 +65,13 @@
                $request->setRequestURL( $url );
                $request->setCookies( $cookies, '' );
 
-               $context = new DerivativeContext( RequestContext::getMain() );
+               $context = new DerivativeContext( $this->baseContext );
                $context->setRequest( $request );
                $context->setOutput( new OutputPage( $context ) );
-               $instance = unserialize( 'O:13:"MobileContext":0:{}' );
+
+               $instance = new MobileContext( $context, $this->config );
                $instance->setContext( $context );
+
                return $instance;
        }
 
@@ -653,11 +672,26 @@
                );
                $req->setRequestURL( 
'/w/index.php?title=Special:Search&mobileaction=toggle_view_mobile' );
                RequestContext::getMain()->setRequest( $req );
-               MobileContext::setInstance( null );
+               MobileContext::resetServiceForTesting();
                $this->setMwGlobals( 'wgTitle', null );
                SpecialPage::getTitleFor( 'Search' );
                $this->assertTrue( true, 'In case of failure this test just 
crashes' );
        }
+
+       public function test_singleton_is_constructed_by_MediaWikiServices() {
+               MobileContext::resetServiceForTesting();
+
+               $expectedInstance = MediaWikiServices::getInstance()
+                       ->getService( 'MobileFrontend.MobileContext' );
+
+               $this->assertSame( $expectedInstance, 
MobileContext::singleton() );
+       }
+
+       public function test_getMFConfig_returns_the_config() {
+               $context = $this->makeContext();
+
+               $this->assertSame( $this->config, $context->getMFConfig() );
+       }
 }
 
 class BogusMobileContext {
diff --git a/tests/phpunit/MobileFrontend.hooksTest.php 
b/tests/phpunit/MobileFrontend.hooksTest.php
index b5b5d8a..430a9b4 100644
--- a/tests/phpunit/MobileFrontend.hooksTest.php
+++ b/tests/phpunit/MobileFrontend.hooksTest.php
@@ -108,8 +108,7 @@
         * SkinTemplate (sk) and OutputPage (out)
         */
        protected function getContextSetup( $mode, $mfXAnalyticsItems, $title = 
null ) {
-               // Create a new MobileContext object for this test
-               MobileContext::setInstance( null );
+               MobileContext::resetServiceForTesting();
                // create a new instance of MobileContext
                $context = MobileContext::singleton();
                // create a DerivativeContext to use in MobileContext later
@@ -141,8 +140,6 @@
                foreach ( $mfXAnalyticsItems as $key => $val ) {
                        $context->addAnalyticsLogItem( $key, $val );
                }
-               // set the newly created MobileContext object as the current 
instance to use
-               MobileContext::setInstance( $context );
 
                // return the stuff
                return [
@@ -183,7 +180,7 @@
                        'wgScriptPath' => '/w',
                        'wgScript' => '/w/index.php',
                ] );
-               MobileContext::setInstance( null );
+               MobileContext::resetServiceForTesting();
 
                $title = Title::newFromText( 'PurgeTest' );
 
diff --git a/tests/phpunit/specials/SpecialMobileDiffTest.php 
b/tests/phpunit/specials/SpecialMobileDiffTest.php
index bea2133..37d4a70 100644
--- a/tests/phpunit/specials/SpecialMobileDiffTest.php
+++ b/tests/phpunit/specials/SpecialMobileDiffTest.php
@@ -11,7 +11,7 @@
                foreach ( $this->unsetReqVals as $v ) {
                        MobileContext::singleton()->getRequest()->unsetVal( $v 
);
                }
-               MobileContext::setInstance( null ); // refresh MobileContext 
instance
+               MobileContext::resetServiceForTesting();
                parent::tearDown();
        }
        /**
@@ -142,4 +142,3 @@
                return '';
        }
 }
-

-- 
To view, visit https://gerrit.wikimedia.org/r/311103
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I2f4f31f7ee675029cf5c9849b85fe7a4c46d0be8
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/MobileFrontend
Gerrit-Branch: master
Gerrit-Owner: Phuedx <g...@samsmith.io>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to