Soeren.oldag has uploaded a new change for review.
https://gerrit.wikimedia.org/r/205846
Change subject: Minor tweaks in CrossChecker.
......................................................................
Minor tweaks in CrossChecker.
CrossChecker now takes entity in constructor. Database connection is now first
established, when CrossChecker is executed and closed, when execution finishes.
Change-Id: I9980829aba72be2e0612cac6882e13ab223a9177
---
M api/CrossCheck.php
M includes/CrossCheck/CrossChecker.php
M specials/SpecialCrossCheck.php
M tests/phpunit/CrossCheck/CrossCheckerTest.php
4 files changed, 75 insertions(+), 55 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/WikidataQualityExternalValidation
refs/changes/46/205846/1
diff --git a/api/CrossCheck.php b/api/CrossCheck.php
index aa936fc..3b85bb8 100644
--- a/api/CrossCheck.php
+++ b/api/CrossCheck.php
@@ -122,8 +122,8 @@
$entity = $this->entityLookup->getEntity(
$this->getIdParser()->parse( $entityId ) );
if ( $entity ) {
- $crossChecker = new CrossChecker();
- $resultLists[ (string)$entityId ] =
$crossChecker->crossCheckEntity( $entity, $propertyIds );
+ $crossChecker = new CrossChecker( $entity );
+ $resultLists[ (string)$entityId ] =
$crossChecker->crossCheckEntity( $propertyIds );
} else {
$resultLists[ (string)$entityId ] = null;
}
@@ -159,8 +159,8 @@
}
}
- $crossChecker = new CrossChecker();
- $resultLists[ (string)$entityId ] =
$crossChecker->crossCheckStatements( $entity, $statements );
+ $crossChecker = new CrossChecker( $entity );
+ $resultLists[ (string)$entityId ] =
$crossChecker->crossCheckStatements( $statements );
} else {
$resultLists[ (string)$entityId ] = null;
}
diff --git a/includes/CrossCheck/CrossChecker.php
b/includes/CrossCheck/CrossChecker.php
index 33063db..52a218c 100644
--- a/includes/CrossCheck/CrossChecker.php
+++ b/includes/CrossCheck/CrossChecker.php
@@ -2,6 +2,9 @@
namespace WikidataQuality\ExternalValidation\CrossCheck;
+use LoadBalancer;
+use DatabaseBase;
+use MWException;
use Doctrine\Instantiator\Exception\InvalidArgumentException;
use Wikibase\DataModel\Entity\Entity;
use Wikibase\DataModel\Entity\EntityId;
@@ -27,42 +30,37 @@
*/
class CrossChecker {
+ /**
+ * @var LoadBalancer
+ */
+ private $loadBalancer;
+
/**
- * Database connection
- *
- * @var \DatabaseBase
+ * @var DatabaseBase
*/
private $db;
- /**
- * Wikibase load balancer for database connections
- * If existing connection was passed to constructor, $loadBalancer is
null
- *
- * @var \LoadBalancer
- */
- private $loadBalancer;
+ /**
+ * @var Entity
+ */
+ private $entity;
- public function __construct() {
- wfWaitForSlaves();
- $this->loadBalancer = wfGetLB();
-
- $this->db = $this->loadBalancer->getConnection( DB_SLAVE );
- }
-
- public function __destruct() {
- $this->loadBalancer->reuseConnection( $this->db );
+ /**
+ * @param Entity $entity
+ */
+ public function __construct( Entity $entity ) {
+ $this->entity = $entity;
}
/**
- * Runs cross-check for all statements of a given entity
+ * Runs cross-check for all statements of the entity
*
- * @param Entity $entity - entity that should be cross-checked
* @param PropertyId|array|null $propertyIds - If specified, only
statements with given property ids will be cross-checked
*
* @return CrossCheckResultList
* @throws InvalidArgumentException
*/
- public function crossCheckEntity( Entity $entity, $propertyIds = array
() ) {
+ public function crossCheckEntity( $propertyIds = array () ) {
$statements = new StatementList();
if ( $propertyIds ) {
if ( $propertyIds instanceof PropertyId ) {
@@ -71,7 +69,7 @@
if ( is_array( $propertyIds ) || $propertyIds
instanceof \Traversable ) {
foreach ( $propertyIds as $propertyId ) {
if ( $propertyId instanceof PropertyId
) {
- foreach (
$entity->getStatements()->getWithPropertyId( $propertyId ) as $statement ) {
+ foreach (
$this->entity->getStatements()->getWithPropertyId( $propertyId ) as $statement
) {
$statements->addStatement( $statement );
}
} else {
@@ -82,35 +80,36 @@
throw new InvalidArgumentException(
'$propertyIds must be a PropertyId, array or an instance of Traversable.' );
}
} else {
- $statements = $entity->getStatements();
+ $statements = $this->entity->getStatements();
}
- return $this->crossCheckStatements( $entity, $statements );
+ return $this->crossCheckStatements( $statements );
}
/**
- * Runs cross-check for specific statements of the given entity
+ * Runs cross-check for specific statements of the entity
*
- * @param Entity $entity - entity that should be cross-checked
* @param Statement|StatementList $statements - Statements of the given
entity that should be cross-checked
*
* @return CrossCheckResultList
* @throws InvalidArgumentException
*/
- public function crossCheckStatements( Entity $entity, $statements ) {
+ public function crossCheckStatements( $statements ) {
if ( $statements instanceof Statement ) {
$statements = new StatementList( $statements );
} elseif ( !( $statements instanceof StatementList ) ) {
throw new InvalidArgumentException( '$statements must
be Statement or StatementList.' );
}
- $statementsOfEntity = $entity->getStatements()->toArray();
+ $statementsOfEntity = $this->entity->getStatements()->toArray();
foreach ( $statements as $statement ) {
if ( !in_array( $statement, $statementsOfEntity ) ) {
- throw new InvalidArgumentException( 'All
statements in $statements must belong to $entity.' );
+ throw new InvalidArgumentException( 'All
statements in $statements must belong to the entity.' );
}
}
- $applicableDumpIds = $this->getApplicableDumpIds( $entity );
+ $this->establishDbConnection();
+
+ $applicableDumpIds = $this->getApplicableDumpIds( $this->entity
);
$resultList = new CrossCheckResultList();
foreach ( $applicableDumpIds as $identifierPropertyId =>
$dumpIds ) {
@@ -118,28 +117,29 @@
$resultList->merge(
$this->crossCheckStatementsWithIdentifier(
- $entity,
$statements,
$identifierPropertyId,
$dumpIds
)
);
}
+
+ $this->reuseDbConnection();
+
return $resultList;
}
/**
* Runs cross-check for a single identifier property
*
- * @param Entity $entity
* @param StatementList $statements
* @param PropertyId $identifierPropertyId
* @param array $dumpIds
*
* @return CrossCheckResultList
*/
- private function crossCheckStatementsWithIdentifier( Entity $entity,
StatementList $statements, PropertyId $identifierPropertyId, array $dumpIds ) {
- $externalIds = $this->getExternalIds( $entity,
$identifierPropertyId );
+ private function crossCheckStatementsWithIdentifier( StatementList
$statements, PropertyId $identifierPropertyId, array $dumpIds ) {
+ $externalIds = $this->getExternalIds( $identifierPropertyId );
$externalData = $this->getExternalData( $dumpIds, $externalIds
);
@@ -214,14 +214,12 @@
/**
* Gets those dump ids from database, that are applicable for
cross-checks with the given entity
*
- * @param Entity $entity
- *
* @return array
*/
- private function getApplicableDumpIds( Entity $entity ) {
+ private function getApplicableDumpIds( ) {
$applicableDumps = array ();
- $propertyIds = $entity->getStatements()->getPropertyIds();
+ $propertyIds = $this->entity->getStatements()->getPropertyIds();
if ( $propertyIds ) {
$result = $this->db->select(
DUMP_IDENTIFIER_PROPERTIES_TABLE,
@@ -240,14 +238,13 @@
/**
* Gets external ids for a identifier property of a given entity
*
- * @param Entity $entity
* @param PropertyId $identifierPropertyId
*
* @return array
*/
- private function getExternalIds( Entity $entity, PropertyId
$identifierPropertyId ) {
+ private function getExternalIds( PropertyId $identifierPropertyId ) {
$externalIds = array ();
- $identifierStatements =
$entity->getStatements()->getWithPropertyId( $identifierPropertyId );
+ $identifierStatements =
$this->entity->getStatements()->getWithPropertyId( $identifierPropertyId );
foreach ( $identifierStatements as $identifierStatement ) {
$mainSnak = $identifierStatement->getMainSnak();
@@ -310,4 +307,27 @@
return sprintf( '%s in (%s)', $columnName, implode( ',',
$values ) );
}
+
+ /**
+ * Establishes a database connection using the load balancer
+ *
+ * @throws MWException
+ */
+ protected function establishDbConnection() {
+ if( !$this->loadBalancer ) {
+ $this->loadBalancer = wfGetLB();
+ }
+
+ wfWaitForSlaves();
+ $this->db = $this->loadBalancer->getConnection( DB_SLAVE );
+ }
+
+ /**
+ * Mark database connection as being available for reuse
+ *
+ * @throws MWException
+ */
+ protected function reuseDbConnection( ) {
+ $this->loadBalancer->reuseConnection( $this->db );
+ }
}
\ No newline at end of file
diff --git a/specials/SpecialCrossCheck.php b/specials/SpecialCrossCheck.php
index 4056e83..fe49f83 100755
--- a/specials/SpecialCrossCheck.php
+++ b/specials/SpecialCrossCheck.php
@@ -5,7 +5,7 @@
use Html;
use Linker;
-use Wikibase\DataModel\Entity\EntityDocument;
+use Wikibase\DataModel\Entity\Entity;
use Wikibase\DataModel\Entity\EntityId;
use WikidataQuality\ExternalValidation\CrossCheck\CrossChecker;
use WikidataQuality\ExternalValidation\CrossCheck\Result\CrossCheckResult;
@@ -57,13 +57,13 @@
/**
* @see SpecialCheckResultPage::executeCheck
*
- * @param EntityDocument $entity
+ * @param Entity $entity
*
* @return string
*/
- protected function executeCheck( EntityDocument $entity ) {
- $crossChecker = new CrossChecker();
- $results = $crossChecker->crossCheckEntity( $entity );
+ protected function executeCheck( Entity $entity ) {
+ $crossChecker = new CrossChecker( $entity );
+ $results = $crossChecker->crossCheckEntity();
$this->saveResultsInViolationsTable( $entity, $results );
diff --git a/tests/phpunit/CrossCheck/CrossCheckerTest.php
b/tests/phpunit/CrossCheck/CrossCheckerTest.php
index 17f9939..a813e63 100644
--- a/tests/phpunit/CrossCheck/CrossCheckerTest.php
+++ b/tests/phpunit/CrossCheck/CrossCheckerTest.php
@@ -179,8 +179,8 @@
}
// Run cross-check
- $crossChecker = new CrossChecker();
- $results = $crossChecker->crossCheckEntity( $entity, $propertyIds );
+ $crossChecker = new CrossChecker( $entity );
+ $results = $crossChecker->crossCheckEntity( $propertyIds );
$this->runResultAssertions( $results, $expectedResults );
}
@@ -312,8 +312,8 @@
}
// Run cross-check
- $crossChecker = new CrossChecker();
- $results = $crossChecker->crossCheckStatements( $entity, $statements );
+ $crossChecker = new CrossChecker( $entity );
+ $results = $crossChecker->crossCheckStatements( $statements );
$this->runResultAssertions( $results, $expectedResults );
}
--
To view, visit https://gerrit.wikimedia.org/r/205846
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9980829aba72be2e0612cac6882e13ab223a9177
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/WikidataQualityExternalValidation
Gerrit-Branch: master
Gerrit-Owner: Soeren.oldag <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
