Create factory for MWHttpRequest
authorStanislav Malyshev <smalyshev@gmail.com>
Tue, 16 Jan 2018 23:11:08 +0000 (15:11 -0800)
committerKunal Mehta <legoktm@member.fsf.org>
Tue, 23 Jan 2018 19:39:02 +0000 (11:39 -0800)
This will allow classes that need MWHttpRequest to inject HttpRequestFactory
and thus make it overridable and testable.

Also made MWHttpRequest abstract class since it doesn't implement
execute anyway. Maybe a good idea to move execute to an abstract
method?

Change-Id: I5c0e035542ff5f791a21a95ed13bed4cea6906d0

autoload.php
includes/MediaWikiServices.php
includes/ServiceWiring.php
includes/http/HttpRequestFactory.php [new file with mode: 0644]
includes/http/MWHttpRequest.php
tests/integration/includes/http/MWHttpRequestTestCase.php
tests/phpunit/includes/MediaWikiServicesTest.php

index 5d6104c..3302415 100644 (file)
@@ -878,6 +878,7 @@ $wgAutoloadLocalClasses = [
        'MediaWiki\\EditPage\\TextboxBuilder' => __DIR__ . '/includes/editpage/TextboxBuilder.php',
        'MediaWiki\\Edit\\PreparedEdit' => __DIR__ . '/includes/edit/PreparedEdit.php',
        'MediaWiki\\HeaderCallback' => __DIR__ . '/includes/HeaderCallback.php',
        'MediaWiki\\EditPage\\TextboxBuilder' => __DIR__ . '/includes/editpage/TextboxBuilder.php',
        'MediaWiki\\Edit\\PreparedEdit' => __DIR__ . '/includes/edit/PreparedEdit.php',
        'MediaWiki\\HeaderCallback' => __DIR__ . '/includes/HeaderCallback.php',
+       'MediaWiki\\Http\\HttpRequestFactory' => __DIR__ . '/includes/http/HttpRequestFactory.php',
        'MediaWiki\\Interwiki\\ClassicInterwikiLookup' => __DIR__ . '/includes/interwiki/ClassicInterwikiLookup.php',
        'MediaWiki\\Interwiki\\InterwikiLookup' => __DIR__ . '/includes/interwiki/InterwikiLookup.php',
        'MediaWiki\\Interwiki\\InterwikiLookupAdapter' => __DIR__ . '/includes/interwiki/InterwikiLookupAdapter.php',
        'MediaWiki\\Interwiki\\ClassicInterwikiLookup' => __DIR__ . '/includes/interwiki/ClassicInterwikiLookup.php',
        'MediaWiki\\Interwiki\\InterwikiLookup' => __DIR__ . '/includes/interwiki/InterwikiLookup.php',
        'MediaWiki\\Interwiki\\InterwikiLookupAdapter' => __DIR__ . '/includes/interwiki/InterwikiLookupAdapter.php',
index 5b173cd..00767c7 100644 (file)
@@ -10,6 +10,7 @@ use GenderCache;
 use GlobalVarConfig;
 use Hooks;
 use IBufferingStatsdDataFactory;
 use GlobalVarConfig;
 use Hooks;
 use IBufferingStatsdDataFactory;
+use MediaWiki\Http\HttpRequestFactory;
 use MediaWiki\Preferences\PreferencesFactory;
 use MediaWiki\Shell\CommandFactory;
 use MediaWiki\Storage\BlobStore;
 use MediaWiki\Preferences\PreferencesFactory;
 use MediaWiki\Shell\CommandFactory;
 use MediaWiki\Storage\BlobStore;
@@ -734,6 +735,14 @@ class MediaWikiServices extends ServiceContainer {
                return $this->getService( 'PreferencesFactory' );
        }
 
                return $this->getService( 'PreferencesFactory' );
        }
 
+       /**
+        * @since 1.31
+        * @return HttpRequestFactory
+        */
+       public function getHttpRequestFactory() {
+               return $this->getService( 'HttpRequestFactory' );
+       }
+
        ///////////////////////////////////////////////////////////////////////////
        // NOTE: When adding a service getter here, don't forget to add a test
        // case for it in MediaWikiServicesTest::provideGetters() and in
        ///////////////////////////////////////////////////////////////////////////
        // NOTE: When adding a service getter here, don't forget to add a test
        // case for it in MediaWikiServicesTest::provideGetters() and in
index 79e5b84..dab3b5c 100644 (file)
@@ -508,6 +508,10 @@ return [
                return new DefaultPreferencesFactory( $config, $wgContLang, $authManager, $linkRenderer );
        },
 
                return new DefaultPreferencesFactory( $config, $wgContLang, $authManager, $linkRenderer );
        },
 
