jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/404711 )
Change subject: Add support for “instance or subclass of” relation
......................................................................
Add support for “instance or subclass of” relation
The “instance or subclass of” relation is the union of the “instance of”
and “subclass of” relations: the link from the item to the class can
either be via the “instance of” property or via the “subclass of”
property. After that, as usual, only “subclass of” is processed: the
relation does *not* mean that “instance of” and “subclass of” can be
mixed in the whole hierarchy – only on the first link!
TypeCheckerHelper::hasClassInRelation is changed to accept a list of
relation property IDs instead of a single one, and gains a new helper
function to extract statements of several property IDs from a
StatementList (since StatementList doesn’t have such a function, as far
as I can tell). The two checkers translate the new relation type to a
relation property ID list of both the “instance of” and the “subclass
of” property ID.
Bug: T169858
Change-Id: Iedfe3013a10de0af7f12ea0f6fde7164dbe4f062
---
M i18n/en.json
M i18n/qqq.json
M src/ConstraintCheck/Checker/TypeChecker.php
M src/ConstraintCheck/Checker/ValueTypeChecker.php
M src/ConstraintCheck/Helper/TypeCheckerHelper.php
M tests/phpunit/Checker/TypeChecker/TypeCheckerHelperTest.php
M tests/phpunit/Checker/TypeChecker/TypeCheckerTest.php
M tests/phpunit/Checker/TypeChecker/ValueTypeCheckerTest.php
8 files changed, 160 insertions(+), 19 deletions(-)
Approvals:
WMDE-leszek: Checked; Looks good to me, approved
jenkins-bot: Verified
diff --git a/i18n/en.json b/i18n/en.json
index 2327d3e..090bdac 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -136,8 +136,10 @@
"wbqc-violation-message-symmetric": "$1 should also have the symmetric
statement $2 $3.",
"wbqc-violation-message-type-instance": "Entities using the $1 property
should be instances of {{PLURAL:$3|1=$5|2=$5 or $6|one of the following
classes}} (or of {{PLURAL:$3|1=a subclass of it|2=a subclass of them|one of
their subclasses}}), but $2 currently {{PLURAL:$3|1=isn't.|2=isn't.|isn't:
$4}}",
"wbqc-violation-message-type-subclass": "Entities using the $1 property
should be subclasses of {{PLURAL:$3|1=$5|2=$5 or $6|one of the following
classes}} (or of {{PLURAL:$3|1=a subclass of it|2=a subclass of them|one of
their subclasses}}), but $2 currently {{PLURAL:$3|1=isn't.|2=isn't.|isn't:
$4}}",
+ "wbqc-violation-message-type-instanceOrSubclass": "Entities using the
$1 property should be instances or subclasses of {{PLURAL:$3|1=$5|2=$5 or
$6|one of the following classes}} (or of {{PLURAL:$3|1=a subclass of it|2=a
subclass of them|one of their subclasses}}), but $2 currently
{{PLURAL:$3|1=isn't.|2=isn't.|isn't: $4}}",
"wbqc-violation-message-valueType-instance": "Values of $1 statements
should be instances of {{PLURAL:$3|1=$5|2=$5 or $6|one of the following
classes}} (or of {{PLURAL:$3|1=a subclass of it|2=a subclass of them|one of
their subclasses}}), but $2 currently {{PLURAL:$3|1=isn't.|2=isn't.|isn't:
$4}}",
"wbqc-violation-message-valueType-subclass": "Values of $1 statements
should be subclasses of {{PLURAL:$3|1=$5|2=$5 or $6|one of the following
classes}} (or of {{PLURAL:$3|1=a subclass of it|2=a subclass of them|one of
their subclasses}}), but $2 currently {{PLURAL:$3|1=isn't.|2=isn't.|isn't:
$4}}",
+ "wbqc-violation-message-valueType-instanceOrSubclass": "Values of $1
statements should be instances or subclasses of {{PLURAL:$3|1=$5|2=$5 or $6|one
of the following classes}} (or of {{PLURAL:$3|1=a subclass of it|2=a subclass
of them|one of their subclasses}}), but $2 currently
{{PLURAL:$3|1=isn't.|2=isn't.|isn't: $4}}",
"wbqc-violation-message-target-required-claim": "$1 should have
{{PLURAL:$3|0=a statement $2.|1=a statement $2 $5.|a statement for $2 with one
of the following values:$4}}",
"wbqc-violation-message-unique-value": "This property's value must not
be present on any other item, but is also present on {{PLURAL:$1|1=$3.|2=$3 and
$4.|the following items: $2}}",
"wbqc-violation-message-valueOnly": "This property should only be used
for the main value of a statement, not for qualifiers or references.",
diff --git a/i18n/qqq.json b/i18n/qqq.json
index 0fa388a..283dc8d 100644
--- a/i18n/qqq.json
+++ b/i18n/qqq.json
@@ -134,8 +134,10 @@
"wbqc-violation-message-symmetric": "Message for a violation of the
“Symmetric” constraint, when the symmetric statement of a statement does not
exist. $1, $2 and $3 contain the expected subject entity, property, and target
entity of the missing symmetric statement.",
"wbqc-violation-message-type-instance": "Message for a violation of the
“Type” constraint, when the subject of a statement should be an instance of a
certain type but isn't. $1 is the property of the statement, $2 is the subject
of the statement, $3 is the number of classes, $4 is an HTML list of all
classes, and $5, $6 etc. are the individual
classes.\n{{Related|wbqc-violation-message-type-subclass}}",
"wbqc-violation-message-type-subclass": "Message for a violation of the
“Type” constraint, when the subject of a statement should be a subclass of a
certain type but isn't. $1 is the property of the statement, $2 is the subject
of the statement, $3 is the number of classes, $4 is an HTML list of all
classes, and $5, $6 etc. are the individual
classes.\n{{Related|wbqc-violation-message-type-instance}}",
+ "wbqc-violation-message-type-instanceOrSubclass": "Message for a
violation of the “Type” constraint, when the subject of a statement should be
an instance or a subclass of a certain type but isn't. $1 is the property of
the statement, $2 is the subject of the statement, $3 is the number of classes,
$4 is an HTML list of all classes, and $5, $6 etc. are the individual
classes.\n{{Related|wbqc-violation-message-type-instance}}",
"wbqc-violation-message-valueType-instance": "Message for a violation
of the “Value type” constraint, when the value of a statement should be an
instance of a certain type but isn't. $1 is the property of the statement, $2
is the value of the statement, $3 is the number of classes, $4 is an HTML list
of all classes, and $5, $6 etc. are the individual
classes.\n{{Related|wbqc-violation-message-valueType-subclass}}",
"wbqc-violation-message-valueType-subclass": "Message for a violation
of the “Value type” constraint, when the value of a statement should be a
subclass of a certain type but isn't. $1 is the property of the statement, $2
is the value of the statement, $3 is the number of classes, $4 is an HTML list
of all classes, and $5, $6 etc. are the individual
classes.\n{{Related|wbqc-violation-message-valueType-instance}}",
+ "wbqc-violation-message-valueType-instanceOrSubclass": "Message for a
violation of the “Value type” constraint, when the value of a statement should
be an instance or a subclass of a certain type but isn't. $1 is the property of
the statement, $2 is the value of the statement, $3 is the number of classes,
$4 is an HTML list of all classes, and $5, $6 etc. are the individual
classes.\n{{Related|wbqc-violation-message-valueType-instance}}",
"wbqc-violation-message-target-required-claim": "Message for a
violation of the “Target required claim” constraint, when the target entity of
a statement is missing an expected statement. Parameters:\n* $1 is the subject
entity of the missing statement, i. e. the target entity of the statement that
has the constraint.\n* $2 is the property of the missing statement.\n* $3 is
the number of values permitted for the missing statement (or 0, in which case
the constraint only specifies that there should be a statement but not the
values it should have).\n* $4 is an HTML list of all values permitted for the
missing statement.\n* $5, $6 etc. are the individual values permitted for the
missing statement.\n{{Related|wbqc-violation-message-item}}",
"wbqc-violation-message-unique-value": "Message for violation of the
Unique Value constraint, when other items are found. Parameters:\n* $1 is the
number of other items with the same value.\n* $2 is an HTML list of all other
items found with the same value.\n* $3, $4 etc. are the individual other items
with the same value.",
"wbqc-violation-message-valueOnly": "Message for a violation of the
“used for values only” constraint, when a property intended for the main value
only was used in a qualifier or reference.",
diff --git a/src/ConstraintCheck/Checker/TypeChecker.php
b/src/ConstraintCheck/Checker/TypeChecker.php
index 3ce5359..d405edf 100644
--- a/src/ConstraintCheck/Checker/TypeChecker.php
+++ b/src/ConstraintCheck/Checker/TypeChecker.php
@@ -109,16 +109,18 @@
);
$relation =
$this->constraintParameterParser->parseRelationParameter(
$constraintParameters, $constraint->getConstraintTypeItemId() );
- if ( $relation === 'instance' ) {
- $relationId = $this->config->get(
'WBQualityConstraintsInstanceOfId' );
- } elseif ( $relation === 'subclass' ) {
- $relationId = $this->config->get(
'WBQualityConstraintsSubclassOfId' );
+ $relationIds = [];
+ if ( $relation === 'instance' || $relation ===
'instanceOrSubclass' ) {
+ $relationIds[] = $this->config->get(
'WBQualityConstraintsInstanceOfId' );
+ }
+ if ( $relation === 'subclass' || $relation ===
'instanceOrSubclass' ) {
+ $relationIds[] = $this->config->get(
'WBQualityConstraintsSubclassOfId' );
}
$parameters['relation'] = [ $relation ];
$result = $this->typeCheckerHelper->hasClassInRelation(
$context->getEntity()->getStatements(),
- $relationId,
+ $relationIds,
$classes
);
diff --git a/src/ConstraintCheck/Checker/ValueTypeChecker.php
b/src/ConstraintCheck/Checker/ValueTypeChecker.php
index 91a34c0..5f0ad93 100644
--- a/src/ConstraintCheck/Checker/ValueTypeChecker.php
+++ b/src/ConstraintCheck/Checker/ValueTypeChecker.php
@@ -121,10 +121,12 @@
);
$relation =
$this->constraintParameterParser->parseRelationParameter(
$constraintParameters, $constraint->getConstraintTypeItemId() );
- if ( $relation === 'instance' ) {
- $relationId = $this->config->get(
'WBQualityConstraintsInstanceOfId' );
- } elseif ( $relation === 'subclass' ) {
- $relationId = $this->config->get(
'WBQualityConstraintsSubclassOfId' );
+ $relationIds = [];
+ if ( $relation === 'instance' || $relation ===
'instanceOrSubclass' ) {
+ $relationIds[] = $this->config->get(
'WBQualityConstraintsInstanceOfId' );
+ }
+ if ( $relation === 'subclass' || $relation ===
'instanceOrSubclass' ) {
+ $relationIds[] = $this->config->get(
'WBQualityConstraintsSubclassOfId' );
}
$parameters['relation'] = [ $relation ];
@@ -163,7 +165,7 @@
$result = $this->typeCheckerHelper->hasClassInRelation(
$statements,
- $relationId,
+ $relationIds,
$classes
);
diff --git a/src/ConstraintCheck/Helper/TypeCheckerHelper.php
b/src/ConstraintCheck/Helper/TypeCheckerHelper.php
index 2c2487b..7225bb1 100644
--- a/src/ConstraintCheck/Helper/TypeCheckerHelper.php
+++ b/src/ConstraintCheck/Helper/TypeCheckerHelper.php
@@ -162,21 +162,21 @@
/**
* Checks, if one of the itemId serializations in $classesToCheck
* is contained in the list of $statements
- * via property $relationId or if it is a subclass of
- * one of the items referenced in $statements via $relationId
+ * via properties $relationIds or if it is a subclass of
+ * one of the items referenced in $statements via $relationIds
*
* @param StatementList $statements
- * @param string $relationId
+ * @param string[] $relationIds
* @param string[] $classesToCheck
*
* @return CachedBool
* @throws SparqlHelperException if SPARQL is used and the query times
out or some other error occurs
*/
- public function hasClassInRelation( StatementList $statements,
$relationId, array $classesToCheck ) {
+ public function hasClassInRelation( StatementList $statements, array
$relationIds, array $classesToCheck ) {
$metadatas = [];
/** @var Statement $statement */
- foreach ( $statements->getByPropertyId( new PropertyId(
$relationId ) ) as $statement ) {
+ foreach ( $this->getStatementsByPropertyIds( $statements,
$relationIds ) as $statement ) {
$mainSnak = $statement->getMainSnak();
if ( !$this->hasCorrectType( $mainSnak ) ) {
@@ -215,11 +215,30 @@
}
/**
+ * @param StatementList $statements
+ * @param string[] $propertyIdSerializations
+ * @return StatementList
+ */
+ private function getStatementsByPropertyIds(
+ StatementList $statements,
+ array $propertyIdSerializations
+ ) {
+ $statementArrays = [];
+ foreach ( $propertyIdSerializations as $propertyIdSerialization
) {
+ $propertyId = new PropertyId( $propertyIdSerialization
);
+ $statementArrays[] = $statements->getByPropertyId(
$propertyId )->toArray();
+ }
+ return new StatementList(
+ call_user_func_array( 'array_merge', $statementArrays )
+ );
+ }
+
+ /**
* @param PropertyId $propertyId ID of the property that introduced the
constraint
* @param EntityId $entityId ID of the entity that does not have the
required type
* @param string[] $classes item ID serializations of the classes that
$entityId should have
* @param string $checker "type" or "valueType" (for message key)
- * @param string $relation "instance" or "subclass" (for message key)
+ * @param string $relation "instance", "subclass", or
"instanceOrSubclass" (for message key)
*
* @return string Localized HTML message
*/
@@ -227,8 +246,10 @@
// Possible messages:
// wbqc-violation-message-type-instance
// wbqc-violation-message-type-subclass
+ // wbqc-violation-message-type-instanceOrSubclass
// wbqc-violation-message-valueType-instance
// wbqc-violation-message-valueType-subclass
+ // wbqc-violation-message-valueType-instanceOrSubclass
$message = wfMessage( 'wbqc-violation-message-' . $checker .
'-' . $relation );
$message->rawParams(
diff --git a/tests/phpunit/Checker/TypeChecker/TypeCheckerHelperTest.php
b/tests/phpunit/Checker/TypeChecker/TypeCheckerHelperTest.php
index 79df966..23360f2 100644
--- a/tests/phpunit/Checker/TypeChecker/TypeCheckerHelperTest.php
+++ b/tests/phpunit/Checker/TypeChecker/TypeCheckerHelperTest.php
@@ -97,21 +97,85 @@
$statement1 = new Statement( new PropertyValueSnak( new
PropertyId( 'P1' ), new EntityIdValue( new ItemId( 'Q42' ) ) ) );
$statement2 = new Statement( new PropertyValueSnak( new
PropertyId( 'P31' ), new EntityIdValue( new ItemId( 'Q1' ) ) ) );
$statements = new StatementList( [ $statement1, $statement2 ] );
- $this->assertTrue( $this->getHelper()->hasClassInRelation(
$statements, 'P31', [ 'Q1' ] )->getBool() );
+ $this->assertTrue( $this->getHelper()->hasClassInRelation(
$statements, [ 'P31' ], [ 'Q1' ] )->getBool() );
}
public function testHasClassInRelation_Invalid() {
$statement1 = new Statement( new PropertyValueSnak( new
PropertyId( 'P1' ), new EntityIdValue( new ItemId( 'Q42' ) ) ) );
$statement2 = new Statement( new PropertyValueSnak( new
PropertyId( 'P31' ), new EntityIdValue( new ItemId( 'Q100' ) ) ) );
$statements = new StatementList( [ $statement1, $statement2 ] );
- $this->assertFalse( $this->getHelper()->hasClassInRelation(
$statements, 'P31', [ 'Q1' ] )->getBool() );
+ $this->assertFalse( $this->getHelper()->hasClassInRelation(
$statements, [ 'P31' ], [ 'Q1' ] )->getBool() );
}
public function testHasClassInRelation_ValidWithIndirection() {
$statement1 = new Statement( new PropertyValueSnak( new
PropertyId( 'P1' ), new EntityIdValue( new ItemId( 'Q42' ) ) ) );
$statement2 = new Statement( new PropertyValueSnak( new
PropertyId( 'P31' ), new EntityIdValue( new ItemId( 'Q5' ) ) ) );
$statements = new StatementList( [ $statement1, $statement2 ] );
- $this->assertTrue( $this->getHelper()->hasClassInRelation(
$statements, 'P31', [ 'Q4' ] )->getBool() );
+ $this->assertTrue( $this->getHelper()->hasClassInRelation(
$statements, [ 'P31' ], [ 'Q4' ] )->getBool() );
+ }
+
+ /**
+ * Test the “instance or subclass of” relation.
+ * The statement list being tested links to Q1 with $firstRelation.
+ * If $secondRelation is not null, then Q1 links to Q2 with
$secondRelation.
+ *
+ * @param string $firstRelation 'instance' or 'subclass'
+ * @param string|null $secondRelation 'instance' or 'subclass'
+ * @param string $class item ID serialization
+ * @param bool $expected
+ * @dataProvider getInstanceOrSubclassOfProvider
+ */
+ public function testHasClassInRelation_InstanceOrSubclassOf(
+ $firstRelation,
+ $secondRelation,
+ $class,
+ $expected
+ ) {
+ $instanceOfId = $this->getDefaultConfig()->get(
'WBQualityConstraintsInstanceOfId' );
+ $subclassOfId = $this->getDefaultConfig()->get(
'WBQualityConstraintsSubclassOfId' );
+ $relationIds = [ $instanceOfId, $subclassOfId ];
+
+ $statements = new StatementList(
+ NewStatement::forProperty(
+ $firstRelation === 'instance' ?
+ $instanceOfId :
+ $subclassOfId
+ )
+ ->withValue( new ItemId( 'Q1' ) )
+ ->build()
+ );
+ $lookup = new InMemoryEntityLookup();
+
+ if ( $secondRelation !== null ) {
+ $q1 = NewItem::withId( 'Q1' )
+ ->andStatement(
+ NewStatement::forProperty(
+ $secondRelation === 'instance' ?
+ $instanceOfId :
+ $subclassOfId
+ )
+ ->withValue( new ItemId( 'Q2' )
)
+ )
+ ->build();
+ $lookup->addEntity( $q1 );
+ }
+ $helper = $this->getHelper( $lookup );
+
+ $result = $helper->hasClassInRelation( $statements,
$relationIds, [ $class ] );
+
+ $this->assertSame( $expected, $result->getBool() );
+ }
+
+ public function getInstanceOrSubclassOfProvider() {
+ return [
+ 'direct instance' => [ 'instance', null, 'Q1', true ],
+ 'direct subclass' => [ 'subclass', null, 'Q1', true ],
+ 'direct instance of unrelated' => [ 'instance', null,
'Q100', false ],
+ 'instance of subclass' => [ 'instance', 'subclass',
'Q2', true ],
+ 'subclass of subclass' => [ 'subclass', 'subclass',
'Q2', true ],
+ 'subclass of instance' => [ 'subclass', 'instance',
'Q2', false ],
+ 'instance of subclass of unrelated' => [ 'instance',
'subclass', 'Q200', false ],
+ ];
}
public function testIsSubclassOf_ValidWithIndirection() {
diff --git a/tests/phpunit/Checker/TypeChecker/TypeCheckerTest.php
b/tests/phpunit/Checker/TypeChecker/TypeCheckerTest.php
index 3e269fc..093fd77 100644
--- a/tests/phpunit/Checker/TypeChecker/TypeCheckerTest.php
+++ b/tests/phpunit/Checker/TypeChecker/TypeCheckerTest.php
@@ -139,6 +139,30 @@
$this->assertCompliance( $checkResult );
}
+ public function testTypeConstraintInstanceOrSubclassValidViaInstance() {
+ $entity = $this->lookup->getEntity( new ItemId( 'Q1' ) );
+ $constraintParameters = array_merge(
+ $this->relationParameter( 'instanceOrSubclass' ),
+ $this->classParameter( [ 'Q100', 'Q101' ] )
+ );
+ $constraint = $this->getConstraintMock( $constraintParameters );
+
+ $checkResult = $this->checker->checkConstraint( new
FakeSnakContext( $this->typeSnak, $entity ), $constraint );
+ $this->assertCompliance( $checkResult );
+ }
+
+ public function testTypeConstraintInstanceOrSubclassValidViaSubclass() {
+ $entity = $this->lookup->getEntity( new ItemId( 'Q4' ) );
+ $constraintParameters = array_merge(
+ $this->relationParameter( 'instanceOrSubclass' ),
+ $this->classParameter( [ 'Q100', 'Q101' ] )
+ );
+ $constraint = $this->getConstraintMock( $constraintParameters );
+
+ $checkResult = $this->checker->checkConstraint( new
FakeSnakContext( $this->typeSnak, $entity ), $constraint );
+ $this->assertCompliance( $checkResult );
+ }
+
public function testTypeConstraintInstanceInvalid() {
$entity = $this->lookup->getEntity( new ItemId( 'Q1' ) );
$constraintParameters = array_merge(
diff --git a/tests/phpunit/Checker/TypeChecker/ValueTypeCheckerTest.php
b/tests/phpunit/Checker/TypeChecker/ValueTypeCheckerTest.php
index 01a6067..fa846b3 100644
--- a/tests/phpunit/Checker/TypeChecker/ValueTypeCheckerTest.php
+++ b/tests/phpunit/Checker/TypeChecker/ValueTypeCheckerTest.php
@@ -141,6 +141,30 @@
$this->assertCompliance( $checkResult );
}
+ public function
testValueTypeConstraintInstanceOrSubclassValidViaInstance() {
+ $snak = new PropertyValueSnak( $this->valueTypePropertyId, new
EntityIdValue( new ItemId( 'Q1' ) ) );
+ $constraintParameters = array_merge(
+ $this->relationParameter( 'instanceOrSubclass' ),
+ $this->classParameter( [ 'Q100', 'Q101' ] )
+ );
+ $constraint = $this->getConstraintMock( $constraintParameters );
+
+ $checkResult = $this->checker->checkConstraint( new
FakeSnakContext( $snak ), $constraint );
+ $this->assertCompliance( $checkResult );
+ }
+
+ public function
testValueTypeConstraintInstanceOrSubclassValidViaSubclass() {
+ $snak = new PropertyValueSnak( $this->valueTypePropertyId, new
EntityIdValue( new ItemId( 'Q4' ) ) );
+ $constraintParameters = array_merge(
+ $this->relationParameter( 'instanceOrSubclass' ),
+ $this->classParameter( [ 'Q100', 'Q101' ] )
+ );
+ $constraint = $this->getConstraintMock( $constraintParameters );
+
+ $checkResult = $this->checker->checkConstraint( new
FakeSnakContext( $snak ), $constraint );
+ $this->assertCompliance( $checkResult );
+ }
+
public function testValueTypeConstraintInstanceInvalid() {
$snak = new PropertyValueSnak( $this->valueTypePropertyId, new
EntityIdValue( new ItemId( 'Q1' ) ) );
$constraintParameters = array_merge(
--
To view, visit https://gerrit.wikimedia.org/r/404711
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Iedfe3013a10de0af7f12ea0f6fde7164dbe4f062
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/WikibaseQualityConstraints
Gerrit-Branch: master
Gerrit-Owner: Lucas Werkmeister (WMDE) <[email protected]>
Gerrit-Reviewer: Lucas Werkmeister (WMDE) <[email protected]>
Gerrit-Reviewer: Siebrand <[email protected]>
Gerrit-Reviewer: WMDE-leszek <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
