Addshore has uploaded a new change for review.
https://gerrit.wikimedia.org/r/114466
Change subject: Refactor confusing getting of Ids from Term Object
......................................................................
Refactor confusing getting of Ids from Term Object
The method getEntityId is missleading as this is not what we
actually get... we get the numeric Id.
This also pokes search entities to reduce the ammount of times
we use this numeric ID as we can noe used the serialization from
a regular EntityId object
Change-Id: I895c24ea880ad4aafa2fca48c99b87691574b7e5
---
M lib/includes/Term.php
M lib/includes/store/TermPropertyLabelResolver.php
M lib/tests/phpunit/TermTest.php
M lib/tests/phpunit/store/TermIndexTest.php
M repo/includes/LabelDescriptionDuplicateDetector.php
M repo/includes/api/SearchEntities.php
M repo/includes/content/EntityContent.php
M repo/tests/phpunit/includes/LabelDescriptionDuplicateDetectorTest.php
8 files changed, 29 insertions(+), 22 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/66/114466/1
diff --git a/lib/includes/Term.php b/lib/includes/Term.php
index 036b107..af56014 100644
--- a/lib/includes/Term.php
+++ b/lib/includes/Term.php
@@ -56,7 +56,7 @@
$this->setLanguage( $value );
break;
case 'entityId':
- $this->setEntityId( $value );
+ $this->setNumericId( $value );
break;
case 'entityType':
$this->setEntityType( $value );
@@ -174,7 +174,7 @@
*
* @throws MWException
*/
- public function setEntityId( $id ) {
+ public function setNumericId( $id ) {
if ( !is_int( $id ) ) {
throw new MWException( 'Entity id code can only be an
integer' );
}
@@ -187,7 +187,7 @@
*
* @return integer|null
*/
- public function getEntityId() {
+ public function getNumericId() {
return array_key_exists( 'entityId', $this->fields ) ?
$this->fields['entityId'] : null;
}
diff --git a/lib/includes/store/TermPropertyLabelResolver.php
b/lib/includes/store/TermPropertyLabelResolver.php
index 6f96bbc..dbb8e8e 100644
--- a/lib/includes/store/TermPropertyLabelResolver.php
+++ b/lib/includes/store/TermPropertyLabelResolver.php
@@ -142,7 +142,7 @@
foreach ( $terms as $term ) {
$label = $term->getText();
- $id = PropertyId::newFromNumber( $term->getEntityId() );
+ $id = PropertyId::newFromNumber( $term->getNumericId()
);
$propertiesByLabel[$label] = $id;
}
diff --git a/lib/tests/phpunit/TermTest.php b/lib/tests/phpunit/TermTest.php
index 9662a29..5221564 100644
--- a/lib/tests/phpunit/TermTest.php
+++ b/lib/tests/phpunit/TermTest.php
@@ -52,7 +52,7 @@
$term = new Term( $fields );
$this->assertEquals( isset( $fields['entityType'] ) ?
$fields['entityType'] : null, $term->getEntityType() );
- $this->assertEquals( isset( $fields['entityId'] ) ?
$fields['entityId'] : null, $term->getEntityId() );
+ $this->assertEquals( isset( $fields['entityId'] ) ?
$fields['entityId'] : null, $term->getNumericId() );
$this->assertEquals( isset( $fields['termType'] ) ?
$fields['termType'] : null, $term->getType() );
$this->assertEquals( isset( $fields['termLanguage'] ) ?
$fields['termLanguage'] : null, $term->getLanguage() );
$this->assertEquals( isset( $fields['termText'] ) ?
$fields['termText'] : null, $term->getText() );
@@ -106,7 +106,7 @@
);
$other = clone $term;
- $other->setEntityId( 11 );
+ $other->setNumericId( 11 );
$tests[] = array( // #3
$term,
$other,
diff --git a/lib/tests/phpunit/store/TermIndexTest.php
b/lib/tests/phpunit/store/TermIndexTest.php
index 306f99b..bc9e3f9 100644
--- a/lib/tests/phpunit/store/TermIndexTest.php
+++ b/lib/tests/phpunit/store/TermIndexTest.php
@@ -183,7 +183,7 @@
* @var Term $expected
*/
foreach ( $actual as $term ) {
- $id = $term->getEntityId();
+ $id = $term->getNumericId();
$this->assertTrue( in_array( $id, array( $id0, $id1 ),
true ) );
@@ -264,7 +264,7 @@
* @var Term $expected
*/
foreach ( $actual as $term ) {
- $id = $term->getEntityId();
+ $id = $term->getNumericId();
$this->assertTrue( in_array( $id, array( $id0, $id1 ),
true ) );
@@ -472,7 +472,7 @@
* @var Term $expected
*/
foreach ( $actual as $term ) {
- $id = $term->getEntityId();
+ $id = $term->getNumericId();
$this->assertEquals( $id0, $id );
diff --git a/repo/includes/LabelDescriptionDuplicateDetector.php
b/repo/includes/LabelDescriptionDuplicateDetector.php
index 021b838..7f87bf0 100644
--- a/repo/includes/LabelDescriptionDuplicateDetector.php
+++ b/repo/includes/LabelDescriptionDuplicateDetector.php
@@ -97,7 +97,7 @@
'wikibase-error-label-not-unique-item',
$label->getText(),
$label->getLanguage(),
- $label->getEntityId(),
+ $label->getNumericId(),
$description->getText()
);
}
diff --git a/repo/includes/api/SearchEntities.php
b/repo/includes/api/SearchEntities.php
index c382058..32645f0 100644
--- a/repo/includes/api/SearchEntities.php
+++ b/repo/includes/api/SearchEntities.php
@@ -132,9 +132,8 @@
$entries = array();
foreach ( $ids as $id ) {
- // FIXME: Change this to the actual ID if the Term
class stops returning integers.
- $numericId = $id->getNumericId();
- $entries[ $numericId ] = array(
+ $key = $id->getSerialization();
+ $entries[ $key ] = array(
'id' => $id->getPrefixedId(),
'url' => $entityContentFactory->getTitleForId(
$id )->getFullUrl()
);
@@ -146,13 +145,12 @@
$aliasPattern = '/^' . preg_quote( $params['search'], '/' ) .
'/i';
foreach ( $terms as $term ) {
- // FIXME: Change this to the actual ID if the Term
class stops returning integers.
- $numericId = $term->getEntityId();
- if ( !isset( $entries[ $numericId ] ) ) {
+ $key = $this->getEntityIdSerializationFromTerm( $term );
+ if ( !isset( $entries[$key] ) ) {
continue;
}
- $entry = $entries[$numericId];
+ $entry = $entries[$key];
switch ( $term->getType() ) {
case Term::TYPE_LABEL:
@@ -173,7 +171,7 @@
break;
}
- $entries[$numericId] = $entry;
+ $entries[$key] = $entry;
}
$entries = array_values( $entries );
@@ -335,4 +333,13 @@
return __CLASS__ . '-' . WB_VERSION;
}
+ /**
+ * @param Term $term
+ * @return string
+ */
+ private function getEntityIdSerializationFromTerm( Term $term ) {
+ $id = new EntityId( $term->getEntityType(),
$term->getNumericId() );
+ return $id->getSerialization();
+ }
+
}
diff --git a/repo/includes/content/EntityContent.php
b/repo/includes/content/EntityContent.php
index 9634e2e..05f6d2d 100644
--- a/repo/includes/content/EntityContent.php
+++ b/repo/includes/content/EntityContent.php
@@ -718,14 +718,14 @@
* @var Term $foundLabel
*/
foreach ( $foundLabels as $foundLabel ) {
- if ( $foundLabel->getEntityId() !==
$entity->getId()->getNumericId() ) {
+ if ( $foundLabel->getNumericId() !==
$entity->getId()->getNumericId() ) {
// Messages:
wikibase-error-label-not-unique-wikibase-property,
//
wikibase-error-label-not-unique-wikibase-query
$status->fatal(
'wikibase-error-label-not-unique-wikibase-' . $entity->getType(),
$foundLabel->getText(),
$foundLabel->getLanguage(),
- $foundLabel->getEntityId()
+ $foundLabel->getNumericId()
);
}
}
diff --git
a/repo/tests/phpunit/includes/LabelDescriptionDuplicateDetectorTest.php
b/repo/tests/phpunit/includes/LabelDescriptionDuplicateDetectorTest.php
index adf8713..706c75f 100644
--- a/repo/tests/phpunit/includes/LabelDescriptionDuplicateDetectorTest.php
+++ b/repo/tests/phpunit/includes/LabelDescriptionDuplicateDetectorTest.php
@@ -195,11 +195,11 @@
&& $term->getType() ===
$storedTerm->getType() ) {
if ( $id === null ) {
- $id =
$term->getEntityId();
+ $id =
$term->getNumericId();
$type =
$term->getEntityType();
$matchingTerms[] =
$term;
}
- elseif ( $id ===
$term->getEntityId() && $type === $term->getEntityType() ) {
+ elseif ( $id ===
$term->getNumericId() && $type === $term->getEntityType() ) {
$matchingTerms[] =
$term;
}
}
--
To view, visit https://gerrit.wikimedia.org/r/114466
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I895c24ea880ad4aafa2fca48c99b87691574b7e5
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Addshore <addshorew...@gmail.com>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
