jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/391279 )
Change subject: Add CachedBool container and use in helpers
......................................................................
Add CachedBool container and use in helpers
CachedBool is simply a wrapper around a bool and some CachingMetadata,
analogous to CachedArray. Several methods in SparqlHelper and
TypeCheckerHelper are updated to return a CachedBool (propagating the
CachingMetadata from the underlying runQuery() call). The information is
still thrown away eventually (this time by the constraint checkers
directly), but later commits will fix that.
Bug: T179844
Change-Id: Ie0510d7e36b5ea9e4350160fe4ca25c2293df3b3
---
M extension.json
A includes/ConstraintCheck/Cache/CachedBool.php
M includes/ConstraintCheck/Checker/TypeChecker.php
M includes/ConstraintCheck/Checker/ValueTypeChecker.php
M includes/ConstraintCheck/Helper/SparqlHelper.php
M includes/ConstraintCheck/Helper/TypeCheckerHelper.php
A tests/phpunit/Cache/CachedBoolTest.php
M tests/phpunit/Checker/TypeChecker/TypeCheckerHelperTest.php
M tests/phpunit/Helper/SparqlHelperTest.php
9 files changed, 149 insertions(+), 25 deletions(-)
Approvals:
WMDE-leszek: Looks good to me, approved
jenkins-bot: Verified
diff --git a/extension.json b/extension.json
index 373138a..3f559d0 100644
--- a/extension.json
+++ b/extension.json
@@ -375,6 +375,7 @@
"WikibaseQuality\\ConstraintReport\\CachingConstraintLookup":
"includes/CachingConstraintLookup.php",
"WikibaseQuality\\ConstraintReport\\Constraint":
"includes/Constraint.php",
"WikibaseQuality\\ConstraintReport\\ConstraintCheck\\Cache\\CachedArray":
"includes/ConstraintCheck/Cache/CachedArray.php",
+
"WikibaseQuality\\ConstraintReport\\ConstraintCheck\\Cache\\CachedBool":
"includes/ConstraintCheck/Cache/CachedBool.php",
"WikibaseQuality\\ConstraintReport\\ConstraintCheck\\Cache\\CachedQueryResults":
"includes/ConstraintCheck/Cache/CachedQueryResults.php",
"WikibaseQuality\\ConstraintReport\\ConstraintCheck\\Cache\\CachingMetadata":
"includes/ConstraintCheck/Cache/CachingMetadata.php",
"WikibaseQuality\\ConstraintReport\\ConstraintCheck\\Checker\\CommonsLinkChecker":
"includes/ConstraintCheck/Checker/CommonsLinkChecker.php",
diff --git a/includes/ConstraintCheck/Cache/CachedBool.php
b/includes/ConstraintCheck/Cache/CachedBool.php
new file mode 100644
index 0000000..e838954
--- /dev/null
+++ b/includes/ConstraintCheck/Cache/CachedBool.php
@@ -0,0 +1,46 @@
+<?php
+
+namespace WikibaseQuality\ConstraintReport\ConstraintCheck\Cache;
+
+/**
+ * A bool along with information whether and how it was cached.
+ *
+ * @author Lucas Werkmeister
+ * @license GNU GPL v2+
+ */
+class CachedBool {
+
+ /**
+ * @var bool
+ */
+ private $bool;
+
+ /**
+ * @var CachingMetadata
+ */
+ private $cachingMetadata;
+
+ /**
+ * @param bool $bool
+ * @param CachingMetadata $cachingMetadata
+ */
+ public function __construct( $bool, CachingMetadata $cachingMetadata ) {
+ $this->bool = $bool;
+ $this->cachingMetadata = $cachingMetadata;
+ }
+
+ /**
+ * @return bool
+ */
+ public function getBool() {
+ return $this->bool;
+ }
+
+ /**
+ * @return CachingMetadata
+ */
+ public function getCachingMetadata() {
+ return $this->cachingMetadata;
+ }
+
+}
diff --git a/includes/ConstraintCheck/Checker/TypeChecker.php
b/includes/ConstraintCheck/Checker/TypeChecker.php
index 9b9f046..01cd19d 100644
--- a/includes/ConstraintCheck/Checker/TypeChecker.php
+++ b/includes/ConstraintCheck/Checker/TypeChecker.php
@@ -96,7 +96,13 @@
}
$parameters['relation'] = [ $relation ];
- if ( $this->typeCheckerHelper->hasClassInRelation(
$context->getEntity()->getStatements(), $relationId, $classes ) ) {
+ $result = $this->typeCheckerHelper->hasClassInRelation(
+ $context->getEntity()->getStatements(),
+ $relationId,
+ $classes
+ );
+
+ if ( $result->getBool() ) {
$message = '';
$status = CheckResult::STATUS_COMPLIANCE;
} else {
diff --git a/includes/ConstraintCheck/Checker/ValueTypeChecker.php
b/includes/ConstraintCheck/Checker/ValueTypeChecker.php
index 01a57d7..144cc19 100644
--- a/includes/ConstraintCheck/Checker/ValueTypeChecker.php
+++ b/includes/ConstraintCheck/Checker/ValueTypeChecker.php
@@ -139,7 +139,13 @@
$statements = $item->getStatements();
- if ( $this->typeCheckerHelper->hasClassInRelation( $statements,
$relationId, $classes ) ) {
+ $result = $this->typeCheckerHelper->hasClassInRelation(
+ $statements,
+ $relationId,
+ $classes
+ );
+
+ if ( $result->getBool() ) {
$message = '';
$status = CheckResult::STATUS_COMPLIANCE;
} else {
diff --git a/includes/ConstraintCheck/Helper/SparqlHelper.php
b/includes/ConstraintCheck/Helper/SparqlHelper.php
index d2593f4..dcd18ac 100644
--- a/includes/ConstraintCheck/Helper/SparqlHelper.php
+++ b/includes/ConstraintCheck/Helper/SparqlHelper.php
@@ -18,6 +18,7 @@
use Wikibase\DataModel\Snak\Snak;
use Wikibase\DataModel\Statement\Statement;
use Wikibase\Rdf\RdfVocabulary;
+use WikibaseQuality\ConstraintReport\ConstraintCheck\Cache\CachedBool;
use WikibaseQuality\ConstraintReport\ConstraintCheck\Cache\CachedQueryResults;
use WikibaseQuality\ConstraintReport\ConstraintCheck\Cache\CachingMetadata;
use WikibaseQuality\ConstraintReport\ConstraintCheck\Context\Context;
@@ -109,7 +110,7 @@
* @param string $id entity ID serialization of the entity to check
* @param string[] $classes entity ID serializations of the expected
types
* @param boolean $withInstance true for “instance” relation, false for
“subclass” relation
- * @return boolean
+ * @return CachedBool
* @throws SparqlHelperException if the query times out or some other
error occurs
*/
public function hasType( $id, array $classes, $withInstance ) {
@@ -117,6 +118,8 @@
$subclassOfId = $this->config->get(
'WBQualityConstraintsSubclassOfId' );
$path = ( $withInstance ? "wdt:$instanceOfId/" : "" ) .
"wdt:$subclassOfId*";
+
+ $cachingMetadatas = [];
foreach ( array_chunk( $classes, 20 ) as $classesChunk ) {
$classesValues = implode( ' ', array_map(
@@ -136,12 +139,19 @@
// TODO hint:gearing is a workaround for T168973 and
can hopefully be removed eventually
$result = $this->runQuery( $query );
+ $cachingMetadatas[] = $result->getCachingMetadata();
if ( $result->getArray()['boolean'] ) {
- return true;
+ return new CachedBool(
+ true,
+ CachingMetadata::merge(
$cachingMetadatas )
+ );
}
}
- return false;
+ return new CachedBool(
+ false,
+ CachingMetadata::merge( $cachingMetadatas )
+ );
}
/**
diff --git a/includes/ConstraintCheck/Helper/TypeCheckerHelper.php
b/includes/ConstraintCheck/Helper/TypeCheckerHelper.php
index 089ce85..39bba1d 100644
--- a/includes/ConstraintCheck/Helper/TypeCheckerHelper.php
+++ b/includes/ConstraintCheck/Helper/TypeCheckerHelper.php
@@ -14,6 +14,8 @@
use Wikibase\DataModel\Statement\Statement;
use Wikibase\DataModel\Statement\StatementList;
use Wikibase\DataModel\Statement\StatementListProvider;
+use WikibaseQuality\ConstraintReport\ConstraintCheck\Cache\CachedBool;
+use WikibaseQuality\ConstraintReport\ConstraintCheck\Cache\CachingMetadata;
use WikibaseQuality\ConstraintReport\ConstraintParameterRenderer;
use WikibaseQuality\ConstraintReport\Role;
@@ -125,13 +127,16 @@
* @param string[] $classesToCheck
* @param int &$entitiesChecked
*
- * @return bool
+ * @return CachedBool
*
* @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 );
+ return new CachedBool(
+ $this->isSubclassOf( $comparativeClass,
$classesToCheck ),
+ CachingMetadata::fresh()
+ );
} catch ( OverflowException $e ) {
if ( $this->sparqlHelper !== null ) {
MediaWikiServices::getInstance()->getStatsdDataFactory()
@@ -142,7 +147,7 @@
/* withInstance = */ false
);
} else {
- return false;
+ return new CachedBool( false,
CachingMetadata::fresh() );
}
}
}
@@ -157,11 +162,13 @@
* @param string $relationId
* @param string[] $classesToCheck
*
- * @return bool
+ * @return CachedBool
*
* @throws SparqlHelperException if SPARQL is used and the query times
out or some other error occurs
*/
public function hasClassInRelation( StatementList $statements,
$relationId, array $classesToCheck ) {
+ $cachingMetadatas = [];
+
/** @var Statement $statement */
foreach ( $statements->getByPropertyId( new PropertyId(
$relationId ) ) as $statement ) {
$mainSnak = $statement->getMainSnak();
@@ -176,15 +183,24 @@
$comparativeClass = $dataValue->getEntityId();
if ( in_array( $comparativeClass->getSerialization(),
$classesToCheck ) ) {
- return true;
+ // discard $cachingMetadatas, we know this is
fresh
+ return new CachedBool( true,
CachingMetadata::fresh() );
}
- if ( $this->isSubclassOfWithSparqlFallback(
$comparativeClass, $classesToCheck ) ) {
- return true;
+ $result = $this->isSubclassOfWithSparqlFallback(
$comparativeClass, $classesToCheck );
+ $cachingMetadatas[] = $result->getCachingMetadata();
+ if ( $result->getBool() ) {
+ return new CachedBool(
+ true,
+ CachingMetadata::merge(
$cachingMetadatas )
+ );
}
}
- return false;
+ return new CachedBool(
+ false,
+ CachingMetadata::merge( $cachingMetadatas )
+ );
}
private function hasCorrectType( Snak $mainSnak ) {
diff --git a/tests/phpunit/Cache/CachedBoolTest.php
b/tests/phpunit/Cache/CachedBoolTest.php
new file mode 100644
index 0000000..6af2973
--- /dev/null
+++ b/tests/phpunit/Cache/CachedBoolTest.php
@@ -0,0 +1,37 @@
+<?php
+
+namespace WikibaseQuality\ConstraintReport\Test\Cache;
+
+use WikibaseQuality\ConstraintReport\ConstraintCheck\Cache\CachedBool;
+use WikibaseQuality\ConstraintReport\ConstraintCheck\Cache\CachingMetadata;
+
+/**
+ * @covers \WikibaseQuality\ConstraintReport\ConstraintCheck\Cache\CachedBool
+ * @uses
\WikibaseQuality\ConstraintReport\ConstraintCheck\Cache\CachingMetadata
+ *
+ * @group WikibaseQualityConstraints
+ *
+ * @author Lucas Werkmeister
+ * @license GNU GPL v2+
+ */
+class CachedBoolTest extends \PHPUnit_Framework_TestCase {
+
+ public function testGetBool() {
+ $bool = true;
+ $cm = CachingMetadata::fresh();
+
+ $ca = new CachedBool( $bool, $cm );
+
+ $this->assertSame( $bool, $ca->getBool() );
+ }
+
+ public function testGetCachingMetadata() {
+ $bool = false;
+ $cm = CachingMetadata::ofMaximumAgeInSeconds( 42 );
+
+ $ca = new CachedBool( $bool, $cm );
+
+ $this->assertSame( $cm, $ca->getCachingMetadata() );
+ }
+
+}
diff --git a/tests/phpunit/Checker/TypeChecker/TypeCheckerHelperTest.php
b/tests/phpunit/Checker/TypeChecker/TypeCheckerHelperTest.php
index 746cc8e..c48e9b2 100644
--- a/tests/phpunit/Checker/TypeChecker/TypeCheckerHelperTest.php
+++ b/tests/phpunit/Checker/TypeChecker/TypeCheckerHelperTest.php
@@ -13,6 +13,8 @@
use Wikibase\DataModel\Statement\StatementList;
use Wikibase\Repo\Tests\NewItem;
use Wikibase\Repo\Tests\NewStatement;
+use WikibaseQuality\ConstraintReport\ConstraintCheck\Cache\CachedBool;
+use WikibaseQuality\ConstraintReport\ConstraintCheck\Cache\CachingMetadata;
use WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\SparqlHelper;
use WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\TypeCheckerHelper;
use WikibaseQuality\ConstraintReport\Tests\ConstraintParameters;
@@ -83,7 +85,7 @@
$mock->expects( $this->once() )
->method( 'hasType' )
->withConsecutive( $arguments )
- ->willReturn( $return );
+ ->willReturn( new CachedBool( $return,
CachingMetadata::fresh() ) );
return $mock;
}
@@ -91,49 +93,49 @@
$statement1 = new Statement( new PropertyValueSnak( new
PropertyId( 'P1' ), new EntityIdValue( new ItemId( 'Q42' ) ) ) );
$statement2 = new Statement( new PropertyValueSnak( new
PropertyId( 'P31' ), new EntityIdValue( new ItemId( 'Q1' ) ) ) );
$statements = new StatementList( [ $statement1, $statement2 ] );
- $this->assertTrue( $this->getHelper()->hasClassInRelation(
$statements, 'P31', [ 'Q1' ] ) );
+ $this->assertTrue( $this->getHelper()->hasClassInRelation(
$statements, 'P31', [ 'Q1' ] )->getBool() );
}
public function testCheckHasClassInRelationInvalid() {
$statement1 = new Statement( new PropertyValueSnak( new
PropertyId( 'P1' ), new EntityIdValue( new ItemId( 'Q42' ) ) ) );
$statement2 = new Statement( new PropertyValueSnak( new
PropertyId( 'P31' ), new EntityIdValue( new ItemId( 'Q100' ) ) ) );
$statements = new StatementList( [ $statement1, $statement2 ] );
- $this->assertFalse( $this->getHelper()->hasClassInRelation(
$statements, 'P31', [ 'Q1' ] ) );
+ $this->assertFalse( $this->getHelper()->hasClassInRelation(
$statements, 'P31', [ 'Q1' ] )->getBool() );
}
public function testCheckHasClassInRelationValidWithIndirection() {
$statement1 = new Statement( new PropertyValueSnak( new
PropertyId( 'P1' ), new EntityIdValue( new ItemId( 'Q42' ) ) ) );
$statement2 = new Statement( new PropertyValueSnak( new
PropertyId( 'P31' ), new EntityIdValue( new ItemId( 'Q5' ) ) ) );
$statements = new StatementList( [ $statement1, $statement2 ] );
- $this->assertTrue( $this->getHelper()->hasClassInRelation(
$statements, 'P31', [ 'Q4' ] ) );
+ $this->assertTrue( $this->getHelper()->hasClassInRelation(
$statements, 'P31', [ 'Q4' ] )->getBool() );
}
public function testCheckIsSubclassOfValidWithIndirection() {
- $this->assertTrue(
$this->getHelper()->isSubclassOfWithSparqlFallback( new ItemId( 'Q6' ), [
'Q100', 'Q101' ] ) );
+ $this->assertTrue(
$this->getHelper()->isSubclassOfWithSparqlFallback( new ItemId( 'Q6' ), [
'Q100', 'Q101' ] )->getBool() );
}
public function testCheckIsSubclassOfInvalid() {
- $this->assertFalse(
$this->getHelper()->isSubclassOfWithSparqlFallback( new ItemId( 'Q6' ), [
'Q200', 'Q201' ] ) );
+ $this->assertFalse(
$this->getHelper()->isSubclassOfWithSparqlFallback( new ItemId( 'Q6' ), [
'Q200', 'Q201' ] )->getBool() );
}
public function testCheckIsSubclassCyclic() {
$helper = $this->getHelper( $this->getMaxEntitiesLookup() );
- $this->assertFalse( $helper->isSubclassOfWithSparqlFallback(
new ItemId( 'Q7' ), [ 'Q100', 'Q101' ] ) );
+ $this->assertFalse( $helper->isSubclassOfWithSparqlFallback(
new ItemId( 'Q7' ), [ 'Q100', 'Q101' ] )->getBool() );
}
public function testCheckIsSubclassCyclicWide() {
$helper = $this->getHelper( $this->getMaxEntitiesLookup() );
- $this->assertFalse( $helper->isSubclassOfWithSparqlFallback(
new ItemId( 'Q9' ), [ 'Q100', 'Q101' ] ) );
+ $this->assertFalse( $helper->isSubclassOfWithSparqlFallback(
new ItemId( 'Q9' ), [ 'Q100', 'Q101' ] )->getBool() );
}
public function testCheckIsSubclassCyclicWideWithSparqlTrue() {
$helper = $this->getHelper( $this->getMaxEntitiesLookup(),
$this->getSparqlHelper( true ) );
- $this->assertTrue( $helper->isSubclassOfWithSparqlFallback( new
ItemId( 'Q9' ), [ 'Q100', 'Q101' ] ) );
+ $this->assertTrue( $helper->isSubclassOfWithSparqlFallback( new
ItemId( 'Q9' ), [ 'Q100', 'Q101' ] )->getBool() );
}
public function testCheckIsSubclassCyclicWideWithSparqlFalse() {
$helper = $this->getHelper( $this->getMaxEntitiesLookup(),
$this->getSparqlHelper( false ) );
- $this->assertFalse( $helper->isSubclassOfWithSparqlFallback(
new ItemId( 'Q9' ), [ 'Q100', 'Q101' ] ) );
+ $this->assertFalse( $helper->isSubclassOfWithSparqlFallback(
new ItemId( 'Q9' ), [ 'Q100', 'Q101' ] )->getBool() );
}
public function testCheckIsSubclassTree() {
@@ -183,7 +185,7 @@
);
$helper = $this->getHelper( $this->getMaxEntitiesLookup(
$lookup ), $sparqlHelper );
- $this->assertTrue( $helper->isSubclassOfWithSparqlFallback( new
ItemId( 'Q1' ), [ 'Q5' ] ) );
+ $this->assertTrue( $helper->isSubclassOfWithSparqlFallback( new
ItemId( 'Q1' ), [ 'Q5' ] )->getBool() );
}
}
diff --git a/tests/phpunit/Helper/SparqlHelperTest.php
b/tests/phpunit/Helper/SparqlHelperTest.php
index 6068026..a2af2fb 100644
--- a/tests/phpunit/Helper/SparqlHelperTest.php
+++ b/tests/phpunit/Helper/SparqlHelperTest.php
@@ -82,7 +82,7 @@
->willReturn( $this->askResult( true ) )
->withConsecutive( [ $this->equalTo( $query ) ] );
- $this->assertTrue( $sparqlHelper->hasType( 'Q1', [ 'Q100',
'Q101' ], true ) );
+ $this->assertTrue( $sparqlHelper->hasType( 'Q1', [ 'Q100',
'Q101' ], true )->getBool() );
}
public function testFindEntitiesWithSameStatement() {
--
To view, visit https://gerrit.wikimedia.org/r/391279
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie0510d7e36b5ea9e4350160fe4ca25c2293df3b3
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/WikibaseQualityConstraints
Gerrit-Branch: master
Gerrit-Owner: Lucas Werkmeister (WMDE) <[email protected]>
Gerrit-Reviewer: WMDE-leszek <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
