Jonaskeutel has uploaded a new change for review.
https://gerrit.wikimedia.org/r/218138
Change subject: implements some of Daniels hints
......................................................................
implements some of Daniels hints
done for:
* EvaluateConstraintReportJobService.php
* TypeCheckerHelper.php
* ValueCountCheckerHelper.php
Change-Id: I5d1b11d0269a515ada443da560d354fbb76ef7c6
---
M includes/ConstraintCheck/Checker/MultiValueChecker.php
M includes/ConstraintCheck/Checker/SingleValueChecker.php
M includes/ConstraintCheck/Checker/TypeChecker.php
M includes/ConstraintCheck/Checker/ValueTypeChecker.php
M includes/ConstraintCheck/Helper/TypeCheckerHelper.php
M includes/ConstraintCheck/Helper/ValueCountCheckerHelper.php
M includes/EvaluateConstraintReportJobService.php
M tests/phpunit/Checker/TypeChecker/TypeCheckerHelperTest.php
M tests/phpunit/Checker/TypeChecker/TypeCheckerTest.php
M tests/phpunit/Checker/ValueCountChecker/ValueCountCheckerHelperTest.php
10 files changed, 116 insertions(+), 65 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/WikidataQualityConstraints
refs/changes/38/218138/1
diff --git a/includes/ConstraintCheck/Checker/MultiValueChecker.php
b/includes/ConstraintCheck/Checker/MultiValueChecker.php
index adb753e..0937a02 100755
--- a/includes/ConstraintCheck/Checker/MultiValueChecker.php
+++ b/includes/ConstraintCheck/Checker/MultiValueChecker.php
@@ -44,8 +44,8 @@
$propertyCountArray =
$this->valueCountCheckerHelper->getPropertyCount( $entity->getStatements() );
- if ( $propertyCountArray[ $propertyId->getNumericId() ] <= 1 ) {
- $message = 'This property must have a multiple values,
that is there must be more than one claim using this property.';
+ if ( $propertyCountArray[ $propertyId->getSerialization() ] <=
1 ) {
+ $message = wfMessage(
"wbqc-violation-message-multi-value" )->escaped();
$status = CheckResult::STATUS_VIOLATION;
} else {
$message = '';
diff --git a/includes/ConstraintCheck/Checker/SingleValueChecker.php
b/includes/ConstraintCheck/Checker/SingleValueChecker.php
index 8718772..b829d3d 100755
--- a/includes/ConstraintCheck/Checker/SingleValueChecker.php
+++ b/includes/ConstraintCheck/Checker/SingleValueChecker.php
@@ -44,8 +44,8 @@
$propertyCountArray =
$this->valueCountCheckerHelper->getPropertyCount( $entity->getStatements() );
- if ( $propertyCountArray[ $propertyId->getNumericId() ] > 1 ) {
- $message = 'This property must only have a single
value, that is there must only be one claim using this property.';
+ if ( $propertyCountArray[$propertyId->getSerialization()] > 1 )
{
+ $message = wfMessage(
"wbqc-violation-message-single-value" )->escaped();
$status = CheckResult::STATUS_VIOLATION;
} else {
$message = '';
diff --git a/includes/ConstraintCheck/Checker/TypeChecker.php
b/includes/ConstraintCheck/Checker/TypeChecker.php
index 4b02e40..cf7b60d 100755
--- a/includes/ConstraintCheck/Checker/TypeChecker.php
+++ b/includes/ConstraintCheck/Checker/TypeChecker.php
@@ -39,8 +39,8 @@
*/
private $typeCheckerHelper;
- const instanceId = 31;
- const subclassId = 279;
+ const instanceId = 'P31';
+ const subclassId = 'P279';
/**
* @param EntityLookup $lookup
diff --git a/includes/ConstraintCheck/Checker/ValueTypeChecker.php
b/includes/ConstraintCheck/Checker/ValueTypeChecker.php
index fe6c007..9381bb2 100755
--- a/includes/ConstraintCheck/Checker/ValueTypeChecker.php
+++ b/includes/ConstraintCheck/Checker/ValueTypeChecker.php
@@ -40,8 +40,8 @@
*/
private $typeCheckerHelper;
- const instanceId = 31;
- const subclassId = 279;
+ const instanceId = 'P31';
+ const subclassId = 'P279';
/**
* @param EntityLookup $lookup
diff --git a/includes/ConstraintCheck/Helper/TypeCheckerHelper.php
b/includes/ConstraintCheck/Helper/TypeCheckerHelper.php
index cb62bc0..360edbd 100755
--- a/includes/ConstraintCheck/Helper/TypeCheckerHelper.php
+++ b/includes/ConstraintCheck/Helper/TypeCheckerHelper.php
@@ -2,6 +2,8 @@
namespace WikibaseQuality\ConstraintReport\ConstraintCheck\Helper;
+use Wikibase\DataModel\Entity\PropertyId;
+use Wikibase\DataModel\Statement\StatementList;
use Wikibase\Lib\Store\EntityLookup;
@@ -15,8 +17,8 @@
class TypeCheckerHelper {
const MAX_DEPTH = 20;
- const instanceId = 31;
- const subclassId = 279;
+ const instanceId = 'P31';
+ const subclassId = 'P279';
/**
* @var EntityLookup $entityLookup
@@ -27,6 +29,18 @@
$this->entityLookup = $lookup;
}
+ /**
+ * Checks, if one of the itemId serializations in $classesToCheck
+ * is subclass of $comparativeClass
+ * Due to cyclic dependencies, the checks stops after a certain
+ * depth is reached
+ *
+ * @param EntityId $comparativeClass
+ * @param array $classesToCheck
+ * @param int $depth
+ *
+ * @return bool
+ */
public function isSubclassOf( $comparativeClass, $classesToCheck,
$depth ) {
$compliance = null;
$item = $this->entityLookup->getEntity( $comparativeClass );
@@ -34,64 +48,69 @@
return false; // lookup failed, probably because item
doesn't exist
}
- foreach ( $item->getStatements() as $statement ) {
- $claim = $statement->getClaim();
- $propertyId = $claim->getPropertyId();
- $numericPropertyId = $propertyId->getNumericId();
+ foreach ( $item->getStatements()->getWithPropertyId( new
PropertyId( self::subclassId ) ) as $statement ) {
+ $mainSnak = $statement->getClaim()->getMainSnak();
- if ( $numericPropertyId === self::subclassId ) {
- $mainSnak = $claim->getMainSnak();
-
- if ( $mainSnak->getType() === 'value' &&
$mainSnak->getDataValue()->getType() === 'wikibase-entityid' ) {
- $comparativeClass =
$mainSnak->getDataValue()->getEntityId();
-
- foreach ( $classesToCheck as $class ) {
- if ( $class ===
$comparativeClass->getSerialization() ) {
- return true;
- }
- }
-
- }
-
- if ( $depth > self::MAX_DEPTH ) {
- return false;
- }
-
- $compliance = $this->isSubclassOf(
$comparativeClass, $classesToCheck, $depth + 1 );
-
+ if ( !( $this->hasCorrectType( $mainSnak ) ) ) {
+ continue;
}
+
+ $comparativeClass =
$mainSnak->getDataValue()->getEntityId();
+
+ if( in_array( $comparativeClass->getSerialization(),
$classesToCheck ) ) {
+ return true;
+ }
+
+ if ( $depth > self::MAX_DEPTH ) {
+ return false;
+ }
+
+ $compliance = $this->isSubclassOf( $comparativeClass,
$classesToCheck, $depth + 1 );
if ( $compliance === true ) {
return true;
}
+
}
return false;
}
- public function hasClassInRelation( $statements, $relationId,
$classesToCheck ) {
+ /**
+ * 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
+ *
+ * @param StatementList $statements
+ * @param string $relationId
+ * @param array $classesToCheck
+ *
+ * @return bool
+ */
+ public function hasClassInRelation( StatementList $statements,
$relationId, $classesToCheck ) {
$compliance = null;
- foreach ( $statements as $statement ) {
- $claim = $statement->getClaim();
- $propertyId = $claim->getPropertyId();
- $numericPropertyId = $propertyId->getNumericId();
+ foreach ( $statements->getWithPropertyId( new PropertyId(
$relationId ) ) as $statement ) {
+ $mainSnak = $claim =
$statement->getClaim()->getMainSnak();
- if ( $numericPropertyId === $relationId ) {
- $mainSnak = $claim->getMainSnak();
-
- if ( $mainSnak->getType() === 'value' &&
$mainSnak->getDataValue()->getType() === 'wikibase-entityid' ) {
- $comparativeClass =
$mainSnak->getDataValue()->getEntityId();
-
- foreach ( $classesToCheck as $class ) {
- if ( $class ===
$comparativeClass->getSerialization() ) {
- return true;
- }
- }
- $compliance = $this->isSubclassOf(
$comparativeClass, $classesToCheck, 1 );
- }
+ if ( !$this->hasCorrectType( $mainSnak ) ) {
+ continue;
}
+
+ $comparativeClass =
$mainSnak->getDataValue()->getEntityId();
+
+ if( in_array( $comparativeClass->getSerialization(),
$classesToCheck ) ) {
+ return true;
+ }
+
+ $compliance = $this->isSubclassOf( $comparativeClass,
$classesToCheck, 1 );
if ( $compliance === true ) {
return true;
}
}
return false;
}
+
+ private function hasCorrectType( $mainSnak ) {
+ return $mainSnak->getType() === 'value' &&
$mainSnak->getDataValue()->getType() === 'wikibase-entityid';
+ }
+
}
\ No newline at end of file
diff --git a/includes/ConstraintCheck/Helper/ValueCountCheckerHelper.php
b/includes/ConstraintCheck/Helper/ValueCountCheckerHelper.php
index f863de8..b7a8812 100755
--- a/includes/ConstraintCheck/Helper/ValueCountCheckerHelper.php
+++ b/includes/ConstraintCheck/Helper/ValueCountCheckerHelper.php
@@ -30,13 +30,13 @@
$this->propertyCount = array ();
foreach ( $statements as $statement ) {
$counter = $statement->getRank() ===
Statement::RANK_DEPRECATED ? 0 : 1;
- if ( array_key_exists(
$statement->getPropertyId()->getNumericId(), $this->propertyCount ) ) {
-
$this->propertyCount[$statement->getPropertyId()->getNumericId()] += $counter;
+ if ( array_key_exists(
$statement->getPropertyId()->getSerialization(), $this->propertyCount ) ) {
+
$this->propertyCount[$statement->getPropertyId()->getSerialization()] +=
$counter;
} else {
-
$this->propertyCount[$statement->getPropertyId()->getNumericId()] = $counter;
+
$this->propertyCount[$statement->getPropertyId()->getSerialization()] =
$counter;
}
}
}
return $this->propertyCount;
}
-}
\ No newline at end of file
+}
diff --git a/includes/EvaluateConstraintReportJobService.php
b/includes/EvaluateConstraintReportJobService.php
index 2ebc71f..8857ade 100644
--- a/includes/EvaluateConstraintReportJobService.php
+++ b/includes/EvaluateConstraintReportJobService.php
@@ -8,12 +8,30 @@
use WikibaseQuality\ConstraintReport\ConstraintCheck\Result\CheckResult;
+/**
+ * Class EvaluateConstraintReportJobService
+ *
+ * @package WikibaseQuality\ConstraintReport
+ *
+ * Service that gets used be EvaluateConstraintReportJob
+ * to build a summary to be logged from the results
+ * (or, in case of a deferred job, gets the results from
+ * the ConstraintChecker first)
+ * and finally writes it to wfDebugLog
+ */
class EvaluateConstraintReportJobService {
public function writeToLog( $message ) {
wfDebugLog( 'wbq_evaluation', $message );
}
+ /**
+ * @param string $resultSummary (json representation of checkResults)
+ * @param int $timestamp (TS_UNIX)
+ * @param array $params
+ *
+ * @return string
+ */
public function buildMessageForLog( $resultSummary, $timestamp, $params
) {
return json_encode(
array (
@@ -26,6 +44,11 @@
);
}
+ /**
+ * @param $results
+ *
+ * @return string
+ */
public function buildResultSummary( $results ) {
$summary = array();
@@ -48,6 +71,15 @@
return json_encode( $summary );
}
+ /**
+ * Returns json-representation of CheckResults,
+ * either from params or from ConstraintChecker
+ * (when invoked via a deferred job; see EvaluateConstraintReportJob)
+ *
+ * @param $params
+ *
+ * @return string
+ */
public function getResults( $params ) {
if ( !array_key_exists( 'results', $params ) ) {
$lookup =
WikibaseRepo::getDefaultInstance()->getEntityLookup();
diff --git a/tests/phpunit/Checker/TypeChecker/TypeCheckerHelperTest.php
b/tests/phpunit/Checker/TypeChecker/TypeCheckerHelperTest.php
index 5dd9542..860669a 100755
--- a/tests/phpunit/Checker/TypeChecker/TypeCheckerHelperTest.php
+++ b/tests/phpunit/Checker/TypeChecker/TypeCheckerHelperTest.php
@@ -7,6 +7,7 @@
use Wikibase\DataModel\Entity\EntityIdValue;
use Wikibase\DataModel\Entity\ItemId;
use Wikibase\DataModel\Entity\PropertyId;
+use Wikibase\DataModel\Statement\StatementList;
use WikibaseQuality\ConstraintReport\ConstraintCheck\Checker\TypeChecker;
use
WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\ConstraintReportHelper;
use WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\TypeCheckerHelper;
@@ -40,22 +41,22 @@
public function testCheckHasClassInRelationValid() {
$statement1 = new Statement( new Claim( new PropertyValueSnak(
new PropertyId( 'P1' ), new EntityIdValue( new ItemId( 'Q42' ) ) ) ) );
$statement2 = new Statement( new Claim( new PropertyValueSnak(
new PropertyId( 'P31' ), new EntityIdValue( new ItemId( 'Q1' ) ) ) ) );
- $statements = array( $statement1, $statement2 );
- $this->assertEquals( true, $this->helper->hasClassInRelation(
$statements, 31, array( 'Q1' ) ) );
+ $statements = new StatementList( array( $statement1,
$statement2 ) );
+ $this->assertEquals( true, $this->helper->hasClassInRelation(
$statements, 'P31', array( 'Q1' ) ) );
}
public function testCheckHasClassInRelationInvalid() {
$statement1 = new Statement( new Claim( new PropertyValueSnak(
new PropertyId( 'P1' ), new EntityIdValue( new ItemId( 'Q42' ) ) ) ) );
$statement2 = new Statement( new Claim( new PropertyValueSnak(
new PropertyId( 'P31' ), new EntityIdValue( new ItemId( 'Q100' ) ) ) ) );
- $statements = array( $statement1, $statement2 );
- $this->assertEquals( false, $this->helper->hasClassInRelation(
$statements, 31, array( 'Q1' ) ) );
+ $statements = new StatementList( array( $statement1,
$statement2 ) );
+ $this->assertEquals( false, $this->helper->hasClassInRelation(
$statements, 'P31', array( 'Q1' ) ) );
}
public function testCheckHasClassInRelationValidWithIndirection() {
$statement1 = new Statement( new Claim( new PropertyValueSnak(
new PropertyId( 'P1' ), new EntityIdValue( new ItemId( 'Q42' ) ) ) ) );
$statement2 = new Statement( new Claim( new PropertyValueSnak(
new PropertyId( 'P31' ), new EntityIdValue( new ItemId( 'Q5' ) ) ) ) );
- $statements = array( $statement1, $statement2 );
- $this->assertEquals( true, $this->helper->hasClassInRelation(
$statements, 31, array( 'Q4' ) ) );
+ $statements = new StatementList( array( $statement1,
$statement2 ) );
+ $this->assertEquals( true, $this->helper->hasClassInRelation(
$statements, 'P31', array( 'Q4' ) ) );
}
public function testCheckIsSubclassOfValidWithIndirection() {
diff --git a/tests/phpunit/Checker/TypeChecker/TypeCheckerTest.php
b/tests/phpunit/Checker/TypeChecker/TypeCheckerTest.php
index b9fe1f5..454f1c2 100755
--- a/tests/phpunit/Checker/TypeChecker/TypeCheckerTest.php
+++ b/tests/phpunit/Checker/TypeChecker/TypeCheckerTest.php
@@ -40,7 +40,6 @@
protected function tearDown() {
unset( $this->lookup );
unset( $this->typeStatement );
- unset( $this->typeStatement );
parent::tearDown();
}
diff --git
a/tests/phpunit/Checker/ValueCountChecker/ValueCountCheckerHelperTest.php
b/tests/phpunit/Checker/ValueCountChecker/ValueCountCheckerHelperTest.php
index 67db44b..a6f6b88 100755
--- a/tests/phpunit/Checker/ValueCountChecker/ValueCountCheckerHelperTest.php
+++ b/tests/phpunit/Checker/ValueCountChecker/ValueCountCheckerHelperTest.php
@@ -43,7 +43,7 @@
$checker = new ValueCountCheckerHelper();
$propertyCount = $checker->getPropertyCount(
$this->statementList );
- $this->assertEquals( 1, $propertyCount[1] );
- $this->assertEquals( 2, $propertyCount[2] );
+ $this->assertEquals( 1, $propertyCount['P1'] );
+ $this->assertEquals( 2, $propertyCount['P2'] );
}
}
\ No newline at end of file
--
To view, visit https://gerrit.wikimedia.org/r/218138
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5d1b11d0269a515ada443da560d354fbb76ef7c6
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/WikidataQualityConstraints
Gerrit-Branch: v1
Gerrit-Owner: Jonaskeutel <jonas.keu...@student.hpi.de>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
