Lucas Werkmeister (WMDE) has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/352865 )
Change subject: Parse Type and Value type constraints from statements
......................................................................
Parse Type and Value type constraints from statements
ConstraintReportFactory learns about two more constraint item IDs, and
maps them to the 'Type' and 'Value type' checkers.
The checkers both learn to use parameters that were serialized from
qualifiers instead of taken from a template. Since the logic for this is
entirely identical between them, it is moved to the TypeCheckerHelper.
In order to parse the parameters, TypeCheckerHelper and
ConstraintRepository get a DataValueFactory injected.
TODO: The tests have not yet been updated at all (and will therefore
fail immediately, since several classes are missing parameters).
Change-Id: Id47731c99b6e8ae10d540315fc68994cf2f0f24e
---
M extension.json
M i18n/en.json
M i18n/qqq.json
M includes/ConstraintCheck/Checker/TypeChecker.php
M includes/ConstraintCheck/Checker/ValueTypeChecker.php
M includes/ConstraintCheck/Helper/TypeCheckerHelper.php
M includes/ConstraintReportFactory.php
M includes/ConstraintRepository.php
8 files changed, 189 insertions(+), 60 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/WikibaseQualityConstraints
refs/changes/65/352865/1
diff --git a/extension.json b/extension.json
index 402e74f..9bb0bb9 100644
--- a/extension.json
+++ b/extension.json
@@ -89,6 +89,36 @@
"value": "Q21510862",
"description": "The item ID of the 'symmetric
constraint' item, which, when used in a 'property constraint' statement on a
property, indicates that a referenced entity should refer back to the original
entity.",
"public": true
+ },
+ "WBQualityConstraintsTypeConstraintId": {
+ "value": "Q21503250",
+ "description": "The item ID of the 'type constraint'
item, which, when used in a 'property constraint' statement on a property,
indicates that the subject entity should have a certain type, with the relation
and type given in the parameters.",
+ "public": true
+ },
+ "WBQualityConstraintsValueTypeConstraintId": {
+ "value": "Q21510865",
+ "description": "The item ID of the 'value type
constraint' item, which, when used in a 'property constraint' statement on a
property, indicates that the referenced entity should have a certain type, with
the class and relation given in the parameters.",
+ "public": true
+ },
+ "WBQualityConstraintsClassId": {
+ "value": "P2308",
+ "description": "The property ID of the 'relation'
property, which specifies the class/type of a 'type' or 'value type'
constraint.",
+ "public": true
+ },
+ "WBQualityConstraintsRelationId": {
+ "value": "P2309",
+ "description": "The property ID of the 'relation'
property, which specifies the relation ('instance of' or 'subclass of') of a
'type' or 'value type' constraint.",
+ "public": true
+ },
+ "WBQualityConstraintsInstanceOfRelationId": {
+ "value": "Q21503252",
+ "description": "The item ID of the 'instance of' item,
which, when used in a 'relation' qualifier of a 'property constraint' statement
on a property, indicates that the 'type' or 'value type' constraint defined in
this statement demands an 'instance' relation.",
+ "public": true
+ },
+ "WBQualityConstraintsSubclassOfRelationId": {
+ "value": "Q21514624",
+ "description": "The item ID of the 'subclass of' item,
which, when used in a 'relation' qualifier of a 'property constraint' statement
on a property, indicates that the 'type' or 'value type' constraint defined in
this statement demands a 'subclass' relation.",
+ "public": true
}
},
"manifest_version": 2
diff --git a/i18n/en.json b/i18n/en.json
index 306f224..8db00be 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -51,6 +51,10 @@
"wbqc-violation-message-parameters-needed-3": "Properties with
constraint \"$1\" need parameters \"$2\", \"$3\", and \"$4\".",
"wbqc-violation-message-target-entity-must-exist": "The target entity
must exist.",
"wbqc-violation-message-value-entity-must-exist": "The value entity
must exist.",
+ "wbqc-violation-message-parameter-novalue": "The parameter \"$1\" must
have a real value, not \"no value\".",
+ "wbqc-violation-message-parameter-entity": "The value for the parameter
\"$1\" must be an entity, not \"$2\".",
+ "wbqc-violation-message-parameter-single": "The parameter \"$1\" must
only have a single value.",
+ "wbqc-violation-message-parameter-oneof": "The parameter \"$1\" must be
one of $2.",
"wbqc-violation-message-commons-link-no-existent": "Commons link must
exist.",
"wbqc-violation-message-commons-link-not-well-formed": "Commons link
must be well-formed.",
diff --git a/i18n/qqq.json b/i18n/qqq.json
index 4165c92..f30dd05 100644
--- a/i18n/qqq.json
+++ b/i18n/qqq.json
@@ -46,6 +46,10 @@
"wbqc-violation-message-parameters-needed-3": "Message for when a
constraint needs three specific parameters, but some of them are missing.",
"wbqc-violation-message-target-entity-must-exist": "Message for when an
entity is referenced, but it doesn't exist (any more).",
"wbqc-violation-message-value-entity-must-exist": "Message for when the
property has an entity as its value, but it doesn't exist (any more).",
+ "wbqc-violation-message-parameter-novalue": "Message for when \"no
value\" has been entered as the value of a parameter that must be an actual
value. $1 contains the parameter.",
+ "wbqc-violation-message-parameter-entity": "Message for when the value
of a parameter must be an entity, but is some other kind of data value. $1
contains the parameter, $2 the data value type.",
+ "wbqc-violation-message-parameter-single": "Message for when a
parameter has multiple values but only supports one. $1 contains the
parameter.",
+ "wbqc-violation-message-parameter-oneof": "Message for when a parameter
must be one of several values, but is something different. $1 contains the
parameter, $2 a comma-separated list of allowed values.",
"wbqc-violation-message-commons-link-no-existent": "Message for
violation of Commons link constraint. When linked commons page does not exist.",
"wbqc-violation-message-commons-link-not-well-formed": "Message for
violation of Commons link constraint. When link contains invalid characters.",
"wbqc-violation-message-commons-link-check-for-namespace-not-yet-implemented":
"Message for when the check for the Commons link constraint has not yet been
implemented for a specific namespace.",
diff --git a/includes/ConstraintCheck/Checker/TypeChecker.php
b/includes/ConstraintCheck/Checker/TypeChecker.php
index 425c061..55972d5 100644
--- a/includes/ConstraintCheck/Checker/TypeChecker.php
+++ b/includes/ConstraintCheck/Checker/TypeChecker.php
@@ -4,6 +4,7 @@
use Config;
use Wikibase\DataModel\Entity\EntityDocument;
+use Wikibase\DataModel\Entity\EntityIdValue;
use Wikibase\DataModel\Services\Lookup\EntityLookup;
use Wikibase\DataModel\Statement\StatementListProvider;
use WikibaseQuality\ConstraintReport\Constraint;
@@ -46,7 +47,12 @@
* @param TypeCheckerHelper $typeCheckerHelper
* @param Config $config
*/
- public function __construct( EntityLookup $lookup,
ConstraintParameterParser $helper, TypeCheckerHelper $typeCheckerHelper, Config
$config ) {
+ public function __construct(
+ EntityLookup $lookup,
+ ConstraintParameterParser $helper,
+ TypeCheckerHelper $typeCheckerHelper,
+ Config $config
+ ) {
$this->entityLookup = $lookup;
$this->helper = $helper;
$this->typeCheckerHelper = $typeCheckerHelper;
@@ -66,39 +72,22 @@
$parameters = [];
$constraintParameters = $constraint->getConstraintParameters();
- $classes = false;
- if ( array_key_exists( 'class', $constraintParameters ) ) {
- $classes = explode( ',', $constraintParameters['class']
);
+ $classes = $this->typeCheckerHelper->parseClassParameter(
$constraintParameters, $constraint->getConstraintTypeName() );
+ if ( is_array( $classes ) ) {
$parameters['class'] =
$this->helper->parseParameterArray( $classes );
+ } else {
+ return new CheckResult( $entity->getId(), $statement,
$constraint->getConstraintTypeQid(), $constraint->getConstraintId(),
$parameters, CheckResult::STATUS_VIOLATION, $classes );
}
- $relation = false;
- if ( array_key_exists( 'relation', $constraintParameters ) ) {
- $relation = $constraintParameters['relation'];
- $parameters['relation'] =
$this->helper->parseSingleParameter( $relation, true );
- }
-
- /*
- * error handling:
- * parameter $constraintParameters['class'] must not be null
- */
- if ( !$classes ) {
- $message = wfMessage(
"wbqc-violation-message-parameter-needed" )->params(
$constraint->getConstraintTypeName(), 'class' )->escaped();
- return new CheckResult( $entity->getId(), $statement,
$constraint->getConstraintTypeQid(), $constraint->getConstraintId(),
$parameters, CheckResult::STATUS_VIOLATION, $message );
- }
-
- /*
- * error handling:
- * parameter $constraintParameters['relation'] must be either
'instance' or 'subclass'
- */
- if ( $relation === 'instance' ) {
+ $relation = $this->typeCheckerHelper->parseRelationParameter(
$constraintParameters, $constraint->getConstraintTypeName() );
+ if ( $relation == 'instance' ) {
$relationId = $this->config->get(
'WBQualityConstraintsInstanceOfId' );
- } elseif ( $relation === 'subclass' ) {
+ } elseif ( $relation == 'subclass' ) {
$relationId = $this->config->get(
'WBQualityConstraintsSubclassOfId' );
} else {
- $message = wfMessage(
"wbqc-violation-message-type-relation-instance-or-subclass" )->escaped();
- return new CheckResult( $entity->getId(), $statement,
$constraint->getConstraintTypeQid(), $constraint->getConstraintId(),
$parameters, CheckResult::STATUS_VIOLATION, $message );
+ return new CheckResult( $entity->getId(), $statement,
$constraint->getConstraintTypeQid(), $constraint->getConstraintId(),
$parameters, CheckResult::STATUS_VIOLATION, $relation );
}
+ $parameters['relation'] = $this->helper->parseSingleParameter(
$relation, true );
if ( $this->typeCheckerHelper->hasClassInRelation(
$entity->getStatements(), $relationId, $classes ) ) {
$message = '';
diff --git a/includes/ConstraintCheck/Checker/ValueTypeChecker.php
b/includes/ConstraintCheck/Checker/ValueTypeChecker.php
index 4430d24..6be125f 100644
--- a/includes/ConstraintCheck/Checker/ValueTypeChecker.php
+++ b/includes/ConstraintCheck/Checker/ValueTypeChecker.php
@@ -68,17 +68,22 @@
$parameters = [];
$constraintParameters = $constraint->getConstraintParameters();
- $classes = false;
- if ( array_key_exists( 'class', $constraintParameters ) ) {
- $classes = explode( ',', $constraintParameters['class']
);
+ $classes = $this->typeCheckerHelper->parseClassParameter(
$constraintParameters, $constraint->getConstraintTypeName() );
+ if ( is_array( $classes ) ) {
$parameters['class'] =
$this->helper->parseParameterArray( $classes );
+ } else {
+ return new CheckResult( $entity->getId(), $statement,
$constraint->getConstraintTypeQid(), $constraint->getConstraintId(),
$parameters, CheckResult::STATUS_VIOLATION, $classes );
}
- $relation = false;
- if ( array_key_exists( 'relation', $constraintParameters ) ) {
- $relation = $constraintParameters['relation'];
- $parameters['relation'] =
$this->helper->parseSingleParameter( $relation, true );
+ $relation = $this->typeCheckerHelper->parseRelationParameter(
$constraintParameters, $constraint->getConstraintTypeName() );
+ if ( $relation == 'instance' ) {
+ $relationId = $this->config->get(
'WBQualityConstraintsInstanceOfId' );
+ } elseif ( $relation == 'subclass' ) {
+ $relationId = $this->config->get(
'WBQualityConstraintsSubclassOfId' );
+ } else {
+ return new CheckResult( $entity->getId(), $statement,
$constraint->getConstraintTypeQid(), $constraint->getConstraintId(),
$parameters, CheckResult::STATUS_VIOLATION, $relation );
}
+ $parameters['relation'] = $this->helper->parseSingleParameter(
$relation, true );
if ( array_key_exists( 'constraint_status',
$constraintParameters ) ) {
$parameters['constraint_status'] =
$this->helper->parseSingleParameter(
$constraintParameters['constraint_status'], true );
@@ -100,31 +105,12 @@
/*
* error handling:
* type of $dataValue for properties with 'Value type'
constraint has to be 'wikibase-entityid'
- * parameter $constraintParameters['class'] must not be null
*/
if ( $dataValue->getType() !== 'wikibase-entityid' ) {
$message = wfMessage(
"wbqc-violation-message-value-needed-of-type" )->params(
$constraint->getConstraintTypeQid(), 'wikibase-entityid' )->escaped();
return new CheckResult( $entity->getId(), $statement,
$constraint->getConstraintTypeQid(), $constraint->getConstraintId(),
$parameters, CheckResult::STATUS_VIOLATION, $message );
}
/** @var EntityIdValue $dataValue */
-
- if ( !$classes ) {
- $message = wfMessage(
"wbqc-violation-message-parameter-needed" )->params(
$constraint->getConstraintTypeQid(), 'class' )->escaped();
- return new CheckResult( $entity->getId(), $statement,
$constraint->getConstraintTypeQid(), $constraint->getConstraintId(),
$parameters, CheckResult::STATUS_VIOLATION, $message );
- }
-
- /*
- * error handling:
- * parameter $constraintParameters['relation'] must be either
'instance' or 'subclass'
- */
- if ( $relation === 'instance' ) {
- $relationId = $this->config->get(
'WBQualityConstraintsInstanceOfId' );
- } elseif ( $relation === 'subclass' ) {
- $relationId = $this->config->get(
'WBQualityConstraintsSubclassOfId' );
- } else {
- $message = wfMessage(
"wbqc-violation-message-type-relation-instance-or-subclass" )->escaped();
- return new CheckResult( $entity->getId(), $statement,
$constraint->getConstraintTypeQid(), $constraint->getConstraintId(),
$parameters, CheckResult::STATUS_VIOLATION, $message );
- }
$item = $this->entityLookup->getEntity(
$dataValue->getEntityId() );
diff --git a/includes/ConstraintCheck/Helper/TypeCheckerHelper.php
b/includes/ConstraintCheck/Helper/TypeCheckerHelper.php
index 72a4566..5d06d5f 100644
--- a/includes/ConstraintCheck/Helper/TypeCheckerHelper.php
+++ b/includes/ConstraintCheck/Helper/TypeCheckerHelper.php
@@ -3,6 +3,7 @@
namespace WikibaseQuality\ConstraintReport\ConstraintCheck\Helper;
use Config;
+use DataValues\DataValueFactory;
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Entity\EntityIdValue;
use Wikibase\DataModel\Entity\PropertyId;
@@ -34,9 +35,24 @@
*/
private $config;
- public function __construct( EntityLookup $lookup, Config $config ) {
+ /**
+ * @var DataValueFactory
+ */
+ private $factory;
+
+ /**
+ * @param EntityLookup $lookup
+ * @param Config $config
+ * @param DataValueFactory $factory
+ */
+ public function __construct(
+ EntityLookup $lookup,
+ Config $config,
+ DataValueFactory $factory
+ ) {
$this->entityLookup = $lookup;
$this->config = $config;
+ $this->factory = $factory;
}
/**
@@ -130,4 +146,88 @@
&& $mainSnak->getDataValue()->getType() ===
'wikibase-entityid';
}
+ /**
+ * @param array $constraintParameters
+ * @param string $constraintTypeName
+ * @return array|string class entity ID serializations, or error message
+ */
+ public function parseClassParameter( array $constraintParameters,
$constraintTypeName ) {
+ $classId = $this->config->get( 'WBQualityConstraintsClassId' );
+ if ( array_key_exists( $classId, $constraintParameters ) ) {
+ if ( in_array( null, $constraintParameters[$classId],
true ) ) {
+ // class: novalue in parameters (invalid)
+ $message = wfMessage(
"wbqc-violation-message-parameter-novalue" )
+ ->params( "class ($classId)" )
+ ->escaped();
+ return $message;
+ }
+ $classes = [];
+ foreach ( $constraintParameters[$classId] as $class ) {
+ $value = $this->factory->newFromArray( $class );
+ if ( $value instanceof EntityIdValue ) {
+ $classes[] =
$value->getEntityId()->getSerialization();
+ } else {
+ return wfMessage(
"wbqc-violation-message-parameter-entity" )
+ ->params( "class ($classId)",
$value->getType() )
+ ->escaped();
+ }
+ }
+ return $classes;
+ } elseif ( array_key_exists( 'class', $constraintParameters ) )
{
+ return explode( ',', $constraintParameters['class'] );
+ } else {
+ return wfMessage(
"wbqc-violation-message-parameter-needed" )
+ ->params( $constraintTypeName, 'class' )
+ ->escaped();
+ }
+ }
+
+ /**
+ * @param array $constraintParameters
+ * @param string $constraintTypeName
+ * @return string 'instance', 'subclass', or error message
+ */
+ public function parseRelationParameter( array $constraintParameters,
$constraintTypeName ) {
+ $relationId = $this->config->get(
'WBQualityConstraintsRelationId' );
+ if ( array_key_exists( $relationId, $constraintParameters ) ) {
+ if ( count( $constraintParameters[$relationId] ) !== 1
) {
+ return wfMessage(
"wbqc-violation-message-parameter-single" )->escaped();
+ }
+ $value = $this->factory->newFromArray(
$constraintParameters[$relationId][0] );
+ if ( $value instanceof EntityIdValue ) {
+ $relationEntityId =
$value->getEntityId()->getSerialization();
+ } else {
+ return wfMessage(
"wbqc-violation-message-parameter-entity" )
+ ->params( "relation ($relationId)",
$value->getType() )
+ ->escaped();
+ }
+ $instanceId = $this->config->get(
'WBQualityConstraintsInstanceOfRelationId' );
+ $subclassId = $this->config->get(
'WBQualityConstraintsSubclassOfRelationId' );
+ switch ( $relationEntityId ) {
+ case $instanceId:
+ return 'instance';
+ case $subclassId:
+ return 'subclass';
+ default:
+ return wfMessage(
"wbqc-violation-message-parameter-oneof" )
+ ->params(
+ "relation
($relationId)",
+ Message::listParam( [
$instanceId, $subclassId ], 'comma' )
+ )->escaped();
+ }
+ } elseif ( array_key_exists( 'relation', $constraintParameters
) ) {
+ $relation = $constraintParameters['relation'];
+ if ( $relation === 'instance' || $relation ===
'subclass' ) {
+ return $relation;
+ } else {
+ return wfMessage(
"wbqc-violation-message-type-relation-instance-or-subclass" )
+ ->escaped();
+ }
+ } else {
+ return wfMessage(
"wbqc-violation-message-parameter-needed" )
+ ->params( $constraintTypeName, 'relation' )
+ ->escaped();
+ }
+ }
+
}
diff --git a/includes/ConstraintReportFactory.php
b/includes/ConstraintReportFactory.php
index 9993332..fdbe1e3 100644
--- a/includes/ConstraintReportFactory.php
+++ b/includes/ConstraintReportFactory.php
@@ -3,6 +3,7 @@
namespace WikibaseQuality\ConstraintReport;
use Config;
+use DataValues\DataValueFactory;
use MediaWiki\MediaWikiServices;
use Wikibase\DataModel\Services\Lookup\EntityLookup;
use Wikibase\Repo\WikibaseRepo;
@@ -70,6 +71,11 @@
private $config;
/**
+ * @var DataValueFactory
+ */
+ private $factory;
+
+ /**
* Returns the default instance.
* IMPORTANT: Use only when it is not feasible to inject an instance
properly.
*
@@ -79,20 +85,28 @@
static $instance = null;
if ( $instance === null ) {
+ $repo = WikibaseRepo::getDefaultInstance();
$instance = new self(
-
WikibaseRepo::getDefaultInstance()->getEntityLookup(),
-
WikibaseRepo::getDefaultInstance()->getStatementGuidParser(),
-
MediaWikiServices::getInstance()->getMainConfig()
+ $repo->getEntityLookup(),
+ $repo->getStatementGuidParser(),
+
MediaWikiServices::getInstance()->getMainConfig(),
+ $repo->getDataValueFactory()
);
}
return $instance;
}
- public function __construct( EntityLookup $lookup, StatementGuidParser
$statementGuidParser, Config $config ) {
+ public function __construct(
+ EntityLookup $lookup,
+ StatementGuidParser $statementGuidParser,
+ Config $config,
+ DataValueFactory $factory
+ ) {
$this->lookup = $lookup;
$this->statementGuidParser = $statementGuidParser;
$this->config = $config;
+ $this->factory = $factory;
}
/**
@@ -118,7 +132,7 @@
$constraintParameterParser = new
ConstraintParameterParser();
$connectionCheckerHelper = new
ConnectionCheckerHelper();
$rangeCheckerHelper = new RangeCheckerHelper();
- $typeCheckerHelper = new TypeCheckerHelper(
$this->lookup, $this->config );
+ $typeCheckerHelper = new TypeCheckerHelper(
$this->lookup, $this->config, $this->factory );
$this->constraintCheckerMap = [
'Conflicts with' => new ConflictsWithChecker(
$this->lookup, $constraintParameterParser, $connectionCheckerHelper ),
@@ -146,6 +160,8 @@
$this->config->get(
'WBQualityConstraintsUsedAsQualifierConstraintId' ) =>
$this->constraintCheckerMap['Qualifier'],
$this->config->get(
'WBQualityConstraintsSingleValueConstraintId' ) =>
$this->constraintCheckerMap['Single value'],
$this->config->get(
'WBQualityConstraintsSymmetricConstraintId' ) =>
$this->constraintCheckerMap['Symmetric'],
+ $this->config->get(
'WBQualityConstraintsTypeConstraintId' ) => $this->constraintCheckerMap['Type'],
+ $this->config->get(
'WBQualityConstraintsValueTypeConstraintId' ) =>
$this->constraintCheckerMap['Value type']
];
}
diff --git a/includes/ConstraintRepository.php
b/includes/ConstraintRepository.php
index 6b47c70..1831790 100644
--- a/includes/ConstraintRepository.php
+++ b/includes/ConstraintRepository.php
@@ -135,7 +135,7 @@
$constraints = [];
foreach ( $results as $result ) {
$constraintTypeQid = $result->constraint_type_qid;
- $constraintParameters = (array) json_decode(
$result->constraint_parameters );
+ $constraintParameters = (array) json_decode(
$result->constraint_parameters, true );
$constraints[] = new Constraint(
$result->constraint_guid,
--
To view, visit https://gerrit.wikimedia.org/r/352865
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id47731c99b6e8ae10d540315fc68994cf2f0f24e
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/WikibaseQualityConstraints
Gerrit-Branch: master
Gerrit-Owner: Lucas Werkmeister (WMDE) <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
