Dominic.sauer has submitted this change and it was merged.
Change subject: Implemented further hints from review from Daniel.
......................................................................
Implemented further hints from review from Daniel.
Bug: T100991
Change-Id: I711f5541b427da9344736aca34124f35c24b53b1
---
M WikibaseQuality.php
M api/GetViolationMessages.php
M api/GetViolationTypes.php
M api/ModifyViolation.php
M includes/Serializer/ViolationSerializer.php
M includes/Violations/SqlViolationRepo.php
M includes/Violations/Violation.php
M includes/Violations/ViolationQuery.php
M includes/Violations/ViolationStore.php
M includes/WikibaseQualityFactory.php
M specials/SpecialViolationsPage.php
M tests/phpunit/Api/GetViolationMessagesTest.php
M tests/phpunit/Api/ModifyViolationTest.php
M tests/phpunit/Serializer/ViolationSerializerTest.php
M tests/phpunit/Specials/SpecialViolationsPageTest.php
M tests/phpunit/Violations/DispatchingViolationFormatterTest.php
M tests/phpunit/Violations/SqlViolationRepoTest.php
M tests/phpunit/Violations/ViolationQueryTest.php
M tests/phpunit/Violations/ViolationTest.php
19 files changed, 409 insertions(+), 194 deletions(-)
Approvals:
Dominic.sauer: Verified; Looks good to me, approved
diff --git a/WikibaseQuality.php b/WikibaseQuality.php
index 081ed76..64d565c 100755
--- a/WikibaseQuality.php
+++ b/WikibaseQuality.php
@@ -6,12 +6,12 @@
call_user_func( function () {
// Set credits
$GLOBALS['wgExtensionCredits']['specialpage'][] = array(
- 'path' => __FILE__,
+ 'path' => __DIR__,
'name' => 'WikibaseQuality',
'author' => 'BP2014N1',
'url' =>
'https://www.mediawiki.org/wiki/Extension:WikibaseQuality',
'descriptionmsg' => 'wbq-desc',
- 'version' => '1.0.0'
+ 'version' => '0.1.0'
);
// Initialize localization and aliases
@@ -80,9 +80,6 @@
'class' => 'WikibaseQuality\Api\GetViolationTypes',
'factory' =>
'WikibaseQuality\ApiModuleFactory::newGetViolationTypes'
);
-
- // Define database table names
- define( 'VIOLATION_TABLE', 'wbq_violations' );
// Define user right
$GLOBALS['wgGroupPermissions']['autoconfirmed']['wikibase-violation-exception']
= true;
diff --git a/api/GetViolationMessages.php b/api/GetViolationMessages.php
index 459b522..9af10fe 100755
--- a/api/GetViolationMessages.php
+++ b/api/GetViolationMessages.php
@@ -2,9 +2,10 @@
namespace WikibaseQuality\Api;
+use MWException;
use ApiMain;
use DataValues\Serializers;
-use Doctrine\Instantiator\Exception\UnexpectedValueException;
+use UnexpectedValueException;
use Wikibase\Api\ApiWikibase;
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Entity\EntityIdParser;
@@ -99,9 +100,13 @@
$query = new ViolationQuery();
$query->setEntityId( $entityId );
$query->setStatuses( array( Violation::STATUS_VIOLATION ) );
- $violations = $this->violationLookup->get( $query );
- return $violations;
+ try {
+ return $this->violationLookup->get( $query );
+ }
+ catch( MWException $e ) {
+ $this->dieException( $e, 'database-error' );
+ }
}
/**
diff --git a/api/GetViolationTypes.php b/api/GetViolationTypes.php
index 5b322cc..c905363 100644
--- a/api/GetViolationTypes.php
+++ b/api/GetViolationTypes.php
@@ -3,7 +3,7 @@
namespace WikibaseQuality\Api;
use ApiMain;
-use Doctrine\Instantiator\Exception\InvalidArgumentException;
+use InvalidArgumentException;
use Wikibase\Api\ApiWikibase;
use WikibaseQuality\Violations\DispatchingViolationContext;
use WikibaseQuality\WikibaseQualityFactory;
diff --git a/api/ModifyViolation.php b/api/ModifyViolation.php
index 918b3b4..12068ae 100755
--- a/api/ModifyViolation.php
+++ b/api/ModifyViolation.php
@@ -111,18 +111,38 @@
$query->setClaimGuid( $claimGuid );
$query->setConstraintId( $constraintId );
- $violations = $this->violationLookup->get( $query );
- if ( !$violations || count( $violations ) !== 1 ) {
+ try {
+ $violationsFromDb = $this->violationLookup->get( $query
);
+ }
+ catch( MWException $e ) {
+ $this->dieException( $e, 'database-error' );
}
+
+ if ( !$violationsFromDb || count( $violationsFromDb ) !== 1 ) {
$this->dieError( 'Violations with given identifiers
does not exist!', 'missing-violation' );
}
- $violation = $violations[0];
- if ( !$this->checkPermission( $violation ) ) {
+ $violationFromDb = $violationsFromDb[0];
+ if ( !$this->checkPermission( $violationFromDb ) ) {
$this->dieError( 'You do not have the permission to
change this violation!', 'permissiondenied' );
}
- $violation->setStatus( $newStatus );
- return $this->violationStore->save( $violation, true );
+ $violation = new Violation(
+ $violationFromDb->getEntityId(),
+ $violationFromDb->getPropertyId(),
+ $violationFromDb->getClaimGuid(),
+ $violationFromDb->getConstraintId(),
+ $violationFromDb->getConstraintTypeEntityId(),
+ $violationFromDb->getRevisionId(),
+ $newStatus,
+ $violationFromDb->getAdditionalInfo()
+ );
+
+ try {
+ return $this->violationStore->update( $violation, true
);
+ }
+ catch( MWException $e ) {
+ $this->dieException( $e, 'database-error' );
+ }
}
/**
diff --git a/includes/Serializer/ViolationSerializer.php
b/includes/Serializer/ViolationSerializer.php
index 47ac439..59379cf 100755
--- a/includes/Serializer/ViolationSerializer.php
+++ b/includes/Serializer/ViolationSerializer.php
@@ -49,7 +49,7 @@
// TODO: ->getSerialization (when it really is an
EntityId and not just a string)
'constraintTypeEntityId' =>
$violation->getConstraintTypeEntityId(),
'additionalInfo' => $violation->getAdditionalInfo(),
- 'updatedAt' =>
$violation->getUpdatedAt()->getTimestamp( TS_MW ),
+ 'updatedAt' => $violation->getUpdatedAt(),
'revisionId' => $violation->getRevisionId(),
'status' => $violation->getStatus()
);
diff --git a/includes/Violations/SqlViolationRepo.php
b/includes/Violations/SqlViolationRepo.php
index ca1f982..54f6e35 100755
--- a/includes/Violations/SqlViolationRepo.php
+++ b/includes/Violations/SqlViolationRepo.php
@@ -4,7 +4,7 @@
use DatabaseBase;
use ResultWrapper;
-use Doctrine\Instantiator\Exception\InvalidArgumentException;
+use InvalidArgumentException;
use MWTimestamp;
use Wikibase\DataModel\Entity\EntityIdParser;
use Wikibase\DataModel\Entity\PropertyId;
@@ -21,26 +21,17 @@
const ORDER_ASCENDING = 'ASC';
const ORDER_DESCENDING = 'DESC';
- /**
- * @var string
- */
- private $tableName;
+ const TABLE_NAME = 'wbq_violations';
/**
* @var EntityIdParser
*/
private $entityIdParser;
- /**
- * @param $tableName
- * @param EntityIdParser $entityIdParser
- */
- public function __construct( $tableName, EntityIdParser $entityIdParser ) {
- if ( !is_string( $tableName ) ) {
- throw new InvalidArgumentException( '$tableName msut be string.' );
- }
-
- $this->tableName = $tableName;
+ /**
+ * @param EntityIdParser $entityIdParser
+ */
+ public function __construct( EntityIdParser $entityIdParser ) {
$this->entityIdParser = $entityIdParser;
}
@@ -53,7 +44,7 @@
public function get( ViolationQuery $query ) {
$db = wfGetDB( DB_SLAVE );
$result = $db->select(
- $this->tableName,
+ self::TABLE_NAME,
'*',
$this->buildConditions( $db, $query )
);
@@ -74,7 +65,7 @@
* @param string $direction
* @param string $claimGuid
* @param string $constraintId
- * @return array
+ * @return array list( $violations, $prevPageAvailable,
$nextPageAvailable )
*/
public function getForPage( ViolationQuery $query, $maxNumber, $direction,
$claimGuid, $constraintId ) {
if ( !is_int( $maxNumber ) ) {
@@ -100,7 +91,7 @@
$firstKey = $this->getFirstKey( $db, $queryConditions, $direction );
$result = $db->select(
- $this->tableName,
+ self::TABLE_NAME,
'*',
$conditions,
__METHOD__,
@@ -113,42 +104,49 @@
return $this->getViolationsForPageFromResult( $result, $firstKey,
$direction, $maxNumber );
}
- /**
- * @see ViolationStore::save
- *
- * @param Violation $violation
- * @param bool $overwriteException
- * @return bool
- */
- public function save( Violation $violation, $overwriteException = false ) {
- $db = wfGetDB( DB_MASTER );
+ /**
+ * @param Violation $violation
+ * @return bool
+ */
+ public function insert( Violation $violation ) {
+ $db = wfGetDB( DB_MASTER );
- $conditions = array(
- sprintf( 'claim_guid="%s"', $violation->getClaimGuid() ),
- sprintf( 'constraint_id="%s"', $violation->getConstraintId() )
- );
- $existing = $db->selectRow(
- $this->tableName,
- '*',
- $conditions
- );
- $violation->setUpdatedAt( new MWTimestamp( $db->timestamp() ) );
- if ( $existing ) {
- if ( !$overwriteException && $this->getViolationFromRow( $existing
)->getStatus() === Violation::STATUS_EXCEPTION ) {
- $violation->setStatus( Violation::STATUS_EXCEPTION );
- }
- return $db->update(
- $this->tableName,
- $this->getRowFromViolation( $db, $violation ),
- $conditions
- );
- } else {
- return $db->insert(
- $this->tableName,
- $this->getRowFromViolation( $db, $violation )
- );
- }
- }
+ if( $this->violationExists( $db, $violation ) ) {
+ throw new InvalidArgumentException( 'Given violation
already exists in database.' );
+ }
+
+ return $db->insert(
+ self::TABLE_NAME,
+ $this->getRowFromViolation( $db, $violation )
+ );
+ }
+
+ /**
+ * @param Violation $violation
+ * @param bool $overwriteException
+ * @return bool
+ */
+ public function update( Violation $violation, $overwriteException =
false ) {
+ $db = wfGetDB( DB_MASTER );
+
+ $violationRow = $this->violationExists( $db, $violation );
+ if( $violationRow ) {
+ if ( !$overwriteException &&
$this->getViolationFromRow( $violationRow )->getStatus() ===
Violation::STATUS_EXCEPTION ) {
+ $violationRow = $this->getRowFromViolation(
$db, $violation, Violation::STATUS_EXCEPTION );
+ }
+ else {
+ $violationRow = $this->getRowFromViolation(
$db, $violation);
+ }
+
+ return $db->update(
+ self::TABLE_NAME,
+ $violationRow,
+ $this->buildKeyConditions( $violation )
+ );
+ }
+
+ throw new InvalidArgumentException( 'Given violation does not
exist in database.' );
+ }
/**
* @see ViolationStore::delete
@@ -160,13 +158,41 @@
public function delete( Violation $violation ) {
$db = wfGetDB( DB_MASTER );
return $db->delete(
- $this->tableName,
+ self::TABLE_NAME,
array(
sprintf( 'claim_guid="%s"', $violation->getClaimGuid() ),
sprintf( 'constraint_id="%s"', $violation->getConstraintId() )
)
);
}
+
+ /**
+ * Checks, if violation exists in database. In this case, database row
of it is returned; otherwise false.
+ *
+ * @param DatabaseBase $db
+ * @param Violation $violation
+ * @return bool|\stdClass
+ */
+ private function violationExists( DatabaseBase $db, Violation
$violation ) {
+ return $db->selectRow(
+ self::TABLE_NAME,
+ '*',
+ $this->buildKeyConditions( $violation )
+ );
+ }
+
+ /**
+ * Builds conditions array for primary key of a violation.
+ *
+ * @param Violation $violation
+ * @return array
+ */
+ private function buildKeyConditions( Violation $violation ) {
+ return array(
+ 'claim_guid' => $violation->getClaimGuid(),
+ 'constraint_id' => $violation->getConstraintId()
+ );
+ }
/**
* Builds array of query conditions for pagination.
@@ -209,7 +235,7 @@
*/
private function getFirstKey( DatabaseBase $db, array $conditions,
$direction ) {
$row = $db->selectRow(
- $this->tableName,
+ self::TABLE_NAME,
array(
'claim_guid',
'constraint_id'
@@ -287,7 +313,7 @@
* @param array $firstKey
* @param string $direction
* @param int $maxNumber
- * @return array
+ * @return array list( $violations, $prevPageAvailable,
$nextPageAvailable )
*/
private function getViolationsForPageFromResult( ResultWrapper $result,
array $firstKey, $direction, $maxNumber ) {
$violations = array();
@@ -339,27 +365,30 @@
(int)$row->revision_id,
$row->status,
$additionalInformation,
- new MWTimestamp( $row->updated_at )
+ $row->updated_at
);
}
- /**
- * Gets array from Violation object for database insertions.
- *
- * @param Violation $violation
- * @return array
- */
- private function getRowFromViolation( DatabaseBase $db, Violation
$violation ) {
+ /**
+ * Gets array from Violation object for database insertions.
+ * If optional status parameter is provided, status of violation will
be overridden.
+ *
+ * @param DatabaseBase $db
+ * @param Violation $violation
+ * @param string|null $status
+ * @return array
+ */
+ private function getRowFromViolation( DatabaseBase $db, Violation
$violation, $status = null ) {
return array(
'entity_id' => $violation->getEntityId()->getSerialization(),
'pid' => $violation->getPropertyId()->getSerialization(),
'claim_guid' => $violation->getClaimGuid(),
'constraint_id' => $violation->getConstraintId(),
'constraint_type_entity_id' =>
$violation->getConstraintTypeEntityId(),
- 'status' => $violation->getStatus(),
+ 'status' => $status ?: $violation->getStatus(),
'revision_id' => $violation->getRevisionId(),
'additional_info' => json_encode( $violation->getAdditionalInfo()
),
- 'updated_at' => $db->timestamp(
$violation->getUpdatedAt()->getTimestamp() )
+ 'updated_at' => $db->timestamp()
);
}
}
\ No newline at end of file
diff --git a/includes/Violations/Violation.php
b/includes/Violations/Violation.php
index 42a298e..26265f1 100755
--- a/includes/Violations/Violation.php
+++ b/includes/Violations/Violation.php
@@ -3,7 +3,6 @@
namespace WikibaseQuality\Violations;
use InvalidArgumentException;
-use MWTimestamp;
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Entity\PropertyId;
@@ -97,11 +96,11 @@
* @param int $revisionId
* @param string $status
* @param array $additionalInfo
- * @param MWTimestamp $updatedAt
+ * @param string $updatedAt
*/
// TODO: Argument 5 --> EntityId as TypeHint
- public function __construct( EntityId $entityId, PropertyId
$propertyId, $claimGuid, $constraintId,
-
$constraintTypeEntityId, $revisionId, $status, array $additionalInfo = array(),
MWTimestamp $updatedAt = null ) {
+ public function __construct( EntityId $entityId, PropertyId
$propertyId, $claimGuid, $constraintId, $constraintTypeEntityId,
+ $revisionId,
$status, array $additionalInfo = array(), $updatedAt = null ) {
$this->entityId = $entityId;
$this->propertyId = $propertyId;
$this->setClaimGuid( $claimGuid );
@@ -137,7 +136,7 @@
*/
private function setRevisionId( $revisionId ) {
if ( !is_int( $revisionId ) ) {
- throw new InvalidArgumentException( '$revisionId must
be of type int' );
+ throw new InvalidArgumentException( '$revisionId must
be int.' );
}
$this->revisionId = $revisionId;
}
@@ -185,17 +184,26 @@
}
/**
- * @return MWTimestamp
+ * @return string
*/
public function getUpdatedAt() {
return $this->updatedAt;
}
/**
- * @param MWTimestamp $updatedAt
+ * @param string $updatedAt
*/
- public function setUpdatedAt( MWTimestamp $updatedAt ) {
- $this->updatedAt = $updatedAt;
+ private function setUpdatedAt( $updatedAt ) {
+ if ( !is_string( $updatedAt ) ) {
+ throw new InvalidArgumentException( '$updatedAt must be
string.' );
+ }
+
+ $timestamp = wfTimestamp( TS_MW, $updatedAt );
+ if( !$timestamp ) {
+ throw new InvalidArgumentException( '$updatedAt has
invalid timestamp format!' );
+ }
+
+ $this->updatedAt = $timestamp;
}
/**
@@ -215,7 +223,7 @@
/**
* @param string $status
*/
- public function setStatus( $status ) {
+ private function setStatus( $status ) {
$violationStatuses = array(
Violation::STATUS_EXCEPTION,
Violation::STATUS_UNVERIFIED,
diff --git a/includes/Violations/ViolationQuery.php
b/includes/Violations/ViolationQuery.php
index a0e7c64..4777f4f 100755
--- a/includes/Violations/ViolationQuery.php
+++ b/includes/Violations/ViolationQuery.php
@@ -2,8 +2,7 @@
namespace WikibaseQuality\Violations;
-use Doctrine\Instantiator\Exception\InvalidArgumentException;
-use MWTimestamp;
+use InvalidArgumentException;
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Entity\PropertyId;
@@ -47,7 +46,7 @@
private $constraintTypeEntityId;
/**
- * @var MwTimeStamp
+ * @var string
*/
private $updatedAt;
@@ -110,7 +109,7 @@
}
/**
- * @return MWTimestamp
+ * @return string
*/
public function getUpdatedAt() {
return $this->updatedAt;
@@ -190,10 +189,19 @@
}
/**
- * @param MWTimestamp $updatedAt
+ * @param string $updatedAt
*/
- public function setUpdatedAt( MWTimestamp $updatedAt ) {
- $this->updatedAt = $updatedAt;
+ public function setUpdatedAt( $updatedAt ) {
+ if ( !is_string( $updatedAt ) ) {
+ throw new InvalidArgumentException( '$updatedAt must be
string!' );
+ }
+
+ $timestamp = wfTimestamp( TS_MW, $updatedAt );
+ if( !$timestamp ) {
+ throw new InvalidArgumentException( '$updatedAt has
invalid timestamp format!' );
+ }
+
+ $this->updatedAt = $timestamp;
}
/**
diff --git a/includes/Violations/ViolationStore.php
b/includes/Violations/ViolationStore.php
index 8821f99..8893cd5 100644
--- a/includes/Violations/ViolationStore.php
+++ b/includes/Violations/ViolationStore.php
@@ -12,13 +12,21 @@
interface ViolationStore {
/**
- * Saves given violation in database.
+ * Inserts given violation into database.
+ *
+ * @param Violation $violation
+ * @return mixed
+ */
+ public function insert( Violation $violation );
+
+ /**
+ * Updates given violation in database.
*
* @param Violation $violation
* @param bool $overwriteException
- * @return bool
+ * @return mixed
*/
- public function save( Violation $violation, $overwriteException = false
);
+ public function update( Violation $violation, $overwriteException =
false );
/**
* Deletes given violation from database.
diff --git a/includes/WikibaseQualityFactory.php
b/includes/WikibaseQualityFactory.php
index 072064c..0894665 100644
--- a/includes/WikibaseQualityFactory.php
+++ b/includes/WikibaseQualityFactory.php
@@ -67,10 +67,7 @@
*/
private function getSqlViolationRepo() {
if( $this->violationRepo === null ) {
- $this->violationRepo = new SqlViolationRepo(
- VIOLATION_TABLE,
-
WikibaseRepo::getDefaultInstance()->getEntityIdParser()
- );
+ $this->violationRepo = new SqlViolationRepo(
WikibaseRepo::getDefaultInstance()->getEntityIdParser() );
}
return $this->violationRepo;
diff --git a/specials/SpecialViolationsPage.php
b/specials/SpecialViolationsPage.php
index 100222b..6b135a5 100644
--- a/specials/SpecialViolationsPage.php
+++ b/specials/SpecialViolationsPage.php
@@ -2,7 +2,7 @@
namespace WikibaseQuality\Specials;
-use Doctrine\Instantiator\Exception\UnexpectedValueException;
+use UnexpectedValueException;
use Html;
use InvalidArgumentException;
use Linker;
@@ -408,7 +408,7 @@
}
/**
- * @return array
+ * @return array list( $violations, $prevPageAvailable,
$nextPageAvailable )
*/
private function getViolations() {
$request = $this->getRequest();
@@ -544,7 +544,7 @@
$claim = $this->formatClaimGuid(
$violation->getEntityId(), $violation->getClaimGuid() );
$type = $violation->getConstraintTypeEntityId(); //
TODO better format when format of type is clear
$status = $this->formatStatus( $violation );
- $updatedAt =
$violation->getUpdatedAt()->getHumanTimestamp();
+ $updatedAt = $this->getLanguage()->date(
$violation->getUpdatedAt(), true );
try {
$additionalInformation =
$this->violationFormatter->formatAdditionalInformation( $violation );
} catch ( UnexpectedValueException $e ) {
diff --git a/tests/phpunit/Api/GetViolationMessagesTest.php
b/tests/phpunit/Api/GetViolationMessagesTest.php
index 6010344..a6a6da7 100755
--- a/tests/phpunit/Api/GetViolationMessagesTest.php
+++ b/tests/phpunit/Api/GetViolationMessagesTest.php
@@ -3,6 +3,7 @@
namespace WikibaseQuality\Tests\Api;
use Wikibase\Test\Api\WikibaseApiTestCase;
+use WikibaseQuality\Violations\SqlViolationRepo;
/**
@@ -20,7 +21,7 @@
protected function setup() {
parent::setup();
- $this->tablesUsed[] = VIOLATION_TABLE;
+ $this->tablesUsed[] = SqlViolationRepo::TABLE_NAME;
$mock = $this->getViolationFormatterMock( 'foo', 'bar',
'foobar' );
$subExtensions = array(
@@ -35,12 +36,12 @@
public function addDBData() {
$this->db->delete(
- VIOLATION_TABLE,
+ SqlViolationRepo::TABLE_NAME,
'*'
);
$this->db->insert(
- VIOLATION_TABLE,
+ SqlViolationRepo::TABLE_NAME,
array(
array(
'entity_id' => 'Q1',
diff --git a/tests/phpunit/Api/ModifyViolationTest.php
b/tests/phpunit/Api/ModifyViolationTest.php
index 5aae6e8..91ebbec 100755
--- a/tests/phpunit/Api/ModifyViolationTest.php
+++ b/tests/phpunit/Api/ModifyViolationTest.php
@@ -4,6 +4,7 @@
use TestUser;
use Wikibase\Test\Api\WikibaseApiTestCase;
+use WikibaseQuality\Violations\SqlViolationRepo;
/**
* @covers WikibaseQuality\Api\ModifyViolation
@@ -25,7 +26,7 @@
protected function setup() {
parent::setup();
- $this->tablesUsed[] = VIOLATION_TABLE;
+ $this->tablesUsed[] = SqlViolationRepo::TABLE_NAME;
$user = new TestUser( 'ModifyViolationTestUser' );
$this->testUser = $user->getUser();
@@ -40,12 +41,12 @@
public function addDBData() {
$this->db->delete(
- VIOLATION_TABLE,
+ SqlViolationRepo::TABLE_NAME,
'*'
);
$this->db->insert(
- VIOLATION_TABLE,
+ SqlViolationRepo::TABLE_NAME,
array(
array(
'entity_id' => 'Q1',
@@ -142,7 +143,7 @@
$result = $this->doApiRequestWithToken( $params, null,
$this->testUser );
$this->assertSelect(
- VIOLATION_TABLE,
+ SqlViolationRepo::TABLE_NAME,
'status',
array(
'claim_guid="P1$aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"',
diff --git a/tests/phpunit/Serializer/ViolationSerializerTest.php
b/tests/phpunit/Serializer/ViolationSerializerTest.php
index 10921c5..8f83c9b 100755
--- a/tests/phpunit/Serializer/ViolationSerializerTest.php
+++ b/tests/phpunit/Serializer/ViolationSerializerTest.php
@@ -46,7 +46,7 @@
1234,
'exception',
array( 'foo' => 'bar' ),
- new MWTimestamp( '2014-10-15T15:00:00Z' )
+ '20141015150000'
);
$serializedViolation = $this->violationSerializer->serialize(
$violation );
@@ -59,7 +59,7 @@
$this->assertEquals( $violation->getRevisionId(),
$serializedViolation['revisionId'] );
$this->assertEquals( $violation->getStatus(),
$serializedViolation['status'] );
$this->assertEquals( $violation->getAdditionalInfo(),
$serializedViolation['additionalInfo'] );
- $this->assertEquals( $violation->getUpdatedAt()->getTimestamp(
TS_MW ), $serializedViolation['updatedAt'] );
+ $this->assertEquals( $violation->getUpdatedAt(),
$serializedViolation['updatedAt'] );
}
public function testSerializeWithInvalidParam() {
diff --git a/tests/phpunit/Specials/SpecialViolationsPageTest.php
b/tests/phpunit/Specials/SpecialViolationsPageTest.php
index ee489c8..240dd52 100644
--- a/tests/phpunit/Specials/SpecialViolationsPageTest.php
+++ b/tests/phpunit/Specials/SpecialViolationsPageTest.php
@@ -14,6 +14,7 @@
use Wikibase\Repo\WikibaseRepo;
use Wikibase\Test\SpecialPageTestBase;
use WikibaseQuality\Specials\SpecialViolationsPage;
+use WikibaseQuality\Violations\SqlViolationRepo;
use WikibaseQuality\Violations\Violation;
use WikibaseQuality\WikibaseQualityFactory;
@@ -57,7 +58,7 @@
public function setUp() {
parent::setUp();
- $this->tablesUsed[] = VIOLATION_TABLE;
+ $this->tablesUsed[] = SqlViolationRepo::TABLE_NAME;
}
public function addDBData() {
@@ -106,11 +107,11 @@
}
$this->db->delete(
- VIOLATION_TABLE,
+ SqlViolationRepo::TABLE_NAME,
'*'
);
$this->db->insert(
- VIOLATION_TABLE,
+ SqlViolationRepo::TABLE_NAME,
array(
array(
'entity_id' => self::$idMap['Q1'],
diff --git a/tests/phpunit/Violations/DispatchingViolationFormatterTest.php
b/tests/phpunit/Violations/DispatchingViolationFormatterTest.php
index ce6f2d3..be77cc6 100755
--- a/tests/phpunit/Violations/DispatchingViolationFormatterTest.php
+++ b/tests/phpunit/Violations/DispatchingViolationFormatterTest.php
@@ -88,6 +88,36 @@
}
/**
+ * @dataProvider isFormatterForDataProvider
+ */
+ public function testIsFormatterFor( $violation, $expectedResult ) {
+ $actualResult =
$this->dispatchingViolationFormatter->isFormatterFor( $violation );
+
+ $this->assertEquals( $expectedResult, $actualResult );
+ }
+
+ /**
+ * Test cases for testIsFormatterFor
+ * @return array
+ */
+ public function isFormatterForDataProvider() {
+ return array(
+ array(
+ $this->getViolationMock( 'foo' ),
+ true
+ ),
+ array(
+ $this->getViolationMock( 'bar' ),
+ true
+ ),
+ array(
+ $this->getViolationMock( 'foobar' ),
+ false
+ )
+ );
+ }
+
+ /**
* @dataProvider formatAdditionalInformationDataProvider
*/
public function testFormatAdditionalInformation( $expectedResult,
$violation, $expectedException = null ) {
diff --git a/tests/phpunit/Violations/SqlViolationRepoTest.php
b/tests/phpunit/Violations/SqlViolationRepoTest.php
index 76f132d..8f60629 100755
--- a/tests/phpunit/Violations/SqlViolationRepoTest.php
+++ b/tests/phpunit/Violations/SqlViolationRepoTest.php
@@ -2,7 +2,6 @@
namespace WikibaseQuality\Tests\Violation;
-use MWTimestamp;
use Wikibase\DataModel\Entity\BasicEntityIdParser;
use Wikibase\DataModel\Entity\ItemId;
use Wikibase\DataModel\Entity\PropertyId;
@@ -48,7 +47,7 @@
123456,
'violation',
array( 'foo' => 'bar' ),
- new MWTimestamp( '20141015150000' )
+ '20141015150000'
),
new Violation(
new ItemId( 'Q84' ),
@@ -59,7 +58,7 @@
123456,
'violation',
array( 'fu' => 'bar' ),
- new MWTimestamp( '20141015150000' )
+ '20141015150000'
),
new Violation(
new ItemId( 'Q21' ),
@@ -70,7 +69,7 @@
123456,
'exception',
array( 'foo' => 'baz' ),
- new MWTimestamp( '20141015150000' )
+ '20141015150000'
),
// Violations for testGetForPage
new Violation(
@@ -82,7 +81,7 @@
123456,
'violation',
array(),
- new MWTimestamp( '20141015150000' )
+ '20141015150000'
),
new Violation(
new ItemId( 'Q1' ),
@@ -93,7 +92,7 @@
123456,
'violation',
array(),
- new MWTimestamp( '20141015150000' )
+ '20141015150000'
),
new Violation(
new ItemId( 'Q1' ),
@@ -104,7 +103,7 @@
123456,
'violation',
array(),
- new MWTimestamp( '20141015150000' )
+ '20141015150000'
),
new Violation(
new ItemId( 'Q1' ),
@@ -115,7 +114,7 @@
123456,
'violation',
array(),
- new MWTimestamp( '20141015150000' )
+ '20141015150000'
),
new Violation(
new ItemId( 'Q1' ),
@@ -126,14 +125,14 @@
123456,
'violation',
array(),
- new MWTimestamp( '20141015150000' )
+ '20141015150000'
)
);
}
public function setUp() {
parent::setUp();
- $this->tablesUsed[] = VIOLATION_TABLE;
- $this->violationRepo = new SqlViolationRepo( VIOLATION_TABLE,
new BasicEntityIdParser() );
+ $this->tablesUsed[] = SqlViolationRepo::TABLE_NAME;
+ $this->violationRepo = new SqlViolationRepo( new
BasicEntityIdParser() );
}
public function tearDown() {
unset( $this->violationRepo, $this->violations );
@@ -141,12 +140,12 @@
}
public function addDBData() {
$this->db->delete(
- VIOLATION_TABLE,
+ SqlViolationRepo::TABLE_NAME,
'*'
);
foreach( $this->violations as $violation ) {
$this->db->insert(
- VIOLATION_TABLE,
+ SqlViolationRepo::TABLE_NAME,
array (
'entity_id' =>
$violation->getEntityId()->getSerialization(),
'pid' =>
$violation->getPropertyId()->getSerialization(),
@@ -156,16 +155,12 @@
'revision_id' =>
$violation->getRevisionId(),
'status' => $violation->getStatus(),
'additional_info' => json_encode(
$violation->getAdditionalInfo() ),
- 'updated_at' => $this->db->timestamp(
$violation->getUpdatedAt()->getTimestamp() )
+ 'updated_at' => $this->db->timestamp(
$violation->getUpdatedAt() )
)
);
}
}
- public function testConstructInvalidArguments() {
- $this->setExpectedException( 'InvalidArgumentException' );
- $entityIdParserMock = $this->getMockForAbstractClass(
'Wikibase\DataModel\Entity\EntityIdParser' );
- new SqlViolationRepo( 42, $entityIdParserMock );
- }
+
/**
* @dataProvider getDataProvider
*/
@@ -188,7 +183,7 @@
$queryEverything->setConstraintTypeEntityId( 'Q84' );
$queryEverything->setRevisionId( 123456 );
$queryEverything->setStatuses( array(
Violation::STATUS_VIOLATION ) );
- $queryEverything->setUpdatedAt( new MWTimestamp(
'20141015150000' ) );
+ $queryEverything->setUpdatedAt( '20141015150000' );
$queryEmptyResult = new ViolationQuery();
$queryEmptyResult->setClaimGuid( 'none existing id' );
$queryEntityId = new ViolationQuery();
@@ -403,12 +398,14 @@
/**
- * @dataProvider saveDataProvider
+ * @dataProvider insertDataProvider
*/
- public function testSave( Violation $violation, $overwriteException,
$expectedStatus ) {
- $this->violationRepo->save( $violation, $overwriteException );
+ public function testInsert( Violation $violation, $expectedException =
null ) {
+ $this->setExpectedException( $expectedException );
+
+ $this->violationRepo->insert( $violation );
$this->assertSelect(
- VIOLATION_TABLE,
+ SqlViolationRepo::TABLE_NAME,
array(
'entity_id',
'pid',
@@ -416,13 +413,11 @@
'constraint_id',
'constraint_type_entity_id',
'revision_id',
- 'status',
- 'additional_info',
- 'updated_at'
+ 'status'
),
array(
- sprintf( 'claim_guid="%s"',
$violation->getClaimGuid() ),
- sprintf( 'constraint_id="%s"',
$violation->getConstraintId() )
+ 'claim_guid' => $violation->getClaimGuid(),
+ 'constraint_id' => $violation->getConstraintId()
),
array(
array(
@@ -432,50 +427,126 @@
$violation->getConstraintId(),
$violation->getConstraintTypeEntityId(),
(string)$violation->getRevisionId(),
- $expectedStatus,
- json_encode(
$violation->getAdditionalInfo() ),
- $this->db->timestamp(
$violation->getUpdatedAt()->getTimestamp() )
+ $violation->getStatus()
)
)
);
}
+
/**
- * Test cases for testSave
+ * Test cases for testInsert
* @return array
*/
- public function saveDataProvider() {
- $this->violations[1]->setStatus( Violation::STATUS_UNVERIFIED );
- $this->violations[2]->setStatus( Violation::STATUS_EXCEPTION );
+ public function insertDataProvider() {
return array(
- // Update violation
array(
- $this->violations[1],
+ new Violation(
+ new ItemId( 'Q1' ),
+ new PropertyId( 'P1' ),
+ 'P1$new',
+ 'newone',
+ 'Q84',
+ 123456,
+ Violation::STATUS_VIOLATION,
+ array( 'foo' => 'bar' )
+ )
+ ),
+ array(
+ new Violation(
+ new ItemId( 'Q1' ),
+ new PropertyId( 'P1' ),
+ 'P42$foobar',
+ 'foo' .
Violation::CONSTRAINT_ID_DELIMITER . 'foobar',
+ 'Q84',
+ 123456,
+ Violation::STATUS_VIOLATION,
+ array( 'foo' => 'bar' )
+ ),
+ 'InvalidArgumentException'
+ )
+ );
+ }
+
+
+ /**
+ * @dataProvider updateDataProvider
+ */
+ public function testUpdate( Violation $violation, $overWriteException,
$expectedStatus, $expectedException = null ) {
+ $this->setExpectedException( $expectedException );
+
+ $this->violationRepo->update( $violation, $overWriteException );
+ $this->assertSelect(
+ SqlViolationRepo::TABLE_NAME,
+ array(
+ 'entity_id',
+ 'pid',
+ 'claim_guid',
+ 'constraint_id',
+ 'constraint_type_entity_id',
+ 'revision_id',
+ 'status'
+ ),
+ array(
+ 'claim_guid' => $violation->getClaimGuid(),
+ 'constraint_id' => $violation->getConstraintId()
+ ),
+ array(
+ array(
+
$violation->getEntityId()->getSerialization(),
+
$violation->getPropertyId()->getSerialization(),
+ $violation->getClaimGuid(),
+ $violation->getConstraintId(),
+ $violation->getConstraintTypeEntityId(),
+ (string)$violation->getRevisionId(),
+ $expectedStatus
+ )
+ )
+ );
+ }
+
+ /**
+ * Test cases for testUpdate
+ * @return array
+ */
+ public function updateDataProvider() {
+ return array(
+ array(
+ new Violation(
+ $this->violations[1]->getEntityId(),
+ $this->violations[1]->getPropertyId(),
+ $this->violations[1]->getClaimGuid(),
+ $this->violations[1]->getConstraintId(),
+
$this->violations[1]->getConstraintTypeEntityId(),
+ $this->violations[1]->getRevisionId(),
+ Violation::STATUS_UNVERIFIED
+ ),
false,
Violation::STATUS_UNVERIFIED
),
array(
- $this->violations[2],
- false,
- Violation::STATUS_EXCEPTION
- ),
- array(
- $this->violations[2],
- true,
- Violation::STATUS_EXCEPTION
- ),
- // Insert new violation
- array(
new Violation(
- new ItemId( 'Q1' ),
- new PropertyId( 'P1' ),
- 'P1$new',
- 'newone',
- 'Q84',
- 123456,
- Violation::STATUS_VIOLATION,
- array( 'foo' => 'bar' )
+ $this->violations[2]->getEntityId(),
+ $this->violations[2]->getPropertyId(),
+ $this->violations[2]->getClaimGuid(),
+ $this->violations[2]->getConstraintId(),
+
$this->violations[2]->getConstraintTypeEntityId(),
+ $this->violations[2]->getRevisionId(),
+ Violation::STATUS_VIOLATION
),
false,
+ Violation::STATUS_EXCEPTION
+ ),
+ array(
+ new Violation(
+ $this->violations[2]->getEntityId(),
+ $this->violations[2]->getPropertyId(),
+ $this->violations[2]->getClaimGuid(),
+ $this->violations[2]->getConstraintId(),
+
$this->violations[2]->getConstraintTypeEntityId(),
+ $this->violations[2]->getRevisionId(),
+ Violation::STATUS_VIOLATION
+ ),
+ true,
Violation::STATUS_VIOLATION
),
array(
@@ -483,17 +554,19 @@
new ItemId( 'Q1' ),
new PropertyId( 'P1' ),
'P1$new',
- 'newone',
+ 'completelynewone',
'Q84',
123456,
- Violation::STATUS_VIOLATION,
- array( 'foo' => 'bar' )
+ Violation::STATUS_VIOLATION
),
true,
- Violation::STATUS_VIOLATION
+ null,
+ 'InvalidArgumentException'
)
);
}
+
+
public function testDelete() {
$violation = new Violation(
new ItemId( 'Q12345' ),
@@ -504,10 +577,10 @@
123456,
'violation',
array(),
- new MWTimestamp()
+ '20150101000000'
);
$this->db->insert(
- VIOLATION_TABLE,
+ SqlViolationRepo::TABLE_NAME,
array (
'entity_id' =>
$violation->getEntityId()->getSerialization(),
'pid' =>
$violation->getPropertyId()->getSerialization(),
@@ -517,12 +590,12 @@
'revision_id' => $violation->getRevisionId(),
'status' => $violation->getStatus(),
'additional_info' => json_encode(
$violation->getAdditionalInfo() ),
- 'updated_at' => $this->db->timestamp(
$violation->getUpdatedAt()->getTimestamp() )
+ 'updated_at' => $this->db->timestamp(
$violation->getUpdatedAt() )
)
);
$this->violationRepo->delete( $violation );
$this->assertSelect(
- VIOLATION_TABLE,
+ SqlViolationRepo::TABLE_NAME,
array( 'COUNT(claim_guid)' ),
array(
'claim_guid="deleteTest"',
diff --git a/tests/phpunit/Violations/ViolationQueryTest.php
b/tests/phpunit/Violations/ViolationQueryTest.php
index 497d9c4..db4260e 100755
--- a/tests/phpunit/Violations/ViolationQueryTest.php
+++ b/tests/phpunit/Violations/ViolationQueryTest.php
@@ -3,7 +3,6 @@
namespace WikibaseQuality\Tests\Violation;
-use MWTimestamp;
use Wikibase\DataModel\Entity\ItemId;
use Wikibase\DataModel\Entity\PropertyId;
use WikibaseQuality\Violations\Violation;
@@ -112,10 +111,13 @@
$this->assertArrayEquals( $status,
$this->violationQuery->getStatuses() );
}
- public function testUpdatedAt() {
- $timeStamp = new MWTimestamp();
- $this->violationQuery->setUpdatedAt( $timeStamp );
- $this->assertEquals( $timeStamp,
$this->violationQuery->getUpdatedAt() );
+ /**
+ * @dataProvider updatedAtDataProvider
+ */
+ public function testUpdatedAt( $timestamp, $expectedException = null ) {
+ $this->setExpectedException( $expectedException );
+ $this->violationQuery->setUpdatedAt( $timestamp );
+ $this->assertEquals( $timestamp,
$this->violationQuery->getUpdatedAt() );
}
public function stringValueDataProvider() {
@@ -178,4 +180,18 @@
)
);
}
+
+ public function updatedAtDataProvider() {
+ return array(
+ array( '20150101000000' ),
+ array(
+ 'foobar',
+ 'InvalidArgumentException'
+ ),
+ array(
+ 2015,
+ 'InvalidArgumentException'
+ )
+ );
+ }
}
diff --git a/tests/phpunit/Violations/ViolationTest.php
b/tests/phpunit/Violations/ViolationTest.php
index 93a5a00..c9abe62 100755
--- a/tests/phpunit/Violations/ViolationTest.php
+++ b/tests/phpunit/Violations/ViolationTest.php
@@ -2,7 +2,6 @@
namespace WikibaseQuality\Tests\Violation;
-use MWTimestamp;
use Wikibase\DataModel\Entity\ItemId;
use Wikibase\DataModel\Entity\PropertyId;
use WikibaseQuality\Violations\Violation;
@@ -61,7 +60,7 @@
'type' => 'JSON',
'mandatory' => 'false'
),
- new MWTimestamp( '2014-10-15T15:00:00Z' )
+ '20141015150000'
),
array(
$entityId,
@@ -93,7 +92,7 @@
$constraintId = 'P666$aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee';
$constraintTypeEntityId = new ItemId( 'Q666' );
$revisionId = 1234;
- $status = 'verified';
+ $status = Violation::STATUS_UNVERIFIED;
return array(
array(
@@ -122,6 +121,28 @@
$entityId,
$pid,
$claimGuid,
+ $claimGuid,
+ $constraintTypeEntityId,
+ $revisionId,
+ $status,
+ array(),
+ 42
+ ),
+ array(
+ $entityId,
+ $pid,
+ $claimGuid,
+ $claimGuid,
+ $constraintTypeEntityId,
+ $revisionId,
+ $status,
+ array(),
+ 'foobar'
+ ),
+ array(
+ $entityId,
+ $pid,
+ $claimGuid,
$constraintId,
$constraintTypeEntityId,
'1234',
--
To view, visit https://gerrit.wikimedia.org/r/218319
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I711f5541b427da9344736aca34124f35c24b53b1
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/WikidataQuality
Gerrit-Branch: master
Gerrit-Owner: Soeren.oldag <[email protected]>
Gerrit-Reviewer: Dominic.sauer <[email protected]>
Gerrit-Reviewer: Siebrand <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
