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

Change subject: Add ConstraintChecker::checkConstraintParameters method
......................................................................

Add ConstraintChecker::checkConstraintParameters method

This void method throws a ConstraintParameterException if the constraint
parameters are invalid. The implementation is straightforward: copy the
calls to ConstraintParameterParser from the checkConstraint method and
discard the result.

The only complication is in RangeChecker, where the parameter parsing
depends on the type of the data (quantity or time). To solve this,
RangeChecker gets an EntityLookup injected, where it can look up the
data type of the property that the constraint belongs to. (Strictly
speaking, this is not exactly equivalent to the checkConstraint method,
which checks the data value type, but for quantity and time types there
is no difference between the two.)

This new method constitutes the first part of T169531; a second commit
will add a new API action to call this method, and a third commit will
use that new API action in the gadget.

Bug: T169531
Change-Id: I338577883d9bccfc2ea990f1ccc7173a3a503136
---
M includes/ConstraintCheck/Checker/CommonsLinkChecker.php
M includes/ConstraintCheck/Checker/ConflictsWithChecker.php
M includes/ConstraintCheck/Checker/DiffWithinRangeChecker.php
M includes/ConstraintCheck/Checker/FormatChecker.php
M includes/ConstraintCheck/Checker/InverseChecker.php
M includes/ConstraintCheck/Checker/ItemChecker.php
M includes/ConstraintCheck/Checker/MandatoryQualifiersChecker.php
M includes/ConstraintCheck/Checker/MultiValueChecker.php
M includes/ConstraintCheck/Checker/OneOfChecker.php
M includes/ConstraintCheck/Checker/QualifierChecker.php
M includes/ConstraintCheck/Checker/QualifiersChecker.php
M includes/ConstraintCheck/Checker/RangeChecker.php
M includes/ConstraintCheck/Checker/SingleValueChecker.php
M includes/ConstraintCheck/Checker/SymmetricChecker.php
M includes/ConstraintCheck/Checker/TargetRequiredClaimChecker.php
M includes/ConstraintCheck/Checker/TypeChecker.php
M includes/ConstraintCheck/Checker/UniqueValueChecker.php
M includes/ConstraintCheck/Checker/ValueTypeChecker.php
M includes/ConstraintCheck/ConstraintChecker.php
M includes/ConstraintReportFactory.php
M tests/phpunit/Checker/RangeChecker/RangeCheckerTest.php
M tests/phpunit/Fake/FakeChecker.php
22 files changed, 130 insertions(+), 1 deletion(-)


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

diff --git a/includes/ConstraintCheck/Checker/CommonsLinkChecker.php 
b/includes/ConstraintCheck/Checker/CommonsLinkChecker.php
index d84762c..ff34984 100644
--- a/includes/ConstraintCheck/Checker/CommonsLinkChecker.php
+++ b/includes/ConstraintCheck/Checker/CommonsLinkChecker.php
@@ -124,6 +124,11 @@
                return new CheckResult( $entity->getId(), $statement, 
$constraint,  $parameters, $status, $message );
        }
 
