Thiemo Mättig (WMDE) has uploaded a new change for review.
https://gerrit.wikimedia.org/r/177772
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/includes/store/TermLookup.php
M lib/tests/phpunit/formatters/EntityIdHtmlLinkFormatterTest.php
M lib/tests/phpunit/store/EntityInfoTest.php
6 files changed, 128 insertions(+), 74 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/72/177772/1
diff --git a/lib/includes/store/EntityInfo.php
b/lib/includes/store/EntityInfo.php
index 4228660..be38f1f 100644
--- a/lib/includes/store/EntityInfo.php
+++ b/lib/includes/store/EntityInfo.php
@@ -53,129 +53,135 @@
}
/**
- * @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.
* 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 label 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 labels.
* @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, or null if
no such term is known.
+ * @return string|null The term value of the requested type, 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
* @throws OutOfBoundsException If nothing is known about the entity 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..e151230 100644
--- a/lib/includes/store/EntityInfoTermLookup.php
+++ b/lib/includes/store/EntityInfoTermLookup.php
@@ -38,12 +38,16 @@
* @param EntityId $entityId
* @param string $languageCode
*
+ * @throws OutOfBoundsException
* @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 +56,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 +69,16 @@
* @param EntityId $entityId
* @param string $languageCode
*
+ * @throws OutOfBoundsException
* @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 +87,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 63c4495..dc644b4 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
* @return string
*/
public function getLabel( EntityId $entityId, $languageCode ) {
@@ -57,6 +58,7 @@
* @param EntityId $entityId
* @param string $languageCode
*
+ * @throws OutOfBoundsException
* @return string
*/
public function getDescription( EntityId $entityId, $languageCode ) {
diff --git a/lib/includes/store/TermLookup.php
b/lib/includes/store/TermLookup.php
index 7fd16eb..bb87872 100644
--- a/lib/includes/store/TermLookup.php
+++ b/lib/includes/store/TermLookup.php
@@ -35,7 +35,7 @@
*
* @param EntityId $entityId
*
- * @throws OutOfBoundsException if the entity eas not found (not
guaranteed).
+ * @throws OutOfBoundsException if the entity was not found (not
guaranteed).
* @return string[] labels, keyed by language.
* An empty array may or may not indicate that the entity does
not exist.
*/
@@ -57,7 +57,7 @@
*
* @param EntityId $entityId
*
- * @throws OutOfBoundsException if the entity eas not found (not
guaranteed).
+ * @throws OutOfBoundsException if the entity was not found (not
guaranteed).
* @return string[] descriptions, keyed by language.
* An empty array may or may not indicate that the entity does
not exist.
*/
diff --git a/lib/tests/phpunit/formatters/EntityIdHtmlLinkFormatterTest.php
b/lib/tests/phpunit/formatters/EntityIdHtmlLinkFormatterTest.php
index bb17962..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 ) {
@@ -110,10 +112,7 @@
if ( $hasLabel ) {
$labelLookup = $this->getLabelLookup();
- } elseif ( !$exists ) {
- $labelLookup = $this->getLabelLookupNoLabel();
} else {
- // Exists, but w/o label
$labelLookup = $this->getLabelLookupNoLabel();
}
@@ -124,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: newchange
Gerrit-Change-Id: I9391f2c5d33eab8f3a34c407396387dac018ec06
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
