Aude has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/353279 )
Change subject: Fix type/subclass check limit
......................................................................
Fix type/subclass check limit
Instead of counting only recursion depth, and effectively decrementing
the counter on every return, count total invocations of the method and
don’t decrement the counter in one check operation.
Since this results in a higher count for classes with more than one
superclass, slightly increase the limit.
Change-Id: I39429d37734f25748f8b7e194246613592edb241
(cherry picked from commit 0408a71186e353703d855e558e0288b3da3a7f41)
---
M includes/ConstraintCheck/Helper/TypeCheckerHelper.php
M tests/phpunit/Checker/TypeChecker/TypeCheckerHelperTest.php
2 files changed, 16 insertions(+), 16 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/WikibaseQualityConstraints
refs/changes/79/353279/1
diff --git a/includes/ConstraintCheck/Helper/TypeCheckerHelper.php
b/includes/ConstraintCheck/Helper/TypeCheckerHelper.php
index 72a4566..261e31f 100644
--- a/includes/ConstraintCheck/Helper/TypeCheckerHelper.php
+++ b/includes/ConstraintCheck/Helper/TypeCheckerHelper.php
@@ -22,7 +22,7 @@
*/
class TypeCheckerHelper {
- const MAX_DEPTH = 20;
+ const MAX_ENTITIES = 50;
/**
* @var EntityLookup
@@ -40,18 +40,22 @@
}
/**
- * Checks if one of the itemId serializations in $classesToCheck
- * is subclass of $comparativeClass
- * Due to cyclic dependencies, the checks stops after a certain
- * depth is reached
+ * Checks if $comparativeClass is a subclass
+ * of one of the item ID serializations in $classesToCheck.
+ * If the class hierarchy is not exhausted after checking MAX_ENTITIES
entities,
+ * the check aborts and returns false.
*
* @param EntityId $comparativeClass
* @param string[] $classesToCheck
- * @param int $depth
+ * @param int &$entitiesChecked
*
* @return bool
*/
- public function isSubclassOf( EntityId $comparativeClass, array
$classesToCheck, $depth ) {
+ public function isSubclassOf( EntityId $comparativeClass, array
$classesToCheck, &$entitiesChecked = 0 ) {
+ if ( $entitiesChecked++ > self::MAX_ENTITIES ) {
+ return false;
+ }
+
$item = $this->entityLookup->getEntity( $comparativeClass );
if ( !( $item instanceof StatementListProvider ) ) {
return false; // lookup failed, probably because item
doesn't exist
@@ -75,11 +79,7 @@
return true;
}
- if ( $depth > self::MAX_DEPTH ) {
- return false;
- }
-
- if ( $this->isSubclassOf( $comparativeClass,
$classesToCheck, $depth + 1 ) ) {
+ if ( $this->isSubclassOf( $comparativeClass,
$classesToCheck, $entitiesChecked ) ) {
return true;
}
}
@@ -117,7 +117,7 @@
return true;
}
- if ( $this->isSubclassOf( $comparativeClass,
$classesToCheck, 1 ) ) {
+ if ( $this->isSubclassOf( $comparativeClass,
$classesToCheck ) ) {
return true;
}
}
diff --git a/tests/phpunit/Checker/TypeChecker/TypeCheckerHelperTest.php
b/tests/phpunit/Checker/TypeChecker/TypeCheckerHelperTest.php
index ac9e7c7..8d6a7d3 100644
--- a/tests/phpunit/Checker/TypeChecker/TypeCheckerHelperTest.php
+++ b/tests/phpunit/Checker/TypeChecker/TypeCheckerHelperTest.php
@@ -65,15 +65,15 @@
}
public function testCheckIsSubclassOfValidWithIndirection() {
- $this->assertEquals( true, $this->helper->isSubclassOf( new
ItemId( 'Q6' ), [ 'Q100', 'Q101' ], 1) );
+ $this->assertEquals( true, $this->helper->isSubclassOf( new
ItemId( 'Q6' ), [ 'Q100', 'Q101' ] ) );
}
public function testCheckIsSubclassOfInvalid() {
- $this->assertEquals( false, $this->helper->isSubclassOf( new
ItemId( 'Q6' ), [ 'Q200', 'Q201' ], 1) );
+ $this->assertEquals( false, $this->helper->isSubclassOf( new
ItemId( 'Q6' ), [ 'Q200', 'Q201' ] ) );
}
public function testCheckIsSubclassCyclic() {
- $this->assertEquals( false, $this->helper->isSubclassOf( new
ItemId( 'Q7' ), [ 'Q100', 'Q101' ], 1) );
+ $this->assertEquals( false, $this->helper->isSubclassOf( new
ItemId( 'Q7' ), [ 'Q100', 'Q101' ] ) );
}
}
--
To view, visit https://gerrit.wikimedia.org/r/353279
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I39429d37734f25748f8b7e194246613592edb241
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/WikibaseQualityConstraints
Gerrit-Branch: wmf/1.30.0-wmf.1
Gerrit-Owner: Aude <aude.w...@gmail.com>
Gerrit-Reviewer: Lucas Werkmeister (WMDE) <lucas.werkmeis...@wikimedia.de>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