+       public function checkConstraintParameters( Constraint $constraint ) {
+               $constraintParameters = $constraint->getConstraintParameters();
+               $this->constraintParameterParser->parseNamespaceParameter( 
$constraintParameters, $constraint->getConstraintTypeItemId() );
+       }
+
        /**
         * @param TitleValue $title
         *
diff --git a/includes/ConstraintCheck/Checker/ConflictsWithChecker.php 
b/includes/ConstraintCheck/Checker/ConflictsWithChecker.php
index 6f0e794..264f762 100644
--- a/includes/ConstraintCheck/Checker/ConflictsWithChecker.php
+++ b/includes/ConstraintCheck/Checker/ConflictsWithChecker.php
@@ -115,4 +115,10 @@
                return new CheckResult( $entity->getId(), $statement, 
$constraint,  $parameters, $status, $message );
        }
 
+       public function checkConstraintParameters( Constraint $constraint ) {
+               $constraintParameters = $constraint->getConstraintParameters();
+               $this->constraintParameterParser->parsePropertyParameter( 
$constraintParameters, $constraint->getConstraintTypeItemId() );
+               $this->constraintParameterParser->parseItemsParameter( 
$constraintParameters, $constraint->getConstraintTypeItemId(), false );
+       }
+
 }
diff --git a/includes/ConstraintCheck/Checker/DiffWithinRangeChecker.php 
b/includes/ConstraintCheck/Checker/DiffWithinRangeChecker.php
index 9597437..a1edeea 100644
--- a/includes/ConstraintCheck/Checker/DiffWithinRangeChecker.php
+++ b/includes/ConstraintCheck/Checker/DiffWithinRangeChecker.php
@@ -156,4 +156,14 @@
                return new CheckResult( $entity->getId(), $statement, 
$constraint,  $parameters, $status, $message );
        }
 
+       public function checkConstraintParameters( Constraint $constraint ) {
+               $constraintParameters = $constraint->getConstraintParameters();
+               $this->constraintParameterParser->parseRangeParameter(
+                       $constraintParameters,
+                       $constraint->getConstraintTypeItemId(),
+                       'quantity'
+               );
+               $this->constraintParameterParser->parsePropertyParameter( 
$constraintParameters, $constraint->getConstraintTypeItemId() );
+       }
+
 }
diff --git a/includes/ConstraintCheck/Checker/FormatChecker.php 
b/includes/ConstraintCheck/Checker/FormatChecker.php
index 7a6d6c2..cf7777a 100644
--- a/includes/ConstraintCheck/Checker/FormatChecker.php
+++ b/includes/ConstraintCheck/Checker/FormatChecker.php
@@ -94,4 +94,9 @@
                return new CheckResult( $entity->getId(), $statement, 
$constraint, $parameters, CheckResult::STATUS_TODO, $message );
        }
 
+       public function checkConstraintParameters( Constraint $constraint ) {
+               $constraintParameters = $constraint->getConstraintParameters();
+               $this->constraintParameterParser->parseFormatParameter( 
$constraintParameters, $constraint->getConstraintTypeItemId() );
+       }
+
 }
diff --git a/includes/ConstraintCheck/Checker/InverseChecker.php 
b/includes/ConstraintCheck/Checker/InverseChecker.php
index b8b9b5c..5ef78b2 100644
--- a/includes/ConstraintCheck/Checker/InverseChecker.php
+++ b/includes/ConstraintCheck/Checker/InverseChecker.php
@@ -130,4 +130,9 @@
                return new CheckResult( $entity->getId(), $statement, 
$constraint, $parameters, $status, $message );
        }
 
+       public function checkConstraintParameters( Constraint $constraint ) {
+               $constraintParameters = $constraint->getConstraintParameters();
+               $this->constraintParameterParser->parsePropertyParameter( 
$constraintParameters, $constraint->getConstraintTypeItemId() );
+       }
+
 }
diff --git a/includes/ConstraintCheck/Checker/ItemChecker.php 
b/includes/ConstraintCheck/Checker/ItemChecker.php
index cf554a3..5195a33 100644
--- a/includes/ConstraintCheck/Checker/ItemChecker.php
+++ b/includes/ConstraintCheck/Checker/ItemChecker.php
@@ -112,4 +112,10 @@
                return new CheckResult( $entity->getId(), $statement, 
$constraint, $parameters, $status, $message );
        }
 
+       public function checkConstraintParameters( Constraint $constraint ) {
+               $constraintParameters = $constraint->getConstraintParameters();
+               $this->constraintParameterParser->parsePropertyParameter( 
$constraintParameters, $constraint->getConstraintTypeItemId() );
+               $this->constraintParameterParser->parseItemsParameter( 
$constraintParameters, $constraint->getConstraintTypeItemId(), false );
+       }
+
 }
diff --git a/includes/ConstraintCheck/Checker/MandatoryQualifiersChecker.php 
b/includes/ConstraintCheck/Checker/MandatoryQualifiersChecker.php
index c9c676b..667cea8 100644
--- a/includes/ConstraintCheck/Checker/MandatoryQualifiersChecker.php
+++ b/includes/ConstraintCheck/Checker/MandatoryQualifiersChecker.php
@@ -77,4 +77,9 @@
                return new CheckResult( $entity->getId(), $statement, 
$constraint, $parameters, $status, $message );
        }
 
+       public function checkConstraintParameters( Constraint $constraint ) {
+               $constraintParameters = $constraint->getConstraintParameters();
+               $this->constraintParameterParser->parsePropertyParameter( 
$constraintParameters, $constraint->getConstraintTypeItemId() );
+       }
+
 }
diff --git a/includes/ConstraintCheck/Checker/MultiValueChecker.php 
b/includes/ConstraintCheck/Checker/MultiValueChecker.php
index 7edaa21..58ed1ed 100644
--- a/includes/ConstraintCheck/Checker/MultiValueChecker.php
+++ b/includes/ConstraintCheck/Checker/MultiValueChecker.php
@@ -53,4 +53,8 @@
                return new CheckResult( $entity->getId(), $statement, 
$constraint, $parameters, $status, $message );
        }
 
+       public function checkConstraintParameters( Constraint $constraint ) {
+               // no parameters
+       }
+
 }
diff --git a/includes/ConstraintCheck/Checker/OneOfChecker.php 
b/includes/ConstraintCheck/Checker/OneOfChecker.php
index a289593..f455f42 100644
--- a/includes/ConstraintCheck/Checker/OneOfChecker.php
+++ b/includes/ConstraintCheck/Checker/OneOfChecker.php
@@ -76,4 +76,9 @@
                return new CheckResult( $entity->getId(), $statement, 
$constraint, $parameters, $status, $message );
        }
 
+       public function checkConstraintParameters( Constraint $constraint ) {
+               $constraintParameters = $constraint->getConstraintParameters();
+               $this->constraintParameterParser->parseItemsParameter( 
$constraintParameters, $constraint->getConstraintTypeItemId(), true );
+       }
+
 }
diff --git a/includes/ConstraintCheck/Checker/QualifierChecker.php 
b/includes/ConstraintCheck/Checker/QualifierChecker.php
index a3095bb..78d8d3f 100644
--- a/includes/ConstraintCheck/Checker/QualifierChecker.php
+++ b/includes/ConstraintCheck/Checker/QualifierChecker.php
@@ -34,4 +34,8 @@
                return new CheckResult( $entity->getId(), $statement, 
$constraint, [], CheckResult::STATUS_VIOLATION, $message );
        }
 
+       public function checkConstraintParameters( Constraint $constraint ) {
+               // no parameters
+       }
+
 }
diff --git a/includes/ConstraintCheck/Checker/QualifiersChecker.php 
b/includes/ConstraintCheck/Checker/QualifiersChecker.php
index a1d6ed3..581f054 100644
--- a/includes/ConstraintCheck/Checker/QualifiersChecker.php
+++ b/includes/ConstraintCheck/Checker/QualifiersChecker.php
@@ -93,4 +93,9 @@
                return new CheckResult( $entity->getId(), $statement, 
$constraint, $parameters, $status, $message );
        }
 
+       public function checkConstraintParameters( Constraint $constraint ) {
+               $constraintParameters = $constraint->getConstraintParameters();
+               $this->constraintParameterParser->parsePropertiesParameter( 
$constraintParameters, $constraint->getConstraintTypeItemId() );
+       }
+
 }
diff --git a/includes/ConstraintCheck/Checker/RangeChecker.php 
b/includes/ConstraintCheck/Checker/RangeChecker.php
index 12758c3..01d486a 100644
--- a/includes/ConstraintCheck/Checker/RangeChecker.php
+++ b/includes/ConstraintCheck/Checker/RangeChecker.php
@@ -3,10 +3,12 @@
 namespace WikibaseQuality\ConstraintReport\ConstraintCheck\Checker;
 
 use Wikibase\DataModel\Entity\EntityDocument;
+use Wikibase\DataModel\Services\Lookup\EntityLookup;
 use Wikibase\DataModel\Snak\PropertyValueSnak;
 use Wikibase\DataModel\Statement\StatementListProvider;
 use WikibaseQuality\ConstraintReport\Constraint;
 use WikibaseQuality\ConstraintReport\ConstraintCheck\ConstraintChecker;
+use 
WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\ConstraintParameterException;
 use 
WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\ConstraintParameterParser;
 use WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\RangeCheckerHelper;
 use WikibaseQuality\ConstraintReport\ConstraintCheck\Result\CheckResult;
@@ -19,6 +21,11 @@
  * @license GNU GPL v2+
  */
 class RangeChecker implements ConstraintChecker {
+
+       /**
+        * @var EntityLookup
+        */
+       private $entityLookup;
 
        /**
         * @var ConstraintParameterParser
@@ -36,15 +43,18 @@
        private $constraintParameterRenderer;
 
        /**
+        * @param EntityLookup $lookup
         * @param ConstraintParameterParser $constraintParameterParser
         * @param RangeCheckerHelper $rangeCheckerHelper
         * @param ConstraintParameterRenderer $constraintParameterRenderer
         */
        public function __construct(
+               EntityLookup $lookup,
                ConstraintParameterParser $constraintParameterParser,
                RangeCheckerHelper $rangeCheckerHelper,
                ConstraintParameterRenderer $constraintParameterRenderer
        ) {
+               $this->entityLookup = $lookup;
                $this->constraintParameterParser = $constraintParameterParser;
                $this->rangeCheckerHelper = $rangeCheckerHelper;
                $this->constraintParameterRenderer = 
$constraintParameterRenderer;
@@ -124,4 +134,17 @@
                );
        }
 
+       public function checkConstraintParameters( Constraint $constraint ) {
+               $constraintParameters = $constraint->getConstraintParameters();
+               // we don’t have a data value here, so get the type from the 
property instead
+               // (the distinction between data type and data value type is 
irrelevant for 'quantity' and 'time')
+               $property = $this->entityLookup->getEntity( 
$constraint->getPropertyId() );
+               $type = $property->getDataTypeId();
+               $this->constraintParameterParser->parseRangeParameter(
+                       $constraintParameters,
+                       $constraint->getConstraintTypeItemId(),
+                       $type
+               );
+       }
+
 }
diff --git a/includes/ConstraintCheck/Checker/SingleValueChecker.php 
b/includes/ConstraintCheck/Checker/SingleValueChecker.php
index 0c880e4..fa9621f 100644
--- a/includes/ConstraintCheck/Checker/SingleValueChecker.php
+++ b/includes/ConstraintCheck/Checker/SingleValueChecker.php
@@ -53,4 +53,8 @@
                return new CheckResult( $entity->getId(), $statement, 
$constraint, $parameters, $status, $message );
        }
 
+       public function checkConstraintParameters( Constraint $constraint ) {
+               // no parameters
+       }
+
 }
diff --git a/includes/ConstraintCheck/Checker/SymmetricChecker.php 
b/includes/ConstraintCheck/Checker/SymmetricChecker.php
index 3b6be78..52b384e 100644
--- a/includes/ConstraintCheck/Checker/SymmetricChecker.php
+++ b/includes/ConstraintCheck/Checker/SymmetricChecker.php
@@ -118,4 +118,8 @@
                return new CheckResult( $entity->getId(), $statement, 
$constraint, $parameters, $status, $message );
        }
 
+       public function checkConstraintParameters( Constraint $constraint ) {
+               // no parameters
+       }
+
 }
diff --git a/includes/ConstraintCheck/Checker/TargetRequiredClaimChecker.php 
b/includes/ConstraintCheck/Checker/TargetRequiredClaimChecker.php
index 516a381..f2a198a 100644
--- a/includes/ConstraintCheck/Checker/TargetRequiredClaimChecker.php
+++ b/includes/ConstraintCheck/Checker/TargetRequiredClaimChecker.php
@@ -152,4 +152,10 @@
                return new CheckResult( $entity->getId(), $statement, 
$constraint, $parameters, $status, $message );
        }
 
+       public function checkConstraintParameters( Constraint $constraint ) {
+               $constraintParameters = $constraint->getConstraintParameters();
+               $this->constraintParameterParser->parsePropertyParameter( 
$constraintParameters, $constraint->getConstraintTypeItemId() );
+               $this->constraintParameterParser->parseItemsParameter( 
$constraintParameters, $constraint->getConstraintTypeItemId(), false );
+       }
+
 }
diff --git a/includes/ConstraintCheck/Checker/TypeChecker.php 
b/includes/ConstraintCheck/Checker/TypeChecker.php
index d561098..9f3382f 100644
--- a/includes/ConstraintCheck/Checker/TypeChecker.php
+++ b/includes/ConstraintCheck/Checker/TypeChecker.php
@@ -108,4 +108,10 @@
                return new CheckResult( $entity->getId(), $statement, 
$constraint, $parameters, $status, $message );
        }
 
+       public function checkConstraintParameters( Constraint $constraint ) {
+               $constraintParameters = $constraint->getConstraintParameters();
+               $this->constraintParameterParser->parseClassParameter( 
$constraintParameters, $constraint->getConstraintTypeItemId() );
+               $this->constraintParameterParser->parseRelationParameter( 
$constraintParameters, $constraint->getConstraintTypeItemId() );
+       }
+
 }
diff --git a/includes/ConstraintCheck/Checker/UniqueValueChecker.php 
b/includes/ConstraintCheck/Checker/UniqueValueChecker.php
index 9d8fc53..f0a397d 100644
--- a/includes/ConstraintCheck/Checker/UniqueValueChecker.php
+++ b/includes/ConstraintCheck/Checker/UniqueValueChecker.php
@@ -78,4 +78,8 @@
                return new CheckResult( $entity->getId(), $statement, 
$constraint, $parameters, $status, $message );
        }
 
+       public function checkConstraintParameters( Constraint $constraint ) {
+               // no parameters
+       }
+
 }
