Thiemo Mättig (WMDE) has uploaded a new change for review.
https://gerrit.wikimedia.org/r/232279
Change subject: Narrow interfaces to StatementListProvider
......................................................................
Narrow interfaces to StatementListProvider
Change-Id: Ie8dfbcb9b752cf4e264591427d2d5d7aa88eb2d1
---
M includes/CrossCheck/CrossCheckInteractor.php
M includes/CrossCheck/CrossChecker.php
M includes/Violations/CrossCheckResultToViolationTranslator.php
M specials/SpecialCrossCheck.php
M tests/phpunit/CrossCheck/CrossCheckInteractorTest.php
M tests/phpunit/CrossCheck/CrossCheckerTest.php
M tests/phpunit/Violations/CrossCheckResultToViolationTranslatorTest.php
7 files changed, 99 insertions(+), 74 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/WikibaseQualityExternalValidation
refs/changes/79/232279/1
diff --git a/includes/CrossCheck/CrossCheckInteractor.php
b/includes/CrossCheck/CrossCheckInteractor.php
index fd04dae..e89471b 100644
--- a/includes/CrossCheck/CrossCheckInteractor.php
+++ b/includes/CrossCheck/CrossCheckInteractor.php
@@ -62,9 +62,11 @@
public function crossCheckEntityById( EntityId $entityId ) {
$entity = $this->entityLookup->getEntity( $entityId );
- if ( $entity ) {
- return $this->crossCheckEntity( $entity );
+ if ( !( $entity instanceof StatementListProvider ) ) {
+ return new CrossCheckResultList();
}
+
+ return $this->crossCheckEntity( $entity );
}
/**
@@ -88,17 +90,12 @@
/**
* Runs cross-check for all statements of a single entity.
*
- * @param Entity $entity
+ * @param StatementListProvider $entity
*
* @return CrossCheckResultList
*/
- public function crossCheckEntity( Entity $entity ) {
- if ( $entity instanceof StatementListProvider ) {
- $statementList = $entity->getStatements();
- } else {
- $statementList = new StatementList();
- }
- return $this->crossChecker->crossCheckStatements( $entity,
$statementList );
+ public function crossCheckEntity( StatementListProvider $entity ) {
+ return $this->crossChecker->crossCheckStatements( $entity,
$entity->getStatements() );
}
/**
@@ -135,9 +132,11 @@
$entity = $this->entityLookup->getEntity( $entityId );
- if ( $entity ) {
- return $this->crossCheckEntityWithProperties( $entity,
$propertyIds );
+ if ( !( $entity instanceof StatementListProvider ) ) {
+ return new CrossCheckResultList();
}
+
+ return $this->crossCheckEntityWithProperties( $entity,
$propertyIds );
}
/**
@@ -164,22 +163,18 @@
/**
* Runs cross-check for all statements with any of the given property
ids of a single entity.
*
- * @param Entity $entity
+ * @param StatementListProvider $entity
* @param PropertyId[] $propertyIds
*
* @return CrossCheckResultList
* @throws InvalidArgumentException
*/
- public function crossCheckEntityWithProperties( Entity $entity, array
$propertyIds ) {
+ public function crossCheckEntityWithProperties( StatementListProvider
$entity, array $propertyIds ) {
Assert::parameterElementType(
'Wikibase\DataModel\Entity\PropertyId', $propertyIds, '$propertyIds' );
- $entityStatements = array();
- if ( $entity instanceof StatementListProvider ) {
- $entityStatements = $entity->getStatements()->toArray();
- }
-
$statementList = new StatementList();
- foreach ( $entityStatements as $statement ) {
+
+ foreach ( $entity->getStatements()->toArray() as $statement ) {
if ( in_array( $statement->getPropertyId(),
$propertyIds ) ) {
$statementList->addStatement( $statement );
}
@@ -257,26 +252,25 @@
/**
* @param EntityId $entityId
* @param string[] $clamGuids
+ *
* @return CrossCheckResultList
*/
private function crossCheckClaimsOfEntity( EntityId $entityId,
$clamGuids ) {
$entity = $this->entityLookup->getEntity( $entityId );
- if ( $entity ) {
- $entityStatements = array();
- if ( $entity instanceof StatementListProvider ) {
- $entityStatements =
$entity->getStatements()->toArray();
- }
-
- $statementList = new StatementList();
- foreach ( $entityStatements as $statement ) {
- if ( in_array( $statement->getGuid(),
$clamGuids ) ) {
- $statementList->addStatement(
$statement );
- }
- }
-
- return $this->crossChecker->crossCheckStatements(
$entity, $statementList );
+ if ( !( $entity instanceof StatementListProvider ) ) {
+ return new CrossCheckResultList();
}
+
+ $statementList = new StatementList();
+
+ foreach ( $entity->getStatements()->toArray() as $statement ) {
+ if ( in_array( $statement->getGuid(), $clamGuids ) ) {
+ $statementList->addStatement( $statement );
+ }
+ }
+
+ return $this->crossChecker->crossCheckStatements( $entity,
$statementList );
}
/**
diff --git a/includes/CrossCheck/CrossChecker.php
b/includes/CrossCheck/CrossChecker.php
index 75f61b8..3e6ec03 100644
--- a/includes/CrossCheck/CrossChecker.php
+++ b/includes/CrossCheck/CrossChecker.php
@@ -93,17 +93,15 @@
/**
* Runs cross-check for specific statements of a entity.
*
- * @param Entity $entity
+ * @param StatementListProvider $entity
* @param StatementList $statements
*
* @return CrossCheckResultList
* @throws InvalidArgumentException
*/
- public function crossCheckStatements( Entity $entity, StatementList
$statements ) {
- $statementsOfEntity = array();
- if ( $entity instanceof StatementListProvider ) {
- $statementsOfEntity =
$entity->getStatements()->toArray();
- }
+ public function crossCheckStatements( StatementListProvider $entity,
StatementList $statements ) {
+ $statementsOfEntity = $entity->getStatements()->toArray();
+
foreach ( $statements as $statement ) {
if ( !in_array( $statement, $statementsOfEntity ) ) {
throw new InvalidArgumentException( 'All
statements in $statements must belong to the entity.' );
@@ -135,16 +133,12 @@
/**
* Gets those dump ids from database, that are applicable for
cross-checks with the given entity
*
- * @param Entity $entity
+ * @param StatementListProvider $entity
*
* @return array
*/
- private function getApplicableDumps( Entity $entity ) {
+ private function getApplicableDumps( StatementListProvider $entity ) {
$applicableDumps = array();
- if( !$entity instanceof StatementListProvider ) {
- return $applicableDumps;
- }
-
$identifierPropertyIds =
$entity->getStatements()->getPropertyIds();
$dumpMetaInformation =
$this->dumpMetaInformationLookup->getWithIdentifierProperties(
$identifierPropertyIds
@@ -163,6 +157,7 @@
/**
* Runs cross-check for a single identifier property
*
+ * @param StatementListProvider $entity
* @param StatementList $statements
* @param PropertyId $identifierPropertyId
* @param DumpMetaInformation[] $dumpMetaInformationList
@@ -170,7 +165,7 @@
* @return CrossCheckResultList
*/
private function crossCheckStatementsWithIdentifier(
- Entity $entity,
+ StatementListProvider $entity,
StatementList $statements,
PropertyId $identifierPropertyId,
array $dumpMetaInformationList
@@ -332,19 +327,16 @@
/**
* Gets external ids for a identifier property of a given entity
*
- * @param Entity $entity
+ * @param StatementListProvider $entity
* @param PropertyId $identifierPropertyId
*
- * @return array
+ * @return string[]
*/
- private function getExternalIds( Entity $entity, PropertyId
$identifierPropertyId ) {
+ private function getExternalIds( StatementListProvider $entity,
PropertyId $identifierPropertyId ) {
$externalIds = array();
- if( !$entity instanceof StatementListProvider ) {
- return $externalIds;
- }
-
$identifierStatements =
$entity->getStatements()->getByPropertyId( $identifierPropertyId );
$values = $this->getDataValues( $identifierStatements );
+
foreach ( $values as $value ) {
if ( $value instanceof StringValue ) {
$externalIds[] = $value->getValue();
diff --git a/includes/Violations/CrossCheckResultToViolationTranslator.php
b/includes/Violations/CrossCheckResultToViolationTranslator.php
index ca022a6..f06c62d 100644
--- a/includes/Violations/CrossCheckResultToViolationTranslator.php
+++ b/includes/Violations/CrossCheckResultToViolationTranslator.php
@@ -4,6 +4,7 @@
use DataValues\Serializers\DataValueSerializer;
use Wikibase\DataModel\Entity\Entity;
+use Wikibase\DataModel\Entity\EntityId;
use Wikibase\Lib\Store\EntityRevisionLookup;
use WikibaseQuality\ExternalValidation\CrossCheck\Result\ComparisonResult;
use WikibaseQuality\ExternalValidation\CrossCheck\Result\CrossCheckResult;
@@ -26,21 +27,20 @@
}
/**
- * @param Entity $entity
+ * @param EntityId $entityId
* @param CrossCheckResultList $crossCheckResultList
*
* @return Violation[]
*/
- public function translateToViolation( Entity $entity,
CrossCheckResultList $crossCheckResultList ) {
-
+ public function translateToViolation( EntityId $entityId,
CrossCheckResultList $crossCheckResultList ) {
$violationArray = array();
+
+ /** @var CrossCheckResult $crossCheckResult */
foreach ( $crossCheckResultList as $crossCheckResult ) {
if (
$crossCheckResult->getComparisonResult()->getStatus() !==
ComparisonResult::STATUS_MISMATCH ) {
continue;
}
-
- $entityId = $entity->getId();
$propertyId = $crossCheckResult->getPropertyId();
$claimGuid = $crossCheckResult->getClaimGuid();
//TODO: Use real ClaimGuid and TypeEntityId
diff --git a/specials/SpecialCrossCheck.php b/specials/SpecialCrossCheck.php
index dc7d8cb..6d3b3b3 100644
--- a/specials/SpecialCrossCheck.php
+++ b/specials/SpecialCrossCheck.php
@@ -14,6 +14,7 @@
use ValueFormatters\FormatterOptions;
use ValueFormatters\ValueFormatter;
use Wikibase\DataModel\Entity\Entity;
+use Wikibase\DataModel\Entity\EntityDocument;
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Entity\EntityIdValue;
use Wikibase\DataModel\Services\EntityId\EntityIdFormatter;
@@ -22,6 +23,7 @@
use Wikibase\DataModel\Services\Lookup\EntityLookup;
use Wikibase\DataModel\Services\Lookup\LanguageLabelDescriptionLookup;
use Wikibase\DataModel\Services\Lookup\TermLookup;
+use Wikibase\DataModel\Statement\StatementListProvider;
use Wikibase\Lib\OutputFormatValueFormatterFactory;
use Wikibase\Lib\SnakFormatter;
use Wikibase\Repo\EntityIdHtmlLinkFormatterFactory;
@@ -199,7 +201,7 @@
return;
}
- if ( !$entity ) {
+ if ( !( $entity instanceof StatementListProvider ) ) {
$out->addHTML(
$this->buildNotice(
'wbqev-crosscheck-not-existent-entity', true )
);
@@ -473,13 +475,19 @@
return $table->toHtml();
}
-
/**
- * @param Entity $entity
- * @param array $results
+ * @param EntityDocument $entity
+ * @param CrossCheckResultList $results
*/
- protected function saveResultsInViolationsTable( $entity, $results ) {
- $violations =
$this->crossCheckResultToViolationTranslator->translateToViolation( $entity,
$results );
+ protected function saveResultsInViolationsTable(
+ EntityDocument $entity,
+ CrossCheckResultList $results
+ ) {
+ $violations =
$this->crossCheckResultToViolationTranslator->translateToViolation(
+ $entity->getId(),
+ $results
+ );
+
foreach( $violations as $violation ) {
$this->violationStore->insert( $violation, true );
}
diff --git a/tests/phpunit/CrossCheck/CrossCheckInteractorTest.php
b/tests/phpunit/CrossCheck/CrossCheckInteractorTest.php
index d875ee1..39d3784 100644
--- a/tests/phpunit/CrossCheck/CrossCheckInteractorTest.php
+++ b/tests/phpunit/CrossCheck/CrossCheckInteractorTest.php
@@ -10,6 +10,7 @@
use Wikibase\DataModel\Statement\Statement;
use Wikibase\DataModel\Entity\Entity;
use Wikibase\DataModel\Statement\StatementList;
+use Wikibase\DataModel\Statement\StatementListProvider;
use WikibaseQuality\ExternalValidation\CrossCheck\CrossCheckInteractor;
use WikibaseQuality\Tests\Helper\JsonFileEntityLookup;
@@ -178,7 +179,7 @@
/**
* @dataProvider crossCheckEntityDataProvider
*/
- public function testCrossCheckEntity( $entity, $expectedResult ) {
+ public function testCrossCheckEntity( StatementListProvider $entity,
array $expectedResult ) {
$actualResult = $this->crossCheckInteractor->crossCheckEntity(
$entity );
$this->runAssertions( $expectedResult, $actualResult );
@@ -433,7 +434,12 @@
/**
* @dataProvider crossCheckEntityWithPropertiesDataProvider
*/
- public function testCrossCheckEntityWithProperties( $entity,
$propertyIds, $expectedResult, $expectedException = null ) {
+ public function testCrossCheckEntityWithProperties(
+ StatementListProvider $entity,
+ array $propertyIds,
+ array $expectedResult,
+ $expectedException = null
+ ) {
if ( $expectedException ) {
$this->setExpectedException( $expectedException );
}
@@ -486,7 +492,12 @@
/**
* @dataProvider crossCheckEntitiesWithPropertiesDataProvider
*/
- public function testCrossCheckEntitiesWithProperties( $entities,
$propertyIds, $expectedResult, $expectedException = null ) {
+ public function testCrossCheckEntitiesWithProperties(
+ array $entities,
+ array $propertyIds,
+ array $expectedResult,
+ $expectedException = null
+ ) {
if ( $expectedException ) {
$this->setExpectedException( $expectedException );
}
diff --git a/tests/phpunit/CrossCheck/CrossCheckerTest.php
b/tests/phpunit/CrossCheck/CrossCheckerTest.php
index 73b5b3e..2a98903 100644
--- a/tests/phpunit/CrossCheck/CrossCheckerTest.php
+++ b/tests/phpunit/CrossCheck/CrossCheckerTest.php
@@ -5,6 +5,8 @@
use Wikibase\DataModel\Entity\Item;
use Wikibase\DataModel\Entity\ItemId;
use Wikibase\DataModel\Entity\PropertyId;
+use Wikibase\DataModel\Statement\StatementList;
+use Wikibase\DataModel\Statement\StatementListProvider;
use WikibaseQuality\ExternalValidation\CrossCheck\CrossChecker;
use WikibaseQuality\ExternalValidation\CrossCheck\Result\ComparisonResult;
use WikibaseQuality\ExternalValidation\CrossCheck\Result\CrossCheckResult;
@@ -143,8 +145,12 @@
/**
* @dataProvider crossCheckStatementsDataProvider
*/
- public function testCrossCheckStatements( $entity, $statementList,
$expectedResults, $expectedException = null ) {
- // If exception is expected, set it so
+ public function testCrossCheckStatements(
+ StatementListProvider $entity,
+ StatementList $statementList,
+ array $expectedResults,
+ $expectedException = null
+ ) {
if ( $expectedException ) {
$this->setExpectedException( $expectedException );
}
diff --git
a/tests/phpunit/Violations/CrossCheckResultToViolationTranslatorTest.php
b/tests/phpunit/Violations/CrossCheckResultToViolationTranslatorTest.php
index f2c014a..10148bc 100644
--- a/tests/phpunit/Violations/CrossCheckResultToViolationTranslatorTest.php
+++ b/tests/phpunit/Violations/CrossCheckResultToViolationTranslatorTest.php
@@ -117,8 +117,15 @@
ComparisonResult::STATUS_MATCH
);
- $crossCheckResultList = new CrossCheckResultList( array( new
CrossCheckResult( $this->propertyId, $this->claimGuid, $this->externalId,
$this->dumpMetaInformation, $comparisonResult, $this->referenceResult ) ) );
- $violations = $this->translator->translateToViolation(
$this->entity, $crossCheckResultList );
+ $crossCheckResultList = new CrossCheckResultList( array( new
CrossCheckResult(
+ $this->propertyId,
+ $this->claimGuid,
+ $this->externalId,
+ $this->dumpMetaInformation,
+ $comparisonResult,
+ $this->referenceResult
+ ) ) );
+ $violations = $this->translator->translateToViolation(
$this->entity->getId(), $crossCheckResultList );
$this->assertEquals( array(), $violations );
}
@@ -129,8 +136,15 @@
ComparisonResult::STATUS_MISMATCH
);
- $crossCheckResultList = new CrossCheckResultList( array( new
CrossCheckResult( $this->propertyId, $this->claimGuid, $this->externalId,
$this->dumpMetaInformation, $comparisonResult, $this->referenceResult ) ) );
- $violations = $this->translator->translateToViolation(
$this->entity, $crossCheckResultList );
+ $crossCheckResultList = new CrossCheckResultList( array( new
CrossCheckResult(
+ $this->propertyId,
+ $this->claimGuid,
+ $this->externalId,
+ $this->dumpMetaInformation,
+ $comparisonResult,
+ $this->referenceResult
+ ) ) );
+ $violations = $this->translator->translateToViolation(
$this->entity->getId(), $crossCheckResultList );
$this->assertEquals( 1, sizeof( $violations ) );
$violation = $violations[0];
--
To view, visit https://gerrit.wikimedia.org/r/232279
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie8dfbcb9b752cf4e264591427d2d5d7aa88eb2d1
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/WikibaseQualityExternalValidation
Gerrit-Branch: master
Gerrit-Owner: Thiemo Mättig (WMDE) <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
