Lucas Werkmeister (WMDE) has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/358055 )
Change subject: Inject+use SparqlHelper in TypeCheckerHelper
......................................................................
Inject+use SparqlHelper in TypeCheckerHelper
TypeCheckerHelper can optionally be instantiated with a SparqlHelper,
and isSubclassOf delegates to it if the configured limit is exceeded and
the SparqlHelper is present. This should remove all false positives from
the type checks.
(Note that this commit does not remove the dedicated SPARQL checkers.)
Bug: T166379
Change-Id: Ie7c67a88a10c7071b9dbfcb3751a175ba0c304d4
---
M includes/ConstraintCheck/Helper/TypeCheckerHelper.php
M includes/ConstraintReportFactory.php
M tests/phpunit/Checker/TypeChecker/TypeCheckerHelperTest.php
3 files changed, 61 insertions(+), 7 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/WikibaseQualityConstraints
refs/changes/55/358055/1
diff --git a/includes/ConstraintCheck/Helper/TypeCheckerHelper.php
b/includes/ConstraintCheck/Helper/TypeCheckerHelper.php
index 88149c2..1136a0f 100644
--- a/includes/ConstraintCheck/Helper/TypeCheckerHelper.php
+++ b/includes/ConstraintCheck/Helper/TypeCheckerHelper.php
@@ -39,18 +39,26 @@
private $constraintParameterRenderer;
/**
+ * @var SparqlHelper
+ */
+ private $sparqlHelper;
+
+ /**
* @param EntityLookup $lookup
* @param Config $config
* @param ConstraintParameterRenderer $constraintParameterRenderer
+ * @param SparqlHelper|null $sparqlHelper
*/
public function __construct(
EntityLookup $lookup,
Config $config,
- ConstraintParameterRenderer $constraintParameterRenderer
+ ConstraintParameterRenderer $constraintParameterRenderer,
+ SparqlHelper $sparqlHelper = null
) {
$this->entityLookup = $lookup;
$this->config = $config;
$this->constraintParameterRenderer =
$constraintParameterRenderer;
+ $this->sparqlHelper = $sparqlHelper;
}
/**
@@ -58,7 +66,8 @@
* of one of the item ID serializations in $classesToCheck.
* If the class hierarchy is not exhausted before
* the configured limit (WBQualityConstraintsTypeCheckMaxEntities) is
reached,
- * the check aborts and returns false.
+ * the injected {@link SparqlHelper} is consulted if present,
+ * otherwise the check aborts and returns false.
*
* @param EntityId $comparativeClass
* @param string[] $classesToCheck
@@ -67,8 +76,17 @@
* @return bool
*/
public function isSubclassOf( EntityId $comparativeClass, array
$classesToCheck, &$entitiesChecked = 0 ) {
- if ( ++$entitiesChecked > $this->config->get(
'WBQualityConstraintsTypeCheckMaxEntities' ) ) {
- return false;
+ $maxEntities = $this->config->get(
'WBQualityConstraintsTypeCheckMaxEntities' );
+ if ( ++$entitiesChecked > $maxEntities ) {
+ if ( $entitiesChecked === $maxEntities + 1 &&
$this->sparqlHelper !== null ) {
+ return $this->sparqlHelper->hasType(
+ $comparativeClass->getSerialization(),
+ $classesToCheck,
+ /* withInstance = */ false
+ );
+ } else {
+ return false;
+ }
}
$item = $this->entityLookup->getEntity( $comparativeClass );
diff --git a/includes/ConstraintReportFactory.php
b/includes/ConstraintReportFactory.php
index 544faff..4e5a7ee 100644
--- a/includes/ConstraintReportFactory.php
+++ b/includes/ConstraintReportFactory.php
@@ -166,7 +166,6 @@
$constraintParameterParser = new
ConstraintParameterParser();
$connectionCheckerHelper = new
ConnectionCheckerHelper();
$rangeCheckerHelper = new RangeCheckerHelper();
- $typeCheckerHelper = new TypeCheckerHelper(
$this->lookup, $this->config, $this->constraintParameterRenderer );
if ( $this->config->get(
'WBQualityConstraintsSparqlEndpoint' ) !== '' ) {
$sparqlHelper = new SparqlHelper(
$this->config,
@@ -176,6 +175,12 @@
} else {
$sparqlHelper = null;
}
+ $typeCheckerHelper = new TypeCheckerHelper(
+ $this->lookup,
+ $this->config,
+ $this->constraintParameterRenderer,
+ $sparqlHelper
+ );
$this->constraintCheckerMap = [
'Conflicts with' => new ConflictsWithChecker(
$this->lookup, $constraintParameterParser, $connectionCheckerHelper,
$this->constraintParameterRenderer ),
diff --git a/tests/phpunit/Checker/TypeChecker/TypeCheckerHelperTest.php
b/tests/phpunit/Checker/TypeChecker/TypeCheckerHelperTest.php
index df456c8..b12da3e 100644
--- a/tests/phpunit/Checker/TypeChecker/TypeCheckerHelperTest.php
+++ b/tests/phpunit/Checker/TypeChecker/TypeCheckerHelperTest.php
@@ -10,6 +10,7 @@
use Wikibase\DataModel\Entity\ItemId;
use Wikibase\DataModel\Entity\PropertyId;
use Wikibase\DataModel\Statement\StatementList;
+use WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\SparqlHelper;
use WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\TypeCheckerHelper;
use WikibaseQuality\ConstraintReport\ConstraintParameterRenderer;
use WikibaseQuality\ConstraintReport\Tests\ConstraintParameters;
@@ -29,14 +30,19 @@
/**
* @param EntityLookup|null $entityLookup
+ * @param SparqlHelper|null $sparqlHelper
*
* @return TypeCheckerHelper
*/
- private function getHelper( EntityLookup $entityLookup = null ) {
+ private function getHelper(
+ EntityLookup $entityLookup = null,
+ SparqlHelper $sparqlHelper = null
+ ) {
return new TypeCheckerHelper(
$entityLookup ?: new JsonFileEntityLookup( __DIR__ ),
$this->getDefaultConfig(),
- $this->getConstraintParameterRenderer()
+ $this->getConstraintParameterRenderer(),
+ $sparqlHelper
);
}
@@ -54,6 +60,21 @@
->will( $this->returnCallback( [ $lookup, 'getEntity' ]
) );
return $spy;
+ }
+
+ /**
+ * @param boolean $return
+ * @return SparqlHelper expects that {@link SparqlHelper::hasType}
+ * is called exactly once and returns $return.
+ */
+ private function getSparqlHelper( $return ) {
+ $mock = $this->getMockBuilder( SparqlHelper::class )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $mock->expects( $this->once() )
+ ->method( 'hasType' )
+ ->willReturn( $return );
+ return $mock;
}
public function testCheckHasClassInRelationValid() {
@@ -95,4 +116,14 @@
$this->assertFalse( $helper->isSubclassOf( 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' ] ) );
+ }
+
+ public function testCheckIsSubclassCyclicWideWithSparqlFalse() {
+ $helper = $this->getHelper( $this->getMaxEntitiesLookup(),
$this->getSparqlHelper( false ) );
+ $this->assertFalse( $helper->isSubclassOf( new ItemId( 'Q9' ),
[ 'Q100', 'Q101' ] ) );
+ }
+
}
--
To view, visit https://gerrit.wikimedia.org/r/358055
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie7c67a88a10c7071b9dbfcb3751a175ba0c304d4
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/WikibaseQualityConstraints
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
