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 <smalys...@wikimedia.org> Gerrit-Reviewer: Addshore <addshorew...@gmail.com> Gerrit-Reviewer: Fomafix <foma...@googlemail.com> Gerrit-Reviewer: Gergő Tisza <gti...@wikimedia.org> Gerrit-Reviewer: Krinkle <krinklem...@gmail.com> Gerrit-Reviewer: Legoktm <lego...@member.fsf.org> Gerrit-Reviewer: Smalyshev <smalys...@wikimedia.org> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits