Lucas Werkmeister (WMDE) has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/351628 )
Change subject: Parse and correctly compare time values
......................................................................
Parse and correctly compare time values
RangeCheckerHelper::getComparativeValue() now returns an array for time
values; comparing two such arrays correctly compares the dates.
DiffWithinRangeChecker uses the first component of the array to compute
the difference in years.
RangeCheckerHelper also gains a new method, getComparativeTimeValue(),
which parses a time value using Wikibase’s standard time parser chain.
RangeChecker uses this method to parse its min/max parameters, which
fixes T164279.
The test skipped in 1644986 can be unskipped again, since the parameters
are now properly parsed, and the test added in 7644ede can now use a
zero-padded timestamp like all other timestamps in the file. Two new
tests for RangeChecker are also added, which test that parameters
containing only a year or only a date work correctly, and two new tests
for RangeCheckerHelper test the new function.
Bug: T164279
Change-Id: I902156e4e013e6c5f3c9c73bf35919de9264650a
---
M includes/ConstraintCheck/Checker/DiffWithinRangeChecker.php
M includes/ConstraintCheck/Checker/RangeChecker.php
M includes/ConstraintCheck/Helper/RangeCheckerHelper.php
M includes/ConstraintReportFactory.php
M tests/phpunit/Checker/RangeChecker/DiffWithinRangeCheckerTest.php
M tests/phpunit/Checker/RangeChecker/RangeCheckerHelperTest.php
M tests/phpunit/Checker/RangeChecker/RangeCheckerTest.php
7 files changed, 91 insertions(+), 9 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/WikibaseQualityConstraints
refs/changes/28/351628/1
diff --git a/includes/ConstraintCheck/Checker/DiffWithinRangeChecker.php
b/includes/ConstraintCheck/Checker/DiffWithinRangeChecker.php
index 3f39227..a2acb1d 100644
--- a/includes/ConstraintCheck/Checker/DiffWithinRangeChecker.php
+++ b/includes/ConstraintCheck/Checker/DiffWithinRangeChecker.php
@@ -114,6 +114,13 @@
if ( $mainSnak->getDataValue()->getType() ===
$dataValue->getType() && $mainSnak->getType() === 'value' ) {
$thatValue =
$this->rangeCheckerHelper->getComparativeValue( $mainSnak->getDataValue() );
+ if ( $dataValue->getType() === 'time' )
{
+ // getComparativeValue()
returned an array, take only the year and convert it back to an integer
+ // TODO very large years might
not fit in an integer – TimeValue itself carefully avoids casting the year to
integer
+ $thisValue = (int)$thisValue[0];
+ $thatValue = (int)$thatValue[0];
+ }
+
// negative difference is possible
$diff = $thisValue - $thatValue;
diff --git a/includes/ConstraintCheck/Checker/RangeChecker.php
b/includes/ConstraintCheck/Checker/RangeChecker.php
index 560d21a..4f3dd89 100644
--- a/includes/ConstraintCheck/Checker/RangeChecker.php
+++ b/includes/ConstraintCheck/Checker/RangeChecker.php
@@ -122,6 +122,8 @@
if ( $max === 'now' ) {
$max = $now;
}
+ $min =
$this->rangeCheckerHelper->getComparativeTimeValue( $min );
+ $max =
$this->rangeCheckerHelper->getComparativeTimeValue( $max );
}
} else {
$message = wfMessage(
'wbqc-violation-message-value-needed-of-types-2' )
diff --git a/includes/ConstraintCheck/Helper/RangeCheckerHelper.php
b/includes/ConstraintCheck/Helper/RangeCheckerHelper.php
index 00338e7..8751760 100644
--- a/includes/ConstraintCheck/Helper/RangeCheckerHelper.php
+++ b/includes/ConstraintCheck/Helper/RangeCheckerHelper.php
@@ -7,6 +7,7 @@
use DataValues\TimeValue;
use DataValues\UnboundedQuantityValue;
use InvalidArgumentException;
+use ValueParsers\ValueParser;
/**
* Class for helper functions for range checkers.
@@ -17,9 +18,32 @@
*/
class RangeCheckerHelper {
+ /**
+ * @var ValueParser
+ */
+ private $timeParser;
+
+ /**
+ * @param ValueParser $timeParser
+ */
+ public function __construct( ValueParser $timeParser ) {
+ $this->timeParser = $timeParser;
+ }
+
public function getComparativeValue( DataValue $dataValue ) {
if ( $dataValue instanceof TimeValue ) {
- return $dataValue->getTime();
+ if ( !preg_match(
+ // copied from TimeValue::normalizeIsoTimestamp
+
'/^([-+])(\d{1,16})-(\d\d)-(\d\d)T(\d\d):(\d\d):(\d\d)Z$/',
+ $dataValue->getTime(),
+ $matches
+ ) ) {
+ throw new InvalidArgumentException(
'TimeValue::getTime() did not match expected format' );
+ }
+
+ list( , $sign, $year, $month, $day, $hour, $minute,
$second ) = $matches;
+
+ return [ $sign . $year, $sign . $month, $sign . $day,
$sign . $hour, $sign . $minute, $sign . $second ];
} elseif ( $dataValue instanceof QuantityValue ) {
return $dataValue->getAmount()->getValue();
} elseif ( $dataValue instanceof UnboundedQuantityValue ) {
@@ -29,4 +53,12 @@
throw new InvalidArgumentException( 'Unsupported data value
type' );
}
+ /**
+ * @param string $timeString
+ * @return string
+ */
+ public function getComparativeTimeValue( $timeString ) {
+ return $this->getComparativeValue( $this->timeParser->parse(
$timeString ) );
+ }
+
}
diff --git a/includes/ConstraintReportFactory.php
b/includes/ConstraintReportFactory.php
index 89f2356..d7d78bd 100644
--- a/includes/ConstraintReportFactory.php
+++ b/includes/ConstraintReportFactory.php
@@ -5,6 +5,7 @@
use Config;
use MediaWiki\MediaWikiServices;
use Wikibase\DataModel\Services\Lookup\EntityLookup;
+use Wikibase\Repo\Parsers\TimeParserFactory;
use Wikibase\Repo\WikibaseRepo;
use
WikibaseQuality\ConstraintReport\ConstraintCheck\DelegatingConstraintChecker;
use
WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\ConstraintParameterParser;
@@ -117,7 +118,7 @@
if ( $this->constraintCheckerMap === null ) {
$constraintParameterParser = new
ConstraintParameterParser();
$connectionCheckerHelper = new
ConnectionCheckerHelper();
- $rangeCheckerHelper = new RangeCheckerHelper();
+ $rangeCheckerHelper = new RangeCheckerHelper( (new
TimeParserFactory())->getTimeParser() );
$typeCheckerHelper = new TypeCheckerHelper(
$this->lookup, $this->config );
$this->constraintCheckerMap = array(
diff --git a/tests/phpunit/Checker/RangeChecker/DiffWithinRangeCheckerTest.php
b/tests/phpunit/Checker/RangeChecker/DiffWithinRangeCheckerTest.php
index 9b329a4..a7759ac 100644
--- a/tests/phpunit/Checker/RangeChecker/DiffWithinRangeCheckerTest.php
+++ b/tests/phpunit/Checker/RangeChecker/DiffWithinRangeCheckerTest.php
@@ -9,6 +9,7 @@
use DataValues\TimeValue;
use Wikibase\DataModel\Entity\ItemId;
use Wikibase\DataModel\Entity\PropertyId;
+use Wikibase\Repo\Parsers\TimeParserFactory;
use WikibaseQuality\ConstraintReport\Constraint;
use
WikibaseQuality\ConstraintReport\ConstraintCheck\Checker\DiffWithinRangeChecker;
use
WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\ConstraintParameterParser;
@@ -53,7 +54,7 @@
$this->helper = new ConstraintParameterParser();
$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 DiffWithinRangeChecker( $this->helper, new
RangeCheckerHelper() );
+ $this->checker = new DiffWithinRangeChecker( $this->helper, new
RangeCheckerHelper( (new TimeParserFactory())->getTimeParser() ) );
}
protected function tearDown() {
diff --git a/tests/phpunit/Checker/RangeChecker/RangeCheckerHelperTest.php
b/tests/phpunit/Checker/RangeChecker/RangeCheckerHelperTest.php
index 02139e8..8728e46 100644
--- a/tests/phpunit/Checker/RangeChecker/RangeCheckerHelperTest.php
+++ b/tests/phpunit/Checker/RangeChecker/RangeCheckerHelperTest.php
@@ -10,6 +10,7 @@
use DataValues\UnboundedQuantityValue;
use InvalidArgumentException;
use PHPUnit_Framework_TestCase;
+use Wikibase\Repo\Parsers\TimeParserFactory;
use WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\RangeCheckerHelper;
/**
@@ -29,7 +30,7 @@
* @dataProvider getComparativeValueProvider
*/
public function testGetComparativeValue( $expected, DataValue
$dataValue ) {
- $rangeCheckHelper = new RangeCheckerHelper();
+ $rangeCheckHelper = new RangeCheckerHelper( (new
TimeParserFactory())->getTimeParser() );
$comparativeValue = $rangeCheckHelper->getComparativeValue(
$dataValue );
$this->assertSame( $expected, $comparativeValue );
@@ -37,7 +38,7 @@
public function getComparativeValueProvider() {
$cases = [
- [ '+1970-01-01T00:00:00Z', $this->getTimeValue() ],
+ [ [ '+1970', '+01', '+01', '+00', '+00', '+00' ],
$this->getTimeValue() ],
[ '+42', $this->getQuantityValue() ],
[ '+9000', UnboundedQuantityValue::newFromNumber(
'+9000' ) ]
];
@@ -49,10 +50,24 @@
* @expectedException InvalidArgumentException
*/
public function
testGetComparativeValue_unsupportedDataValueTypeThrowsException() {
- $rangeCheckHelper = new RangeCheckerHelper();
+ $rangeCheckHelper = new RangeCheckerHelper( (new
TimeParserFactory())->getTimeParser() );
$rangeCheckHelper->getComparativeValue( new StringValue(
'kittens' ) );
}
+ public function testGetComparativeTimeValueY() {
+ $rangeCheckHelper = new RangeCheckerHelper( (new
TimeParserFactory())->getTimeParser() );
+ $expected = [ '+1970', '+00', '+00', '+00', '+00', '+00' ];
+ $actual = $rangeCheckHelper->getComparativeTimeValue( '1970' );
+ $this->assertSame( $expected, $actual );
+ }
+
+ public function testGetComparativeTimeValueYMD() {
+ $rangeCheckHelper = new RangeCheckerHelper( (new
TimeParserFactory())->getTimeParser() );
+ $expected = [ '+1970', '+05', '+03', '+00', '+00', '+00' ];
+ $actual = $rangeCheckHelper->getComparativeTimeValue(
'1970-05-03' );
+ $this->assertSame( $expected, $actual );
+ }
+
private function getTimeValue() {
return new TimeValue(
'+00000001970-01-01T00:00:00Z',
diff --git a/tests/phpunit/Checker/RangeChecker/RangeCheckerTest.php
b/tests/phpunit/Checker/RangeChecker/RangeCheckerTest.php
index 72aff88..d56b471 100644
--- a/tests/phpunit/Checker/RangeChecker/RangeCheckerTest.php
+++ b/tests/phpunit/Checker/RangeChecker/RangeCheckerTest.php
@@ -13,6 +13,7 @@
use Wikibase\DataModel\Entity\Item;
use Wikibase\DataModel\Entity\ItemId;
use Wikibase\DataModel\Entity\PropertyId;
+use Wikibase\Repo\Parsers\TimeParserFactory;
use WikibaseQuality\ConstraintReport\Constraint;
use WikibaseQuality\ConstraintReport\ConstraintCheck\Checker\RangeChecker;
use
WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\ConstraintParameterParser;
@@ -57,7 +58,7 @@
$this->helper = new ConstraintParameterParser();
$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->helper, new
RangeCheckerHelper() );
+ $this->checker = new RangeChecker( $this->helper, new
RangeCheckerHelper( (new TimeParserFactory())->getTimeParser() ) );
}
protected function tearDown() {
@@ -102,7 +103,6 @@
}
public function testRangeConstraintTimeWithinRange() {
- $this->markTestSkipped( 'RangeChecker does not correctly parse
the parameters to compare – see T164279.' );
$min = '+00000001960-01-01T00:00:00Z';
$max = '+00000001980-01-01T00:00:00Z';
$statement = new Statement( new PropertyValueSnak( new
PropertyId( 'P1457' ), $this->timeValue ) );
@@ -115,7 +115,7 @@
}
public function testRangeConstraintTimeWithinRangeToNow() {
- $min = '+1960-01-01T00:00:00Z'; // TODO add leading zeroes as
in testRangeConstraintTimeWithinRange
+ $min = '+00000001960-01-01T00:00:00Z';
$max = 'now';
$statement = new Statement( new PropertyValueSnak( new
PropertyId( 'P1457' ), $this->timeValue ) );
$constraintParameters = array(
@@ -126,6 +126,30 @@
$this->assertEquals( 'compliance', $checkResult->getStatus(),
'check should comply' );
}
+ public function testRangeConstraintTimeWithinYearRange() {
+ $min = '1960';
+ $max = '1980';
+ $statement = new Statement( new PropertyValueSnak( new
PropertyId( 'P1457' ), $this->timeValue ) );
+ $constraintParameters = array(
+ 'minimum_date' => $min,
+ 'maximum_date' => $max
+ );
+ $checkResult = $this->checker->checkConstraint( $statement,
$this->getConstraintMock( $constraintParameters ), $this->getEntity() );
+ $this->assertEquals( 'compliance', $checkResult->getStatus(),
'check should comply' );
+ }
+
+ public function testRangeConstraintTimeWithinYearMonthDayRange() {
+ $min = '1969-12-31';
+ $max = '1970-01-02';
+ $statement = new Statement( new PropertyValueSnak( new
PropertyId( 'P1457' ), $this->timeValue ) );
+ $constraintParameters = array(
+ 'minimum_date' => $min,
+ 'maximum_date' => $max
+ );
+ $checkResult = $this->checker->checkConstraint( $statement,
$this->getConstraintMock( $constraintParameters ), $this->getEntity() );
+ $this->assertEquals( 'compliance', $checkResult->getStatus(),
'check should comply' );
+ }
+
public function testRangeConstraintTimeTooSmall() {
$min = '+00000001975-01-01T00:00:00Z';
$max = '+00000001980-01-01T00:00:00Z';
--
To view, visit https://gerrit.wikimedia.org/r/351628
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I902156e4e013e6c5f3c9c73bf35919de9264650a
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
