jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/403966 )
Change subject: Add $statuses parameter to ResultsBuilder::getResults
......................................................................
Add $statuses parameter to ResultsBuilder::getResults
This parameter filters the check results represented in the final result
to only those listed in the $statuses. CheckingResultsBuilder implements
this filtering, and CachingResultsBuilder caches only those results
where the $statuses are exactly those three statuses which are also
displayed in the gadget.
For this commit, CheckConstraints passes the list of all statuses to
ResultsBuilder::getResults(). The next commit will add a dedicated API
parameter instead which CheckConstraints just passes through.
Bug: T183927
Change-Id: I3ff5f8d6b0a16a4aa2417b0f1e03b8634376a6dd
---
M src/Api/CachingResultsBuilder.php
M src/Api/CheckConstraints.php
M src/Api/CheckingResultsBuilder.php
M src/Api/ResultsBuilder.php
M tests/phpunit/Api/CachingResultsBuilderTest.php
M tests/phpunit/Api/CheckingResultsBuilderTest.php
6 files changed, 292 insertions(+), 38 deletions(-)
Approvals:
Ladsgroup: Looks good to me, approved
jenkins-bot: Verified
diff --git a/src/Api/CachingResultsBuilder.php
b/src/Api/CachingResultsBuilder.php
index 00b788a..fc98fc6 100644
--- a/src/Api/CachingResultsBuilder.php
+++ b/src/Api/CachingResultsBuilder.php
@@ -11,6 +11,7 @@
use WikibaseQuality\ConstraintReport\ConstraintCheck\Cache\CachingMetadata;
use WikibaseQuality\ConstraintReport\ConstraintCheck\Cache\DependencyMetadata;
use WikibaseQuality\ConstraintReport\ConstraintCheck\Cache\Metadata;
+use WikibaseQuality\ConstraintReport\ConstraintCheck\Result\CheckResult;
/**
* A wrapper around another ResultsBuilder that caches results in a
ResultsCache.
@@ -76,6 +77,15 @@
private $microtime = 'microtime';
/**
+ * TODO: In PHP 5.6, make this a public class constant instead,
+ * and also use it in CheckConstraints::getAllowedParams()
+ * and in some of the tests.
+ *
+ * @var string[]
+ */
+ private $cachedStatuses;
+
+ /**
* @param ResultsBuilder $resultsBuilder The ResultsBuilder that cache
misses are delegated to.
* @param ResultsCache $cache The cache where results can be stored.
* @param WikiPageEntityMetaDataAccessor
$wikiPageEntityMetaDataAccessor Used to get the latest revision ID.
@@ -101,22 +111,30 @@
$this->ttlInSeconds = $ttlInSeconds;
$this->possiblyStaleConstraintTypes =
$possiblyStaleConstraintTypes;
$this->dataFactory = $dataFactory;
+
+ $this->cachedStatuses = [
+ CheckResult::STATUS_VIOLATION,
+ CheckResult::STATUS_WARNING,
+ CheckResult::STATUS_BAD_PARAMETERS,
+ ];
}
/**
* @param EntityId[] $entityIds
* @param string[] $claimIds
* @param string[]|null $constraintIds
+ * @param string[] $statuses
* @return CachedCheckConstraintsResponse
*/
public function getResults(
array $entityIds,
array $claimIds,
- array $constraintIds = null
+ array $constraintIds = null,
+ array $statuses
) {
$results = [];
$metadatas = [];
- if ( $this->canUseStoredResults( $entityIds, $claimIds,
$constraintIds ) ) {
+ if ( $this->canUseStoredResults( $entityIds, $claimIds,
$constraintIds, $statuses ) ) {
$storedEntityIds = [];
foreach ( $entityIds as $entityId ) {
$storedResults = $this->getStoredResults(
$entityId );
@@ -136,7 +154,7 @@
'wikibase.quality.constraints.cache.entity.miss',
count( $entityIds )
);
- $response = $this->getAndStoreResults( $entityIds,
$claimIds, $constraintIds );
+ $response = $this->getAndStoreResults( $entityIds,
$claimIds, $constraintIds, $statuses );
$results += $response->getArray();
$metadatas[] = $response->getMetadata();
}
@@ -147,37 +165,58 @@
}
/**
- * We can only use cached constraint results if full constraint check
results were requested:
+ * We can only use cached constraint results
+ * if exactly the problematic results of a full constraint check were
requested:
* constraint checks for the full entity (not just individual
statements),
- * and without restricting the set of constraints to check.
+ * without restricting the set of constraints to check,
+ * and with exactly the 'violation', 'warning' and 'bad-parameters'
statuses.
+ *
+ * (In theory, we could also use results for requests
+ * that asked for a subset of these result statuses,
+ * but removing the extra results from the cached value is tricky,
+ * especially if you consider that they might have added qualifier
contexts to the output
+ * that should not only be empty, but completely absent.)
*
* @param EntityId[] $entityIds
* @param string[] $claimIds
* @param string[]|null $constraintIds
+ * @param string[] $statuses
* @return bool
*/
private function canUseStoredResults(
array $entityIds,
array $claimIds,
- array $constraintIds = null
+ array $constraintIds = null,
+ array $statuses
) {
- return $claimIds === [] && $constraintIds === null;
+ if ( $claimIds !== [] ) {
+ return false;
+ }
+ if ( $constraintIds !== null ) {
+ return false;
+ }
+ if ( $statuses != $this->cachedStatuses ) {
+ return false;
+ }
+ return true;
}
/**
* @param EntityId[] $entityIds
* @param string[] $claimIds
* @param string[]|null $constraintIds
+ * @param string[] $statuses
* @return CachedCheckConstraintsResponse
*/
public function getAndStoreResults(
array $entityIds,
array $claimIds,
- array $constraintIds = null
+ array $constraintIds = null,
+ array $statuses
) {
- $results = $this->resultsBuilder->getResults( $entityIds,
$claimIds, $constraintIds );
+ $results = $this->resultsBuilder->getResults( $entityIds,
$claimIds, $constraintIds, $statuses );
- if ( $this->canStoreResults( $entityIds, $claimIds,
$constraintIds ) ) {
+ if ( $this->canStoreResults( $entityIds, $claimIds,
$constraintIds, $statuses ) ) {
foreach ( $entityIds as $entityId ) {
$value = [
'results' =>
$results->getArray()[$entityId->getSerialization()],
@@ -193,22 +232,37 @@
}
/**
- * We can only store constraint results if the set of constraints to
check was not restricted.
+ * We can only store constraint results
+ * if the set of constraints to check was not restricted
+ * and exactly the problematic results were requested.
* However, it doesn’t matter whether constraint checks on individual
statements were requested:
* we only store results for the mentioned entity IDs,
* and those will be complete regardless of what’s in the statement IDs.
*
+ * (In theory, we could also store results of checks that requested
extra result statuses,
+ * but removing the extra results before caching the value is tricky,
+ * especially if you consider that they might have added qualifier
contexts to the output
+ * that should not only be empty, but completely absent.)
+ *
* @param EntityId[] $entityIds
* @param string[] $claimIds
* @param string[]|null $constraintIds
+ * @param string[] $statuses
* @return bool
*/
private function canStoreResults(
array $entityIds,
array $claimIds,
- array $constraintIds = null
+ array $constraintIds = null,
+ array $statuses
) {
- return $constraintIds === null;
+ if ( $constraintIds !== null ) {
+ return false;
+ }
+ if ( $statuses != $this->cachedStatuses ) {
+ return false;
+ }
+ return true;
}
/**
diff --git a/src/Api/CheckConstraints.php b/src/Api/CheckConstraints.php
index 5f0676f..9d8801a 100644
--- a/src/Api/CheckConstraints.php
+++ b/src/Api/CheckConstraints.php
@@ -20,6 +20,7 @@
use Wikibase\Repo\EntityIdLabelFormatterFactory;
use Wikibase\Repo\WikibaseRepo;
use
WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\ConstraintParameterParser;
+use WikibaseQuality\ConstraintReport\ConstraintCheck\Result\CheckResult;
use WikibaseQuality\ConstraintReport\ConstraintParameterRenderer;
use WikibaseQuality\ConstraintReport\ConstraintReportFactory;
@@ -204,7 +205,21 @@
$this->getResult()->addValue(
null,
$this->getModuleName(),
- $this->resultsBuilder->getResults( $entityIds,
$claimIds, $constraintIDs )->getArray()
+ $this->resultsBuilder->getResults(
+ $entityIds,
+ $claimIds,
+ $constraintIDs,
+ [
+ CheckResult::STATUS_COMPLIANCE,
+ CheckResult::STATUS_VIOLATION,
+ CheckResult::STATUS_WARNING,
+ CheckResult::STATUS_EXCEPTION,
+ CheckResult::STATUS_NOT_IN_SCOPE,
+ CheckResult::STATUS_DEPRECATED,
+ CheckResult::STATUS_BAD_PARAMETERS,
+ CheckResult::STATUS_TODO,
+ ]
+ )->getArray()
);
// ensure that result contains the given entity IDs even if
they have no statements
foreach ( $entityIds as $entityId ) {
diff --git a/src/Api/CheckingResultsBuilder.php
b/src/Api/CheckingResultsBuilder.php
index 23494b7..e4f84f1 100644
--- a/src/Api/CheckingResultsBuilder.php
+++ b/src/Api/CheckingResultsBuilder.php
@@ -64,21 +64,25 @@
* @param EntityId[] $entityIds
* @param string[] $claimIds
* @param string[]|null $constraintIds
+ * @param string[] $statuses
* @return CachedCheckConstraintsResponse
*/
public function getResults(
array $entityIds,
array $claimIds,
- array $constraintIds = null
+ array $constraintIds = null,
+ array $statuses
) {
$response = [];
$metadatas = [];
+ $statusesFlipped = array_flip( $statuses );
foreach ( $entityIds as $entityId ) {
$results =
$this->delegatingConstraintChecker->checkAgainstConstraintsOnEntityId(
$entityId,
$constraintIds,
[ $this, 'defaultResults' ]
);
+ $results = array_filter( $results,
$this->statusSelected( $statusesFlipped ) );
foreach ( $results as $result ) {
$metadatas[] = $result->getMetadata();
$resultArray = $this->checkResultToArray(
$result );
@@ -91,6 +95,7 @@
$constraintIds,
[ $this, 'defaultResults' ]
);
+ $results = array_filter( $results,
$this->statusSelected( $statusesFlipped ) );
foreach ( $results as $result ) {
$metadatas[] = $result->getMetadata();
$resultArray = $this->checkResultToArray(
$result );
@@ -109,6 +114,13 @@
[];
}
+ public function statusSelected( array $statusesFlipped ) {
+ return function( CheckResult $result ) use ( $statusesFlipped )
{
+ return array_key_exists( $result->getStatus(),
$statusesFlipped ) ||
+ $result instanceof NullResult;
+ };
+ }
+
public function checkResultToArray( CheckResult $checkResult ) {
if ( $checkResult instanceof NullResult ) {
return null;
diff --git a/src/Api/ResultsBuilder.php b/src/Api/ResultsBuilder.php
index 6969ed9..b6cfe39 100644
--- a/src/Api/ResultsBuilder.php
+++ b/src/Api/ResultsBuilder.php
@@ -15,12 +15,14 @@
* @param EntityId[] $entityIds
* @param string[] $claimIds
* @param string[]|null $constraintIds
+ * @param string[] $statuses
* @return CachedCheckConstraintsResponse
*/
public function getResults(
array $entityIds,
array $claimIds,
- array $constraintIds = null
+ array $constraintIds = null,
+ array $statuses
);
}
diff --git a/tests/phpunit/Api/CachingResultsBuilderTest.php
b/tests/phpunit/Api/CachingResultsBuilderTest.php
index e6dba7b..9ff74d6 100644
--- a/tests/phpunit/Api/CachingResultsBuilderTest.php
+++ b/tests/phpunit/Api/CachingResultsBuilderTest.php
@@ -19,6 +19,7 @@
use WikibaseQuality\ConstraintReport\ConstraintCheck\Cache\CachingMetadata;
use WikibaseQuality\ConstraintReport\ConstraintCheck\Cache\DependencyMetadata;
use WikibaseQuality\ConstraintReport\ConstraintCheck\Cache\Metadata;
+use WikibaseQuality\ConstraintReport\ConstraintCheck\Result\CheckResult;
include_once __DIR__ .
'/../../../../../tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php';
@@ -35,10 +36,11 @@
Metadata::blank()
);
$q100 = new ItemId( 'Q100' );
+ $statuses = [ 'garbage status', 'other status' ];
$resultsBuilder = $this->getMock( ResultsBuilder::class );
$resultsBuilder->expects( $this->once() )
->method( 'getResults' )
- ->with( [ $q100 ], [], null )
+ ->with( [ $q100 ], [], null, $statuses )
->willReturn( $expectedResults );
$metaDataAccessor = $this->getMock(
WikiPageEntityMetaDataAccessor::class );
$metaDataAccessor->method( 'loadRevisionInformation' )
@@ -53,7 +55,7 @@
new NullStatsdDataFactory()
);
- $results = $cachingResultsBuilder->getAndStoreResults( [ $q100
], [], null );
+ $results = $cachingResultsBuilder->getAndStoreResults( [ $q100
], [], null, $statuses );
$this->assertSame( $expectedResults, $results );
}
@@ -63,10 +65,11 @@
[ 'Q100' => 'garbage data, should not matter' ],
Metadata::blank()
);
+ $statuses = [ 'garbage status', 'other status' ];
$resultsBuilder = $this->getMock( ResultsBuilder::class );
$resultsBuilder->expects( $this->once() )
->method( 'getResults' )
- ->with( [], [ 'fake' ], null )
+ ->with( [], [ 'fake' ], null, $statuses )
->willReturn( $expectedResults );
$cache = $this->getMockBuilder( WANObjectCache::class )
->disableOriginalConstructor()
@@ -84,7 +87,7 @@
new NullStatsdDataFactory()
);
- $cachingResultsBuilder->getAndStoreResults( [], [ 'fake' ],
null );
+ $cachingResultsBuilder->getAndStoreResults( [], [ 'fake' ],
null, $statuses );
}
public function testGetAndStoreResults_DontCacheWithConstraintIds() {
@@ -93,10 +96,11 @@
Metadata::blank()
);
$q100 = new ItemId( 'Q100' );
+ $statuses = [ 'garbage status', 'other status' ];
$resultsBuilder = $this->getMock( ResultsBuilder::class );
$resultsBuilder->expects( $this->once() )
->method( 'getResults' )
- ->with( [ $q100 ], [], [ 'fake' ] )
+ ->with( [ $q100 ], [], [ 'fake' ], $statuses )
->willReturn( $expectedResults );
$cache = $this->getMockBuilder( WANObjectCache::class )
->disableOriginalConstructor()
@@ -114,7 +118,7 @@
new NullStatsdDataFactory()
);
- $cachingResultsBuilder->getAndStoreResults( [ $q100 ], [], [
'fake' ] );
+ $cachingResultsBuilder->getAndStoreResults( [ $q100 ], [], [
'fake' ], $statuses );
}
public function testGetAndStoreResults_StoreResults() {
@@ -123,10 +127,15 @@
Metadata::blank()
);
$q100 = new ItemId( 'Q100' );
+ $statuses = [
+ CheckResult::STATUS_VIOLATION,
+ CheckResult::STATUS_WARNING,
+ CheckResult::STATUS_BAD_PARAMETERS
+ ];
$resultsBuilder = $this->getMock( ResultsBuilder::class );
$resultsBuilder->expects( $this->once() )
->method( 'getResults' )
- ->with( [ $q100 ], [], null )
+ ->with( [ $q100 ], [], null, $statuses )
->willReturn( $expectedResults );
$cache = new WANObjectCache( [ 'cache' => new HashBagOStuff() ]
);
$resultsCache = new ResultsCache( $cache );
@@ -143,7 +152,7 @@
new NullStatsdDataFactory()
);
- $cachingResultsBuilder->getAndStoreResults( [ $q100 ], [], null
);
+ $cachingResultsBuilder->getAndStoreResults( [ $q100 ], [],
null, $statuses );
$cachedResults = $resultsCache->get( $q100 );
$this->assertNotNull( $cachedResults );
@@ -168,10 +177,15 @@
$q101->getSerialization() => 1337,
$p102->getSerialization() => 42,
];
+ $statuses = [
+ CheckResult::STATUS_VIOLATION,
+ CheckResult::STATUS_WARNING,
+ CheckResult::STATUS_BAD_PARAMETERS
+ ];
$resultsBuilder = $this->getMock( ResultsBuilder::class );
$resultsBuilder->expects( $this->once() )
->method( 'getResults' )
- ->with( [ $q100 ], [], null )
+ ->with( [ $q100 ], [], null, $statuses )
->willReturn( $expectedResults );
$cache = new WANObjectCache( [ 'cache' => new HashBagOStuff() ]
);
$resultsCache = new ResultsCache( $cache );
@@ -202,12 +216,43 @@
new NullStatsdDataFactory()
);
- $cachingResultsBuilder->getAndStoreResults( [ $q100 ], [], null
);
+ $cachingResultsBuilder->getAndStoreResults( [ $q100 ], [],
null, $statuses );
$cachedResults = $resultsCache->get( $q100 );
$this->assertNotNull( $cachedResults );
$this->assertArrayHasKey( 'latestRevisionIds', $cachedResults );
$this->assertSame( $revisionIds,
$cachedResults['latestRevisionIds'] );
+ }
+
+ public function testGetAndStoreResults_DontStoreWithWrongStatuses() {
+ $expectedResults = new CachedCheckConstraintsResponse(
+ [ 'Q100' => 'garbage data, should not matter' ],
+ Metadata::blank()
+ );
+ $q100 = new ItemId( 'Q100' );
+ $statuses = [ CheckResult::STATUS_VIOLATION ];
+ $resultsBuilder = $this->getMock( ResultsBuilder::class );
+ $resultsBuilder->expects( $this->once() )
+ ->method( 'getResults' )
+ ->with( [ $q100 ], [], null, $statuses )
+ ->willReturn( $expectedResults );
+ $cache = $this->getMockBuilder( WANObjectCache::class )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $cache->expects( $this->never() )->method( 'set' );
+ $metaDataAccessor = $this->getMock(
WikiPageEntityMetaDataAccessor::class );
+ $metaDataAccessor->expects( $this->never() )->method(
'loadRevisionInformation ' );
+ $cachingResultsBuilder = new CachingResultsBuilder(
+ $resultsBuilder,
+ new ResultsCache( $cache ),
+ $metaDataAccessor,
+ new ItemIdParser(),
+ 86400,
+ [],
+ new NullStatsdDataFactory()
+ );
+
+ $cachingResultsBuilder->getAndStoreResults( [ $q100 ], [],
null, $statuses );
}
public function testGetStoredResults_CacheMiss() {
@@ -306,6 +351,7 @@
public function testGetResults_EmptyCache() {
$entityIds = [ new ItemId( 'Q5' ), new ItemId( 'Q10' ) ];
+ $statuses = [ 'garbage status', 'other status' ];
$cachingResultsBuilder = $this->getMockBuilder(
CachingResultsBuilder::class )
->setConstructorArgs( [
@@ -321,7 +367,7 @@
->getMock();
$cachingResultsBuilder->method( 'getStoredResults'
)->willReturn( null );
$cachingResultsBuilder->method( 'getAndStoreResults' )
- ->with( $entityIds, [], null )
+ ->with( $entityIds, [], null, $statuses )
->willReturnCallback( function( $entityIds, $claimIds,
$constraintIds ) {
$results = [];
foreach ( $entityIds as $entityId ) {
@@ -332,7 +378,12 @@
} );
/** @var CachingResultsBuilder $cachingResultsBuilder */
- $results = $cachingResultsBuilder->getResults( $entityIds, [],
null );
+ $results = $cachingResultsBuilder->getResults(
+ $entityIds,
+ [],
+ null,
+ $statuses
+ );
$expected = [ 'Q5' => 'garbage of Q5', 'Q10' => 'garbage of
Q10' ];
$actual = $results->getArray();
@@ -346,6 +397,7 @@
$entityIds = [ new ItemId( 'Q5' ), new ItemId( 'Q10' ) ];
$statementIds = [];
$constraintIds = [ 'P12$11a14ea5-10dc-425b-b94d-6e65997be983' ];
+ $statuses = [ 'garbage status', 'other status' ];
$expected = new CachedCheckConstraintsResponse(
[ 'Q5' => 'garbage of Q5', 'Q10' => 'some garbage of
Q10' ],
Metadata::ofCachingMetadata(
CachingMetadata::ofMaximumAgeInSeconds( 5 * 60 ) )
@@ -364,11 +416,16 @@
->getMock();
$cachingResultsBuilder->expects( $this->never() )->method(
'getStoredResults' );
$cachingResultsBuilder->expects( $this->once() )->method(
'getAndStoreResults' )
- ->with( $entityIds, $statementIds, $constraintIds )
+ ->with( $entityIds, $statementIds, $constraintIds,
$statuses )
->willReturn( $expected );
/** @var CachingResultsBuilder $cachingResultsBuilder */
- $results = $cachingResultsBuilder->getResults( $entityIds,
$statementIds, $constraintIds );
+ $results = $cachingResultsBuilder->getResults(
+ $entityIds,
+ $statementIds,
+ $constraintIds,
+ $statuses
+ );
$this->assertSame( $expected->getArray(), $results->getArray()
);
$this->assertEquals( $expected->getMetadata(),
$results->getMetadata() );
@@ -378,6 +435,7 @@
$entityIds = [];
$statementIds = [ 'Q5$9c009c6f-fdf5-41d1-86e9-e790427e3dc6' ];
$constraintIds = [];
+ $statuses = [ 'garbage status', 'other status' ];
$expected = new CachedCheckConstraintsResponse(
[ 'Q5' => 'some garbage of Q5' ],
Metadata::ofCachingMetadata(
CachingMetadata::ofMaximumAgeInSeconds( 5 * 60 ) )
@@ -396,11 +454,16 @@
->getMock();
$cachingResultsBuilder->expects( $this->never() )->method(
'getStoredResults' );
$cachingResultsBuilder->expects( $this->once() )->method(
'getAndStoreResults' )
- ->with( $entityIds, $statementIds, $constraintIds )
+ ->with( $entityIds, $statementIds, $constraintIds,
$statuses )
->willReturn( $expected );
/** @var CachingResultsBuilder $cachingResultsBuilder */
- $results = $cachingResultsBuilder->getResults( $entityIds,
$statementIds, $constraintIds );
+ $results = $cachingResultsBuilder->getResults(
+ $entityIds,
+ $statementIds,
+ $constraintIds,
+ $statuses
+ );
$this->assertSame( $expected->getArray(), $results->getArray()
);
$this->assertEquals( $expected->getMetadata(),
$results->getMetadata() );
@@ -408,6 +471,11 @@
public function testGetResults_FullyCached() {
$entityIds = [ new ItemId( 'Q5' ), new ItemId( 'Q10' ) ];
+ $statuses = [
+ CheckResult::STATUS_VIOLATION,
+ CheckResult::STATUS_WARNING,
+ CheckResult::STATUS_BAD_PARAMETERS
+ ];
$cachingResultsBuilder = $this->getMockBuilder(
CachingResultsBuilder::class )
->setConstructorArgs( [
$this->getMock( ResultsBuilder::class ),
@@ -431,7 +499,12 @@
$cachingResultsBuilder->expects( $this->never() )->method(
'getAndStoreResults' );
/** @var CachingResultsBuilder $cachingResultsBuilder */
- $results = $cachingResultsBuilder->getResults( $entityIds, [],
null );
+ $results = $cachingResultsBuilder->getResults(
+ $entityIds,
+ [],
+ null,
+ $statuses
+ );
$expected = [ 'Q5' => 'garbage of Q5', 'Q10' => 'garbage of
Q10' ];
$actual = $results->getArray();
@@ -443,6 +516,11 @@
public function testGetResults_PartiallyCached() {
$entityIds = [ new ItemId( 'Q5' ), new ItemId( 'Q10' ) ];
+ $statuses = [
+ CheckResult::STATUS_VIOLATION,
+ CheckResult::STATUS_WARNING,
+ CheckResult::STATUS_BAD_PARAMETERS
+ ];
$cachingResultsBuilder = $this->getMockBuilder(
CachingResultsBuilder::class )
->setConstructorArgs( [
$this->getMock( ResultsBuilder::class ),
@@ -467,14 +545,19 @@
}
} );
$cachingResultsBuilder->expects( $this->once() )->method(
'getAndStoreResults' )
- ->with( [ $entityIds[1] ], [], null )
+ ->with( [ $entityIds[1] ], [], null, $statuses )
->willReturn( new CachedCheckConstraintsResponse(
[ 'Q10' => 'garbage of Q10' ],
Metadata::ofDependencyMetadata(
DependencyMetadata::ofEntityId( $entityIds[1] ) )
) );
/** @var CachingResultsBuilder $cachingResultsBuilder */
- $results = $cachingResultsBuilder->getResults( $entityIds, [],
null );
+ $results = $cachingResultsBuilder->getResults(
+ $entityIds,
+ [],
+ null,
+ $statuses
+ );
$expected = [ 'Q5' => 'garbage of Q5', 'Q10' => 'garbage of
Q10' ];
$actual = $results->getArray();
@@ -486,6 +569,44 @@
$this->assertSame( [ $entityIds[1] ],
$metadata->getDependencyMetadata()->getEntityIds() );
}
+ public function testGetResults_SkipCacheWithWrongStatuses() {
+ $entityIds = [ new ItemId( 'Q5' ), new ItemId( 'Q10' ) ];
+ $statuses = [
+ CheckResult::STATUS_VIOLATION,
+ CheckResult::STATUS_WARNING,
+ CheckResult::STATUS_BAD_PARAMETERS,
+ CheckResult::STATUS_TODO,
+ ];
+ $expected = new CachedCheckConstraintsResponse(
+ [ 'Q5' => 'some garbage of Q5' ],
+ Metadata::ofCachingMetadata(
CachingMetadata::ofMaximumAgeInSeconds( 5 * 60 ) )
+ );
+ $cachingResultsBuilder = $this->getMockBuilder(
CachingResultsBuilder::class )
+ ->setConstructorArgs( [
+ $this->getMock( ResultsBuilder::class ),
+ new ResultsCache( WANObjectCache::newEmpty() ),
+ $this->getMock(
WikiPageEntityMetaDataAccessor::class ),
+ new ItemIdParser(),
+ 86400,
+ [],
+ new NullStatsdDataFactory()
+ ] )
+ ->setMethods( [ 'getStoredResults',
'getAndStoreResults' ] )
+ ->getMock();
+ $cachingResultsBuilder->expects( $this->never() )->method(
'getStoredResults' );
+ $cachingResultsBuilder->expects( $this->once() )->method(
'getAndStoreResults' )
+ ->with( $entityIds, [], null, $statuses )
+ ->willReturn( $expected );
+ /** @var CachingResultsBuilder $cachingResultsBuilder */
+
+ $cachingResultsBuilder->getResults(
+ $entityIds,
+ [],
+ null,
+ $statuses
+ );
+ }
+
public function testUpdateCachingMetadata_CachedToplevel() {
$cachingResultsBuilder = new CachingResultsBuilder(
$this->getMock( ResultsBuilder::class ),
diff --git a/tests/phpunit/Api/CheckingResultsBuilderTest.php
b/tests/phpunit/Api/CheckingResultsBuilderTest.php
index 49ee639..d1a517e 100644
--- a/tests/phpunit/Api/CheckingResultsBuilderTest.php
+++ b/tests/phpunit/Api/CheckingResultsBuilderTest.php
@@ -10,6 +10,7 @@
use Wikibase\DataModel\Entity\PropertyId;
use Wikibase\DataModel\Services\EntityId\PlainEntityIdFormatter;
use Wikibase\DataModel\Snak\PropertyNoValueSnak;
+use Wikibase\DataModel\Snak\PropertySomeValueSnak;
use Wikibase\DataModel\Statement\Statement;
use Wikibase\Lib\Store\EntityTitleLookup;
use WikibaseQuality\ConstraintReport\Constraint;
@@ -129,7 +130,8 @@
$result = $this->getResultsBuilder(
$delegatingConstraintChecker )->getResults(
[ $q1, $q2 ],
[ $s1, $s2 ],
- $constraintIds
+ $constraintIds,
+ [ CheckResult::STATUS_TODO ]
)->getArray();
$this->assertSame( [ 'Q1', 'Q2', 'Q3', 'Q4' ], array_keys(
$result ) );
@@ -153,7 +155,8 @@
$result = $this->getResultsBuilder(
$delegatingConstraintChecker )->getResults(
[ new ItemId( self::NONEXISTENT_ITEM ) ],
[ self::NONEXISTENT_CLAIM ],
- []
+ [],
+ [ CheckResult::STATUS_TODO ]
)->getArray();
$this->assertEmpty( $result );
@@ -200,7 +203,8 @@
$metadata = $this->getResultsBuilder(
$delegatingConstraintChecker )->getResults(
[ new ItemId( 'Q1' ) ],
[ 'Q2$73408a9b-b1b0-4035-bf36-1e65ecf8772d' ],
- null
+ null,
+ [ CheckResult::STATUS_TODO ]
)->getMetadata();
$expected = [ new ItemId( 'Q100' ), new PropertyId( 'P100' ) ];
@@ -210,6 +214,52 @@
$this->assertEquals( $expected, $actual );
}
+ public function testGetResults_FilterStatuses() {
+ $q1 = new ItemId( 'Q1' );
+ $mock = $this->getMockBuilder(
DelegatingConstraintChecker::class )
+ ->disableOriginalConstructor()
+ ->setMethods( [ 'checkAgainstConstraintsOnEntityId' ] );
+ $delegatingConstraintChecker = $mock->getMock();
+ $constraint = new Constraint(
+ 'P1$47681880-d5f5-417d-96c3-570d6e94d234',
+ new PropertyId( 'P1' ),
+ 'Q1',
+ []
+ );
+ $delegatingConstraintChecker->method(
'checkAgainstConstraintsOnEntityId' )
+ ->willReturn( [
+ new CheckResult(
+ new MainSnakContext(
+ new Item( $q1 ),
+ new Statement( new
PropertyNoValueSnak( new PropertyId( 'P1' ) ) )
+ ),
+ $constraint,
+ [],
+ CheckResult::STATUS_VIOLATION
+ ),
+ new CheckResult(
+ new MainSnakContext(
+ new Item( $q1 ),
+ new Statement( new
PropertySomeValueSnak( new PropertyId( 'P1' ) ) )
+ ),
+ $constraint,
+ [],
+ CheckResult::STATUS_COMPLIANCE
+ ),
+ ] );
+
+ $result = $this->getResultsBuilder(
$delegatingConstraintChecker )->getResults(
+ [ $q1 ],
+ [],
+ [],
+ [ CheckResult::STATUS_VIOLATION ]
+ )->getArray();
+
+ $statementResults =
$result['Q1']['claims']['P1'][0]['mainsnak']['results'];
+ $this->assertCount( 1, $statementResults );
+ $this->assertSame( CheckResult::STATUS_VIOLATION,
$statementResults[0]['status'] );
+ }
+
public function testCheckResultToArray_NullResult() {
$checkResult = new NullResult(
new FakeSnakContext( new PropertyNoValueSnak( new
PropertyId( 'P1' ) ) )
--
To view, visit https://gerrit.wikimedia.org/r/403966
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I3ff5f8d6b0a16a4aa2417b0f1e03b8634376a6dd
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/WikibaseQualityConstraints
Gerrit-Branch: master
Gerrit-Owner: Lucas Werkmeister (WMDE) <[email protected]>
Gerrit-Reviewer: Ladsgroup <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
