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

Reply via email to