Mailing List Archive

Lucas Werkmeister (WMDE) has uploaded a new change for review. ( https://gerrit.wikimedia.org/r/405293 )

Change subject: Check “difference within range” on qualifiers and references
......................................................................

Check “difference within range” on qualifiers and references

To implement this, we can simply use Context::getSiblingSnaks(), since
the semantics of that method are to ignore deprecated statements, just
like DiffWithinRangeChecker also used to do.

Unfortunately, a lot of changes in the tests are necessary:
getSiblingSnaks() does not include the current snak, and in the case of
MainSnakContext, this is achieved by removing statements with the same
GUID – so all the test cases have to be adjusted to assign some GUID to
the statements they create, otherwise too many statements are removed.

Two new tests are also added, one for qualifiers and one for references.

Bug: T175565
Change-Id: I21c4b0ec82baf461b38aca0022db3fbf70ebb651
---
M src/ConstraintCheck/Checker/DiffWithinRangeChecker.php
M tests/phpunit/Checker/RangeChecker/DiffWithinRangeCheckerTest.php
2 files changed, 168 insertions(+), 57 deletions(-)


git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/WikibaseQualityConstraints refs/changes/93/405293/1

diff --git a/src/ConstraintCheck/Checker/DiffWithinRangeChecker.php b/src/ConstraintCheck/Checker/DiffWithinRangeChecker.php
index f516bbd..e7cd7d5 100644
--- a/src/ConstraintCheck/Checker/DiffWithinRangeChecker.php
+++ b/src/ConstraintCheck/Checker/DiffWithinRangeChecker.php
@@ -67,9 +67,8 @@
public function getSupportedContextTypes() {
return [.
Context::TYPE_STATEMENT => CheckResult::STATUS_COMPLIANCE,
- // TODO T175565
- Context::TYPE_QUALIFIER => CheckResult::STATUS_TODO,
- Context::TYPE_REFERENCE => CheckResult::STATUS_TODO,
+ Context::TYPE_QUALIFIER => CheckResult::STATUS_COMPLIANCE,
+ Context::TYPE_REFERENCE => CheckResult::STATUS_COMPLIANCE,
];
}

@@ -79,9 +78,8 @@
public function getDefaultContextTypes() {
return [.
Context::TYPE_STATEMENT,
- // TODO T175565
- // Context::TYPE_QUALIFIER,
- // Context::TYPE_REFERENCE,
+ Context::TYPE_QUALIFIER,
+ Context::TYPE_REFERENCE,
];
}

@@ -162,19 +160,15 @@
list ( $min, $max, $property, $parameters ) = $this->parseConstraintParameters( $constraint );

// checks only the first occurrence of the referenced property (this constraint implies a single value constraint on that property)
- /** @var Statement $otherStatement */
- foreach ( $context->getEntity()->getStatements() as $otherStatement ) {
- $otherMainSnak = $otherStatement->getMainSnak();
-
+ foreach ( $context->getSiblingSnaks() as $otherSnak ) {
if (
- !$property->equals( $otherStatement->getPropertyId() ) ||
- $otherStatement->getRank() === Statement::RANK_DEPRECATED ||
- !$otherMainSnak instanceof PropertyValueSnak
+ !$property->equals( $otherSnak->getPropertyId() ) ||
+ !$otherSnak instanceof PropertyValueSnak
) {
continue;
}

- $subtrahend = $otherMainSnak->getDataValue();
+ $subtrahend = $otherSnak->getDataValue();
if ( $subtrahend->getType() === $minuend->getType() ) {
$diff = $this->rangeInYears( $min, $max ) ?
$this->rangeCheckerHelper->getDifferenceInYears( $minuend, $subtrahend ) :
@@ -189,7 +183,7 @@
$message->rawParams(
$this->constraintParameterRenderer->formatEntityId( $context->getSnak()->getPropertyId(), Role::PREDICATE ),
$this->constraintParameterRenderer->formatDataValue( $minuend, Role::OBJECT ),
- $this->constraintParameterRenderer->formatEntityId( $otherStatement->getPropertyId(), Role::PREDICATE ),
+ $this->constraintParameterRenderer->formatEntityId( $otherSnak->getPropertyId(), Role::PREDICATE ),
$this->constraintParameterRenderer->formatDataValue( $subtrahend, Role::OBJECT )
);
if ( $min !== null ) {
diff --git a/tests/phpunit/Checker/RangeChecker/DiffWithinRangeCheckerTest.php b/tests/phpunit/Checker/RangeChecker/DiffWithinRangeCheckerTest.php
index afe2459..daa012d 100644
--- a/tests/phpunit/Checker/RangeChecker/DiffWithinRangeCheckerTest.php
+++ b/tests/phpunit/Checker/RangeChecker/DiffWithinRangeCheckerTest.php
@@ -5,7 +5,11 @@
use DataValues\TimeValue;
use DataValues\UnboundedQuantityValue;
use Wikibase\DataModel\Entity\PropertyId;
+use Wikibase\DataModel\Reference;
+use Wikibase\DataModel\ReferenceList;
+use Wikibase\DataModel\Snak\PropertySomeValueSnak;
use Wikibase\DataModel\Snak\PropertyValueSnak;
+use Wikibase\DataModel\Snak\SnakList;
use Wikibase\DataModel\Statement\Statement;
use Wikibase\Lib\Units\CSVUnitStorage;
use Wikibase\Lib\Units\UnitConverter;
@@ -14,6 +18,8 @@
use WikibaseQuality\ConstraintReport\Constraint;
use WikibaseQuality\ConstraintReport\ConstraintCheck\Checker\DiffWithinRangeChecker;
use WikibaseQuality\ConstraintReport\ConstraintCheck\Context\MainSnakContext;
+use WikibaseQuality\ConstraintReport\ConstraintCheck\Context\QualifierContext;
+use WikibaseQuality\ConstraintReport\ConstraintCheck\Context\ReferenceContext;
use WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\RangeCheckerHelper;
use WikibaseQuality\ConstraintReport\Tests\ConstraintParameters;
use WikibaseQuality\ConstraintReport\Tests\ResultAssertions;
@@ -112,7 +118,10 @@
self::$t1900 = new TimeValue( '+00000001900-01-01T00:00:00Z', 0, 0, 0, 11, 'http://www.wikidata.org/entity/Q1985727' );
self::$t1970 = new TimeValue( '+00000001970-01-01T00:00:00Z', 0, 0, 0, 11, 'http://www.wikidata.org/entity/Q1985727' );
self::$t2000 = new TimeValue( '+00000002000-01-01T00:00:00Z', 0, 0, 0, 11, 'http://www.wikidata.org/entity/Q1985727' );
- self::$s1970 = NewStatement::forProperty( 'P570' )->withValue( self::$t1970 )->build();
+ self::$s1970 = NewStatement::forProperty( 'P570' )
+ ->withValue( self::$t1970 )
+ ->withSomeGuid()
+ ->build();
self::$i1970 = NewItem::withId( 'Q1' )->andStatement( self::$s1970 );
}

@@ -158,7 +167,11 @@

public function testDiffWithinRangeConstraintWithinRange() {
$entity = self::$i1970
- ->andStatement( NewStatement::forProperty( 'P569' )->withValue( self::$t1900 ) )
+ ->andStatement(
+ NewStatement::forProperty( 'P569' )
+ ->withValue( self::$t1900 )
+ ->withSomeGuid()
+ )
->build();
$constraint = $this->getConstraintMock( $this->dob0to150Parameters );

@@ -169,7 +182,11 @@

public function testDiffWithinRangeConstraintTooSmall() {
$entity = self::$i1970
- ->andStatement( NewStatement::forProperty( 'P569' )->withValue( self::$t2000 ) )
+ ->andStatement(
+ NewStatement::forProperty( 'P569' )
+ ->withValue( self::$t2000 )
+ ->withSomeGuid()
+ )
->build();
$constraint = $this->getConstraintMock( $this->dob0to150Parameters );

@@ -180,7 +197,11 @@

public function testDiffWithinRangeConstraintTooBig() {
$entity = self::$i1970
- ->andStatement( NewStatement::forProperty( 'P569' )->withValue( self::$t1800 ) )
+ ->andStatement(
+ NewStatement::forProperty( 'P569' )
+ ->withValue( self::$t1800 )
+ ->withSomeGuid()
+ )
->build();
$constraint = $this->getConstraintMock( $this->dob0to150Parameters );

@@ -191,9 +212,13 @@

public function testDiffWithinRangeConstraintWithinDaysRange() {
$entity = self::$i1970
- ->andStatement( NewStatement::forProperty( 'P569' )->withValue(
- new TimeValue( '+00000001969-12-24T00:00:00Z', 0, 0, 0, 11, 'http://www.wikidata.org/entity/Q1985727' )
- ) )
+ ->andStatement(
+ NewStatement::forProperty( 'P569' )
+ ->withValue(
+ new TimeValue( '+00000001969-12-24T00:00:00Z', 0, 0, 0, 11, 'http://www.wikidata.org/entity/Q1985727' )
+ )
+ ->withSomeGuid()
+ )
->build();
$constraint = $this->getConstraintMock( $this->dob5to10DaysParameters );

@@ -204,9 +229,13 @@

public function testDiffWithinRangeConstraintTooFewDays() {
$entity = self::$i1970
- ->andStatement( NewStatement::forProperty( 'P569' )->withValue(
- new TimeValue( '+00000001969-12-31T00:00:00Z', 0, 0, 0, 11, 'http://www.wikidata.org/entity/Q1985727' )
- ) )
+ ->andStatement(
+ NewStatement::forProperty( 'P569' )
+ ->withValue(
+ new TimeValue( '+00000001969-12-31T00:00:00Z', 0, 0, 0, 11, 'http://www.wikidata.org/entity/Q1985727' )
+ )
+ ->withSomeGuid()
+ )
->build();
$constraint = $this->getConstraintMock( $this->dob5to10DaysParameters );

@@ -217,9 +246,13 @@

public function testDiffWithinRangeConstraintTooManyDays() {
$entity = self::$i1970
- ->andStatement( NewStatement::forProperty( 'P569' )->withValue(
- new TimeValue( '+00000001969-10-31T00:00:00Z', 0, 0, 0, 11, 'http://www.wikidata.org/entity/Q1985727' )
- ) )
+ ->andStatement(
+ NewStatement::forProperty( 'P569' )
+ ->withValue(
+ new TimeValue( '+00000001969-10-31T00:00:00Z', 0, 0, 0, 11, 'http://www.wikidata.org/entity/Q1985727' )
+ )
+ ->withSomeGuid()
+ )
->build();
$constraint = $this->getConstraintMock( $this->dob5to10DaysParameters );

@@ -231,9 +264,13 @@
public function testDiffWithinRangeConstraintWithinYearsRange() {
// DoB June 1820 and DoD January 1970 has diff within range [0, 150] years
$entity = self::$i1970
- ->andStatement( NewStatement::forProperty( 'P569' )->withValue(
- new TimeValue( '+00000001820-06-01T00:00:00Z', 0, 0, 0, 11, 'http://www.wikidata.org/entity/Q1985727' )
- ) )
+ ->andStatement(
+ NewStatement::forProperty( 'P569' )
+ ->withValue(
+ new TimeValue( '+00000001820-06-01T00:00:00Z', 0, 0, 0, 11, 'http://www.wikidata.org/entity/Q1985727' )
+ )
+ ->withSomeGuid()
+ )
->build();
$constraint = $this->getConstraintMock( $this->dob0to150YearsParameters );
$context = new MainSnakContext( $entity, self::$s1970 );
@@ -247,9 +284,13 @@
// DoB June 1970 and DoD January 1970 violates diff within range [0, 150] years
// because June is after January, even though the year difference is 0 (within range)
$entity = self::$i1970
- ->andStatement( NewStatement::forProperty( 'P569' )->withValue(
- new TimeValue( '+00000001970-06-01T00:00:00Z', 0, 0, 0, 11, 'http://www.wikidata.org/entity/Q1985727' )
- ) )
+ ->andStatement(
+ NewStatement::forProperty( 'P569' )
+ ->withValue(
+ new TimeValue( '+00000001970-06-01T00:00:00Z', 0, 0, 0, 11, 'http://www.wikidata.org/entity/Q1985727' )
+ )
+ ->withSomeGuid()
+ )
->build();
$constraint = $this->getConstraintMock( $this->dob0to150YearsParameters );
$context = new MainSnakContext( $entity, self::$s1970 );
@@ -262,14 +303,21 @@
public function testDiffWithinRangeConstraintTooManyYears() {
// DoB March 1820 and DoD September 1970 violates diff within range [0, 150] years
// because March is before September, even though the year difference is 150 (within range)
- $fall1970Statement = NewStatement::forProperty( 'P570' )->withValue(
- new TimeValue( '+00000001970-09-01T00:00:00Z', 0, 0, 0, 11, 'http://www.wikidata.org/entity/Q1985727' )
- )->build();
+ $fall1970Statement = NewStatement::forProperty( 'P570' )
+ ->withValue(
+ new TimeValue( '+00000001970-09-01T00:00:00Z', 0, 0, 0, 11, 'http://www.wikidata.org/entity/Q1985727' )
+ )
+ ->withSomeGuid()
+ ->build();
$entity = NewItem::withId( 'Q1' )
->andStatement( $fall1970Statement )
- ->andStatement( NewStatement::forProperty( 'P569' )->withValue(
- new TimeValue( '+00000001820-03-01T00:00:00Z', 0, 0, 0, 11, 'http://www.wikidata.org/entity/Q1985727' )
- ) )
+ ->andStatement(
+ NewStatement::forProperty( 'P569' )
+ ->withValue(
+ new TimeValue( '+00000001820-03-01T00:00:00Z', 0, 0, 0, 11, 'http://www.wikidata.org/entity/Q1985727' )
+ )
+ ->withSomeGuid()
+ )
->build();
$constraint = $this->getConstraintMock( $this->dob0to150YearsParameters );
$context = new MainSnakContext( $entity, $fall1970Statement );
@@ -282,10 +330,15 @@
public function testDiffWithinRangeConstraintWithinRangeWithDeprecatedStatement() {
$deprecatedStatement = NewStatement::forProperty( 'P569' )
->withValue( self::$t1800 )
- ->withRank( Statement::RANK_DEPRECATED );
+ ->withRank( Statement::RANK_DEPRECATED )
+ ->withSomeGuid();
$entity = self::$i1970
->andStatement( $deprecatedStatement ) // should be ignored
- ->andStatement( NewStatement::forProperty( 'P569' )->withValue( self::$t1900 ) )
+ ->andStatement(
+ NewStatement::forProperty( 'P569' )
+ ->withValue( self::$t1900 )
+ ->withSomeGuid()
+ )
->build();
$constraint = $this->getConstraintMock( $this->dob0to150Parameters );

@@ -299,6 +352,7 @@
->andStatement(
NewStatement::forProperty( 'P2067' )
->withValue( UnboundedQuantityValue::newFromNumber( 5000, 'g' ) )
+ ->withSomeGuid()
)
->build();
$snak = new PropertyValueSnak(
@@ -326,6 +380,7 @@
->andStatement(
NewStatement::forProperty( 'P2067' )
->withValue( UnboundedQuantityValue::newFromNumber( 5000, 'g' ) )
+ ->withSomeGuid()
)
->build();
$snak = new PropertyValueSnak(
@@ -353,6 +408,7 @@
->andStatement(
NewStatement::forProperty( 'P2067' )
->withValue( UnboundedQuantityValue::newFromNumber( 5000, 'g' ) )
+ ->withSomeGuid()
)
->build();
$snak = new PropertyValueSnak(
@@ -376,12 +432,16 @@
}

public function testDiffWithinRangeConstraintWithinRangeWithOtherSnakTypes() {
- $noValueStatement = NewStatement::noValueFor( 'P569' );
- $someValueStatement = NewStatement::someValueFor( 'P569' );
+ $noValueStatement = NewStatement::noValueFor( 'P569' )->withSomeGuid();
+ $someValueStatement = NewStatement::someValueFor( 'P569' )->withSomeGuid();
$entity = self::$i1970
->andStatement( $noValueStatement ) // should be ignored
->andStatement( $someValueStatement ) // should be ignored
- ->andStatement( NewStatement::forProperty( 'P569' )->withValue( self::$t1900 ) )
+ ->andStatement(
+ NewStatement::forProperty( 'P569' )
+ ->withValue( self::$t1900 )
+ ->withSomeGuid()
+ )
->build();
$constraint = $this->getConstraintMock( $this->dob0to150Parameters );

@@ -402,7 +462,8 @@
public function testDiffWithinRangeConstraintWithOnlyDeprecatedStatement() {
$deprecatedStatement = NewStatement::forProperty( 'P569' )
->withValue( self::$t1900 )
- ->withRank( Statement::RANK_DEPRECATED );
+ ->withRank( Statement::RANK_DEPRECATED )
+ ->withSomeGuid();
$entity = self::$i1970
->andStatement( $deprecatedStatement ) // should be ignored
->build();
@@ -414,8 +475,8 @@
}

public function testDiffWithinRangeConstraintWithOnlyOtherSnakTypes() {
- $noValueStatement = NewStatement::noValueFor( 'P569' );
- $someValueStatement = NewStatement::someValueFor( 'P569' );
+ $noValueStatement = NewStatement::noValueFor( 'P569' )->withSomeGuid();
+ $someValueStatement = NewStatement::someValueFor( 'P569' )->withSomeGuid();
$entity = self::$i1970
->andStatement( $noValueStatement ) // should be ignored
->andStatement( $someValueStatement ) // should be ignored
@@ -429,7 +490,11 @@

public function testDiffWithinRangeConstraintWrongTypeOfProperty() {
$entity = self::$i1970
- ->andStatement( NewStatement::forProperty( 'P569' )->withValue( '1900' ) )
+ ->andStatement(
+ NewStatement::forProperty( 'P569' )
+ ->withValue( '1900' )
+ ->withSomeGuid()
+ )
->build();
$constraint = $this->getConstraintMock( $this->dob0to150Parameters );

@@ -440,7 +505,7 @@

public function testDiffWithinRangeConstraintNoValueSnak() {
$entity = self::$i1970->build();
- $statement = NewStatement::noValueFor( 'P1' )->build();
+ $statement = NewStatement::noValueFor( 'P1' )->withSomeGuid()->build();
$constraint = $this->getConstraintMock( $this->dob0to150Parameters );

$checkResult = $this->checker->checkConstraint( new MainSnakContext( $entity, $statement ), $constraint );
@@ -450,7 +515,11 @@

public function testDiffWithinRangeConstraintLeftOpenWithinRange() {
$entity = self::$i1970
- ->andStatement( NewStatement::forProperty( 'P569' )->withValue( self::$t1970 ) )
+ ->andStatement(
+ NewStatement::forProperty( 'P569' )
+ ->withValue( self::$t1970 )
+ ->withSomeGuid()
+ )
->build();
$constraintParameters = array_merge(
$this->propertyParameter( 'P569' ),
@@ -465,7 +534,11 @@

public function testDiffWithinRangeConstraintLeftOpenTooBig() {
$entity = self::$i1970
- ->andStatement( NewStatement::forProperty( 'P569' )->withValue( self::$t0001 ) )
+ ->andStatement(
+ NewStatement::forProperty( 'P569' )
+ ->withValue( self::$t0001 )
+ ->withSomeGuid()
+ )
->build();
$constraintParameters = array_merge(
$this->propertyParameter( 'P569' ),
@@ -480,7 +553,11 @@

public function testDiffWithinRangeConstraintRightOpenWithinRange() {
$entity = self::$i1970
- ->andStatement( NewStatement::forProperty( 'P569' )->withValue( self::$t0001 ) )
+ ->andStatement(
+ NewStatement::forProperty( 'P569' )
+ ->withValue( self::$t0001 )
+ ->withSomeGuid()
+ )
->build();
$constraintParameters = array_merge(
$this->propertyParameter( 'P569' ),
@@ -495,7 +572,11 @@

public function testDiffWithinRangeConstraintRightOpenTooSmall() {
$entity = self::$i1970
- ->andStatement( NewStatement::forProperty( 'P569' )->withValue( self::$t2000 ) )
+ ->andStatement(
+ NewStatement::forProperty( 'P569' )
+ ->withValue( self::$t2000 )
+ ->withSomeGuid()
+ )
->build();
$constraintParameters = array_merge(
$this->propertyParameter( 'P569' ),
@@ -508,13 +589,49 @@
$this->assertViolation( $checkResult, 'wbqc-violation-message-diff-within-range-rightopen' );
}

+ public function testDiffWithinRangeConstraint_Qualifier() {
+ $qualifier1 = new PropertyValueSnak( new PropertyId( 'P569' ), self::$t1900 );
+ $qualifier2 = new PropertyValueSnak( new PropertyId( 'P570' ), self::$t1970 );
+ $statement = new Statement(
+ new PropertySomeValueSnak( new PropertyId( 'P10' ) ),
+ new SnakList( [ $qualifier1, $qualifier2 ] ),
+ null,
+ 'Q1$c5f1968c-c8f9-4edd-9f2d-f12c93cd8b2b'
+ );
+ $entity = NewItem::withStatement( $statement )->build();
+ $constraint = $this->getConstraintMock( $this->dob0to150Parameters );
+
+ $checkResult = $this->checker->checkConstraint( new QualifierContext( $entity, $statement, $qualifier2 ), $constraint );
+
+ $this->assertCompliance( $checkResult );
+ }
+
+ public function testDiffWithinRangeConstraint_Reference() {
+ $snak1 = new PropertyValueSnak( new PropertyId( 'P569' ), self::$t2000 );
+ $snak2 = new PropertyValueSnak( new PropertyId( 'P570' ), self::$t1970 );
+ $reference = new Reference( [ $snak1, $snak2 ] );
+ $statement = new Statement(
+ new PropertySomeValueSnak( new PropertyId( 'P10' ) ),
+ null,
+ new ReferenceList( [ $reference ] ),
+ 'Q1$80a5fb6c-8b14-4f18-a60e-2c853d5dfbd1'
+ );
+ $entity = NewItem::withStatement( $statement )->build();
+ $constraint = $this->getConstraintMock( $this->dob0to150Parameters );
+
+ $checkResult = $this->checker->checkConstraint( new ReferenceContext( $entity, $statement, $reference, $snak2 ), $constraint );
+
+ $this->assertViolation( $checkResult, 'wbqc-violation-message-diff-within-range' );
+ }
+
public function testDiffWithinRangeConstraintDeprecatedStatement() {
$statement = NewStatement::noValueFor( 'P1' )
- ->withDeprecatedRank()
- ->build();
+ ->withDeprecatedRank()
+ ->withSomeGuid()
+ ->build();
$constraint = $this->getConstraintMock( [] );
$entity = NewItem::withId( 'Q1' )
- ->build();
+ ->build();

$checkResult = $this->checker->checkConstraint( new MainSnakContext( $entity, $statement ), $constraint );


--
To view, visit https://gerrit.wikimedia.org/r/405293
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I21c4b0ec82baf461b38aca0022db3fbf70ebb651
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/WikibaseQualityConstraints
Gerrit-Branch: master
Gerrit-Owner: Lucas Werkmeister (WMDE) <lucas.werkmeister@wikimedia.de>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits