Lucas Werkmeister (WMDE) has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/369555 )
Change subject: Update Wikidata - fix constraint type checks
......................................................................
Update Wikidata - fix constraint type checks
This adds change I2aaf3d6fa6 to the Constraints extension.
Bug: T169326
Change-Id: I49b0f4342b9060bf189f17561e7e3e2e02f536de
---
M composer.lock
M extensions/Constraints/includes/ConstraintCheck/Helper/TypeCheckerHelper.php
M
extensions/Constraints/tests/phpunit/Checker/TypeChecker/TypeCheckerHelperTest.php
M vendor/composer/installed.json
4 files changed, 113 insertions(+), 30 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikidata
refs/changes/55/369555/1
diff --git a/composer.lock b/composer.lock
index 9cd6b13..878c16b 100644
--- a/composer.lock
+++ b/composer.lock
@@ -965,7 +965,7 @@
"source": {
"type": "git",
"url":
"https://gerrit.wikimedia.org/r/mediawiki/extensions/WikibaseQualityConstraints";,
- "reference": "7b42a909fabb4a2abccdc913cffedd74937a62c4"
+ "reference": "6c32f105be631dcbf876c713bf298381428afea4"
},
"require": {
"php": ">=5.5.9",
@@ -1026,7 +1026,7 @@
"support": {
"issues":
"https://phabricator.wikimedia.org/project/profile/1202/";
},
- "time": "2017-08-01T11:10:09+00:00"
+ "time": "2017-08-01T14:04:16+00:00"
},
{
"name": "wikibase/data-model",
diff --git
a/extensions/Constraints/includes/ConstraintCheck/Helper/TypeCheckerHelper.php
b/extensions/Constraints/includes/ConstraintCheck/Helper/TypeCheckerHelper.php
index d7cb13a..b3b698a 100644
---
a/extensions/Constraints/includes/ConstraintCheck/Helper/TypeCheckerHelper.php
+++
b/extensions/Constraints/includes/ConstraintCheck/Helper/TypeCheckerHelper.php
@@ -4,6 +4,7 @@
use Config;
use MediaWiki\MediaWikiServices;
+use OverflowException;
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Entity\EntityIdValue;
use Wikibase\DataModel\Entity\PropertyId;
@@ -68,31 +69,19 @@
* of one of the item ID serializations in $classesToCheck.
* If the class hierarchy is not exhausted before
* the configured limit (WBQualityConstraintsTypeCheckMaxEntities) is
reached,
- * the injected {@link SparqlHelper} is consulted if present,
- * otherwise the check aborts and returns false.
+ * an OverflowException is thrown.
*
* @param EntityId $comparativeClass
* @param string[] $classesToCheck
* @param int &$entitiesChecked
*
* @return bool
- *
- * @throws SparqlHelperException if SPARQL is used and the query times
out or some other error occurs
+ * @throws OverflowException if $entitiesChecked exceeds the configured
limit
*/
- public function isSubclassOf( EntityId $comparativeClass, array
$classesToCheck, &$entitiesChecked = 0 ) {
+ private function isSubclassOf( EntityId $comparativeClass, array
$classesToCheck, &$entitiesChecked = 0 ) {
$maxEntities = $this->config->get(
'WBQualityConstraintsTypeCheckMaxEntities' );
if ( ++$entitiesChecked > $maxEntities ) {
- if ( $entitiesChecked === $maxEntities + 1 &&
$this->sparqlHelper !== null ) {
-
MediaWikiServices::getInstance()->getStatsdDataFactory()
- ->increment(
'wikibase.quality.constraints.sparql.typeFallback' );
- return $this->sparqlHelper->hasType(
- $comparativeClass->getSerialization(),
- $classesToCheck,
- /* withInstance = */ false
- );
- } else {
- return false;
- }
+ throw new OverflowException( 'Too many entities to
check' );
}
$item = $this->entityLookup->getEntity( $comparativeClass );
@@ -127,6 +116,39 @@
}
/**
+ * Checks if $comparativeClass is a subclass
+ * of one of the item ID serializations in $classesToCheck.
+ * If isSubclassOf() aborts due to hitting the configured limit,
+ * the injected {@link SparqlHelper} is consulted if present,
+ * otherwise the check returns false.
+ *
+ * @param EntityId $comparativeClass
+ * @param string[] $classesToCheck
+ * @param int &$entitiesChecked
+ *
+ * @return bool
+ *
+ * @throws SparqlHelperException if SPARQL is used and the query times
out or some other error occurs
+ */
+ public function isSubclassOfWithSparqlFallback( EntityId
$comparativeClass, array $classesToCheck ) {
+ try {
+ return $this->isSubclassOf( $comparativeClass,
$classesToCheck );
+ } catch ( OverflowException $e ) {
+ if ( $this->sparqlHelper !== null ) {
+
MediaWikiServices::getInstance()->getStatsdDataFactory()
+ ->increment(
'wikibase.quality.constraints.sparql.typeFallback' );
+ return $this->sparqlHelper->hasType(
+ $comparativeClass->getSerialization(),
+ $classesToCheck,
+ /* withInstance = */ false
+ );
+ } else {
+ return false;
+ }
+ }
+ }
+
+ /**
* Checks, if one of the itemId serializations in $classesToCheck
* is contained in the list of $statements
* via property $relationId or if it is a subclass of
@@ -158,7 +180,7 @@
return true;
}
- if ( $this->isSubclassOf( $comparativeClass,
$classesToCheck ) ) {
+ if ( $this->isSubclassOfWithSparqlFallback(
$comparativeClass, $classesToCheck ) ) {
return true;
}
}
diff --git
a/extensions/Constraints/tests/phpunit/Checker/TypeChecker/TypeCheckerHelperTest.php
b/extensions/Constraints/tests/phpunit/Checker/TypeChecker/TypeCheckerHelperTest.php
index c4173a6..746cc8e 100644
---
a/extensions/Constraints/tests/phpunit/Checker/TypeChecker/TypeCheckerHelperTest.php
+++
b/extensions/Constraints/tests/phpunit/Checker/TypeChecker/TypeCheckerHelperTest.php
@@ -4,12 +4,15 @@
use PHPUnit_Framework_TestCase;
use Wikibase\DataModel\Services\Lookup\EntityLookup;
+use Wikibase\DataModel\Services\Lookup\InMemoryEntityLookup;
use Wikibase\DataModel\Statement\Statement;
use Wikibase\DataModel\Snak\PropertyValueSnak;
use Wikibase\DataModel\Entity\EntityIdValue;
use Wikibase\DataModel\Entity\ItemId;
use Wikibase\DataModel\Entity\PropertyId;
use Wikibase\DataModel\Statement\StatementList;
+use Wikibase\Repo\Tests\NewItem;
+use Wikibase\Repo\Tests\NewStatement;
use WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\SparqlHelper;
use WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\TypeCheckerHelper;
use WikibaseQuality\ConstraintReport\Tests\ConstraintParameters;
@@ -46,11 +49,14 @@
}
/**
+ * @param EntityLookup $lookup The backing lookup of the mock (defaults
to JsonFileEntityLookup).
* @return EntityLookup Expects that getEntity is called
* exactly WBQualityConstraintsTypeCheckMaxEntities times.
*/
- private function getMaxEntitiesLookup() {
- $lookup = new JsonFileEntityLookup( __DIR__ );
+ private function getMaxEntitiesLookup( EntityLookup $lookup = null ) {
+ if ( $lookup === null ) {
+ $lookup = new JsonFileEntityLookup( __DIR__ );
+ }
$maxEntities = $this->getDefaultConfig()->get(
'WBQualityConstraintsTypeCheckMaxEntities' );
$spy = $this->getMock( EntityLookup::class );
@@ -63,15 +69,20 @@
/**
* @param boolean $return
+ * @param array|null $arguments
* @return SparqlHelper expects that {@link SparqlHelper::hasType}
* is called exactly once and returns $return.
*/
- private function getSparqlHelper( $return ) {
+ private function getSparqlHelper( $return, array $arguments = null ) {
+ if ( $arguments === null ) {
+ $arguments = [ $this->anything(), $this->anything(),
$this->anything() ];
+ }
$mock = $this->getMockBuilder( SparqlHelper::class )
->disableOriginalConstructor()
->getMock();
$mock->expects( $this->once() )
->method( 'hasType' )
+ ->withConsecutive( $arguments )
->willReturn( $return );
return $mock;
}
@@ -98,31 +109,81 @@
}
public function testCheckIsSubclassOfValidWithIndirection() {
- $this->assertTrue( $this->getHelper()->isSubclassOf( new
ItemId( 'Q6' ), [ 'Q100', 'Q101' ] ) );
+ $this->assertTrue(
$this->getHelper()->isSubclassOfWithSparqlFallback( new ItemId( 'Q6' ), [
'Q100', 'Q101' ] ) );
}
public function testCheckIsSubclassOfInvalid() {
- $this->assertFalse( $this->getHelper()->isSubclassOf( new
ItemId( 'Q6' ), [ 'Q200', 'Q201' ] ) );
+ $this->assertFalse(
$this->getHelper()->isSubclassOfWithSparqlFallback( new ItemId( 'Q6' ), [
'Q200', 'Q201' ] ) );
}
public function testCheckIsSubclassCyclic() {
$helper = $this->getHelper( $this->getMaxEntitiesLookup() );
- $this->assertFalse( $helper->isSubclassOf( new ItemId( 'Q7' ),
[ 'Q100', 'Q101' ] ) );
+ $this->assertFalse( $helper->isSubclassOfWithSparqlFallback(
new ItemId( 'Q7' ), [ 'Q100', 'Q101' ] ) );
}
public function testCheckIsSubclassCyclicWide() {
$helper = $this->getHelper( $this->getMaxEntitiesLookup() );
- $this->assertFalse( $helper->isSubclassOf( new ItemId( 'Q9' ),
[ 'Q100', 'Q101' ] ) );
+ $this->assertFalse( $helper->isSubclassOfWithSparqlFallback(
new ItemId( 'Q9' ), [ 'Q100', 'Q101' ] ) );
}
public function testCheckIsSubclassCyclicWideWithSparqlTrue() {
$helper = $this->getHelper( $this->getMaxEntitiesLookup(),
$this->getSparqlHelper( true ) );
- $this->assertTrue( $helper->isSubclassOf( new ItemId( 'Q9' ), [
'Q100', 'Q101' ] ) );
+ $this->assertTrue( $helper->isSubclassOfWithSparqlFallback( new
ItemId( 'Q9' ), [ 'Q100', 'Q101' ] ) );
}
public function testCheckIsSubclassCyclicWideWithSparqlFalse() {
$helper = $this->getHelper( $this->getMaxEntitiesLookup(),
$this->getSparqlHelper( false ) );
- $this->assertFalse( $helper->isSubclassOf( new ItemId( 'Q9' ),
[ 'Q100', 'Q101' ] ) );
+ $this->assertFalse( $helper->isSubclassOfWithSparqlFallback(
new ItemId( 'Q9' ), [ 'Q100', 'Q101' ] ) );
+ }
+
+ public function testCheckIsSubclassTree() {
+ $lookup = new InMemoryEntityLookup();
+ $subclassPid = $this->getDefaultConfig()->get(
'WBQualityConstraintsSubclassOfId' );
+
+ $q1 = NewItem::withId( 'Q1' )
+ ->andStatement(
+ NewStatement::forProperty( $subclassPid )
+ ->withValue( new ItemId( 'Q2' ) )
+ )
+ ->build();
+ $lookup->addEntity( $q1 );
+ $q2 = NewItem::withId( 'Q2' )
+ ->andStatement(
+ NewStatement::forProperty( $subclassPid )
+ ->withValue( new ItemId( 'Q3' ) )
+ )
+ ->andStatement(
+ NewStatement::forProperty( $subclassPid )
+ ->withValue( new ItemId( 'Q5' ) )
+ )
+ ->build();
+ $lookup->addEntity( $q2 );
+ $q3 = NewItem::withId( 'Q3' )
+ ->andStatement(
+ NewStatement::forProperty( $subclassPid )
+ ->withValue( new ItemId( 'Q4' ) )
+ )
+ ->build();
+ $lookup->addEntity( $q3 );
+ $q4 = NewItem::withId( 'Q4' )
+ ->andStatement(
+ NewStatement::forProperty( $subclassPid )
+ ->withValue( new ItemId( 'Q3' ) )
+ )
+ ->build();
+ $lookup->addEntity( $q4 );
+
+ $sparqlHelper = $this->getSparqlHelper(
+ true,
+ [
+ $this->identicalTo( 'Q1' ),
+ $this->identicalTo( [ 'Q5' ] ),
+ $this->identicalTo( false )
+ ]
+ );
+ $helper = $this->getHelper( $this->getMaxEntitiesLookup(
$lookup ), $sparqlHelper );
+
+ $this->assertTrue( $helper->isSubclassOfWithSparqlFallback( new
ItemId( 'Q1' ), [ 'Q5' ] ) );
}
}
diff --git a/vendor/composer/installed.json b/vendor/composer/installed.json
index 88b7c4e..18dfcf1 100644
--- a/vendor/composer/installed.json
+++ b/vendor/composer/installed.json
@@ -1711,7 +1711,7 @@
"source": {
"type": "git",
"url":
"https://gerrit.wikimedia.org/r/mediawiki/extensions/WikibaseQualityConstraints";,
- "reference": "7b42a909fabb4a2abccdc913cffedd74937a62c4"
+ "reference": "6c32f105be631dcbf876c713bf298381428afea4"
},
"require": {
"php": ">=5.5.9",
@@ -1727,7 +1727,7 @@
"satooshi/php-coveralls": "master-dev",
"wikibase/wikibase-codesniffer": "^0.1.0"
},
- "time": "2017-08-01T11:10:09+00:00",
+ "time": "2017-08-01T14:04:16+00:00",
"type": "mediawiki-extension",
"installation-source": "source",
"autoload": {
--
To view, visit https://gerrit.wikimedia.org/r/369555
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I49b0f4342b9060bf189f17561e7e3e2e02f536de
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikidata
Gerrit-Branch: master
Gerrit-Owner: Lucas Werkmeister (WMDE) <lucas.werkmeis...@wikimedia.de>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
