From: Stanislav Malyshev <smalyshev@gmail.com>
Date: Tue, 16 Jan 2018 23:11:08 +0000 (-0800)
Subject: Create factory for MWHttpRequest
X-Git-Tag: 1.31.0-rc.0~819^2
X-Git-Url: https://git.heureux-cyclage.org/?p=lhc%2Fweb%2Fwiklou.git;a=commitdiff_plain;h=ebbfe592fe02e38ab2201c0141e7468284e8ff68

Create factory for MWHttpRequest

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
---

diff --git a/autoload.php b/autoload.php
index 5d6104cc04..3302415ba2 100644
--- a/autoload.php
+++ b/autoload.php
@@ -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\\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',
diff --git a/includes/MediaWikiServices.php b/includes/MediaWikiServices.php
index 5b173cd122..00767c7f6c 100644
--- a/includes/MediaWikiServices.php
+++ b/includes/MediaWikiServices.php
@@ -10,6 +10,7 @@ use GenderCache;
 use GlobalVarConfig;
 use Hooks;
 use IBufferingStatsdDataFactory;
+use MediaWiki\Http\HttpRequestFactory;
 use MediaWiki\Preferences\PreferencesFactory;
 use MediaWiki\Shell\CommandFactory;
 use MediaWiki\Storage\BlobStore;
@@ -734,6 +735,14 @@ class MediaWikiServices extends ServiceContainer {
 		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
diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php
index 79e5b848f7..dab3b5cf6c 100644
--- a/includes/ServiceWiring.php
+++ b/includes/ServiceWiring.php
@@ -508,6 +508,10 @@ return [
 		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
diff --git a/includes/http/HttpRequestFactory.php b/includes/http/HttpRequestFactory.php
new file mode 100644
index 0000000000..80f9b68862
--- /dev/null
+++ b/includes/http/HttpRequestFactory.php
@@ -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' );
+	}
+
+}
diff --git a/includes/http/MWHttpRequest.php b/includes/http/MWHttpRequest.php
index 0f0118ce00..fff72ec09f 100644
--- a/includes/http/MWHttpRequest.php
+++ b/includes/http/MWHttpRequest.php
@@ -18,7 +18,6 @@
  * @file
  */
 
-use MediaWiki\Logger\LoggerFactory;
 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.
  */
-class MWHttpRequest implements LoggerAwareInterface {
+abstract class MWHttpRequest implements LoggerAwareInterface {
 	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
 	 */
-	protected function __construct(
-		$url, $options = [], $caller = __METHOD__, $profiler = null
+	public function __construct(
+		$url, array $options = [], $caller = __METHOD__, $profiler = null
 	) {
 		global $wgHTTPTimeout, $wgHTTPConnectTimeout;
 
@@ -174,44 +173,18 @@ class MWHttpRequest implements LoggerAwareInterface {
 
 	/**
 	 * 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
-	 * @return CurlHttpRequest|PhpHttpRequest
+	 * @return MWHttpRequest
 	 * @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 );
 	}
 
 	/**
diff --git a/tests/integration/includes/http/MWHttpRequestTestCase.php b/tests/integration/includes/http/MWHttpRequestTestCase.php
index 81473df227..3b02e28b44 100644
--- a/tests/integration/includes/http/MWHttpRequestTestCase.php
+++ b/tests/integration/includes/http/MWHttpRequestTestCase.php
@@ -2,7 +2,7 @@
 
 use Wikimedia\TestingAccessWrapper;
 
-class MWHttpRequestTestCase extends PHPUnit_Framework_TestCase {
+abstract class MWHttpRequestTestCase extends PHPUnit_Framework_TestCase {
 	protected static $httpEngine;
 	protected $oldHttpEngine;
 
diff --git a/tests/phpunit/includes/MediaWikiServicesTest.php b/tests/phpunit/includes/MediaWikiServicesTest.php
index dbb7799b55..d19340b462 100644
--- a/tests/phpunit/includes/MediaWikiServicesTest.php
+++ b/tests/phpunit/includes/MediaWikiServicesTest.php
@@ -1,4 +1,6 @@
 <?php
+
+use Mediawiki\Http\HttpRequestFactory;
 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 ],
+			'HttpRequestFactory' => [ 'HttpRequestFactory', HttpRequestFactory::class ],
 		];
 	}