+       'HttpRequestFactory' => function ( MediaWikiServices $services ) {
+               return new \MediaWiki\Http\HttpRequestFactory();
+       },
+
        ///////////////////////////////////////////////////////////////////////////
        // NOTE: When adding a service here, don't forget to add a getter function
        // in the MediaWikiServices class. The convenience getter should just call
        ///////////////////////////////////////////////////////////////////////////
        // NOTE: When adding a service here, don't forget to add a getter function
        // in the MediaWikiServices class. The convenience getter should just call
diff --git a/includes/http/HttpRequestFactory.php b/includes/http/HttpRequestFactory.php
new file mode 100644 (file)
index 0000000..80f9b68
--- /dev/null
@@ -0,0 +1,82 @@
+<?php
+/**
+ * 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
+ */
+namespace MediaWiki\Http;
+
+use CurlHttpRequest;
+use DomainException;
+use Http;
+use MediaWiki\Logger\LoggerFactory;
+use MWHttpRequest;
+use PhpHttpRequest;
+use Profiler;
+
+/**
+ * Factory creating MWHttpRequest objects.
+ */
+class HttpRequestFactory {
+
+       /**
+        * Generate a new MWHttpRequest object
+        * @param string $url Url to use
+        * @param array $options (optional) extra params to pass (see Http::request())
+        * @param string $caller The method making this request, for profiling
+        * @throws DomainException
+        * @return MWHttpRequest
+        * @see MWHttpRequest::__construct
+        */
+       public function create( $url, array $options = [], $caller = __METHOD__ ) {
+               if ( !Http::$httpEngine ) {
+                       Http::$httpEngine = function_exists( 'curl_init' ) ? 'curl' : 'php';
+               } elseif ( Http::$httpEngine == 'curl' && !function_exists( 'curl_init' ) ) {
+                       throw new DomainException( __METHOD__ . ': curl (http://php.net/curl) is not installed, but' .
+                          ' Http::$httpEngine is set to "curl"' );
+               }
+
+               if ( !isset( $options['logger'] ) ) {
+                       $options['logger'] = LoggerFactory::getInstance( 'http' );
+               }
+
+               switch ( Http::$httpEngine ) {
+                       case 'curl':
+                               return new CurlHttpRequest( $url, $options, $caller, Profiler::instance() );
+                       case 'php':
+                               if ( !wfIniGetBool( 'allow_url_fopen' ) ) {
+                                       throw new DomainException( __METHOD__ . ': allow_url_fopen ' .
+                                          'needs to be enabled for pure PHP http requests to ' .
+                                          'work. If possible, curl should be used instead. See ' .
+                                          'http://php.net/curl.'
+                                       );
+                               }
+                               return new PhpHttpRequest( $url, $options, $caller, Profiler::instance() );
+                       default:
+                               throw new DomainException( __METHOD__ . ': The setting of Http::$httpEngine is not valid.' );
+               }
+       }
+
+       /**
+        * Simple function to test if we can make any sort of requests at all, using
+        * cURL or fopen()
+        * @return bool
+        */
+       public function canMakeRequests() {
+               return function_exists( 'curl_init' ) || wfIniGetBool( 'allow_url_fopen' );
+       }
+
+}
index 0f0118c..fff72ec 100644 (file)
@@ -18,7 +18,6 @@
  * @file
  */
 
  * @file
  */
 
-use MediaWiki\Logger\LoggerFactory;
 use Psr\Log\LoggerInterface;
 use Psr\Log\LoggerAwareInterface;
 use Psr\Log\NullLogger;
 use Psr\Log\LoggerInterface;
 use Psr\Log\LoggerAwareInterface;
 use Psr\Log\NullLogger;
@@ -30,7 +29,7 @@ use Psr\Log\NullLogger;
  * Renamed from HttpRequest to MWHttpRequest to avoid conflict with
  * PHP's HTTP extension.
  */
  * Renamed from HttpRequest to MWHttpRequest to avoid conflict with
  * PHP's HTTP extension.
  */
