jenkins-bot has submitted this change and it was merged. Change subject: VirtualRESTServiceClient management cleanups ......................................................................
VirtualRESTServiceClient management cleanups * Add getVirtualRESTServiceClient() to MediaWikiServices. * Support auto-mounting services that are usable by the main MediaWikiServices instance. * Support lazy-loading in mount(), where only class/args are set until the service is needed. This avoids excess overhead. Change-Id: I5c22be59664b3f5716c957e2c3d7c8e70d5fdc6c --- M includes/DefaultSettings.php M includes/MediaWikiServices.php M includes/ServiceWiring.php M includes/libs/virtualrest/VirtualRESTServiceClient.php M tests/phpunit/includes/MediaWikiServicesTest.php 5 files changed, 93 insertions(+), 15 deletions(-) Approvals: Legoktm: Looks good to me, approved jenkins-bot: Verified diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 87003f0..eee007c 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -8250,11 +8250,29 @@ /** * Global configuration variable for Virtual REST Services. - * Parameters for different services are to be declared inside - * $wgVirtualRestConfig['modules'], which is to be treated as an associative - * array. Global parameters will be merged with service-specific ones. The - * result will then be passed to VirtualRESTService::__construct() in the - * module. + * + * Use the 'path' key to define automatically mounted services. The value for this + * key is a map of path prefixes to service configuration. The later is an array of: + * - class : the fully qualified class name + * - options : map of arguments to the class constructor + * Such services will be available to handle queries under their path from the VRS + * singleton, e.g. MediaWikiServices::getInstance()->getVirtualRESTServiceClient(); + * + * Auto-mounting example for Parsoid: + * + * $wgVirtualRestConfig['paths']['/parsoid/'] = [ + * 'class' => 'ParsoidVirtualRESTService', + * 'options' => [ + * 'url' => 'http://localhost:8000', + * 'prefix' => 'enwiki', + * 'domain' => 'en.wikipedia.org' + * ] + * ]; + * + * Parameters for different services can also be declared inside the 'modules' value, + * which is to be treated as an associative array. The parameters in 'global' will be + * merged with service-specific ones. The result will then be passed to + * VirtualRESTService::__construct() in the module. * * Example config for Parsoid: * @@ -8268,6 +8286,7 @@ * @since 1.25 */ $wgVirtualRestConfig = [ + 'paths' => [], 'modules' => [], 'global' => [ # Timeout in seconds diff --git a/includes/MediaWikiServices.php b/includes/MediaWikiServices.php index ac5fbe0..49891df 100644 --- a/includes/MediaWikiServices.php +++ b/includes/MediaWikiServices.php @@ -28,6 +28,7 @@ use SkinFactory; use TitleFormatter; use TitleParser; +use VirtualRESTServiceClient; use MediaWiki\Interwiki\InterwikiLookup; /** @@ -571,6 +572,14 @@ return $this->getService( 'TitleParser' ); } + /** + * @since 1.28 + * @return VirtualRESTServiceClient + */ + public function getVirtualRESTServiceClient() { + return $this->getService( 'VirtualRESTServiceClient' ); + } + /////////////////////////////////////////////////////////////////////////// // 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 21c6377..33569e6 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -209,6 +209,24 @@ return $services->getService( '_MediaWikiTitleCodec' ); }, + 'VirtualRESTServiceClient' => function( MediaWikiServices $services ) { + $config = $services->getMainConfig()->get( 'VirtualRestConfig' ); + + $vrsClient = new VirtualRESTServiceClient( new MultiHttpClient( [] ) ); + foreach ( $config['paths'] as $prefix => $serviceConfig ) { + $class = $serviceConfig['class']; + // Merge in the global defaults + $constructArg = isset( $serviceConfig['options'] ) + ? $serviceConfig['options'] + : []; + $constructArg += $config['global']; + // Make the VRS service available at the mount point + $vrsClient->mount( $prefix, [ 'class' => $class, 'config' => $constructArg ] ); + } + + return $vrsClient; + }, + /////////////////////////////////////////////////////////////////////////// // 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/libs/virtualrest/VirtualRESTServiceClient.php b/includes/libs/virtualrest/VirtualRESTServiceClient.php index a333d75..0864e5c 100644 --- a/includes/libs/virtualrest/VirtualRESTServiceClient.php +++ b/includes/libs/virtualrest/VirtualRESTServiceClient.php @@ -45,9 +45,9 @@ */ class VirtualRESTServiceClient { /** @var MultiHttpClient */ - protected $http; - /** @var VirtualRESTService[] Map of (prefix => VirtualRESTService) */ - protected $instances = []; + private $http; + /** @var array Map of (prefix => VirtualRESTService|array) */ + private $instances = []; const VALID_MOUNT_REGEX = '#^/[0-9a-z]+/([0-9a-z]+/)*$#'; @@ -61,14 +61,23 @@ /** * Map a prefix to service handler * + * If $instance is in array, it must have these keys: + * - class : string; fully qualified VirtualRESTService class name + * - config : array; map of parameters that is the first __construct() argument + * * @param string $prefix Virtual path - * @param VirtualRESTService $instance + * @param VirtualRESTService|array $instance Service or info to yield the service */ - public function mount( $prefix, VirtualRESTService $instance ) { + public function mount( $prefix, $instance ) { if ( !preg_match( self::VALID_MOUNT_REGEX, $prefix ) ) { throw new UnexpectedValueException( "Invalid service mount point '$prefix'." ); } elseif ( isset( $this->instances[$prefix] ) ) { throw new UnexpectedValueException( "A service is already mounted on '$prefix'." ); + } + if ( !( $instance instanceof VirtualRESTService ) ) { + if ( !isset( $instance['class'] ) || !isset( $instance['config'] ) ) { + throw new UnexpectedValueException( "Missing 'class' or 'config' ('$prefix')." ); + } } $this->instances[$prefix] = $instance; } @@ -104,7 +113,7 @@ }; $matches = []; // matching prefixes (mount points) - foreach ( $this->instances as $prefix => $service ) { + foreach ( $this->instances as $prefix => $unused ) { if ( strpos( $path, $prefix ) === 0 ) { $matches[] = $prefix; } @@ -112,8 +121,8 @@ usort( $matches, $cmpFunc ); // Return the most specific prefix and corresponding service - return isset( $matches[0] ) - ? [ $matches[0], $this->instances[$matches[0]] ] + return $matches + ? [ $matches[0], $this->getInstance( $matches[0] ) ] : [ null, null ]; } @@ -216,7 +225,7 @@ // defer the original or to set a proxy response to the original. $newReplaceReqsByService = []; foreach ( $replaceReqsByService as $prefix => $servReqs ) { - $service = $this->instances[$prefix]; + $service = $this->getInstance( $prefix ); foreach ( $service->onRequests( $servReqs, $idFunc ) as $index => $req ) { // Services use unique IDs for replacement requests if ( isset( $servReqs[$index] ) || isset( $origPending[$index] ) ) { @@ -250,7 +259,7 @@ // forced by setting 'response' rather than actually be sent over the wire. $newReplaceReqsByService = []; foreach ( $checkReqIndexesByPrefix as $prefix => $servReqIndexes ) { - $service = $this->instances[$prefix]; + $service = $this->getInstance( $prefix ); // $doneReqs actually has the requests (with 'response' set) $servReqs = array_intersect_key( $doneReqs, $servReqIndexes ); foreach ( $service->onResponses( $servReqs, $idFunc ) as $index => $req ) { @@ -288,4 +297,26 @@ return $responses; } + + /** + * @param string $prefix + * @return VirtualRESTService + */ + private function getInstance( $prefix ) { + if ( !isset( $this->instances[$prefix] ) ) { + throw new RunTimeException( "No service registered at prefix '{$prefix}'." ); + } + + if ( !( $this->instances[$prefix] instanceof VirtualRESTService ) ) { + $config = $this->instances[$prefix]['config']; + $class = $this->instances[$prefix]['class']; + $service = new $class( $config ); + if ( !( $service instanceof VirtualRESTService ) ) { + throw new UnexpectedValueException( "Registered service has the wrong class." ); + } + $this->instances[$prefix] = $service; + } + + return $this->instances[$prefix]; + } } diff --git a/tests/phpunit/includes/MediaWikiServicesTest.php b/tests/phpunit/includes/MediaWikiServicesTest.php index ac8c43b..8c2b143 100644 --- a/tests/phpunit/includes/MediaWikiServicesTest.php +++ b/tests/phpunit/includes/MediaWikiServicesTest.php @@ -324,6 +324,7 @@ '_MediaWikiTitleCodec' => [ '_MediaWikiTitleCodec', MediaWikiTitleCodec::class ], 'TitleFormatter' => [ 'TitleFormatter', TitleFormatter::class ], 'TitleParser' => [ 'TitleParser', TitleParser::class ], + 'VirtualRESTServiceClient' => [ 'VirtualRESTServiceClient', VirtualRESTServiceClient::class ] ]; } -- To view, visit https://gerrit.wikimedia.org/r/305943 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I5c22be59664b3f5716c957e2c3d7c8e70d5fdc6c Gerrit-PatchSet: 5 Gerrit-Project: mediawiki/core Gerrit-Branch: master Gerrit-Owner: Aaron Schulz <[email protected]> Gerrit-Reviewer: Aaron Schulz <[email protected]> Gerrit-Reviewer: Cscott <[email protected]> Gerrit-Reviewer: GWicke <[email protected]> Gerrit-Reviewer: KartikMistry <[email protected]> Gerrit-Reviewer: Legoktm <[email protected]> Gerrit-Reviewer: Nikerabbit <[email protected]> Gerrit-Reviewer: Ppchelko <[email protected]> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
