Lucas Werkmeister (WMDE) has uploaded a new change for review. ( 
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.

Bug: T183927
Change-Id: I3ff5f8d6b0a16a4aa2417b0f1e03b8634376a6dd
---
M src/Api/CachingResultsBuilder.php
M src/Api/CheckingResultsBuilder.php
M src/Api/ResultsBuilder.php
M tests/phpunit/Api/CachingResultsBuilderTest.php
M tests/phpunit/Api/CheckingResultsBuilderTest.php
5 files changed, 276 insertions(+), 37 deletions(-)


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

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/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..7514d7a 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: newchange
Gerrit-Change-Id: I3ff5f8d6b0a16a4aa2417b0f1e03b8634376a6dd
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/WikibaseQualityConstraints
Gerrit-Branch: master
Gerrit-Owner: Lucas Werkmeister (WMDE) <lucas.werkmeis...@wikimedia.de>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to