diff --git a/includes/ConstraintCheck/Checker/ValueTypeChecker.php 
b/includes/ConstraintCheck/Checker/ValueTypeChecker.php
index 5a773ed..4b37257 100644
--- a/includes/ConstraintCheck/Checker/ValueTypeChecker.php
+++ b/includes/ConstraintCheck/Checker/ValueTypeChecker.php
@@ -142,4 +142,10 @@
                return new CheckResult( $entity->getId(), $statement, 
$constraint, $parameters, $status, $message );
        }
 
+       public function checkConstraintParameters( Constraint $constraint ) {
+               $constraintParameters = $constraint->getConstraintParameters();
+               $this->constraintParameterParser->parseClassParameter( 
$constraintParameters, $constraint->getConstraintTypeItemId() );
+               $this->constraintParameterParser->parseRelationParameter( 
$constraintParameters, $constraint->getConstraintTypeItemId() );
+       }
+
 }
diff --git a/includes/ConstraintCheck/ConstraintChecker.php 
b/includes/ConstraintCheck/ConstraintChecker.php
index b2d60d5..fb251c6 100644
--- a/includes/ConstraintCheck/ConstraintChecker.php
+++ b/includes/ConstraintCheck/ConstraintChecker.php
@@ -24,4 +24,16 @@
         */
        public function checkConstraint( Statement $statement, Constraint 
$constraint, EntityDocument $entity );
 
+       /**
+        * Check if the constraint parameters of $constraint are valid,
+        * and throw a ConstraintParameterException with an appropriate message 
if they are not.
+        *
+        * @param Constraint $constraint
+        *
+        * @return void
+        *
+        * @throws ConstraintParameterException if the constraint parameters 
are invalid
+        */
+       public function checkConstraintParameters( Constraint $constraint );
+
 }
diff --git a/includes/ConstraintReportFactory.php 
b/includes/ConstraintReportFactory.php
index 96cd99a..dab2e12 100644
--- a/includes/ConstraintReportFactory.php
+++ b/includes/ConstraintReportFactory.php
@@ -215,7 +215,7 @@
                                'Qualifier' => new QualifierChecker(),
                                'Qualifiers' => new QualifiersChecker( 
$this->constraintParameterParser, $this->constraintParameterRenderer ),
                                'Mandatory qualifiers' => new 
MandatoryQualifiersChecker( $this->constraintParameterParser, 
$this->constraintParameterRenderer ),
-                               'Range' => new RangeChecker( 
$this->constraintParameterParser, $rangeCheckerHelper, 
$this->constraintParameterRenderer ),
+                               'Range' => new RangeChecker( $this->lookup, 
$this->constraintParameterParser, $rangeCheckerHelper, 
$this->constraintParameterRenderer ),
                                'Diff within range' => new 
DiffWithinRangeChecker( $this->constraintParameterParser, $rangeCheckerHelper, 
$this->constraintParameterRenderer ),
                                'Type' => new TypeChecker( $this->lookup, 
$this->constraintParameterParser, $typeCheckerHelper, $this->config ),
                                'Value type' => new ValueTypeChecker( 
$this->lookup, $this->constraintParameterParser, $typeCheckerHelper, 
$this->config ),
diff --git a/tests/phpunit/Checker/RangeChecker/RangeCheckerTest.php 
b/tests/phpunit/Checker/RangeChecker/RangeCheckerTest.php
index 0cb2920..519fcf7 100644
--- a/tests/phpunit/Checker/RangeChecker/RangeCheckerTest.php
+++ b/tests/phpunit/Checker/RangeChecker/RangeCheckerTest.php
@@ -55,6 +55,7 @@
                $this->lookup = new JsonFileEntityLookup( __DIR__ );
                $this->timeValue = new TimeValue( 
'+00000001970-01-01T00:00:00Z', 0, 0, 0, 11, 
'http://www.wikidata.org/entity/Q1985727' );
                $this->checker = new RangeChecker(
+                       $this->lookup,
                        $this->getConstraintParameterParser(),
                        new RangeCheckerHelper( $this->getDefaultConfig() ),
                        $this->getConstraintParameterRenderer()
diff --git a/tests/phpunit/Fake/FakeChecker.php 
b/tests/phpunit/Fake/FakeChecker.php
index d0e317e..eefd96a 100644
--- a/tests/phpunit/Fake/FakeChecker.php
+++ b/tests/phpunit/Fake/FakeChecker.php
@@ -35,4 +35,7 @@
                );
        }
 
+       public function checkConstraintParameters( Constraint $constraint ) {
+       }
+
 }

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I338577883d9bccfc2ea990f1ccc7173a3a503136
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/WikibaseQualityConstraints
Gerrit-Branch: master
Gerrit-Owner: Lucas Werkmeister (WMDE) <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to