Soeren.oldag has submitted this change and it was merged.
Change subject: Applied some hints from Scrutinizer
......................................................................
Applied some hints from Scrutinizer
Change-Id: I53097cfff316760b1c2f214dbe4e780ca9f654fe
---
M includes/CrossCheck/Comparer/DataValueComparer.php
M includes/CrossCheck/CrossChecker.php
M includes/CrossCheck/ReferenceHandler.php
M includes/CrossCheck/Result/CrossCheckResultList.php
M includes/DumpMetaInformation.php
M includes/Serializer/IndexedTagsSerializer.php
M includes/UpdateTable/ImportContext.php
M includes/UpdateTable/Importer.php
M maintenance/UpdateTable.php
M specials/SpecialCrossCheck.php
M specials/SpecialExternalDbs.php
11 files changed, 60 insertions(+), 59 deletions(-)
Approvals:
Soeren.oldag: Verified; Looks good to me, approved
diff --git a/includes/CrossCheck/Comparer/DataValueComparer.php
b/includes/CrossCheck/Comparer/DataValueComparer.php
index 2f42a5a..205ace6 100755
--- a/includes/CrossCheck/Comparer/DataValueComparer.php
+++ b/includes/CrossCheck/Comparer/DataValueComparer.php
@@ -46,7 +46,7 @@
/**
* Wikibase data value for comparison.
*
- * @var array
+ * @var DataValue
*/
protected $localValue;
@@ -79,7 +79,7 @@
*
* @return CompareResult
*/
- public abstract function execute();
+ abstract public function execute();
/**
* Builds CompareResult instance for current comparison with given value
diff --git a/includes/CrossCheck/CrossChecker.php
b/includes/CrossCheck/CrossChecker.php
index 302ab25..c8d0b18 100644
--- a/includes/CrossCheck/CrossChecker.php
+++ b/includes/CrossCheck/CrossChecker.php
@@ -110,7 +110,7 @@
$resultList = new CrossCheckResultList();
if( $statements->count() > 0 ) {
$this->establishDbConnection();
- $applicableDumpIds = $this->getApplicableDumpIds( $this->entity );
+ $applicableDumpIds = $this->getApplicableDumpIds();
foreach ( $applicableDumpIds as $identifierPropertyId => $dumpIds
) {
$identifierPropertyId = new PropertyId( $identifierPropertyId
);
@@ -202,7 +202,7 @@
*
* @param Statement $statement
* @param PropertyId $identifierPropertyId
- * @param array $externalId
+ * @param string $externalId
*
* @return ReferenceResult
*/
@@ -217,7 +217,7 @@
*
* @return array
*/
- private function getApplicableDumpIds( ) {
+ private function getApplicableDumpIds() {
$applicableDumps = array ();
$propertyIds = $this->entity->getStatements()->getPropertyIds();
diff --git a/includes/CrossCheck/ReferenceHandler.php
b/includes/CrossCheck/ReferenceHandler.php
index 82fd04b..c4e0b34 100644
--- a/includes/CrossCheck/ReferenceHandler.php
+++ b/includes/CrossCheck/ReferenceHandler.php
@@ -54,7 +54,7 @@
* @param PropertyId $identifierPropertyId
* @param DumpMetaInformation $dumpMetaInformation
*/
- function __construct( Statement $statement, $externalId, PropertyId
$identifierPropertyId, DumpMetaInformation $dumpMetaInformation ) {
+ public function __construct( Statement $statement, $externalId,
PropertyId $identifierPropertyId, DumpMetaInformation $dumpMetaInformation ) {
if ( !is_string( $externalId ) ) {
throw new InvalidArgumentException( '$externalId must
be string.' );
}
diff --git a/includes/CrossCheck/Result/CrossCheckResultList.php
b/includes/CrossCheck/Result/CrossCheckResultList.php
index 6994e88..fe76755 100644
--- a/includes/CrossCheck/Result/CrossCheckResultList.php
+++ b/includes/CrossCheck/Result/CrossCheckResultList.php
@@ -48,7 +48,7 @@
/**
* Merges another CrossCheckResultList to the current one
*
- * @param CrossCheckResultList $list
+ * @param CrossCheckResultList $resultList
*/
public function merge( CrossCheckResultList $resultList ) {
$this->results = array_merge( $this->results,
$resultList->results );
@@ -89,7 +89,7 @@
*
* @return array
*/
- function getPropertyIds() {
+ public function getPropertyIds() {
$propertyIds = array ();
foreach ( $this->results as $result ) {
@@ -109,7 +109,7 @@
*
* @return CrossCheckResultList
*/
- function getWithPropertyId( PropertyId $propertyId ) {
+ public function getWithPropertyId( PropertyId $propertyId ) {
$results = array ();
foreach ( $this->results as $result ) {
diff --git a/includes/DumpMetaInformation.php b/includes/DumpMetaInformation.php
index fd61f05..25c57d8 100644
--- a/includes/DumpMetaInformation.php
+++ b/includes/DumpMetaInformation.php
@@ -6,6 +6,7 @@
use DateTime;
use DateTimeZone;
use Doctrine\Instantiator\Exception\InvalidArgumentException;
+use Doctrine\Instantiator\Exception\UnexpectedValueException;
use Wikibase\DataModel\Entity\ItemId;
@@ -230,6 +231,9 @@
$dumpId = $row->dump_id;
$sourceItemId = new ItemId( $row->source_qid );
$importDate = DateTime::createFromFormat( 'YmdHis',
$row->import_date, new DateTimeZone( 'UTC' ) );
+ if ( !$importDate ) {
+ throw new UnexpectedValueException( 'Cannot
convert import_date from violations table to YmdHis format.' );
+ }
$language = $row->language;
$sourceUrl = $row->source_url;
$size = (int)$row->size;
diff --git a/includes/Serializer/IndexedTagsSerializer.php
b/includes/Serializer/IndexedTagsSerializer.php
index f519270..06aa86a 100644
--- a/includes/Serializer/IndexedTagsSerializer.php
+++ b/includes/Serializer/IndexedTagsSerializer.php
@@ -39,7 +39,7 @@
* @param mixed $object
* @return array|int|string|bool|float A possibly nested structure
consisting of only arrays and scalar values
*/
- public abstract function serialize( $object );
+ abstract public function serialize( $object );
/**
* @return bool
diff --git a/includes/UpdateTable/ImportContext.php
b/includes/UpdateTable/ImportContext.php
index 8d995ce..b674877 100644
--- a/includes/UpdateTable/ImportContext.php
+++ b/includes/UpdateTable/ImportContext.php
@@ -45,7 +45,7 @@
* @param bool $quiet
* @param string $tarFilePath
*/
- function __construct( LoadBalancer $loadBalancer, $batchSize, $quiet,
$tarFilePath ) {
+ public function __construct( LoadBalancer $loadBalancer, $batchSize,
$quiet, $tarFilePath ) {
$this->setLoadBalancer( $loadBalancer );
$this->setBatchSize( $batchSize );
$this->setQuiet( $quiet );
diff --git a/includes/UpdateTable/Importer.php
b/includes/UpdateTable/Importer.php
index 56b9704..60d0e39 100644
--- a/includes/UpdateTable/Importer.php
+++ b/includes/UpdateTable/Importer.php
@@ -55,7 +55,7 @@
/**
* Starts the whole import process
*/
- function import() {
+ public function import() {
$this->extractTarFile();
$db = $this->establishDbConnection();
@@ -254,35 +254,17 @@
print "Insert new identifier properties\n";
}
- $csvFile = fopen( $this->identifierPropertiesFilePath, 'rb' );
-
- $i = 0;
- $accumulator = array();
- while ( true ) {
- $data = fgetcsv( $csvFile );
- if ( $data === false || ++$i %
$this->importContext->getBatchSize() === 0 ) {
- $db->commit( __METHOD__, 'flush' );
- wfWaitForSlaves();
- $db->insert( DUMP_IDENTIFIER_PROPERTIES_TABLE, $accumulator );
- if ( !$this->importContext->isQuiet() ) {
- print "\r\033[K";
- print "$i rows inserted";
- }
-
- $accumulator = array();
-
- if ( $data === false ) {
- break;
- }
+ $this->insertData(
+ $db,
+ $this->identifierPropertiesFilePath,
+ DUMP_IDENTIFIER_PROPERTIES_TABLE,
+ function( $data ) {
+ return array(
+ 'identifier_pid' => $data[ 0 ],
+ 'dump_id' => $data[ 1 ]
+ );
}
-
- $accumulator[ ] = array(
- 'identifier_pid' => $data[ 0 ],
- 'dump_id' => $data[ 1 ]
- );
- }
-
- fclose( $csvFile );
+ );
if ( !$this->importContext->isQuiet() ) {
print "\n";
@@ -299,7 +281,32 @@
print "Insert new data values\n";
}
- $csvFile = fopen( $this->externalValuesFilePath, 'rb' );
+ $this->insertData(
+ $db,
+ $this->externalValuesFilePath,
+ DUMP_DATA_TABLE,
+ function( $data ) {
+ return array(
+ 'dump_id' => $data[ 0 ],
+ 'external_id' => $data[ 1 ],
+ 'pid' => $data[ 2 ],
+ 'external_value' => $data[ 3 ]
+ );
+ }
+ );
+
+ if ( !$this->importContext->isQuiet() ) {
+ print "\n";
+ }
+ }
+
+ /**
+ * Inserts values stored in csv file into database
+ *
+ * @param $csvFile
+ */
+ protected function insertData( DatabaseBase $db, $filePath, $tableName,
$accumulatorBuilder ) {
+ $csvFile = fopen( $filePath, 'rb' );
$i = 0;
$accumulator = array();
@@ -308,7 +315,7 @@
if ( $data === false || ++$i %
$this->importContext->getBatchSize() === 0 ) {
$db->commit( __METHOD__, 'flush' );
wfWaitForSlaves();
- $db->insert( DUMP_DATA_TABLE, $accumulator );
+ $db->insert( $tableName, $accumulator );
if ( !$this->importContext->isQuiet() ) {
print "\r\033[K";
print "$i rows inserted";
@@ -321,18 +328,9 @@
}
}
- $accumulator[ ] = array(
- 'dump_id' => $data[ 0 ],
- 'external_id' => $data[ 1 ],
- 'pid' => $data[ 2 ],
- 'external_value' => $data[ 3 ],
- );
+ $accumulator[ ] = $accumulatorBuilder( $data );
}
fclose( $csvFile );
-
- if ( !$this->importContext->isQuiet() ) {
- print "\n";
- }
}
}
\ No newline at end of file
diff --git a/maintenance/UpdateTable.php b/maintenance/UpdateTable.php
index 42a3b8b..a6640c8 100644
--- a/maintenance/UpdateTable.php
+++ b/maintenance/UpdateTable.php
@@ -23,14 +23,14 @@
*/
class UpdateTable extends Maintenance {
- function __construct() {
+ public function __construct() {
parent::__construct();
$this->mDescription = "Downloads dumps of external databases and
imports the entities into the local database.";
$this->addOption( 'tar-file', 'TAR file that contains external data
for import.', true, true );
$this->setBatchSize( 1000 );
}
- function execute() {
+ public function execute() {
wfWaitForSlaves();
$loadBalancer = wfGetLB();
diff --git a/specials/SpecialCrossCheck.php b/specials/SpecialCrossCheck.php
index 8a84b7b..64dc025 100755
--- a/specials/SpecialCrossCheck.php
+++ b/specials/SpecialCrossCheck.php
@@ -19,7 +19,7 @@
class SpecialCrossCheck extends SpecialCheckResultPage {
- function __construct() {
+ public function __construct() {
parent::__construct( 'CrossCheck' );
}
diff --git a/specials/SpecialExternalDbs.php b/specials/SpecialExternalDbs.php
index ce455f6..6897fcf 100644
--- a/specials/SpecialExternalDbs.php
+++ b/specials/SpecialExternalDbs.php
@@ -17,7 +17,7 @@
class SpecialExternalDbs extends SpecialWikidataQualityPage {
- function __construct() {
+ public function __construct() {
parent::__construct( 'ExternalDbs' );
}
@@ -35,7 +35,7 @@
*
* @param string|null $subPage
*/
- function execute( $subPage ) {
+ public function execute( $subPage ) {
$out = $this->getOutput();
$this->setHeaders();
@@ -155,7 +155,7 @@
$dateFormatPreference = $this->getUser()->getDatePreference();
switch ( $dateFormatPreference ) {
- case 'mdy';
+ case 'mdy':
$dateFormat = 'H:i, F d, Y';
break;
@@ -172,7 +172,6 @@
break;
default:
- case 'default':
$dateFormat = 'Y-m-d H:i:s';
break;
}
--
To view, visit https://gerrit.wikimedia.org/r/206110
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I53097cfff316760b1c2f214dbe4e780ca9f654fe
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/WikidataQualityExternalValidation
Gerrit-Branch: master
Gerrit-Owner: Tamslo <tamaraslosa...@gmail.com>
Gerrit-Reviewer: Soeren.oldag <soeren_ol...@freenet.de>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