-class MWHttpRequest implements LoggerAwareInterface {
+abstract class MWHttpRequest implements LoggerAwareInterface {
        const SUPPORTS_FILE_POSTS = false;
 
        /**
        const SUPPORTS_FILE_POSTS = false;
 
        /**
@@ -90,8 +89,8 @@ class MWHttpRequest implements LoggerAwareInterface {
         * @param string $caller The method making this request, for profiling
         * @param Profiler $profiler An instance of the profiler for profiling, or null
         */
         * @param string $caller The method making this request, for profiling
         * @param Profiler $profiler An instance of the profiler for profiling, or null
         */
-       protected function __construct(
-               $url, $options = [], $caller = __METHOD__, $profiler = null
+       public function __construct(
+               $url, array $options = [], $caller = __METHOD__, $profiler = null
        ) {
                global $wgHTTPTimeout, $wgHTTPConnectTimeout;
 
        ) {
                global $wgHTTPTimeout, $wgHTTPConnectTimeout;
 
@@ -174,44 +173,18 @@ class MWHttpRequest implements LoggerAwareInterface {
 
        /**
         * Generate a new request object
 
        /**
         * Generate a new request object
+        * Deprecated: @see HttpRequestFactory::create
         * @param string $url Url to use
         * @param array $options (optional) extra params to pass (see Http::request())
         * @param string $caller The method making this request, for profiling
         * @throws DomainException
         * @param string $url Url to use
         * @param array $options (optional) extra params to pass (see Http::request())
         * @param string $caller The method making this request, for profiling
         * @throws DomainException
-        * @return CurlHttpRequest|PhpHttpRequest
+        * @return MWHttpRequest
         * @see MWHttpRequest::__construct
         */
        public static function factory( $url, $options = null, $caller = __METHOD__ ) {
         * @see MWHttpRequest::__construct
         */
        public static function factory( $url, $options = null, $caller = __METHOD__ ) {
-               if ( !Http::$httpEngine ) {
-                       Http::$httpEngine = function_exists( 'curl_init' ) ? 'curl' : 'php';
-               } elseif ( Http::$httpEngine == 'curl' && !function_exists( 'curl_init' ) ) {
-                       throw new DomainException( __METHOD__ . ': curl (http://php.net/curl) is not installed, but' .
-                               ' Http::$httpEngine is set to "curl"' );
-               }
-
-               if ( !is_array( $options ) ) {
-                       $options = [];
-               }
-
-               if ( !isset( $options['logger'] ) ) {
-                       $options['logger'] = LoggerFactory::getInstance( 'http' );
-               }
-
-               switch ( Http::$httpEngine ) {
-                       case 'curl':
-                               return new CurlHttpRequest( $url, $options, $caller, Profiler::instance() );
-                       case 'php':
-                               if ( !wfIniGetBool( 'allow_url_fopen' ) ) {
-                                       throw new DomainException( __METHOD__ . ': allow_url_fopen ' .
-                                               'needs to be enabled for pure PHP http requests to ' .
-                                               'work. If possible, curl should be used instead. See ' .
-                                               'http://php.net/curl.'
-                                       );
-                               }
-                               return new PhpHttpRequest( $url, $options, $caller, Profiler::instance() );
-                       default:
-                               throw new DomainException( __METHOD__ . ': The setting of Http::$httpEngine is not valid.' );
-               }
+               return \MediaWiki\MediaWikiServices::getInstance()
+                       ->getHttpRequestFactory()
+                       ->create( $url, $options, $caller );
        }
 
        /**
        }
 
        /**
index 81473df..3b02e28 100644 (file)
@@ -2,7 +2,7 @@
 
 use Wikimedia\TestingAccessWrapper;
 
 
 use Wikimedia\TestingAccessWrapper;
 
-class MWHttpRequestTestCase extends PHPUnit_Framework_TestCase {
+abstract class MWHttpRequestTestCase extends PHPUnit_Framework_TestCase {
        protected static $httpEngine;
        protected $oldHttpEngine;
 
        protected static $httpEngine;
        protected $oldHttpEngine;
 
index dbb7799..d19340b 100644 (file)
@@ -1,4 +1,6 @@
 <?php
 <?php
+
+use Mediawiki\Http\HttpRequestFactory;
 use MediaWiki\Interwiki\InterwikiLookup;
 use MediaWiki\Linker\LinkRenderer;
 use MediaWiki\Linker\LinkRendererFactory;
 use MediaWiki\Interwiki\InterwikiLookup;
 use MediaWiki\Linker\LinkRenderer;
 use MediaWiki\Linker\LinkRendererFactory;
@@ -339,6 +341,7 @@ class MediaWikiServicesTest extends MediaWikiTestCase {
                        'BlobStore' => [ 'BlobStore', BlobStore::class ],
                        '_SqlBlobStore' => [ '_SqlBlobStore', SqlBlobStore::class ],
                        'RevisionStore' => [ 'RevisionStore', RevisionStore::class ],
                        'BlobStore' => [ 'BlobStore', BlobStore::class ],
                        '_SqlBlobStore' => [ '_SqlBlobStore', SqlBlobStore::class ],
                        'RevisionStore' => [ 'RevisionStore', RevisionStore::class ],
+                       'HttpRequestFactory' => [ 'HttpRequestFactory', HttpRequestFactory::class ],
                ];
        }
 
                ];
        }