jenkins-bot has submitted this change and it was merged. Change subject: Switch ValueParser definition to $wgWBRepoDataTypes ......................................................................
Switch ValueParser definition to $wgWBRepoDataTypes Bug: T110212 Change-Id: I32e129badd42febd8d4c9055a444c2471399569a --- M docs/datatypes.wiki M lib/includes/DataTypeDefinitions.php M lib/tests/phpunit/DataTypeDefinitionsTest.php M repo/Wikibase.php M repo/WikibaseRepo.datatypes.php M repo/includes/ValueParserFactory.php M repo/includes/WikibaseRepo.php M repo/includes/api/ParseValue.php M repo/tests/phpunit/includes/ValueParserFactoryTest.php M repo/tests/phpunit/includes/WikibaseRepoTest.php M repo/tests/phpunit/includes/api/ParseValueTest.php 11 files changed, 134 insertions(+), 87 deletions(-) Approvals: Jeroen De Dauw: Looks good to me, approved jenkins-bot: Verified diff --git a/docs/datatypes.wiki b/docs/datatypes.wiki index 5fba488..6219575 100644 --- a/docs/datatypes.wiki +++ b/docs/datatypes.wiki @@ -27,7 +27,7 @@ data type definition record. Each such record has the following fields: * value-type (repo and client): the data value type ID identifying the low level value type to use for this data type. Logically, the value type defines the structure of the value, while the data type defines constraints on the value. * validator-factory-callback (repo only): a callable that acts as a factory for the list of validators that should be used to check any user supplied values of the given data type. The callable will be called without any arguments, and must return a list of ValueValidator objects. -* parser-factory-callback (repo only): (PLANNED) a callable that acts as a factory for a ValueParser for this data type. +* parser-factory-callback (repo only): a callable that acts as a factory for a ValueParser for this data type. * formatter-factory-callback (repo and client): (PLANNED) a callable that acts as a factory for ValueFormatters for use with this data type. * rdf-builder-factory-callback (repo only): (PLANNED) a callable that acts as a factory for DataValueRdfBuilder for use with this data type. @@ -47,6 +47,5 @@ == Caveats == -* ValueParsers are still defined using the $wgValueParsers global. * the Methods getSnakFormatterFactory() does not yet use $wgWikibaseDataTypeDefinitions. * RDF mappings for the different data types are still hardcoded. diff --git a/lib/includes/DataTypeDefinitions.php b/lib/includes/DataTypeDefinitions.php index 57d03eb..5093874 100644 --- a/lib/includes/DataTypeDefinitions.php +++ b/lib/includes/DataTypeDefinitions.php @@ -106,7 +106,15 @@ return $this->getMapForDefinitionField( 'validator-factory-callback' ); } - //TODO: getParserFactoryCallbacks() + /** + * @see BuilderBasedDataTypeValidatorFactory + * + * @return callable[]|string[] + */ + public function getParserFactoryCallbacks() { + return $this->getMapForDefinitionField( 'parser-factory-callback' ); + } + //TODO: getFormatterFactoryCallbacks() //TODO: getRdfBuilderFactoryCallbacks() diff --git a/lib/tests/phpunit/DataTypeDefinitionsTest.php b/lib/tests/phpunit/DataTypeDefinitionsTest.php index edac67d..cb91f88 100644 --- a/lib/tests/phpunit/DataTypeDefinitionsTest.php +++ b/lib/tests/phpunit/DataTypeDefinitionsTest.php @@ -48,6 +48,11 @@ $this->assertEquals( array( 'foo' => 'DataTypeDefinitionsTest::getFooValidators' ), $defs->getValidatorFactoryCallbacks() ); } + public function testGetParserFactoryCallbacks() { + $defs = $this->getDataTypeDefinitions(); + $this->assertEquals( array( 'foo' => 'DataTypeDefinitionsTest::getFooParser' ), $defs->getParserFactoryCallbacks() ); + } + public function testRegisterDataTypes() { $defs = $this->getDataTypeDefinitions(); diff --git a/repo/Wikibase.php b/repo/Wikibase.php index cac6f75..444f587 100644 --- a/repo/Wikibase.php +++ b/repo/Wikibase.php @@ -139,43 +139,17 @@ ); }; - // all entity types use the same parser - $wgValueParsers['wikibase-item'] = $newEntityIdParser; - $wgValueParsers['wikibase-property'] = $newEntityIdParser; + /** + * @var callable[] $wgValueParsers Defines parser factory callbacks by parser name (not data type name). + * @deprecated use $wgWBRepoDataTypes instead. + */ + $wgValueParsers['wikibase-entityid'] = $wgWBRepoDataTypes['wikibase-item']['parser-factory-callback']; + $wgValueParsers['globecoordinate'] = $wgWBRepoDataTypes['globe-coordinate']['parser-factory-callback']; - // deprecated: 'wikibase-entityid' is not a datatype. Alias kept for backwards compatibility. - $wgValueParsers['wikibase-entityid'] = $newEntityIdParser; - - $wgValueParsers['quantity'] = function( ValueParsers\ParserOptions $options ) { - $language = Language::factory( $options->getOption( ValueParser::OPT_LANG ) ); - $unlocalizer = new Wikibase\Lib\MediaWikiNumberUnlocalizer( $language ); - return new \ValueParsers\QuantityParser( $options, $unlocalizer ); + // 'null' is not a datatype. Kept for backwards compatibility. + $wgValueParsers['null'] = function() { + return new \ValueParsers\NullParser(); }; - - $wgValueParsers['time'] = function( ValueParsers\ParserOptions $options ) { - $factory = new Wikibase\Lib\Parsers\TimeParserFactory( $options ); - return $factory->getTimeParser(); - }; - - $wgValueParsers['globe-coordinate'] = 'DataValues\Geo\Parsers\GlobeCoordinateParser'; - - // deprecated: 'globecoordinate' is not a datatype. Alias kept for backwards compatibility. - $wgValueParsers['globecoordinate'] = 'DataValues\Geo\Parsers\GlobeCoordinateParser'; - - $wgValueParsers['monolingualtext'] = 'Wikibase\Parsers\MonolingualTextParser'; - - // Use StringParser for datatypes that use StringValue - $stringParserFactoryFunction = function( ValueParsers\ParserOptions $options ) { - $normalizer = \Wikibase\Repo\WikibaseRepo::getDefaultInstance()->getStringNormalizer(); - return new \ValueParsers\StringParser( new Wikibase\Lib\WikibaseStringValueNormalizer( $normalizer ) ); - }; - - $wgValueParsers['commonsMedia'] = $stringParserFactoryFunction; - $wgValueParsers['string'] = $stringParserFactoryFunction; - $wgValueParsers['url'] = $stringParserFactoryFunction; - - // deprecated: 'null' is not a datatype. Alias kept for backwards compatibility. - $wgValueParsers['null'] = 'ValueParsers\NullParser'; // API module registration $wgAPIModules['wbgetentities'] = 'Wikibase\Repo\Api\GetEntities'; diff --git a/repo/WikibaseRepo.datatypes.php b/repo/WikibaseRepo.datatypes.php index 5a96124..99eed1a 100644 --- a/repo/WikibaseRepo.datatypes.php +++ b/repo/WikibaseRepo.datatypes.php @@ -18,6 +18,13 @@ * @author Daniel Kinzler */ +use DataValues\Geo\Parsers\GlobeCoordinateParser; +use ValueParsers\NullParser; +use ValueParsers\QuantityParser; +use ValueParsers\ValueParser; +use Wikibase\Lib\EntityIdValueParser; +use Wikibase\Lib\Parsers\TimeParserFactory; +use Wikibase\Parsers\MonolingualTextParser; use Wikibase\Repo\WikibaseRepo; return call_user_func( function() { @@ -27,23 +34,40 @@ // ValidatorBuilders should be used *only* here, program logic should use a // DataValueValidatorFactory as returned by WikibaseRepo::getDataTypeValidatorFactory(). + $newEntityIdParser = function( ValueParsers\ParserOptions $options ) { + $repo = WikibaseRepo::getDefaultInstance(); + return new EntityIdValueParser( $repo->getEntityIdParser() ); + }; + + $newNullParser = function( ValueParsers\ParserOptions $options ) { + return new NullParser(); + }; + return array( 'commonsMedia' => array( 'validator-factory-callback' => function () { $factory = WikibaseRepo::getDefaultValidatorBuilders(); return $factory->buildStringValidators(); }, + 'parser-factory-callback' => $newNullParser, //TODO: use StringParser + ), 'globe-coordinate' => array( 'validator-factory-callback' => function () { $factory = WikibaseRepo::getDefaultValidatorBuilders(); return $factory->buildCoordinateValidators(); }, + 'parser-factory-callback' => function( ValueParsers\ParserOptions $options ) { + return new GlobeCoordinateParser( $options ); + } ), 'monolingualtext' => array( 'validator-factory-callback' => function () { $factory = WikibaseRepo::getDefaultValidatorBuilders(); return $factory->buildMonolingualTextValidators(); + }, + 'parser-factory-callback' => function( ValueParsers\ParserOptions $options ) { + return new MonolingualTextParser( $options ); } ), 'quantity' => array( @@ -51,17 +75,27 @@ $factory = WikibaseRepo::getDefaultValidatorBuilders(); return $factory->buildQuantityValidators(); }, + 'parser-factory-callback' => function( ValueParsers\ParserOptions $options ) { + $language = Language::factory( $options->getOption( ValueParser::OPT_LANG ) ); + $unlocalizer = new Wikibase\Lib\MediaWikiNumberUnlocalizer( $language); + return new QuantityParser( $options, $unlocalizer ); + }, ), 'string' => array( 'validator-factory-callback' => function () { $factory = WikibaseRepo::getDefaultValidatorBuilders(); return $factory->buildStringValidators(); }, + 'parser-factory-callback' => $newNullParser, //TODO: use StringParser ), 'time' => array( 'validator-factory-callback' => function () { $factory = WikibaseRepo::getDefaultValidatorBuilders(); return $factory->buildTimeValidators(); + }, + 'parser-factory-callback' => function( ValueParsers\ParserOptions $options ) { + $factory = new TimeParserFactory( $options ); + return $factory->getTimeParser(); }, ), 'url' => array( @@ -69,18 +103,21 @@ $factory = WikibaseRepo::getDefaultValidatorBuilders(); return $factory->buildUrlValidators(); }, + 'parser-factory-callback' => $newNullParser, //TODO: use StringParser ), 'wikibase-item' => array( 'validator-factory-callback' => function () { $factory = WikibaseRepo::getDefaultValidatorBuilders(); return $factory->buildItemValidators(); }, + 'parser-factory-callback' => $newEntityIdParser, ), 'wikibase-property' => array( 'validator-factory-callback' => function () { $factory = WikibaseRepo::getDefaultValidatorBuilders(); return $factory->buildPropertyValidators(); }, + 'parser-factory-callback' => $newEntityIdParser, ), ); diff --git a/repo/includes/ValueParserFactory.php b/repo/includes/ValueParserFactory.php index a54acc3..8c46540 100644 --- a/repo/includes/ValueParserFactory.php +++ b/repo/includes/ValueParserFactory.php @@ -20,17 +20,17 @@ /** * Maps parser id to ValueParser class or builder callback. * - * @since 0.1 + * @since 0.5 * - * @var array + * @var callable[] */ protected $parsers = array(); /** - * @since 0.1 + * @since 0.5 * - * @param string|callable[] $valueParsers An associative array mapping parser ids to - * class names or callable builders. + * @param callable[] $valueParsers An associative array mapping parser ids to + * factory functions. * * @throws InvalidArgumentException */ @@ -40,8 +40,8 @@ throw new InvalidArgumentException( 'Parser id needs to be a string' ); } - if ( !is_string( $parserBuilder ) && !is_callable( $parserBuilder ) ) { - throw new InvalidArgumentException( 'Parser class needs to be a class name or callable' ); + if ( !is_callable( $parserBuilder ) ) { + throw new InvalidArgumentException( 'Parser class needs to be a callable' ); } $this->parsers[$parserId] = $parserBuilder; @@ -57,24 +57,6 @@ */ public function getParserIds() { return array_keys( $this->parsers ); - } - - /** - * Returns the parser builder (class name or callable) for $parserId, or null if - * no builder was registered for that id. - * - * @since 0.1 - * - * @param string $parserId - * - * @return string|callable|null - */ - public function getParserBuilder( $parserId ) { - if ( array_key_exists( $parserId, $this->parsers ) ) { - return $this->parsers[$parserId]; - } - - return null; } /** @@ -100,20 +82,15 @@ } /** - * @param string|callable $builder Either a classname of an implementation of ValueParser, - * or a callable that returns a ValueParser. $options will be passed to the constructor - * or callable, respectively. + * @param callable $builder A callable that returns a ValueParser. + * $options will be passed to the constructor or callable, respectively. * @param ParserOptions $options * * @throws LogicException if the builder did not create a ValueParser * @return ValueParser */ private function instantiateParser( $builder, ParserOptions $options ) { - if ( is_string( $builder ) ) { - $parser = new $builder( $options ); - } else { - $parser = call_user_func( $builder, $options ); - } + $parser = call_user_func( $builder, $options ); if ( !( $parser instanceof ValueParser ) ) { throw new LogicException( "Invalid parser builder, did not create an instance of ValueParser." ); diff --git a/repo/includes/WikibaseRepo.php b/repo/includes/WikibaseRepo.php index 2ce523b..a844700 100644 --- a/repo/includes/WikibaseRepo.php +++ b/repo/includes/WikibaseRepo.php @@ -120,6 +120,11 @@ private $dataTypeFactory = null; /** + * @var ValueParserFactory|null + */ + private $valueParserFactory = null; + + /** * @var SnakConstructionService|null */ private $snakConstructionService = null; @@ -292,6 +297,26 @@ } /** + * @since 0.5 + * + * @return ValueParserFactory + */ + public function getValueParserFactory() { + global $wgValueParsers; + + if ( $this->valueParserFactory === null ) { + $callbacks = $this->dataTypeDefinitions->getParserFactoryCallbacks(); + + // For backwards-compatibility, also register parsers under legacy names. + $callbacks = array_merge( $wgValueParsers, $callbacks ); + + $this->valueParserFactory = new ValueParserFactory( $callbacks ); + } + + return $this->valueParserFactory; + } + + /** * @since 0.4 * * @return DataValueFactory diff --git a/repo/includes/api/ParseValue.php b/repo/includes/api/ParseValue.php index 1321e4d..29b83ff 100644 --- a/repo/includes/api/ParseValue.php +++ b/repo/includes/api/ParseValue.php @@ -81,7 +81,7 @@ $this->setServices( $wikibaseRepo->getDataTypeFactory(), - new ValueParserFactory( $GLOBALS['wgValueParsers'] ), + $wikibaseRepo->getValueParserFactory(), $wikibaseRepo->getDataTypeValidatorFactory(), $wikibaseRepo->getExceptionLocalizer(), $wikibaseRepo->getValidatorErrorLocalizer(), @@ -135,9 +135,9 @@ $options = $this->getOptionsObject( $params['options'] ); // Parsers are registered by datatype. - // Note: parser used to be addressed by a name independant of datatype, using the 'parser' + // Note: parser used to be addressed by a name independent of datatype, using the 'parser' // parameter. For backwards compatibility, parsers are also registered under their old names - // in $wgValueParsers, and this in the ValueParserFactory. + // in $wgValueParsers, and thus in the ValueParserFactory. $name = $params['datatype'] ?: $params['parser']; if ( empty( $name ) ) { diff --git a/repo/tests/phpunit/includes/ValueParserFactoryTest.php b/repo/tests/phpunit/includes/ValueParserFactoryTest.php index 261fc1f..937480e 100644 --- a/repo/tests/phpunit/includes/ValueParserFactoryTest.php +++ b/repo/tests/phpunit/includes/ValueParserFactoryTest.php @@ -2,6 +2,8 @@ namespace Wikibase\Tests\Repo; +use ValueParsers\NullParser; +use ValueParsers\ParserOptions; use Wikibase\Repo\ValueParserFactory; /** @@ -14,26 +16,42 @@ * @licence GNU GPL v2+ * @author Adrian Lang <adrian.l...@wikimedia.de> */ -class ValueParserFactoryTest extends \MediaWikiTestCase { +class ValueParserFactoryTest extends \PHPUnit_Framework_TestCase { /** - * @dataProvider getParserIdsProvider + * @dataProvider provideFactoryFunctions */ - public function testGetParserIds( $valueParsers, $expected ) { - $valueParserFactory = new ValueParserFactory( $valueParsers ); + public function testGetParserIds( $factoryFunctions ) { + $valueParserFactory = new ValueParserFactory( $factoryFunctions ); $returnValue = $valueParserFactory->getParserIds(); - $this->assertEquals( $expected, $returnValue ); + $this->assertEquals( array_keys( $factoryFunctions ), $returnValue ); } - public function getParserIdsProvider() { + public function provideFactoryFunctions() { return array( array( - array( 'key' => 'value' ), - array( 'key' ) + array( + 'foo' => function() { + return new NullParser(); + } + ), ) ); } + /** + * @dataProvider provideFactoryFunctions + */ + public function testNewParser( $factoryFunctions ) { + $valueParserFactory = new ValueParserFactory( $factoryFunctions ); + $options = new ParserOptions(); + + foreach ( $valueParserFactory->getParserIds() as $id ) { + $parser = $valueParserFactory->newParser( $id, $options ); + $this->assertInstanceOf( 'ValueParsers\ValueParser', $parser ); + } + } + } diff --git a/repo/tests/phpunit/includes/WikibaseRepoTest.php b/repo/tests/phpunit/includes/WikibaseRepoTest.php index bff52cf..d28e05d 100644 --- a/repo/tests/phpunit/includes/WikibaseRepoTest.php +++ b/repo/tests/phpunit/includes/WikibaseRepoTest.php @@ -25,9 +25,9 @@ $this->assertInstanceOf( 'DataTypes\DataTypeFactory', $returnValue ); } - public function testGetDataTypeValidatorFactoryReturnType() { - $returnValue = $this->getWikibaseRepo()->getDataTypeValidatorFactory(); - $this->assertInstanceOf( 'Wikibase\Repo\DataTypeValidatorFactory', $returnValue ); + public function testGetValueParserFactoryReturnType() { + $returnValue = $this->getWikibaseRepo()->getValueParserFactory(); + $this->assertInstanceOf( 'Wikibase\Repo\ValueParserFactory', $returnValue ); } public function testGetDataValueFactoryReturnType() { diff --git a/repo/tests/phpunit/includes/api/ParseValueTest.php b/repo/tests/phpunit/includes/api/ParseValueTest.php index 13356b1..734394b 100644 --- a/repo/tests/phpunit/includes/api/ParseValueTest.php +++ b/repo/tests/phpunit/includes/api/ParseValueTest.php @@ -3,8 +3,8 @@ namespace Wikibase\Test\Repo\Api; use ApiMain; -use DataTypes\DataType; use DataTypes\DataTypeFactory; +use DataValues\Geo\Parsers\GlobeCoordinateParser; use FauxRequest; use Language; use ValueParsers\NullParser; @@ -60,7 +60,7 @@ 'null' => array( $this, 'newNullParser' ), 'string' => array( $this, 'newNullParser' ), 'url' => array( $this, 'newNullParser' ), - 'globe-coordinate' => 'DataValues\Geo\Parsers\GlobeCoordinateParser', + 'globe-coordinate' => array( $this, 'newGlobeCoordinateParser' ), ) ); $validatorFactory = new BuilderBasedDataTypeValidatorFactory( array( @@ -91,6 +91,10 @@ return new NullParser(); } + public function newGlobeCoordinateParser() { + return new GlobeCoordinateParser(); + } + private function callApiModule( array $params ) { $module = $this->newApiModule( $params ); -- To view, visit https://gerrit.wikimedia.org/r/228865 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I32e129badd42febd8d4c9055a444c2471399569a Gerrit-PatchSet: 18 Gerrit-Project: mediawiki/extensions/Wikibase Gerrit-Branch: master Gerrit-Owner: Daniel Kinzler <daniel.kinz...@wikimedia.de> Gerrit-Reviewer: Addshore <addshorew...@gmail.com> Gerrit-Reviewer: Daniel Kinzler <daniel.kinz...@wikimedia.de> Gerrit-Reviewer: Jeroen De Dauw <jeroended...@gmail.com> Gerrit-Reviewer: Jonas Kress (WMDE) <jonas.kr...@wikimedia.de> Gerrit-Reviewer: Thiemo Mättig (WMDE) <thiemo.maet...@wikimedia.de> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits