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) <lucas.werkmeis...@wikimedia.de> Gerrit-Reviewer: Ladsgroup <ladsgr...@gmail.com> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits