jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/356421 )
Change subject: Import Inverse constraints from statements
......................................................................
Import Inverse constraints from statements
InverseChecker gets a ConstraintStatementParameterParser injected, which
it uses to parse the property parameter. A config variable for the
constraint type item is added, and ConstraintReportFactory aliases it to
the “Inverse” constraint type.
The test for a missing property parameter is removed, since it now
throws an exception and is covered by the dedicated
ConstraintStatementParameterParser tests. A new test is added which
checks that a constraint from a statement also works.
Bug: T167107
Change-Id: Idf365b7445af7918208cc56342b8646fb6a13e97
---
M extension.json
M includes/ConstraintCheck/Checker/InverseChecker.php
M includes/ConstraintReportFactory.php
M tests/phpunit/Checker/ConnectionChecker/InverseCheckerTest.php
4 files changed, 34 insertions(+), 39 deletions(-)
Approvals:
WMDE-leszek: Looks good to me, but someone else must approve
Jonas Kress (WMDE): Looks good to me, approved
jenkins-bot: Verified
diff --git a/extension.json b/extension.json
index 857295f..8381eb4 100644
--- a/extension.json
+++ b/extension.json
@@ -121,6 +121,11 @@
"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
},
+ "WBQualityConstraintsInverseConstraintId": {
+ "value": "Q21510855",
+ "description": "The item ID of the 'inverse 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
with the property 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.",
diff --git a/includes/ConstraintCheck/Checker/InverseChecker.php
b/includes/ConstraintCheck/Checker/InverseChecker.php
index 81fd918..9a537a3 100644
--- a/includes/ConstraintCheck/Checker/InverseChecker.php
+++ b/includes/ConstraintCheck/Checker/InverseChecker.php
@@ -9,7 +9,7 @@
use Wikibase\DataModel\Statement\StatementListProvider;
use WikibaseQuality\ConstraintReport\Constraint;
use WikibaseQuality\ConstraintReport\ConstraintCheck\ConstraintChecker;
-use
WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\ConstraintParameterParser;
+use
WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\ConstraintStatementParameterParser;
use
WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\ConnectionCheckerHelper;
use WikibaseQuality\ConstraintReport\ConstraintCheck\Result\CheckResult;
use WikibaseQuality\ConstraintReport\ConstraintParameterRenderer;
@@ -28,7 +28,7 @@
private $entityLookup;
/**
- * @var ConstraintParameterParser
+ * @var ConstraintStatementParameterParser
*/
private $constraintParameterParser;
@@ -44,18 +44,18 @@
/**
* @param EntityLookup $lookup
- * @param ConstraintParameterParser $helper
+ * @param ConstraintStatementParameterParser $constraintParameterParser
* @param ConnectionCheckerHelper $connectionCheckerHelper
* @param ConstraintParameterRenderer $constraintParameterRenderer
*/
public function __construct(
EntityLookup $lookup,
- ConstraintParameterParser $helper,
+ ConstraintStatementParameterParser $constraintParameterParser,
ConnectionCheckerHelper $connectionCheckerHelper,
ConstraintParameterRenderer $constraintParameterRenderer
) {
$this->entityLookup = $lookup;
- $this->constraintParameterParser = $helper;
+ $this->constraintParameterParser = $constraintParameterParser;
$this->connectionCheckerHelper = $connectionCheckerHelper;
$this->constraintParameterRenderer =
$constraintParameterRenderer;
}
@@ -73,9 +73,8 @@
$parameters = [];
$constraintParameters = $constraint->getConstraintParameters();
- if ( array_key_exists( 'property', $constraintParameters ) ) {
- $parameters['property'] =
$this->constraintParameterParser->parseSingleParameter(
$constraintParameters['property'] );
- };
+ $propertyId =
$this->constraintParameterParser->parsePropertyParameter(
$constraintParameters, $constraint->getConstraintTypeName() );
+ $parameters['property'] = [ $propertyId ];
if ( array_key_exists( 'constraint_status',
$constraintParameters ) ) {
$parameters['constraint_status'] =
$this->constraintParameterParser->parseSingleParameter(
$constraintParameters['constraint_status'], true );
@@ -97,7 +96,6 @@
/*
* error handling:
* type of $dataValue for properties with 'Inverse'
constraint has to be 'wikibase-entityid'
- * parameter $property must not be null
*/
if ( $dataValue->getType() !== 'wikibase-entityid' ) {
$message = wfMessage(
"wbqc-violation-message-value-needed-of-type" )->params(
$constraint->getConstraintTypeName(), 'wikibase-entityid' )->escaped();
@@ -105,12 +103,6 @@
}
/** @var EntityIdValue $dataValue */
- if ( !array_key_exists( 'property', $constraintParameters ) ) {
- $message = wfMessage(
"wbqc-violation-message-parameter-needed" )->params(
$constraint->getConstraintTypeName(), 'property' )->escaped();
- return new CheckResult( $entity->getId(), $statement,
$constraint->getConstraintTypeQid(), $constraint->getConstraintId(),
$parameters, CheckResult::STATUS_VIOLATION, $message );
- }
-
- $property = $constraintParameters['property'];
$targetItem = $this->entityLookup->getEntity(
$dataValue->getEntityId() );
if ( $targetItem === null ) {
$message = wfMessage(
"wbqc-violation-message-target-entity-must-exist" )->escaped();
@@ -118,14 +110,14 @@
}
$targetItemStatementList = $targetItem->getStatements();
- if ( $this->connectionCheckerHelper->findStatement(
$targetItemStatementList, $property, $entity->getId()->getSerialization() ) !==
null ) {
+ if ( $this->connectionCheckerHelper->findStatement(
$targetItemStatementList, $propertyId->getSerialization(),
$entity->getId()->getSerialization() ) !== null ) {
$message = '';
$status = CheckResult::STATUS_COMPLIANCE;
} else {
$message = wfMessage( 'wbqc-violation-message-inverse' )
->rawParams(
$this->constraintParameterRenderer->formatEntityId( $targetItem->getId() ),
-
$this->constraintParameterRenderer->formatPropertyId( $property ),
+
$this->constraintParameterRenderer->formatEntityId( $propertyId ),
$this->constraintParameterRenderer->formatEntityId( $entity->getId() )
)
->escaped();
diff --git a/includes/ConstraintReportFactory.php
b/includes/ConstraintReportFactory.php
index e63661b..88ca513 100644
--- a/includes/ConstraintReportFactory.php
+++ b/includes/ConstraintReportFactory.php
@@ -201,7 +201,7 @@
'Item' => new ItemChecker( $this->lookup,
$constraintParameterParser, $connectionCheckerHelper,
$this->constraintParameterRenderer ),
'Target required claim' => new
TargetRequiredClaimChecker( $this->lookup, $constraintParameterParser,
$connectionCheckerHelper, $this->constraintParameterRenderer ),
'Symmetric' => new SymmetricChecker(
$this->lookup, $constraintParameterParser, $connectionCheckerHelper,
$this->constraintParameterRenderer ),
- 'Inverse' => new InverseChecker( $this->lookup,
$constraintParameterParser, $connectionCheckerHelper,
$this->constraintParameterRenderer ),
+ 'Inverse' => new InverseChecker( $this->lookup,
$this->constraintStatementParameterParser, $connectionCheckerHelper,
$this->constraintParameterRenderer ),
'Qualifier' => new QualifierChecker(
$constraintParameterParser ),
'Qualifiers' => new QualifiersChecker(
$constraintParameterParser, $this->constraintParameterRenderer ),
'Mandatory qualifiers' => new
MandatoryQualifiersChecker( $constraintParameterParser,
$this->constraintParameterRenderer ),
@@ -226,6 +226,7 @@
$this->config->get(
'WBQualityConstraintsSymmetricConstraintId' ) =>
$this->constraintCheckerMap['Symmetric'],
$this->config->get(
'WBQualityConstraintsTypeConstraintId' ) => $this->constraintCheckerMap['Type'],
$this->config->get(
'WBQualityConstraintsValueTypeConstraintId' ) =>
$this->constraintCheckerMap['Value type'],
+ $this->config->get(
'WBQualityConstraintsInverseConstraintId' ) =>
$this->constraintCheckerMap['Inverse'],
];
}
diff --git a/tests/phpunit/Checker/ConnectionChecker/InverseCheckerTest.php
b/tests/phpunit/Checker/ConnectionChecker/InverseCheckerTest.php
index a46f2d5..08314e6 100644
--- a/tests/phpunit/Checker/ConnectionChecker/InverseCheckerTest.php
+++ b/tests/phpunit/Checker/ConnectionChecker/InverseCheckerTest.php
@@ -9,10 +9,10 @@
use Wikibase\DataModel\Entity\EntityIdValue;
use Wikibase\DataModel\Entity\ItemId;
use Wikibase\DataModel\Entity\PropertyId;
+use Wikibase\Repo\WikibaseRepo;
use WikibaseQuality\ConstraintReport\Constraint;
use WikibaseQuality\ConstraintReport\ConstraintCheck\Checker\InverseChecker;
use
WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\ConnectionCheckerHelper;
-use
WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\ConstraintParameterParser;
use WikibaseQuality\ConstraintReport\Tests\ConstraintParameters;
use WikibaseQuality\ConstraintReport\Tests\ResultAssertions;
use WikibaseQuality\Tests\Helper\JsonFileEntityLookup;
@@ -23,7 +23,7 @@
* @group WikibaseQualityConstraints
*
* @uses \WikibaseQuality\ConstraintReport\ConstraintCheck\Result\CheckResult
- * @uses
\WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\ConstraintParameterParser
+ * @uses
\WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\ConstraintStatementParameterParser
*
* @author BP2014N1
* @license GNU GPL v2+
@@ -38,11 +38,6 @@
private $lookup;
/**
- * @var ConstraintParameterParser
- */
- private $helper;
-
- /**
* @var ConnectionCheckerHelper
*/
private $connectionCheckerHelper;
@@ -55,11 +50,10 @@
protected function setUp() {
parent::setUp();
$this->lookup = new JsonFileEntityLookup( __DIR__ );
- $this->helper = new ConstraintParameterParser();
$this->connectionCheckerHelper = new ConnectionCheckerHelper();
$this->checker = new InverseChecker(
$this->lookup,
- $this->helper,
+ $this->getConstraintParameterParser(),
$this->connectionCheckerHelper,
$this->getConstraintParameterRenderer()
);
@@ -67,7 +61,6 @@
protected function tearDown() {
unset( $this->lookup );
- unset( $this->helper );
unset( $this->connectionCheckerHelper );
unset( $this->checker );
parent::tearDown();
@@ -86,6 +79,21 @@
$this->assertCompliance( $checkResult );
}
+ public function testInverseConstraintValidWithStatement() {
+ $entity = $this->lookup->getEntity( new ItemId( 'Q1' ) );
+
+ $value = new EntityIdValue( new ItemId( 'Q7' ) );
+ $statement = new Statement( new PropertyValueSnak( new
PropertyId( 'P188' ), $value ) );
+
+ $snakSerializer =
WikibaseRepo::getDefaultInstance()->getSerializerFactory()->newSnakSerializer();
+ $propertyId = $this->getDefaultConfig()->get(
'WBQualityConstraintsPropertyId' );
+ $constraintParameters = [
+ $propertyId => [ $snakSerializer->serialize( new
PropertyValueSnak( new PropertyId( $propertyId ), new EntityIdValue( new
PropertyId( 'P1' ) ) ) ) ]
+ ];
+ $checkResult = $this->checker->checkConstraint( $statement,
$this->getConstraintMock( $constraintParameters ), $entity );
+ $this->assertCompliance( $checkResult );
+ }
+
public function testInverseConstraintWrongItem() {
$entity = $this->lookup->getEntity( new ItemId( 'Q1' ) );
@@ -97,17 +105,6 @@
];
$checkResult = $this->checker->checkConstraint( $statement,
$this->getConstraintMock( $constraintParameters ), $entity );
$this->assertViolation( $checkResult,
'wbqc-violation-message-inverse' );
- }
-
- public function testInverseConstraintWithoutProperty() {
- $entity = $this->lookup->getEntity( new ItemId( 'Q1' ) );
-
- $value = new EntityIdValue( new ItemId( 'Q7' ) );
- $statement = new Statement( new PropertyValueSnak( new
PropertyId( 'P188' ), $value ) );
-
- $constraintParameters = [];
- $checkResult = $this->checker->checkConstraint( $statement,
$this->getConstraintMock( $constraintParameters ), $entity );
- $this->assertViolation( $checkResult,
'wbqc-violation-message-parameter-needed' );
}
public function testInverseConstraintWrongDataTypeForItem() {
--
To view, visit https://gerrit.wikimedia.org/r/356421
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Idf365b7445af7918208cc56342b8646fb6a13e97
Gerrit-PatchSet: 11
Gerrit-Project: mediawiki/extensions/WikibaseQualityConstraints
Gerrit-Branch: master
Gerrit-Owner: Lucas Werkmeister (WMDE) <[email protected]>
Gerrit-Reviewer: Jonas Kress (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
