jenkins-bot has submitted this change and it was merged.
Change subject: Catch more edge cases in EntityInfo
......................................................................
Catch more edge cases in EntityInfo
The most important issue this patch fixes is the "backwards
compatibility" that got removed in the previous patch. The
implementation tried to access the first character if the value
was a string (old format) and not an array. But it should throw a
RuntimeException.
Change-Id: I9391f2c5d33eab8f3a34c407396387dac018ec06
---
M lib/includes/store/EntityInfo.php
M lib/includes/store/EntityInfoTermLookup.php
M lib/includes/store/EntityTermLookup.php
M lib/tests/phpunit/formatters/EntityIdHtmlLinkFormatterTest.php
M lib/tests/phpunit/store/EntityInfoTest.php
5 files changed, 130 insertions(+), 75 deletions(-)
Approvals:
Daniel Kinzler: Looks good to me, approved
jenkins-bot: Verified
diff --git a/lib/includes/store/EntityInfo.php
b/lib/includes/store/EntityInfo.php
index 219dd36..6c43361 100644
--- a/lib/includes/store/EntityInfo.php
+++ b/lib/includes/store/EntityInfo.php
@@ -53,110 +53,113 @@
}
/**
- * @param EntityId $id
+ * @param EntityId $entityId
*
* @return bool
*/
- public function hasEntityInfo( EntityId $id ) {
- $key = $id->getSerialization();
+ public function hasEntityInfo( EntityId $entityId ) {
+ $key = $entityId->getSerialization();
- return isset( $this->info[$key] );
+ return array_key_exists( $key, $this->info );
}
/**
- * @param EntityId $id
+ * @param EntityId $entityId
*
+ * @throws RuntimeException
* @throws OutOfBoundsException If this EntityInfo does not have
information about the
* requested Entity. This does say anything about whether the
Entity exists in
* the database.
- * @return array[] An array structure representing information about
the given entity.
+ * @return array An array structure representing information about the
given entity.
* Refer to the class level documentation for information about
the structure.
*/
- public function getEntityInfo( EntityId $id ) {
- $key = $id->getSerialization();
+ public function getEntityInfo( EntityId $entityId ) {
+ $key = $entityId->getSerialization();
- if ( isset( $this->info[$key] ) ) {
- return $this->info[$key];
- } else {
- throw new OutOfBoundsException( 'Unknown entity: ' .
$id );
+ if ( !array_key_exists( $key, $this->info ) ) {
+ throw new OutOfBoundsException( "Unknown entity
$entityId" );
+ } elseif ( !is_array( $this->info[$key] ) ) {
+ throw new RuntimeException( "$key term record is
invalid" );
}
+
+ return $this->info[$key];
}
/**
- * @param EntityId $id
+ * @param EntityId $entityId
* @param string $languageCode
*
* @throws OutOfBoundsException If nothing is known about the entity or
its labels.
* @return string|null The entity's label in the given language,
- * or null if no such label exists on the entity.
+ * or null if a label in that language is not known.
*/
- public function getLabel( EntityId $id, $languageCode ) {
- return $this->getTermValue( $id, 'labels', $languageCode );
+ public function getLabel( EntityId $entityId, $languageCode ) {
+ return $this->getTermValue( $entityId, 'labels', $languageCode
);
}
/**
- * @param EntityId $id
+ * @param EntityId $entityId
* @param string $languageCode
*
* @throws OutOfBoundsException If nothing is known about the entity or
its descriptions.
* @return string|null The entity's description in the given language,
- * or null if no such description exists on the entity.
+ * or null if a description in that language is not known.
*/
- public function getDescription( EntityId $id, $languageCode ) {
- return $this->getTermValue( $id, 'descriptions', $languageCode
);
+ public function getDescription( EntityId $entityId, $languageCode ) {
+ return $this->getTermValue( $entityId, 'descriptions',
$languageCode );
}
/**
- * @param EntityId $id
+ * @param EntityId $entityId
*
* @throws OutOfBoundsException If nothing is known about the entity or
its labels.
* @return string[] The given entity's labels, keyed by language.
*/
- public function getLabels( EntityId $id ) {
- return $this->getTermValues( $id, 'labels' );
+ public function getLabels( EntityId $entityId ) {
+ return $this->getTermValues( $entityId, 'labels' );
}
/**
- * @param EntityId $id
+ * @param EntityId $entityId
*
* @throws OutOfBoundsException If nothing is known about the entity or
its descriptions.
* @return string[] The given entity's descriptions, keyed by language.
*/
- public function getDescriptions( EntityId $id ) {
- return $this->getTermValues( $id, 'descriptions' );
+ public function getDescriptions( EntityId $entityId ) {
+ return $this->getTermValues( $entityId, 'descriptions' );
}
/**
- * @param EntityId $id
+ * @param EntityId $entityId
* @param string $termField The term field (e.g. 'labels' or
'descriptions').
* @param string $languageCode
*
* @throws RuntimeException
* @throws OutOfBoundsException If nothing is known about the entity
* or terms of the requested type.
- * @return string|null The term value of the requested type and
language,
- * or null if no such term is set on the entity.
+ * @return string|null The term value of the requested type and
language, or null if a term in
+ * that language is not known.
*/
- private function getTermValue( EntityId $id, $termField, $languageCode
) {
- $entityInfo = $this->getEntityInfo( $id );
+ private function getTermValue( EntityId $entityId, $termField,
$languageCode ) {
+ $entityInfo = $this->getEntityInfo( $entityId );
- if ( !isset( $entityInfo[$termField] ) ) {
- throw new OutOfBoundsException( 'Term field ' .
$termField . ' is unknown.' );
- }
-
- if ( !isset( $entityInfo[$termField][$languageCode] ) ) {
+ if ( !array_key_exists( $termField, $entityInfo ) ) {
+ throw new OutOfBoundsException( "No '$termField' in
$entityId term record" );
+ } elseif ( !is_array( $entityInfo[$termField] ) ) {
+ throw new RuntimeException( "$entityId term record is
invalid" );
+ } elseif ( !array_key_exists( $languageCode,
$entityInfo[$termField] ) ) {
return null;
- }
-
- if ( !isset( $entityInfo[$termField][$languageCode]['value'] )
) {
- throw new RuntimeException( 'Term record is missing
`value` key (' . $id->getSerialization() . ')' );
+ } elseif ( !is_array( $entityInfo[$termField][$languageCode] )
+ || !array_key_exists( 'value',
$entityInfo[$termField][$languageCode] )
+ ) {
+ throw new RuntimeException( "$entityId term record is
missing the 'value' field" );
}
return $entityInfo[$termField][$languageCode]['value'];
}
/**
- * @param EntityId $id
+ * @param EntityId $entityId
* @param string $termField The term field (e.g. 'labels' or
'descriptions').
*
* @throws RuntimeException
@@ -164,21 +167,23 @@
* or terms of the requested type.
* @return string[] The entity's term values of the requested type,
keyed by language.
*/
- private function getTermValues( EntityId $id, $termField ) {
- $entityInfo = $this->getEntityInfo( $id );
+ private function getTermValues( EntityId $entityId, $termField ) {
+ $entityInfo = $this->getEntityInfo( $entityId );
- if ( !isset( $entityInfo[$termField] ) ) {
- throw new OutOfBoundsException( 'Term field `' .
$termField . '` is unknown.' );
+ if ( !array_key_exists( $termField, $entityInfo ) ) {
+ throw new OutOfBoundsException( "No '$termField' in
$entityId term record" );
+ } elseif ( !is_array( $entityInfo[$termField] ) ) {
+ throw new RuntimeException( "$entityId term record is
invalid" );
}
$values = array();
- foreach ( $entityInfo[$termField] as $key => $entry ) {
- if ( !isset( $entry['value'] ) ) {
- throw new RuntimeException( 'Term record is
missing `value` key (' . $id->getSerialization() . ')' );
+ foreach ( $entityInfo[$termField] as $key => $term ) {
+ if ( !is_array( $term ) || !array_key_exists( 'value',
$term ) ) {
+ throw new RuntimeException( "$entityId term
record is missing the 'value' field" );
}
- $values[$key] = $entry['value'];
+ $values[$key] = $term['value'];
}
return $values;
diff --git a/lib/includes/store/EntityInfoTermLookup.php
b/lib/includes/store/EntityInfoTermLookup.php
index 09c9dcb..d60f017 100644
--- a/lib/includes/store/EntityInfoTermLookup.php
+++ b/lib/includes/store/EntityInfoTermLookup.php
@@ -25,8 +25,7 @@
private $entityInfo;
/**
- * @param EntityInfo $entityInfo An array of entity records, as returned
- * by EntityInfoBuilder::getEntityInfo.
+ * @param EntityInfo $entityInfo
*/
public function __construct( EntityInfo $entityInfo ) {
$this->entityInfo = $entityInfo;
@@ -38,12 +37,16 @@
* @param EntityId $entityId
* @param string $languageCode
*
+ * @throws OutOfBoundsException if no label in that language is known
* @return string
*/
public function getLabel( EntityId $entityId, $languageCode ) {
$label = $this->entityInfo->getLabel( $entityId, $languageCode
);
- $this->checkOutOfBOunds( $label, $languageCode );
+ if ( $label === null ) {
+ throw new OutOfBoundsException( "No label found for
language '$languageCode'" );
+ }
+
return $label;
}
@@ -52,11 +55,11 @@
*
* @param EntityId $entityId
*
+ * @throws OutOfBoundsException
* @return string[]
*/
public function getLabels( EntityId $entityId ) {
- $labels = $this->entityInfo->getLabels( $entityId );
- return $labels;
+ return $this->entityInfo->getLabels( $entityId );
}
/**
@@ -65,12 +68,16 @@
* @param EntityId $entityId
* @param string $languageCode
*
+ * @throws OutOfBoundsException if no description in that language is
known
* @return string
*/
public function getDescription( EntityId $entityId, $languageCode ) {
$description = $this->entityInfo->getDescription( $entityId,
$languageCode );
- $this->checkOutOfBOunds( $description, $languageCode );
+ if ( $description === null ) {
+ throw new OutOfBoundsException( "No description found
for language '$languageCode'" );
+ }
+
return $description;
}
@@ -79,22 +86,11 @@
*
* @param EntityId $entityId
*
+ * @throws OutOfBoundsException
* @return string[]
*/
public function getDescriptions( EntityId $entityId ) {
- $descriptions = $this->entityInfo->getDescriptions( $entityId );
- return $descriptions;
+ return $this->entityInfo->getDescriptions( $entityId );
}
- /**
- * @param string $value
- * @param string $languageCode
- *
- * @throws OutOfBoundsException
- */
- private function checkOutOfBOunds( $value, $languageCode ) {
- if ( $value === null ) {
- throw new OutOfBoundsException( 'No entry found for
language ' . $languageCode );
- }
- }
}
diff --git a/lib/includes/store/EntityTermLookup.php
b/lib/includes/store/EntityTermLookup.php
index c66badf..30abf6d 100644
--- a/lib/includes/store/EntityTermLookup.php
+++ b/lib/includes/store/EntityTermLookup.php
@@ -33,6 +33,7 @@
* @param EntityId $entityId
* @param string $languageCode
*
+ * @throws OutOfBoundsException if no label in that language is known
* @return string
*/
public function getLabel( EntityId $entityId, $languageCode ) {
@@ -57,10 +58,11 @@
* @param EntityId $entityId
* @param string $languageCode
*
+ * @throws OutOfBoundsException if no description in that language is
known
* @return string
*/
public function getDescription( EntityId $entityId, $languageCode ) {
- $descriptions = $this->getTermsOfType( $entityId, 'description'
);
+ $descriptions = $this->getDescriptions( $entityId );
return $this->filterByLanguage( $descriptions, $languageCode );
}
@@ -91,7 +93,7 @@
* @param string[] $terms
* @param string $languageCode
*
- * @throws OutOfBoundsException
+ * @throws OutOfBoundsException if no term in that language is known
* @return string
*/
private function filterByLanguage( array $terms, $languageCode ) {
diff --git a/lib/tests/phpunit/formatters/EntityIdHtmlLinkFormatterTest.php
b/lib/tests/phpunit/formatters/EntityIdHtmlLinkFormatterTest.php
index 1af7e72..086cf2b 100644
--- a/lib/tests/phpunit/formatters/EntityIdHtmlLinkFormatterTest.php
+++ b/lib/tests/phpunit/formatters/EntityIdHtmlLinkFormatterTest.php
@@ -4,11 +4,11 @@
use OutOfBoundsException;
use Title;
+use ValueFormatters\FormatterOptions;
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Entity\ItemId;
use Wikibase\Lib\EntityIdHtmlLinkFormatter;
use Wikibase\Lib\Store\EntityTitleLookup;
-use ValueFormatters\FormatterOptions;
/**
* @covers Wikibase\Lib\EntityIdHtmlLinkFormatter
@@ -41,6 +41,8 @@
}
/**
+ * @param bool $exists
+ *
* @return EntityTitleLookup
*/
private function newEntityTitleLookup( $exists = true ) {
@@ -121,4 +123,5 @@
$this->assertRegExp( $expectedRegex, $result );
}
+
}
diff --git a/lib/tests/phpunit/store/EntityInfoTest.php
b/lib/tests/phpunit/store/EntityInfoTest.php
index 0ff806c..be1cdab 100644
--- a/lib/tests/phpunit/store/EntityInfoTest.php
+++ b/lib/tests/phpunit/store/EntityInfoTest.php
@@ -199,7 +199,6 @@
*/
public function testGetLabel_exception( $data ) {
$info = new EntityInfo( $data );
-
$this->setExpectedException( 'OutOfBoundsException' );
$info->getLabel( new ItemId( 'Q99' ), 'en' );
}
@@ -209,7 +208,6 @@
*/
public function testGetLabels_exception( $data ) {
$info = new EntityInfo( $data );
-
$this->setExpectedException( 'OutOfBoundsException' );
$info->getLabels( new ItemId( 'Q99' ) );
}
@@ -219,7 +217,6 @@
*/
public function testGetDescription_exception( $data ) {
$info = new EntityInfo( $data );
-
$this->setExpectedException( 'OutOfBoundsException' );
$info->getDescription( new ItemId( 'Q99' ), 'en' );
}
@@ -229,9 +226,61 @@
*/
public function testGetDescriptions_exception( $data ) {
$info = new EntityInfo( $data );
-
$this->setExpectedException( 'OutOfBoundsException' );
$info->getDescriptions( new ItemId( 'Q99' ) );
}
+ public function invalidArrayProvider() {
+ return array(
+ 'value incomplete' => array(
+ array( 'Q99' => array( 'labels' => array( 'en'
=> array() ) ) )
+ ),
+ 'value invalid' => array(
+ array( 'Q99' => array( 'labels' => array( 'en'
=> 'not an array' ) ) )
+ ),
+ 'labels invalid' => array(
+ array( 'Q99' => array( 'labels' => 'not an
array' ) )
+ ),
+ 'entity invalid' => array(
+ array( 'Q99' => 'not an array' )
+ ),
+ );
+ }
+
+ /**
+ * @dataProvider invalidArrayProvider
+ */
+ public function testGetLabelWithInvalidArray_throwsRuntimeException(
$array ) {
+ $info = new EntityInfo( $array );
+ $this->setExpectedException( 'RuntimeException' );
+ $info->getLabel( new ItemId( 'Q99' ), 'en' );
+ }
+
+ /**
+ * @dataProvider invalidArrayProvider
+ */
+ public function testGetLabelsWithInvalidArray_throwsRuntimeException(
$array ) {
+ $info = new EntityInfo( $array );
+ $this->setExpectedException( 'RuntimeException' );
+ $info->getLabels( new ItemId( 'Q99' ) );
+ }
+
+ /**
+ * @dataProvider invalidArrayProvider
+ */
+ public function
testGetDescriptionWithInvalidArray_throwsRuntimeException( $array ) {
+ $info = new EntityInfo( $array );
+ $this->setExpectedException( 'RuntimeException' );
+ $info->getDescription( new ItemId( 'Q99' ), 'en' );
+ }
+
+ /**
+ * @dataProvider invalidArrayProvider
+ */
+ public function
testGetDescriptionsWithInvalidArray_throwsRuntimeException( $array ) {
+ $info = new EntityInfo( $array );
+ $this->setExpectedException( 'RuntimeException' );
+ $info->getDescriptions( new ItemId( 'Q99' ) );
+ }
+
}
--
To view, visit https://gerrit.wikimedia.org/r/177772
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I9391f2c5d33eab8f3a34c407396387dac018ec06
Gerrit-PatchSet: 6
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Thiemo Mättig (WMDE) <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: JanZerebecki <[email protected]>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
