Thiemo Mättig (WMDE) has uploaded a new change for review. https://gerrit.wikimedia.org/r/125958
Change subject: Make EntityInfoBuilder method return entity info arrays instead of labels ...................................................................... Make EntityInfoBuilder method return entity info arrays instead of labels This is split from the bigger change I9bbbe8055d68b274fa6141eecff4a8d9414f89d9 to make it easier to review. This change alone may look a bit useless. The idea is to make the existing protected method getPropertyLabels() more generic. Now it returns the full EntityInfoBuilder array instead of the labels only. Since we know it contains only one language it's safe to use reset() to fetch the label. Please note that these reset() calls will go away in the next patch. Change-Id: I912882b09d302f8c74b1138b1aa3ab807d19f094 --- M repo/includes/ClaimHtmlGenerator.php M repo/includes/EntityView.php M repo/includes/view/SnakHtmlGenerator.php M repo/tests/phpunit/includes/view/SnakHtmlGeneratorTest.php 4 files changed, 64 insertions(+), 74 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase refs/changes/58/125958/1 diff --git a/repo/includes/ClaimHtmlGenerator.php b/repo/includes/ClaimHtmlGenerator.php index c882350..aa4fd79 100644 --- a/repo/includes/ClaimHtmlGenerator.php +++ b/repo/includes/ClaimHtmlGenerator.php @@ -2,11 +2,7 @@ namespace Wikibase; -use InvalidArgumentException; -use Wikibase\Lib\FormattingException; -use Wikibase\Lib\PropertyNotFoundException; use Wikibase\Lib\Serializers\ClaimSerializer; -use Wikibase\Lib\SnakFormatter; use Wikibase\View\SnakHtmlGenerator; /** @@ -19,7 +15,7 @@ * * @author H. Snater < [email protected] > * @author Pragunbhutani - * @author Katie Filbert < [email protected]> + * @author Katie Filbert < [email protected] > */ class ClaimHtmlGenerator { @@ -53,31 +49,29 @@ * @since 0.4 * * @param Claim $claim the claim to render + * @param array[] $propertyInfo * @param null|string $editSectionHtml has the html for the edit section * * @return string */ - public function getHtmlForClaim( Claim $claim, $propertyLabels, $editSectionHtml = null ) { + public function getHtmlForClaim( Claim $claim, array $propertyInfo, $editSectionHtml = null ) { wfProfileIn( __METHOD__ ); $mainSnakHtml = $this->snakHtmlGenerator->getSnakHtml( $claim->getMainSnak(), - $propertyLabels, + $propertyInfo, false ); - $claimHtml = ''; - if( !is_a( $claim, 'Wikibase\Statement' ) ) { - $claimHtml = wfTemplate( 'wb-claim', $claim->getGuid(), $mainSnakHtml, - $this->getHtmlForQualifiers( $claim->getQualifiers(), $propertyLabels ), + $this->getHtmlForQualifiers( $claim->getQualifiers(), $propertyInfo ), $editSectionHtml ); - } else { + /** @var \Wikibase\Statement $claim */ $serializedRank = ClaimSerializer::serializeRank( $claim->getRank() ); // Messages: wikibase-statementview-rank-preferred, wikibase-statementview-rank-normal, @@ -100,14 +94,14 @@ $referencesHtml = $this->getHtmlForReferences( $claim->getReferences(), - $propertyLabels + $propertyInfo ); $claimHtml = wfTemplate( 'wb-statement', $rankHtml, $claim->getGuid(), $mainSnakHtml, - $this->getHtmlForQualifiers( $claim->getQualifiers(), $propertyLabels ), + $this->getHtmlForQualifiers( $claim->getQualifiers(), $propertyInfo ), $editSectionHtml, $referencesHeading, $referencesHtml @@ -122,11 +116,11 @@ * Generates and returns the HTML representing a claim's qualifiers. * * @param Snaks $qualifiers - * @param string[] $propertyLabels + * @param array[] $propertyInfo * * @return string */ - protected function getHtmlForQualifiers( Snaks $qualifiers, array $propertyLabels ) { + protected function getHtmlForQualifiers( Snaks $qualifiers, array $propertyInfo ) { $qualifiersByProperty = new ByPropertyIdArray( $qualifiers ); $qualifiersByProperty->buildIndex(); @@ -135,7 +129,7 @@ foreach( $qualifiersByProperty->getPropertyIds() as $propertyId ) { $snaklistviewsHtml .= $this->getSnaklistviewHtml( $qualifiersByProperty->getByPropertyId( $propertyId ), - $propertyLabels + $propertyInfo ); } @@ -146,15 +140,15 @@ * Generates the HTML for a ReferenceList object. * * @param ReferenceList $referenceList - * @param string[] $propertyLabels + * @param array[] $propertyInfo * * @return string */ - protected function getHtmlForReferences( ReferenceList $referenceList, array $propertyLabels ) { + protected function getHtmlForReferences( ReferenceList $referenceList, array $propertyInfo ) { $referencesHtml = ''; foreach( $referenceList as $reference ) { - $referencesHtml .= $this->getHtmlForReference( $reference, $propertyLabels ); + $referencesHtml .= $this->getHtmlForReference( $reference, $propertyInfo ); } return $this->wrapInListview( $referencesHtml ); @@ -172,11 +166,11 @@ * Generates the HTML for a Reference object. * * @param Reference $reference - * @param string[] $propertyLabels + * @param array[] $propertyInfo * * @return string */ - protected function getHtmlForReference( $reference, array $propertyLabels ) { + protected function getHtmlForReference( $reference, array $propertyInfo ) { $referenceSnaksByProperty = new ByPropertyIdArray( $reference->getSnaks() ); $referenceSnaksByProperty->buildIndex(); @@ -185,7 +179,7 @@ foreach( $referenceSnaksByProperty->getPropertyIds() as $propertyId ) { $snaklistviewsHtml .= $this->getSnaklistviewHtml( $referenceSnaksByProperty->getByPropertyId( $propertyId ), - $propertyLabels + $propertyInfo ); } @@ -199,16 +193,16 @@ * Generates the HTML for a list of snaks. * * @param Snak[] $snaks - * @param string[] $propertyLabels + * @param array[] $propertyInfo * * @return string */ - protected function getSnaklistviewHtml( $snaks, array $propertyLabels ) { + protected function getSnaklistviewHtml( $snaks, array $propertyInfo ) { $snaksHtml = ''; $i = 0; foreach( $snaks as $snak ) { - $snaksHtml .= $this->snakHtmlGenerator->getSnakHtml( $snak, $propertyLabels, ( $i++ === 0 ) ); + $snaksHtml .= $this->snakHtmlGenerator->getSnakHtml( $snak, $propertyInfo, ( $i++ === 0 ) ); } return wfTemplate( diff --git a/repo/includes/EntityView.php b/repo/includes/EntityView.php index 6458234..2a05965 100644 --- a/repo/includes/EntityView.php +++ b/repo/includes/EntityView.php @@ -2,14 +2,16 @@ namespace Wikibase; +use ContextSource; use Html; -use ParserOutput; -use Language; use IContextSource; +use InvalidArgumentException; +use Language; +use ParserOutput; +use SpecialPageFactory; use Wikibase\Lib\PropertyDataTypeLookup; use Wikibase\Lib\Serializers\SerializationOptions; use Wikibase\Lib\SnakFormatter; -use Wikibase\Repo\WikibaseRepo; use Wikibase\View\SnakHtmlGenerator; /** @@ -26,7 +28,7 @@ * @author Daniel Werner * @author Daniel Kinzler */ -abstract class EntityView extends \ContextSource { +abstract class EntityView extends ContextSource { /** * @var EntityInfoBuilder @@ -90,7 +92,7 @@ * * @todo: move the $editable flag here, instead of passing it around everywhere * - * @throws \InvalidArgumentException + * @throws InvalidArgumentException */ public function __construct( IContextSource $context, @@ -103,7 +105,7 @@ ) { if ( $snakFormatter->getFormat() !== SnakFormatter::FORMAT_HTML && $snakFormatter->getFormat() !== SnakFormatter::FORMAT_HTML_WIDGET ) { - throw new \InvalidArgumentException( '$snakFormatter is expected to return text/html, not ' + throw new InvalidArgumentException( '$snakFormatter is expected to return text/html, not ' . $snakFormatter->getFormat() ); } @@ -533,7 +535,7 @@ $claimsByProperty[$propertyId->getNumericId()][] = $claim; } - $propertyLabels = $this->getPropertyLabels( $entity, $this->getLanguage()->getCode() ); + $propertyInfo = $this->getPropertyInfo( $entity, $this->getLanguage()->getCode() ); /** * @var string $claimsHtml @@ -545,10 +547,11 @@ $propertyHtml = ''; $propertyId = $claims[0]->getMainSnak()->getPropertyId(); - $propertyKey = $propertyId->getSerialization(); - $propertyLabel = isset( $propertyLabels[$propertyKey] ) - ? $propertyLabels[$propertyKey] - : $propertyKey; + $key = $propertyId->getSerialization(); + $propertyLabel = ( isset( $propertyInfo[$key] ) + && !empty( $propertyInfo[$key]['labels'] ) ) + ? reset( $propertyInfo[$key]['labels'] ) + : $key; $propertyLink = \Linker::link( $this->entityTitleLookup->getTitleForId( $propertyId ), htmlspecialchars( $propertyLabel ) @@ -559,7 +562,7 @@ foreach( $claims as $claim ) { $propertyHtml .= $this->claimHtmlGenerator->getHtmlForClaim( $claim, - $propertyLabels, + $propertyInfo, $htmlForEditSection ); } @@ -615,12 +618,12 @@ * * @param string $specialpagename * @param Entity $entity - * @param \Language $lang|null + * @param Language $language|null * * @return string */ - protected function getEditUrl( $specialpagename, Entity $entity, Language $lang = null ) { - $specialpage = \SpecialPageFactory::getPage( $specialpagename ); + protected function getEditUrl( $specialpagename, Entity $entity, Language $language = null ) { + $specialpage = SpecialPageFactory::getPage( $specialpagename ); if ( $specialpage === null ) { return ''; //XXX: this should throw an exception?! @@ -632,8 +635,8 @@ $subpage = ''; // can't skip this, that would confuse the order of parameters! } - if ( $lang !== null ) { - $subpage .= '/' . $lang->getCode(); + if ( $language !== null ) { + $subpage .= '/' . $language->getCode(); } return $specialpage->getPageTitle( $subpage )->getLocalURL(); } @@ -642,38 +645,30 @@ * Fetches labels for all properties used as properties in snaks in the given entity. * * @param Entity $entity - * @param string $langCode the language code of the labels to fetch. + * @param string $languageCode the language code of the labels to fetch. * - * @todo: we may also want to have the descriptions, in addition to the labels - * @return array maps property IDs to labels. + * @todo: We may also want to have the descriptions, in addition to the labels + * @return array[] Maps property IDs to labels. */ - protected function getPropertyLabels( Entity $entity, $langCode ) { + protected function getPropertyInfo( Entity $entity, $languageCode ) { wfProfileIn( __METHOD__ ); - //TODO: share cache with PropertyLabelResolver - //TODO: ...or share info with getBasicEntityInfo + // TODO: Share cache with PropertyLabelResolver + // TODO: ... or share info with getBasicEntityInfo. - //TODO: make a finder just for properties, so we don't have to filter + // TODO: Make a finder just for properties, so we don't have to filter. $refFinder = new ReferencedEntitiesFinder(); $entityIds = $refFinder->findSnakLinks( $entity->getAllSnaks() ); $propertyIds = array_filter( $entityIds, function ( EntityId $id ) { return $id->getEntityType() === Property::ENTITY_TYPE; } ); - //NOTE: this is a bit hackish,it would be more appropriate to use a TermTable here. + // NOTE: This is a bit hackish, it would be more appropriate to use a TermTable here. $entities = $this->entityInfoBuilder->buildEntityInfo( $propertyIds ); - $this->entityInfoBuilder->addTerms( $entities, array( 'label', 'description' ), array( $langCode ) ); - - //TODO: apply language fallback - $propertyLabels = array(); - foreach ( $entities as $id => $entity ) { - if ( isset( $entity['labels'][$langCode] ) ) { - $label = $entity['labels'][$langCode]['value']; - $propertyLabels[$id] = $label; - } - } + $this->entityInfoBuilder->removeMissing( $entities ); + $this->entityInfoBuilder->addTerms( $entities, array( 'label', 'description' ), array( $languageCode ) ); wfProfileOut( __METHOD__ ); - return $propertyLabels; + return $entities; } } diff --git a/repo/includes/view/SnakHtmlGenerator.php b/repo/includes/view/SnakHtmlGenerator.php index 57cc5b7..2790b7f 100644 --- a/repo/includes/view/SnakHtmlGenerator.php +++ b/repo/includes/view/SnakHtmlGenerator.php @@ -51,12 +51,12 @@ * Generates the HTML for a single snak. * * @param Snak $snak - * @param string[] $propertyLabels - * @param boolean $showPropertyLink + * @param array[] $propertyInfo + * @param bool $showPropertyLink * * @return string */ - public function getSnakHtml( Snak $snak, array $propertyLabels, $showPropertyLink = false ) { + public function getSnakHtml( Snak $snak, array $propertyInfo, $showPropertyLink = false ) { $snakViewVariation = $this->getSnakViewVariation( $snak ); $snakViewCssClass = 'wb-snakview-variation-' . $snakViewVariation; @@ -67,7 +67,7 @@ } $propertyLink = $showPropertyLink ? - $this->makePropertyLink( $snak, $propertyLabels, $showPropertyLink ) : ''; + $this->makePropertyLink( $snak, $propertyInfo, $showPropertyLink ) : ''; $html = wfTemplate( 'wb-snak', // Display property link only once for snaks featuring the same property: @@ -81,16 +81,17 @@ /** * @param Snak $snak - * @param string[] $propertyLabels + * @param array[] $propertyInfo * * @return string */ - private function makePropertyLink( Snak $snak, array $propertyLabels ) { + private function makePropertyLink( Snak $snak, array $propertyInfo ) { $propertyId = $snak->getPropertyId(); - $propertyKey = $propertyId->getSerialization(); - $propertyLabel = isset( $propertyLabels[$propertyKey] ) - ? $propertyLabels[$propertyKey] - : $propertyKey; + $key = $propertyId->getSerialization(); + $propertyLabel = ( isset( $propertyInfo[$key] ) + && !empty( $propertyInfo[$key]['labels'] ) ) + ? reset( $propertyInfo[$key]['labels'] ) + : $key; // @todo use EntityIdHtmlLinkFormatter here $propertyLink = \Linker::link( diff --git a/repo/tests/phpunit/includes/view/SnakHtmlGeneratorTest.php b/repo/tests/phpunit/includes/view/SnakHtmlGeneratorTest.php index 0595da1..8a4d90d 100644 --- a/repo/tests/phpunit/includes/view/SnakHtmlGeneratorTest.php +++ b/repo/tests/phpunit/includes/view/SnakHtmlGeneratorTest.php @@ -28,7 +28,7 @@ public function testGetSnakHtml( $snakFormatter, $entityTitleLookup, - $propertyLabels, + $propertyInfo, $snak, $patterns ) { @@ -37,7 +37,7 @@ $entityTitleLookup ); - $html = $snakHtmlGenerator->getSnakHtml( $snak, $propertyLabels ); + $html = $snakHtmlGenerator->getSnakHtml( $snak, $propertyInfo ); foreach( $patterns as $message => $pattern ) { $this->assertRegExp( $pattern, $html, $message ); -- To view, visit https://gerrit.wikimedia.org/r/125958 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I912882b09d302f8c74b1138b1aa3ab807d19f094 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Wikibase Gerrit-Branch: master Gerrit-Owner: Thiemo Mättig (WMDE) <[email protected]> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
