Daniel Kinzler has uploaded a new change for review.
https://gerrit.wikimedia.org/r/223192
Change subject: Add optional validation to wbparsevalue
......................................................................
Add optional validation to wbparsevalue
Bug: T104330
Change-Id: I315a55db8dff1225cc1077cb312b6d4c7a90d282
---
M repo/i18n/en.json
M repo/i18n/qqq.json
M repo/includes/BuilderBasedDataTypeValidatorFactory.php
M repo/includes/DataTypeValidatorFactory.php
M repo/includes/WikibaseRepo.php
M repo/includes/api/ParseValue.php
M repo/tests/phpunit/includes/ValidatorBuildersTest.php
M repo/tests/phpunit/includes/api/ParseValueTest.php
8 files changed, 191 insertions(+), 23 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/92/223192/1
diff --git a/repo/i18n/en.json b/repo/i18n/en.json
index b83916d..015254f 100644
--- a/repo/i18n/en.json
+++ b/repo/i18n/en.json
@@ -554,7 +554,8 @@
"apihelp-wbparsevalue-param-values": "The values to parse",
"apihelp-wbparsevalue-param-options": "The options the parser should
use. Provided as a JSON object.",
"apihelp-wbparsevalue-example-1": "Parse a plain string into a
StringValue object.",
- "apihelp-wbparsevalue-example-2": "Parse 1994-02-08 to a TimeValue
object with a precision of 9 (the year).",
+ "apihelp-wbparsevalue-example-2": "Parse 1994-02-08 to a TimeValue
object with a precision of 9 (year).",
+ "apihelp-wbparsevalue-example-3": "Parse 1994-02-08 to a TimeValue
object with a precision of 14 (second) with validation enabled, resulting in a
validation failure.",
"apihelp-wbremoveclaims-description": "Removes Wikibase claims.",
"apihelp-wbremoveclaims-param-claim": "One GUID or several
(pipe-separated) GUIDs identifying the claims to be removed.\nAll claims must
belong to the same entity.",
"apihelp-wbremoveclaims-example-1": "Remove claim with GUID of
\"Q42$D8404CDA-25E4-4334-AF13-A3290BCD9C0N\"",
diff --git a/repo/i18n/qqq.json b/repo/i18n/qqq.json
index 04ed02d..341bd8b 100644
--- a/repo/i18n/qqq.json
+++ b/repo/i18n/qqq.json
@@ -583,6 +583,7 @@
"apihelp-wbparsevalue-param-options":
"{{doc-apihelp-param|wbparsevalue|options}}",
"apihelp-wbparsevalue-example-1":
"{{doc-apihelp-example|wbparsevalue}}",
"apihelp-wbparsevalue-example-2":
"{{doc-apihelp-example|wbparsevalue}}",
+ "apihelp-wbparsevalue-example-3":
"{{doc-apihelp-example|wbparsevalue}}",
"apihelp-wbremoveclaims-description":
"{{doc-apihelp-description|wbremoveclaims}}",
"apihelp-wbremoveclaims-param-claim":
"{{doc-apihelp-param|wbremoveclaims|claim}}",
"apihelp-wbremoveclaims-example-1":
"{{doc-apihelp-example|wbremoveclaim}}",
diff --git a/repo/includes/BuilderBasedDataTypeValidatorFactory.php
b/repo/includes/BuilderBasedDataTypeValidatorFactory.php
index 5780b04..02ec071 100644
--- a/repo/includes/BuilderBasedDataTypeValidatorFactory.php
+++ b/repo/includes/BuilderBasedDataTypeValidatorFactory.php
@@ -3,9 +3,12 @@
namespace Wikibase\Repo;
use ValueValidators\ValueValidator;
+use Wikimedia\Assert\Assert;
/**
* A factory providing ValueValidators based on DataType id that uses
ValidatorBuilders.
+ *
+ * @todo: unit tests!
*
* @author Adrian Heine < [email protected] >
*/
@@ -17,10 +20,12 @@
private $validatorBuilders;
/**
- * @param ValidatorBuilders $validatorBuilders
+ * @param callable[] $validatorBuilders
*/
- public function __construct( ValidatorBuilders $validatorBuilders ) {
- $this->validatorBuilders =
$validatorBuilders->getDataTypeValidators();
+ public function __construct( array $validatorBuilders ) {
+ Assert::parameterElementType( 'callable', $validatorBuilders,
'$validatorBuilders' );
+
+ $this->validatorBuilders = $validatorBuilders;
}
/**
@@ -30,9 +35,22 @@
* @return ValueValidator[]
*/
public function getValidators( $dataTypeId ) {
- return call_user_func(
+ if ( !isset( $this->validatorBuilders[ $dataTypeId ] ) ) {
+ //@todo: test me!
+ return array();
+ }
+
+ $validators = call_user_func(
$this->validatorBuilders[ $dataTypeId ]
);
+
+ Assert::postcondition( is_array( $validators ), "Factory
function for $dataTypeId did not return an array of ValueValidator objects!" );
+
+ foreach ( $validators as $v ) {
+ Assert::postcondition( $v instanceof ValueValidator,
"Factory function for $dataTypeId did not return an array of ValueValidator
objects!" );
+ }
+
+ return $validators;
}
}
diff --git a/repo/includes/DataTypeValidatorFactory.php
b/repo/includes/DataTypeValidatorFactory.php
index 69bb30e..4f9ee7b 100644
--- a/repo/includes/DataTypeValidatorFactory.php
+++ b/repo/includes/DataTypeValidatorFactory.php
@@ -12,6 +12,8 @@
interface DataTypeValidatorFactory {
/**
+ * Returns the validators associated with the given $dataTypeId.
+ * For unknown $dataTypeIds, an empty array is returned.
*
* @param string $dataTypeId
*
diff --git a/repo/includes/WikibaseRepo.php b/repo/includes/WikibaseRepo.php
index 86af164..752ce1e 100644
--- a/repo/includes/WikibaseRepo.php
+++ b/repo/includes/WikibaseRepo.php
@@ -1144,16 +1144,18 @@
);
}
- private function getDataTypeValidatorFactory() {
+ public function getDataTypeValidatorFactory() {
$urlSchemes = $this->settings->getSetting( 'urlSchemes' );
+ $builders = new ValidatorBuilders(
+ $this->getEntityLookup(),
+ $this->getEntityIdParser(),
+ $urlSchemes,
+ $this->getMonolingualTextLanguages()
+ );
+
return new BuilderBasedDataTypeValidatorFactory(
- new ValidatorBuilders(
- $this->getEntityLookup(),
- $this->getEntityIdParser(),
- $urlSchemes,
- $this->getMonolingualTextLanguages()
- )
+ $builders->getDataTypeValidators()
);
}
diff --git a/repo/includes/api/ParseValue.php b/repo/includes/api/ParseValue.php
index aed3ff5..0b69752 100644
--- a/repo/includes/api/ParseValue.php
+++ b/repo/includes/api/ParseValue.php
@@ -14,9 +14,15 @@
use ValueParsers\ParseException;
use ValueParsers\ParserOptions;
use ValueParsers\ValueParser;
+use ValueValidators\Error;
+use ValueValidators\NullValidator;
+use ValueValidators\ValueValidator;
use Wikibase\Lib\Localizer\ExceptionLocalizer;
+use Wikibase\Repo\DataTypeValidatorFactory;
use Wikibase\Repo\ValueParserFactory;
use Wikibase\Repo\WikibaseRepo;
+use Wikibase\Validators\CompositeValidator;
+use Wikibase\Validators\ValidatorErrorLocalizer;
/**
* API module for using value parsers.
@@ -39,6 +45,16 @@
* @var ValueParserFactory
*/
private $valueParserFactory;
+
+ /**
+ * @var DataTypeValidatorFactory
+ */
+ private $dataTypeValidatorFactory;
+
+ /**
+ * @var ValidatorErrorLocalizer
+ */
+ private $validatorErrorLocalizer;
/**
* @var ExceptionLocalizer
@@ -66,7 +82,9 @@
$this->setServices(
$wikibaseRepo->getDataTypeFactory(),
new ValueParserFactory( $GLOBALS['wgValueParsers'] ),
+ $wikibaseRepo->getDataTypeValidatorFactory(),
$wikibaseRepo->getExceptionLocalizer(),
+ $wikibaseRepo->getValidatorErrorLocalizer(),
$apiHelperFactory->getErrorReporter( $this )
);
}
@@ -74,12 +92,16 @@
public function setServices(
DataTypeFactory $dataTypeFactory,
ValueParserFactory $valueParserFactory,
+ DataTypeValidatorFactory $dataTypeValidatorFactory,
ExceptionLocalizer $exceptionLocalizer,
+ ValidatorErrorLocalizer $validatorErrorLocalizer,
ApiErrorReporter $errorReporter
) {
$this->dataTypeFactory = $dataTypeFactory;
$this->valueParserFactory = $valueParserFactory;
+ $this->dataTypeValidatorFactory = $dataTypeValidatorFactory;
$this->exceptionLocalizer = $exceptionLocalizer;
+ $this->validatorErrorLocalizer = $validatorErrorLocalizer;
$this->errorReporter = $errorReporter;
}
@@ -94,9 +116,10 @@
$results = array();
$params = $this->extractRequestParams();
+ $validator = $params['validate'] ? $this->getValidator() : null;
foreach ( $params['values'] as $value ) {
- $results[] = $this->parseStringValue( $parser, $value );
+ $results[] = $this->parseStringValue( $parser, $value,
$validator );
}
$this->outputResults( $results );
@@ -119,25 +142,59 @@
if ( empty( $name ) ) {
// If neither 'datatype' not 'parser' is given, tell
the client to use 'datatype'.
- $this->dieUsageMsg( array( 'missingparam', 'datatype' )
);
+ $this->errorReporter->dieError( 'No datatype given',
'param-illegal' );
}
try {
$parser = $this->valueParserFactory->newParser( $name,
$options );
+ return $parser;
} catch ( OutOfBoundsException $ex ) {
- throw new LogicException( "No parser registered for
`$name`" );
+ $this->errorReporter->dieError( "Unknown datatype
`$name`", 'unknown-datatype' );
+ throw new LogicException( 'dieError() did not throw an
exception' );
+ }
+ }
+
+ /**
+ * @return ValueValidator
+ */
+ private function getValidator() {
+ $params = $this->extractRequestParams();
+
+ $name = $params['datatype'];
+
+ if ( empty( $name ) ) {
+ // 'datatype' parameter is required for validation.
+ $this->errorReporter->dieError( 'No datatype given',
'param-illegal' );
}
- return $parser;
+ // Note: For unknown datatype, we'll get an empty list.
+ $validators = $this->dataTypeValidatorFactory->getValidators(
$name );
+ return $this->wrapValidators( $validators );
+ }
+
+ /**
+ * @param ValueValidator[] $validators
+ *
+ * @return ValueValidator
+ */
+ private function wrapValidators( array $validators ) {
+ if ( count( $validators ) === 0 ) {
+ return new NullValidator();
+ } elseif ( count( $validators ) === 1 ) {
+ return reset( $validators );
+ } else {
+ return new CompositeValidator( $validators, true );
+ }
}
/**
* @param ValueParser $parser
* @param string $value
+ * @param ValueValidator $validator
*
* @return array
*/
- private function parseStringValue( ValueParser $parser, $value ) {
+ private function parseStringValue( ValueParser $parser, $value,
ValueValidator $validator = null ) {
$result = array(
'raw' => $value
);
@@ -158,7 +215,34 @@
$result['value'] = $parseResult;
}
+ if ( $validator ) {
+ $validatorResult = $validator->validate( $value );
+ $validationStatus =
$this->validatorErrorLocalizer->getResultStatus( $validatorResult );
+
+ $result['valid'] = $validationStatus->isOK();
+
+ if ( !$validationStatus->isOK() ) {
+ $result['error'] = 'ValidationError';
+ $this->errorReporter->addStatusToResult(
$validationStatus, $result );
+ $result['validation-errors'] =
$this->getValidatorErrorCodes( $validatorResult->getErrors() );
+ }
+ }
+
return $result;
+ }
+
+ /**
+ * @param Error[] $errors
+ *
+ * @return string[]
+ */
+ private function getValidatorErrorCodes( array $errors ) {
+ return array_map(
+ function ( Error $error ) {
+ return $error->getCode();
+ },
+ $errors
+ );
}
private function addParseErrorToResult( array &$result, ParseException
$parseError ) {
@@ -254,6 +338,9 @@
ApiBase::PARAM_TYPE => 'string',
ApiBase::PARAM_REQUIRED => false,
),
+ 'validate' => array(
+ ApiBase::PARAM_TYPE => 'boolean',
+ ),
);
}
@@ -266,6 +353,8 @@
'apihelp-wbparsevalue-example-1',
'action=wbparsevalue&datatype=time&values=1994-02-08&options={"precision":9}' =>
'apihelp-wbparsevalue-example-2',
+
'action=wbparsevalue&datatype=time&validate&values=1994-02-08&options={"precision":14}'
=>
+ 'apihelp-wbparsevalue-example-3',
);
}
diff --git a/repo/tests/phpunit/includes/ValidatorBuildersTest.php
b/repo/tests/phpunit/includes/ValidatorBuildersTest.php
index ec8c30c..a4228dc 100644
--- a/repo/tests/phpunit/includes/ValidatorBuildersTest.php
+++ b/repo/tests/phpunit/includes/ValidatorBuildersTest.php
@@ -56,7 +56,7 @@
$urlSchemes,
$contentLanguages
);
- $validatorFactory = new BuilderBasedDataTypeValidatorFactory(
$builders );
+ $validatorFactory = new BuilderBasedDataTypeValidatorFactory(
$builders->getDataTypeValidators() );
return $validatorFactory;
}
diff --git a/repo/tests/phpunit/includes/api/ParseValueTest.php
b/repo/tests/phpunit/includes/api/ParseValueTest.php
index 28199bc..115be46 100644
--- a/repo/tests/phpunit/includes/api/ParseValueTest.php
+++ b/repo/tests/phpunit/includes/api/ParseValueTest.php
@@ -10,8 +10,10 @@
use ValueParsers\NullParser;
use Wikibase\Api\ApiErrorReporter;
use Wikibase\Api\ParseValue;
+use Wikibase\Repo\BuilderBasedDataTypeValidatorFactory;
use Wikibase\Repo\ValueParserFactory;
use Wikibase\Repo\WikibaseRepo;
+use Wikibase\Validators\RegexValidator;
/**
* @covers Wikibase\Api\ParseValue
@@ -40,6 +42,8 @@
$module = new ParseValue( $main, 'wbparsevalue' );
$exceptionLocalizer =
WikibaseRepo::getDefaultInstance()->getExceptionLocalizer();
+ $validatorErrorLocalizer =
WikibaseRepo::getDefaultInstance()->getValidatorErrorLocalizer();
+
$errorReporter = new ApiErrorReporter(
$module,
$exceptionLocalizer,
@@ -59,21 +63,32 @@
'globe-coordinate' =>
'DataValues\Geo\Parsers\GlobeCoordinateParser',
) );
+ $validatorFactory = new BuilderBasedDataTypeValidatorFactory(
array(
+ 'string' => array( $this, 'newArrayWithStringValidator'
),
+ 'url' => array( $this, 'newArrayWithStringValidator' ),
+ ) );
+
$module->setServices(
$dataTypeFactory,
$valueParserFactory,
+ $validatorFactory,
$exceptionLocalizer,
+ $validatorErrorLocalizer,
$errorReporter
);
return $module;
}
- public function newStringDataType( $spec, $name ) {
+ public function newArrayWithStringValidator() {
+ return array( new RegexValidator( '/INVALID/', true,
'no-kittens' ) );
+ }
+
+ public function newStringDataType( $name ) {
return new DataType( $name, 'string', array() );
}
- public function newCoordinateDataType( $spec, $name ) {
+ public function newCoordinateDataType( $name ) {
return new DataType( $name, 'globecoordinate', array() );
}
@@ -88,7 +103,7 @@
$result = $module->getResult();
$data = $result->getResultData( null, array(
- 'BC' => array(),
+ 'BC' => array( 'nobool' ),
'Types' => array(),
'Strip' => 'all',
) );
@@ -121,6 +136,45 @@
'0/raw' => 'foo',
'0/type' => 'unknown',
'0/value' => 'foo',
+ ),
+ ),
+
+ 'validation' => array(
+ array(
+ 'values' => 'VALID',
+ 'datatype' => 'string',
+ 'validate' => ''
+ ),
+ array(
+ '0/raw' => 'VALID',
+ '0/valid' => true,
+ ),
+ ),
+
+ 'bad value, validation failure' => array(
+ array(
+ 'values' => 'INVALID',
+ 'datatype' => 'string',
+ 'validate' => ''
+ ),
+ array(
+ '0/raw' => 'INVALID',
+ '0/valid' => false,
+ '0/error' => 'ValidationError',
+ '0/messages/0/name' =>
'wikibase-validator-no-kittens',
+ '0/messages/0/html/*' => '/.+/',
+ '0/validation-errors/0' => 'no-kittens',
+ ),
+ ),
+
+ 'bad value, no validation' => array(
+ array(
+ 'values' => 'INVALID',
+ 'datatype' => 'string',
+ ),
+ array(
+ '0/raw' => 'INVALID',
+ '0/type' => 'unknown',
),
),
@@ -193,9 +247,10 @@
protected function assertValueAtPath( $expected, $path, $data ) {
$name = '';
foreach ( $path as $step ) {
- $this->assertArrayHasKey( $step, $data );
- $data = $data[$step];
$name .= '/' . $step;
+ $this->assertInternalType( 'array', $data, $name );
+ $this->assertArrayHasKey( $step, $data, $name );
+ $data = $data[$step];
}
if ( is_string( $expected ) && preg_match(
'/^([^\s\w\d]).*\1[a-zA-Z]*$/', $expected ) ) {
--
To view, visit https://gerrit.wikimedia.org/r/223192
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I315a55db8dff1225cc1077cb312b6d4c7a90d282
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Daniel Kinzler <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
