Jeroen De Dauw has submitted this change and it was merged. Change subject: Simplify SnaksFinder ......................................................................
Simplify SnaksFinder Per Daniel's comments on https://gerrit.wikimedia.org/r/190809 Change-Id: I4bcf79829618331d0d92bb54828f4b71aa14c03a --- M client/includes/DataAccess/PropertyParserFunction/LanguageAwareRenderer.php M client/includes/DataAccess/PropertyParserFunction/PropertyClaimsRendererFactory.php M client/includes/DataAccess/SnaksFinder.php M client/includes/WikibaseClient.php M client/tests/phpunit/includes/DataAccess/PropertyParserFunction/LanguageAwareRendererTest.php M client/tests/phpunit/includes/DataAccess/PropertyParserFunction/PropertyClaimsRendererFactoryTest.php M client/tests/phpunit/includes/DataAccess/SnaksFinderTest.php 7 files changed, 67 insertions(+), 102 deletions(-) Approvals: Jeroen De Dauw: Looks good to me, approved diff --git a/client/includes/DataAccess/PropertyParserFunction/LanguageAwareRenderer.php b/client/includes/DataAccess/PropertyParserFunction/LanguageAwareRenderer.php index af188ca..321231c 100644 --- a/client/includes/DataAccess/PropertyParserFunction/LanguageAwareRenderer.php +++ b/client/includes/DataAccess/PropertyParserFunction/LanguageAwareRenderer.php @@ -13,8 +13,10 @@ use Wikibase\DataModel\Entity\PropertyId; use Wikibase\DataModel\Snak\PropertyValueSnak; use Wikibase\DataModel\Snak\Snak; +use Wikibase\DataModel\StatementListProvider; use Wikibase\Lib\PropertyLabelNotResolvedException; use Wikibase\Lib\SnakFormatter; +use Wikibase\Lib\Store\EntityLookup; /** * PropertyClaimsRenderer of the {{#property}} parser function. @@ -54,24 +56,32 @@ private $usageAccumulator; /** + * @var EntityLookup + */ + private $entityLookup; + + /** * @param Language $language * @param PropertyIdResolver $propertyIdResolver * @param SnaksFinder $snaksFinder * @param SnakFormatter $snakFormatter * @param UsageAccumulator $usageAcc + * @param EntityLookup $entityLookup */ public function __construct( Language $language, PropertyIdResolver $propertyIdResolver, SnaksFinder $snaksFinder, SnakFormatter $snakFormatter, - UsageAccumulator $usageAcc + UsageAccumulator $usageAcc, + EntityLookup $entityLookup ) { $this->language = $language; $this->propertyIdResolver = $propertyIdResolver; $this->snaksFinder = $snaksFinder; $this->snakFormatter = $snakFormatter; $this->usageAccumulator = $usageAcc; + $this->entityLookup = $entityLookup; } /** @@ -166,8 +176,14 @@ private function renderWithStatus( EntityId $entityId, PropertyId $propertyId ) { wfProfileIn( __METHOD__ ); + $entity = $this->entityLookup->getEntity( $entityId ); + + if ( !$entity instanceof StatementListProvider ) { + return Status::newGood( '' ); + } + $snaks = $this->snaksFinder->findSnaks( - $entityId, + $entity, $propertyId, $this->language->getCode() ); diff --git a/client/includes/DataAccess/PropertyParserFunction/PropertyClaimsRendererFactory.php b/client/includes/DataAccess/PropertyParserFunction/PropertyClaimsRendererFactory.php index b817809..8f10e44 100644 --- a/client/includes/DataAccess/PropertyParserFunction/PropertyClaimsRendererFactory.php +++ b/client/includes/DataAccess/PropertyParserFunction/PropertyClaimsRendererFactory.php @@ -12,6 +12,7 @@ use Wikibase\LanguageFallbackChainFactory; use Wikibase\Lib\OutputFormatSnakFormatterFactory; use Wikibase\Lib\SnakFormatter; +use Wikibase\Lib\Store\EntityLookup; /** * @since 0.5 @@ -47,21 +48,29 @@ private $languageAwareRenderers = array(); /** + * @var EntityLookup + */ + private $entityLookup; + + /** * @param PropertyIdResolver $propertyIdResolver * @param SnaksFinder $snaksFinder * @param LanguageFallbackChainFactory $languageFallbackChainFactory * @param OutputFormatSnakFormatterFactory $snakFormatterFactory + * @param EntityLookup $entityLookup */ public function __construct( PropertyIdResolver $propertyIdResolver, SnaksFinder $snaksFinder, LanguageFallbackChainFactory $languageFallbackChainFactory, - OutputFormatSnakFormatterFactory $snakFormatterFactory + OutputFormatSnakFormatterFactory $snakFormatterFactory, + EntityLookup $entityLookup ) { $this->propertyIdResolver = $propertyIdResolver; $this->snaksFinder = $snaksFinder; $this->languageFallbackChainFactory = $languageFallbackChainFactory; $this->snakFormatterFactory = $snakFormatterFactory; + $this->entityLookup = $entityLookup; } /** @@ -93,7 +102,8 @@ $this->propertyIdResolver, $this->snaksFinder, $this->newSnakFormatterForLanguage( $language ), - $usageAccumulator + $usageAccumulator, + $this->entityLookup ); } diff --git a/client/includes/DataAccess/SnaksFinder.php b/client/includes/DataAccess/SnaksFinder.php index 96fb16c..4a8ad17 100644 --- a/client/includes/DataAccess/SnaksFinder.php +++ b/client/includes/DataAccess/SnaksFinder.php @@ -2,61 +2,35 @@ namespace Wikibase\DataAccess; -use Wikibase\DataModel\Entity\EntityId; use Wikibase\DataModel\Entity\PropertyId; use Wikibase\DataModel\Snak\Snak; use Wikibase\DataModel\StatementListProvider; use Wikibase\DataModel\Statement\StatementList; -use Wikibase\Lib\Store\EntityLookup; /** - * Find Snaks for claims in an entity, with EntityId, based on PropertyId. + * Find Snaks for claims in a given Entity, based on PropertyId. * * @since 0.5 * - * @licence GNU GPL v2+ - * @author Katie Filbert < [email protected] > - * @author Jeroen De Dauw < [email protected] > - * @author Daniel Kinzler - * @author Liangent < [email protected] > + * @license GNU GPL v2+ * @author Marius Hoch < [email protected] > */ class SnaksFinder { /** - * @var EntityLookup - */ - private $entityLookup; - - /** - * @param EntityLookup $entityLookup - */ - public function __construct( EntityLookup $entityLookup ) { - $this->entityLookup = $entityLookup; - } - - /** - * @param EntityId $entityId The item or property that the property is used on + * @param StatementListProvider $statementListProvider * @param PropertyId $propertyId The PropertyId for which we want the formatted Snaks * @param int[]|null $acceptableRanks * * @return Snak[] */ - public function findSnaks( EntityId $entityId, PropertyId $propertyId, $acceptableRanks = null ) { - $entity = $this->entityLookup->getEntity( $entityId ); - - if ( !$entity instanceof StatementListProvider ) { - return array(); - } - - $statementList = $this->getStatementsWithPropertyId( $entity, $propertyId ); + public function findSnaks( StatementListProvider $statementListProvider, PropertyId $propertyId, $acceptableRanks = null ) { + $statementList = $this->getStatementsWithPropertyId( $statementListProvider, $propertyId ); if ( $acceptableRanks === null ) { - $snaks = $this->getBestMainSnaks( $statementList ); + return $statementList->getBestStatements()->getMainSnaks(); } else { - $snaks = $this->getMainSnaksByRanks( $statementList, $acceptableRanks ); + return $statementList->getWithRank( $acceptableRanks )->getMainSnaks(); } - - return $snaks; } /** @@ -69,24 +43,5 @@ return $statementListProvider ->getStatements() ->getWithPropertyId( $propertyId ); - } - - /** - * @param StatementList $statementList - * - * @return Snak[] - */ - private function getBestMainSnaks( StatementList $statementList ) { - return $statementList->getBestStatements()->getMainSnaks(); - } - - /** - * @param StatementList $statementList - * @param int[] $acceptableRanks - * - * @return Snak[] - */ - private function getMainSnaksByRanks( StatementList $statementList, array $acceptableRanks ) { - return $statementList->getWithRank( $acceptableRanks )->getMainSnaks(); } } diff --git a/client/includes/WikibaseClient.php b/client/includes/WikibaseClient.php index 2500932..1db3d19 100644 --- a/client/includes/WikibaseClient.php +++ b/client/includes/WikibaseClient.php @@ -698,8 +698,6 @@ private function getPropertyClaimsRendererFactory() { $entityLookup = $this->getEntityLookup(); - $snaksFinder = new SnaksFinder( $entityLookup ); - $propertyIdResolver = new PropertyIdResolver( $entityLookup, $this->getStore()->getPropertyLabelResolver() @@ -707,9 +705,10 @@ return new PropertyClaimsRendererFactory( $propertyIdResolver, - $snaksFinder, + new SnaksFinder(), $this->getLanguageFallbackChainFactory(), - $this->getSnakFormatterFactory() + $this->getSnakFormatterFactory(), + $entityLookup ); } diff --git a/client/tests/phpunit/includes/DataAccess/PropertyParserFunction/LanguageAwareRendererTest.php b/client/tests/phpunit/includes/DataAccess/PropertyParserFunction/LanguageAwareRendererTest.php index ce7cd9d..259a1a0 100644 --- a/client/tests/phpunit/includes/DataAccess/PropertyParserFunction/LanguageAwareRendererTest.php +++ b/client/tests/phpunit/includes/DataAccess/PropertyParserFunction/LanguageAwareRendererTest.php @@ -75,7 +75,8 @@ $propertyIdResolver, $snaksFinder, $this->getSnakFormatter(), - $this->getUsageAccumulator( $usages ) + $this->getUsageAccumulator( $usages ), + $this->getEntityLookup() ); } @@ -212,6 +213,17 @@ return $propertyIdResolver; } + private function getEntityLookup() { + $lookup = $this->getMock( 'Wikibase\Lib\Store\EntityLookup' ); + $lookup->expects( $this->any() ) + ->method( 'getEntity' ) + ->will( $this->returnValue( + $this->getMock( 'Wikibase\DataModel\StatementListProvider' ) + ) ); + + return $lookup; + } + /*** * @return SnakFormatter */ diff --git a/client/tests/phpunit/includes/DataAccess/PropertyParserFunction/PropertyClaimsRendererFactoryTest.php b/client/tests/phpunit/includes/DataAccess/PropertyParserFunction/PropertyClaimsRendererFactoryTest.php index dfdc1e9..1257596 100644 --- a/client/tests/phpunit/includes/DataAccess/PropertyParserFunction/PropertyClaimsRendererFactoryTest.php +++ b/client/tests/phpunit/includes/DataAccess/PropertyParserFunction/PropertyClaimsRendererFactoryTest.php @@ -110,7 +110,8 @@ $this->getPropertyIdResolver(), $this->getSnaksFinder(), $this->getLanguageFallbackChainFactory(), - $this->getSnakFormatterFactory() + $this->getSnakFormatterFactory(), + $this->getMock( 'Wikibase\Lib\Store\EntityLookup' ) ); } diff --git a/client/tests/phpunit/includes/DataAccess/SnaksFinderTest.php b/client/tests/phpunit/includes/DataAccess/SnaksFinderTest.php index 124032d..273ef36 100644 --- a/client/tests/phpunit/includes/DataAccess/SnaksFinderTest.php +++ b/client/tests/phpunit/includes/DataAccess/SnaksFinderTest.php @@ -7,12 +7,10 @@ use Wikibase\DataModel\Claim\Claim; use Wikibase\DataModel\Entity\Item; use Wikibase\DataModel\Entity\ItemId; -use Wikibase\DataModel\Entity\Property; use Wikibase\DataModel\Entity\PropertyId; use Wikibase\DataModel\Snak\PropertyValueSnak; +use Wikibase\DataModel\StatementListProvider; use Wikibase\DataModel\Statement\Statement; -use Wikibase\Lib\Store\EntityLookup; -use Wikibase\Test\MockRepository; /** * @covers Wikibase\DataAccess\SnaksFinder @@ -28,19 +26,18 @@ */ class SnaksFinderTest extends \PHPUnit_Framework_TestCase { - private function getSnaksFinder() { - $entityLookup = $this->getEntityLookup(); + /** + * @dataProvider findSnaksProvider + */ + public function testFindSnaks( array $expected, StatementListProvider $statementListProvider, PropertyId $propertyId, $acceptableRanks = null ) { + $snaksFinder = new SnaksFinder(); - return new SnaksFinder( $entityLookup ); + $snakList = $snaksFinder->findSnaks( $statementListProvider, $propertyId, $acceptableRanks ); + $this->assertEquals( $expected, $snakList ); } - /** - * @return EntityLookup - */ - private function getEntityLookup() { + public function findSnaksProvider() { $propertyId = new PropertyId( 'P1337' ); - - $mockRepository = new MockRepository(); $statement1 = new Statement( new Claim( new PropertyValueSnak( $propertyId, @@ -67,31 +64,6 @@ $item->getStatements()->addStatement( $statement2 ); $item->getStatements()->addStatement( $statement3 ); - $property = Property::newFromType( 'string' ); - $property->setId( $propertyId ); - $property->getFingerprint()->setLabel( 'en', 'a kitten!' ); - - $mockRepository->putEntity( $item ); - $mockRepository->putEntity( $property ); - - return $mockRepository; - } - - /** - * @dataProvider findSnaksProvider - */ - public function testFindSnaks( array $expected, ItemId $itemId, PropertyId $propertyId, $acceptableRanks = null ) { - $snaksFinder = $this->getSnaksFinder(); - - $snakList = $snaksFinder->findSnaks( $itemId, $propertyId, $acceptableRanks ); - $this->assertEquals( $expected, $snakList ); - } - - public function findSnaksProvider() { - $itemId = new ItemId( 'Q42' ); - - $propertyId = new PropertyId( 'P1337' ); - $snaksNormal = array( new PropertyValueSnak( $propertyId, new StringValue( 'a kitten!' ) ), new PropertyValueSnak( $propertyId, new StringValue( 'two kittens!!' ) ) @@ -99,11 +71,11 @@ $snakDeprecated = array( new PropertyValueSnak( $propertyId, new StringValue( 'three kittens!!!' ) ) ); return array( - array( $snaksNormal, $itemId, new PropertyId( 'P1337' ) ), - array( array(), $itemId, new PropertyId( 'P90001' ) ), + array( $snaksNormal, $item, new PropertyId( 'P1337' ) ), + array( array(), $item, new PropertyId( 'P90001' ) ), array( $snakDeprecated, - $itemId, + $item, new PropertyId( 'P1337' ), array( Statement::RANK_DEPRECATED ) ), -- To view, visit https://gerrit.wikimedia.org/r/192089 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I4bcf79829618331d0d92bb54828f4b71aa14c03a Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Wikibase Gerrit-Branch: master Gerrit-Owner: Hoo man <[email protected]> Gerrit-Reviewer: Aude <[email protected]> Gerrit-Reviewer: Daniel Kinzler <[email protected]> Gerrit-Reviewer: Jeroen De Dauw <[email protected]> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
