Mailing List Archive

jenkins-bot has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/404602 )

Change subject: Create factory for MWHttpRequest
......................................................................


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
---
M autoload.php
M includes/MediaWikiServices.php
M includes/ServiceWiring.php
A includes/http/HttpRequestFactory.php
M includes/http/MWHttpRequest.php
M tests/integration/includes/http/MWHttpRequestTestCase.php
M tests/phpunit/includes/MediaWikiServicesTest.php
7 files changed, 108 insertions(+), 36 deletions(-)

Approvals:
Legoktm: Looks good to me, approved
Addshore: Looks good to me, but someone else must approve
jenkins-bot: Verified



diff --git a/autoload.php b/autoload.php
index 5d6104c..3302415 100644
--- a/autoload.php
+++ b/autoload.php
@@ -878,6 +878,7 @@
'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 5b173cd..00767c7 100644
--- a/includes/MediaWikiServices.php
+++ b/includes/MediaWikiServices.php
@@ -10,6 +10,7 @@
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 @@
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 79e5b84..dab3b5c 100644
--- a/includes/ServiceWiring.php
+++ b/includes/ServiceWiring.php
@@ -508,6 +508,10 @@
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 0000000..80f9b68
--- /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 0f0118c..fff72ec 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 @@
* 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 @@
* @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 @@

/**
* 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 81473df..3b02e28 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 dbb7799..d19340b 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 @@
'BlobStore' => [ 'BlobStore', BlobStore::class ],
'_SqlBlobStore' => [ '_SqlBlobStore', SqlBlobStore::class ],
'RevisionStore' => [ 'RevisionStore', RevisionStore::class ],
+ 'HttpRequestFactory' => [ 'HttpRequestFactory', HttpRequestFactory::class ],
];
}


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

Gerrit-MessageType: merged
Gerrit-Change-Id: I5c0e035542ff5f791a21a95ed13bed4cea6906d0
Gerrit-PatchSet: 8
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Smalyshev <smalyshev@wikimedia.org>
Gerrit-Reviewer: Addshore <addshorewiki@gmail.com>
Gerrit-Reviewer: Fomafix <fomafix@googlemail.com>
Gerrit-Reviewer: Gerg? Tisza <gtisza@wikimedia.org>
Gerrit-Reviewer: Krinkle <krinklemail@gmail.com>
Gerrit-Reviewer: Legoktm <legoktm@member.fsf.org>
Gerrit-Reviewer: Smalyshev <smalyshev@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>

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