Tamslo has uploaded a new change for review.
https://gerrit.wikimedia.org/r/220321
Change subject: [WIP] Introduced ExceptionLookup
......................................................................
[WIP] Introduced ExceptionLookup
Bug: T103108
Change-Id: I4accc458077284d86b5636665a74498806f989f7
---
A includes/Violations/ExceptionLookup.php
M includes/Violations/SqlViolationRepo.php
M includes/WikibaseQualityFactory.php
M tests/phpunit/Violations/SqlViolationRepoTest.php
4 files changed, 329 insertions(+), 215 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/WikibaseQuality
refs/changes/21/220321/1
diff --git a/includes/Violations/ExceptionLookup.php
b/includes/Violations/ExceptionLookup.php
new file mode 100644
index 0000000..cf53868
--- /dev/null
+++ b/includes/Violations/ExceptionLookup.php
@@ -0,0 +1,23 @@
+<?php
+
+namespace WikibaseQuality\Violations;
+
+
+/**
+ * Interface ExceptionLookup
+ * @package WikibaseQuality\Violations
+ * @author BP2014N1
+ * @license GNU GPL v2+
+ */
+interface ExceptionLookup {
+
+ /**
+ * Returns whether a violation is an exception.
+ *
+ * @param string $claimGuid
+ * @param string $constraintId
+ *
+ * @return bool
+ */
+ public function isException( $claimGuid, $constraintId );
+}
\ No newline at end of file
diff --git a/includes/Violations/SqlViolationRepo.php
b/includes/Violations/SqlViolationRepo.php
index 40d5181..5628f0d 100755
--- a/includes/Violations/SqlViolationRepo.php
+++ b/includes/Violations/SqlViolationRepo.php
@@ -8,6 +8,7 @@
use MWTimestamp;
use Wikibase\DataModel\Entity\EntityIdParser;
use Wikibase\DataModel\Entity\PropertyId;
+use Wikimedia\Assert\Assert;
/**
@@ -16,93 +17,117 @@
* @author BP2014N1
* @license GNU GPL v2+
*/
-class SqlViolationRepo implements ViolationLookup, ViolationStore {
+class SqlViolationRepo implements ViolationLookup, ViolationStore,
ExceptionLookup {
- const ORDER_ASCENDING = 'ASC';
- const ORDER_DESCENDING = 'DESC';
+ const ORDER_ASCENDING = 'ASC';
+ const ORDER_DESCENDING = 'DESC';
const TABLE_NAME = 'wbq_violations';
- /**
- * @var EntityIdParser
- */
- private $entityIdParser;
+ /**
+ * @var EntityIdParser
+ */
+ private $entityIdParser;
/**
* @param EntityIdParser $entityIdParser
*/
- public function __construct( EntityIdParser $entityIdParser ) {
- $this->entityIdParser = $entityIdParser;
- }
+ public function __construct( EntityIdParser $entityIdParser ) {
+ $this->entityIdParser = $entityIdParser;
+ }
/**
- * @see ViolationLookup::get
+ * @see ExceptionLookup::isException
*
- * @param ViolationQuery $query
- * @return Violation[]
- */
- public function get( ViolationQuery $query ) {
- $db = wfGetDB( DB_SLAVE );
- $result = $db->select(
- self::TABLE_NAME,
- '*',
- $this->buildConditions( $db, $query )
- );
-
- $violations = array();
- foreach ( $result as $row ) {
- $violations[] = $this->getViolationFromRow( $row );
- }
-
- return $violations;
- }
-
- /**
- * @see ViolationRepo::getForPage
- *
- * @param ViolationQuery $query
- * @param int $maxNumber
- * @param string $direction
* @param string $claimGuid
* @param string $constraintId
- * @return array list( $violations, $prevPageAvailable,
$nextPageAvailable )
+ *
+ * @return bool
*/
- public function getForPage( ViolationQuery $query, $maxNumber, $direction,
$claimGuid, $constraintId ) {
- if ( !is_int( $maxNumber ) ) {
- throw new InvalidArgumentException( '$maxNumber must be int.' );
- }
- if ( $direction !== self::ORDER_ASCENDING && $direction !==
self::ORDER_DESCENDING ) {
- throw new InvalidArgumentException( '$direction must a valid
direction.' );
- }
- if ( $claimGuid && !is_string( $claimGuid ) ) {
- throw new InvalidArgumentException( '$claimGuid must be string.' );
- }
- if ( $constraintId && !is_string( $constraintId ) ) {
- throw new InvalidArgumentException( '$constraintId must be
string.' );
- }
- if ( $query->getClaimGuid() || $query->getConstraintId() ) {
- throw new InvalidArgumentException( 'Filtering by claim guid or
constraint id is not supported.' );
- }
+ public function isException( $claimGuid, $constraintId ) {
+ Assert::parameterType( 'string', $claimGuid, '$claimGuid' );
+ Assert::parameterType( 'string', $constraintId, '$constraintId'
);
+ $violationQuery = new ViolationQuery();
+ $violationQuery->setClaimGuid( $claimGuid );
+ $violationQuery->setConstraintId( $constraintId );
- $db = wfGetDB( DB_SLAVE );
- $queryConditions = $this->buildConditions( $db, $query );
- $pageConditions = $this->buildPaginationConditions( $db, $direction,
$claimGuid, $constraintId );
- $conditions = array_merge( $queryConditions, $pageConditions );
+ $db = wfGetDB( DB_SLAVE );
+ $status = $db->selectField(
+ self::TABLE_NAME,
+ 'status',
+ $this->buildConditions( $db, $violationQuery )
+ );
+ return $status === Violation::STATUS_EXCEPTION;
+ }
- $firstKey = $this->getFirstKey( $db, $queryConditions, $direction );
- $result = $db->select(
- self::TABLE_NAME,
- '*',
- $conditions,
- __METHOD__,
- array(
- 'ORDER BY' => "claim_guid $direction, constraint_id
$direction",
- 'LIMIT' => $maxNumber + 1
- )
- );
+ /**
+ * @see ViolationLookup::get
+ *
+ * @param ViolationQuery $query
+ * @return Violation[]
+ */
+ public function get( ViolationQuery $query ) {
+ $db = wfGetDB( DB_SLAVE );
+ $result = $db->select(
+ self::TABLE_NAME,
+ '*',
+ $this->buildConditions( $db, $query )
+ );
- return $this->getViolationsForPageFromResult( $result, $firstKey,
$direction, $maxNumber );
- }
+ $violations = array();
+ foreach ( $result as $row ) {
+ $violations[] = $this->getViolationFromRow( $row );
+ }
+
+ return $violations;
+ }
+
+ /**
+ * @see ViolationRepo::getForPage
+ *
+ * @param ViolationQuery $query
+ * @param int $maxNumber
+ * @param string $direction
+ * @param string $claimGuid
+ * @param string $constraintId
+ * @return array list( $violations, $prevPageAvailable,
$nextPageAvailable )
+ */
+ public function getForPage( ViolationQuery $query, $maxNumber,
$direction, $claimGuid, $constraintId ) {
+ if ( !is_int( $maxNumber ) ) {
+ throw new InvalidArgumentException( '$maxNumber must be
int.' );
+ }
+ if ( $direction !== self::ORDER_ASCENDING && $direction !==
self::ORDER_DESCENDING ) {
+ throw new InvalidArgumentException( '$direction must a
valid direction.' );
+ }
+ if ( $claimGuid && !is_string( $claimGuid ) ) {
+ throw new InvalidArgumentException( '$claimGuid must be
string.' );
+ }
+ if ( $constraintId && !is_string( $constraintId ) ) {
+ throw new InvalidArgumentException( '$constraintId must
be string.' );
+ }
+ if ( $query->getClaimGuid() || $query->getConstraintId() ) {
+ throw new InvalidArgumentException( 'Filtering by claim
guid or constraint id is not supported.' );
+ }
+
+ $db = wfGetDB( DB_SLAVE );
+ $queryConditions = $this->buildConditions( $db, $query );
+ $pageConditions = $this->buildPaginationConditions( $db,
$direction, $claimGuid, $constraintId );
+ $conditions = array_merge( $queryConditions, $pageConditions );
+
+ $firstKey = $this->getFirstKey( $db, $queryConditions,
$direction );
+ $result = $db->select(
+ self::TABLE_NAME,
+ '*',
+ $conditions,
+ __METHOD__,
+ array(
+ 'ORDER BY' => "claim_guid $direction,
constraint_id $direction",
+ 'LIMIT' => $maxNumber + 1
+ )
+ );
+
+ return $this->getViolationsForPageFromResult( $result,
$firstKey, $direction, $maxNumber );
+ }
/**
* @param Violation $violation
@@ -152,23 +177,23 @@
throw new InvalidArgumentException( 'Given violation does not
exist in database.' );
}
- /**
- * @see ViolationStore::delete
- *
- * @param Violation $violation
- * @return bool|\ResultWrapper
- * @throws \DBUnexpectedError
- */
- public function delete( Violation $violation ) {
- $db = wfGetDB( DB_MASTER );
- return $db->delete(
- self::TABLE_NAME,
- array(
- sprintf( 'claim_guid="%s"', $violation->getClaimGuid() ),
- sprintf( 'constraint_id="%s"', $violation->getConstraintId() )
- )
- );
- }
+ /**
+ * @see ViolationStore::delete
+ *
+ * @param Violation $violation
+ * @return bool|\ResultWrapper
+ * @throws \DBUnexpectedError
+ */
+ public function delete( Violation $violation ) {
+ $db = wfGetDB( DB_MASTER );
+ return $db->delete(
+ 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.
@@ -198,117 +223,117 @@
);
}
- /**
- * Builds array of query conditions for pagination.
- *
- * @param DatabaseBase $db
- * @param string $direction
- * @param string $claimGuid
- * @param string $constraintId
- * @return array
- */
- private function buildPaginationConditions( DatabaseBase $db, $direction,
$claimGuid, $constraintId ) {
- $conditions = array();
+ /**
+ * Builds array of query conditions for pagination.
+ *
+ * @param DatabaseBase $db
+ * @param string $direction
+ * @param string $claimGuid
+ * @param string $constraintId
+ * @return array
+ */
+ private function buildPaginationConditions( DatabaseBase $db,
$direction, $claimGuid, $constraintId ) {
+ $conditions = array();
- if ( $claimGuid && $constraintId ) {
- $operator = '>';
- $oppositeOperator = '<';
- if ( $direction === self::ORDER_DESCENDING ) {
- $operator = '<';
- $oppositeOperator = '>';
- }
+ if ( $claimGuid && $constraintId ) {
+ $operator = '>';
+ $oppositeOperator = '<';
+ if ( $direction === self::ORDER_DESCENDING ) {
+ $operator = '<';
+ $oppositeOperator = '>';
+ }
- $quotedClaimGuid = $db->addQuotes( $claimGuid );
- $quotedConstraintId = $db->addQuotes( $constraintId );
+ $quotedClaimGuid = $db->addQuotes( $claimGuid );
+ $quotedConstraintId = $db->addQuotes( $constraintId );
- $conditions[] = sprintf( 'claim_guid%s=%s', $operator,
$quotedClaimGuid );
- $conditions[] = sprintf( 'NOT (claim_guid=%s AND
constraint_id%s=%s)', $quotedClaimGuid, $oppositeOperator, $quotedConstraintId
);
- }
+ $conditions[] = sprintf( 'claim_guid%s=%s', $operator,
$quotedClaimGuid );
+ $conditions[] = sprintf( 'NOT (claim_guid=%s AND
constraint_id%s=%s)', $quotedClaimGuid, $oppositeOperator, $quotedConstraintId
);
+ }
- return $conditions;
- }
+ return $conditions;
+ }
- /**
- * Gets array with primary keys of the first (regarding specified
direction) violation
- * for the query in database.
- *
- * @param DatabaseBase $db
- * @param array $conditions
- * @param string $direction
- * @return array
- */
- private function getFirstKey( DatabaseBase $db, array $conditions,
$direction ) {
- $row = $db->selectRow(
- self::TABLE_NAME,
- array(
- 'claim_guid',
- 'constraint_id'
- ),
- $conditions,
- __METHOD__,
- array(
- 'LIMIT' => 1,
- 'ORDER BY' => "claim_guid $direction, constraint_id $direction"
- )
- );
+ /**
+ * Gets array with primary keys of the first (regarding specified
direction) violation
+ * for the query in database.
+ *
+ * @param DatabaseBase $db
+ * @param array $conditions
+ * @param string $direction
+ * @return array
+ */
+ private function getFirstKey( DatabaseBase $db, array $conditions,
$direction ) {
+ $row = $db->selectRow(
+ self::TABLE_NAME,
+ array(
+ 'claim_guid',
+ 'constraint_id'
+ ),
+ $conditions,
+ __METHOD__,
+ array(
+ 'LIMIT' => 1,
+ 'ORDER BY' => "claim_guid $direction,
constraint_id $direction"
+ )
+ );
- if ( $row ) {
- return $this->getKeyArray( $row );
- }
+ if ( $row ) {
+ return $this->getKeyArray( $row );
+ }
return array();
- }
+ }
- /**
- * Gets array with primary keys of a given row.
- *
- * @param \stdClass $row
- * @return array
- */
- private function getKeyArray( $row ) {
+ /**
+ * Gets array with primary keys of a given row.
+ *
+ * @param \stdClass $row
+ * @return array
+ */
+ private function getKeyArray( $row ) {
return array( $row->claim_guid, $row->constraint_id );
- }
+ }
- /**
- * Build array with query conditions from ViolationQuery object.
- *
- * @param DatabaseBase $db
- * @param ViolationQuery $query
- * @return array
- */
- private function buildConditions( DatabaseBase $db, ViolationQuery $query
) {
- $conditions = array();
- if ( $query->getEntityId() !== null ) {
+ /**
+ * Build array with query conditions from ViolationQuery object.
+ *
+ * @param DatabaseBase $db
+ * @param ViolationQuery $query
+ * @return array
+ */
+ private function buildConditions( DatabaseBase $db, ViolationQuery
$query ) {
+ $conditions = array();
+ if ( $query->getEntityId() !== null ) {
$conditions['entity_id'] =
$query->getEntityId()->getSerialization();
- }
- if ( $query->getPropertyId() !== null ) {
+ }
+ if ( $query->getPropertyId() !== null ) {
$conditions['pid'] =
$query->getPropertyId()->getSerialization();
- }
- if ( $query->getClaimGuid() !== null ) {
- $conditions['claim_guid'] = $query->getClaimGuid();
- }
- if ( $query->getConstraintId() !== null ) {
- $conditions['constraint_id'] = $query->getConstraintId();
- }
- if ( $query->getConstraintGroup() !== null ) {
+ }
+ if ( $query->getClaimGuid() !== null ) {
+ $conditions['claim_guid'] = $query->getClaimGuid();
+ }
+ if ( $query->getConstraintId() !== null ) {
+ $conditions['constraint_id'] =
$query->getConstraintId();
+ }
+ if ( $query->getConstraintGroup() !== null ) {
$prefix = $query->getConstraintGroup() .
Violation::CONSTRAINT_ID_DELIMITER;
$conditions[] = 'constraint_id' . $db->buildLike(
$prefix, $db->anyString() );
- }
- if ( $query->getConstraintTypeEntityId() !== null ) {
- $conditions['constraint_type_entity_id'] =
$query->getConstraintTypeEntityId();
- }
- if ( $query->getUpdatedAt() !== null ) {
- $conditions['updated_at'] = $db->timestamp( $query->getUpdatedAt()
);
- }
- if ( $query->getRevisionId() !== null ) {
- $conditions['revision_id'] = $query->getRevisionId();
- }
- if ( $query->getStatuses() !== null && count( $query->getStatuses() )
> 0 ) {
+ }
+ if ( $query->getConstraintTypeEntityId() !== null ) {
+ $conditions['constraint_type_entity_id'] =
$query->getConstraintTypeEntityId();
+ }
+ if ( $query->getUpdatedAt() !== null ) {
+ $conditions['updated_at'] = $db->timestamp(
$query->getUpdatedAt() );
+ }
+ if ( $query->getRevisionId() !== null ) {
+ $conditions['revision_id'] = $query->getRevisionId();
+ }
+ if ( $query->getStatuses() !== null && count(
$query->getStatuses() ) > 0 ) {
$conditions['status'] = $query->getStatuses();
- }
+ }
- return $conditions;
- }
+ return $conditions;
+ }
/**
* Creates violation objects from database result and checks, whether
previous or next page is available
@@ -351,27 +376,27 @@
return array( $violations, $prevPageAvailable,
$nextPageAvailable );
}
- /**
- * Creates Violation object from single row of a database result.
- *
- * @param $row
- * @return Violation
- */
- private function getViolationFromRow( $row ) {
- $additionalInformation = json_decode( $row->additional_info, true ) ?:
array();
+ /**
+ * Creates Violation object from single row of a database result.
+ *
+ * @param $row
+ * @return Violation
+ */
+ private function getViolationFromRow( $row ) {
+ $additionalInformation = json_decode( $row->additional_info,
true ) ?: array();
- return new Violation(
- $this->entityIdParser->parse( $row->entity_id ),
- new PropertyId( $row->pid ),
- $row->claim_guid,
- $row->constraint_id,
- $row->constraint_type_entity_id,
- (int)$row->revision_id,
- $row->status,
- $additionalInformation,
+ return new Violation(
+ $this->entityIdParser->parse( $row->entity_id ),
+ new PropertyId( $row->pid ),
+ $row->claim_guid,
+ $row->constraint_id,
+ $row->constraint_type_entity_id,
+ (int)$row->revision_id,
+ $row->status,
+ $additionalInformation,
$row->updated_at
- );
- }
+ );
+ }
/**
* Gets array from Violation object for database insertions.
@@ -382,17 +407,17 @@
* @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' => $status ?: $violation->getStatus(),
- 'revision_id' => $violation->getRevisionId(),
- 'additional_info' => json_encode( $violation->getAdditionalInfo()
),
- 'updated_at' => $db->timestamp()
- );
- }
+ 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' => $status ?: $violation->getStatus(),
+ 'revision_id' => $violation->getRevisionId(),
+ 'additional_info' => json_encode(
$violation->getAdditionalInfo() ),
+ 'updated_at' => $db->timestamp()
+ );
+ }
}
\ No newline at end of file
diff --git a/includes/WikibaseQualityFactory.php
b/includes/WikibaseQualityFactory.php
index 0894665..c48b2a4 100644
--- a/includes/WikibaseQualityFactory.php
+++ b/includes/WikibaseQualityFactory.php
@@ -6,13 +6,12 @@
use UnexpectedValueException;
use Wikibase\Repo\WikibaseRepo;
use WikibaseQuality\Serializer\ViolationSerializer;
-use WikibaseQuality\Violations\DispatchingViolationContext;
use WikibaseQuality\Violations\DispatchingViolationFormatter;
-use WikibaseQuality\Violations\ViolationContext;
use WikibaseQuality\Violations\SqlViolationRepo;
use WikibaseQuality\Violations\ViolationFormatter;
use WikibaseQuality\Violations\ViolationLookup;
use WikibaseQuality\Violations\ViolationStore;
+use WikibaseQuality\Violations\ExceptionLookup;
/**
* Class WikibaseQualityFactory
@@ -56,6 +55,13 @@
}
/**
+ * @return ExceptionLookup
+ */
+ public function getExceptionLookup() {
+ return $this->getSqlViolationRepo();
+ }
+
+ /**
* @return ViolationStore
*/
public function getViolationStore() {
diff --git a/tests/phpunit/Violations/SqlViolationRepoTest.php
b/tests/phpunit/Violations/SqlViolationRepoTest.php
index 8f60629..caf830e 100755
--- a/tests/phpunit/Violations/SqlViolationRepoTest.php
+++ b/tests/phpunit/Violations/SqlViolationRepoTest.php
@@ -606,4 +606,64 @@
)
);
}
+
+ public function testIsException() {
+ $violation = new Violation(
+ new ItemId( 'Q12345' ),
+ new PropertyId( 'P12345' ),
+ 'P12345$isExceptionTest',
+ 'isExceptionTestTrue',
+ 'Q42',
+ 123456,
+ 'exception',
+ array(),
+ '201501010000'
+ );
+
+ $anotherViolation = new Violation(
+ new ItemId( 'Q12345' ),
+ new PropertyId( 'P12345' ),
+ 'P12345$isExceptionTest',
+ 'isExceptionTestFalse',
+ 'Q42',
+ 123456,
+ 'exception',
+ array(),
+ '201501010000'
+ );
+
+ $this->db->insert(
+ SqlViolationRepo::TABLE_NAME,
+ array(
+ array (
+ 'entity_id' =>
$violation->getEntityId()->getSerialization(),
+ 'pid' =>
$violation->getPropertyId()->getSerialization(),
+ 'claim_guid' =>
$violation->getClaimGuid(),
+ 'constraint_id' =>
$violation->getConstraintId(),
+ 'constraint_type_entity_id' =>
$violation->getConstraintTypeEntityId(),
+ 'revision_id' =>
$violation->getRevisionId(),
+ 'status' => $violation->getStatus(),
+ 'additional_info' => json_encode(
$violation->getAdditionalInfo() ),
+ 'updated_at' => $this->db->timestamp(
$violation->getUpdatedAt() )
+ ),
+ array (
+ 'entity_id' =>
$anotherViolation->getEntityId()->getSerialization(),
+ 'pid' =>
$anotherViolation->getPropertyId()->getSerialization(),
+ 'claim_guid' =>
$anotherViolation->getClaimGuid(),
+ 'constraint_id' =>
$anotherViolation->getConstraintId(),
+ 'constraint_type_entity_id' =>
$anotherViolation->getConstraintTypeEntityId(),
+ 'revision_id' =>
$anotherViolation->getRevisionId(),
+ 'status' =>
$anotherViolation->getStatus(),
+ 'additional_info' => json_encode(
$anotherViolation->getAdditionalInfo() ),
+ 'updated_at' => $this->db->timestamp(
$anotherViolation->getUpdatedAt() )
+ )
+ )
+ );
+
+ $shouldBeTrue = $this->violationRepo->isException(
'P12345$isExceptionTest', 'isExceptionTestTrue' );
+ $shouldBeFalse = $this->violationRepo->isException(
'P12345$isExceptionTest', 'isExceptionTestFalse' );
+
+ $this->assertTrue( $shouldBeTrue );
+ $this->assertFalse( $shouldBeFalse );
+ }
}
--
To view, visit https://gerrit.wikimedia.org/r/220321
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4accc458077284d86b5636665a74498806f989f7
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/WikibaseQuality
Gerrit-Branch: master
Gerrit-Owner: Tamslo <tamaraslosa...@gmail.com>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
