Daniel Kinzler has uploaded a new change for review.
https://gerrit.wikimedia.org/r/179527
Change subject: Allow filtering by term type in getTermsOfEntities
......................................................................
Allow filtering by term type in getTermsOfEntities
Change-Id: Ie0d92ae917fe6a9ebe112a3a37466c94250334db
---
M client/includes/api/PageTerms.php
M client/tests/phpunit/includes/api/PageTermsTest.php
M lib/includes/store/TermIndex.php
M lib/includes/store/sql/TermSqlIndex.php
M lib/tests/phpunit/store/MockTermIndex.php
M lib/tests/phpunit/store/TermIndexTest.php
M repo/includes/api/SearchEntities.php
7 files changed, 122 insertions(+), 50 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/27/179527/1
diff --git a/client/includes/api/PageTerms.php
b/client/includes/api/PageTerms.php
index e34d700..58c252b 100644
--- a/client/includes/api/PageTerms.php
+++ b/client/includes/api/PageTerms.php
@@ -74,11 +74,7 @@
$pagesToEntityIds = $this->getEntityIdsForTitles( $titles,
$continue );
$entityToPageMap = $this->getEntityToPageMap( $pagesToEntityIds
);
- $terms = $this->getTermsOfEntities( $pagesToEntityIds,
$languageCode );
-
- if ( $params['terms'] ) {
- $terms = $this->filterTerms( $terms, $params['terms'] );
- }
+ $terms = $this->getTermsOfEntities( $pagesToEntityIds,
$languageCode, $params['terms'] );
$termGroups = $this->groupTermsByPageAndType( $entityToPageMap,
$terms );
@@ -104,17 +100,18 @@
/**
* @param EntityID[] $entityIds
* @param string $languageCode
+ * @param string[]|null $termTypes
*
* @return Term[]
*/
- private function getTermsOfEntities( array $entityIds, $languageCode ) {
+ private function getTermsOfEntities( array $entityIds, $languageCode,
array $termTypes = null ) {
$entityIdGroups = $this->splitPageEntityMapByType( $entityIds );
$terms = array();
- foreach ( $entityIdGroups as $type => $entityIds ) {
+ foreach ( $entityIdGroups as $entityType => $entityIds ) {
$terms = array_merge(
$terms,
- $this->termIndex->getTermsOfEntities(
$entityIds, $type, $languageCode )
+ $this->termIndex->getTermsOfEntities(
$entityIds, $entityType, $termTypes, array( $languageCode ) )
);
}
@@ -157,28 +154,6 @@
);
return array_flip( $entityIdsStrings );
- }
-
- /**
- * @param Term[] $terms
- * @param string[] $types
- *
- * @return Term[]
- */
- private function filterTerms( array $terms, array $types = null ) {
- if ( !$types ) {
- return $terms;
- }
-
- $types = array_flip( $types );
-
- return array_filter(
- $terms,
- function( Term $term ) use ( $types ) {
- $key = $term->getType();
- return isset( $types[$key] );
- }
- );
}
/**
diff --git a/client/tests/phpunit/includes/api/PageTermsTest.php
b/client/tests/phpunit/includes/api/PageTermsTest.php
index dc6a52d..487dc2b 100644
--- a/client/tests/phpunit/includes/api/PageTermsTest.php
+++ b/client/tests/phpunit/includes/api/PageTermsTest.php
@@ -127,8 +127,8 @@
$termIndex->expects( $this->any() )
->method( 'getTermsOfEntities' )
->will( $this->returnCallback(
- function ( array $entityIds, $entityType,
$language = null ) use( $termObjectsByEntityId, $this_ ) {
- return $this_->getTermsOfEntities(
$termObjectsByEntityId, $entityIds, $entityType, $language );
+ function ( array $entityIds, $entityType,
$termTypes = null, $languages = null ) use( $termObjectsByEntityId, $this_ ) {
+ return $this_->getTermsOfEntities(
$termObjectsByEntityId, $entityIds, $entityType, $termTypes, $languages );
}
) );
@@ -146,7 +146,7 @@
*
* @return Term[]
*/
- public function getTermsOfEntities( $termObjectsByEntityId, $entityIds,
$entityType, $language = null ) {
+ public function getTermsOfEntities( $termObjectsByEntityId, $entityIds,
$entityType, $termTypes, $languages ) {
$result = array();
foreach ( $entityIds as $id ) {
@@ -158,9 +158,15 @@
/** @var Term $term */
foreach ( $termObjectsByEntityId[$key] as $term ) {
- if ( $term->getLanguage() === $language ) {
- $result[] = $term;
+ if ( $languages !== null && !in_array(
$term->getLanguage(), $languages ) ) {
+ continue;
}
+
+ if ( $termTypes !== null && !in_array(
$term->getType(), $termTypes ) ) {
+ continue;
+ }
+
+ $result[] = $term;
}
}
diff --git a/lib/includes/store/TermIndex.php b/lib/includes/store/TermIndex.php
index 0eff783..4550337 100644
--- a/lib/includes/store/TermIndex.php
+++ b/lib/includes/store/TermIndex.php
@@ -75,11 +75,12 @@
*
* @param EntityId[] $ids
* @param string $entityType
- * @param string|null $languageCode language code
+ * @param string[]|null $termTypes The types of terms to return, e.g.
"label", "description",
+ * or "alias". Compare the Term::TYPE_XXX constants. If null,
all types are returned.
*
* @return Term[]
*/
- public function getTermsOfEntities( array $ids, $entityType,
$languageCode = null );
+ public function getTermsOfEntities( array $ids, $entityType, array
$termTypes = null, array $languageCodes = null );
/**
* Returns if a term with the specified parameters exists.
diff --git a/lib/includes/store/sql/TermSqlIndex.php
b/lib/includes/store/sql/TermSqlIndex.php
index 59657b0..38248d2 100644
--- a/lib/includes/store/sql/TermSqlIndex.php
+++ b/lib/includes/store/sql/TermSqlIndex.php
@@ -373,6 +373,8 @@
* Returns the terms stored for the given entity.
*
* @see TermIndex::getTermsOfEntity
+ * @todo: share more code with getTermsOfEntities. There are only
subtle differences
+ * regarding what fields are loaded.
*
* @param EntityId $entityId
* @param string[]|null $termTypes
@@ -435,24 +437,37 @@
*
* @param EntityId[] $entityIds
* @param string $entityType
- * @param string|null $language Language code
+ * @param string[]|null $termTypes
+ * @param string[]|null $languageCodes Language codes
*
- * @throws MWException
+ * @throws \MWException
* @return Term[]
*/
- public function getTermsOfEntities( array $entityIds, $entityType,
$language = null ) {
- wfProfileIn( __METHOD__ );
-
- if ( empty( $entityIds ) ) {
- wfProfileOut( __METHOD__ );
+ public function getTermsOfEntities( array $entityIds, $entityType,
array $termTypes = null, array $languageCodes = null ) {
+ if ( $entityIds !== null && empty( $entityIds ) ) {
return array();
}
+
+ if ( $languageCodes !== null && empty( $languageCodes ) ) {
+ return array();
+ }
+
+ if ( $termTypes !== null && empty( $termTypes ) ) {
+ return array();
+ }
+
+ wfProfileIn( __METHOD__ );
$entityIdentifiers = array(
'term_entity_type' => $entityType
);
- if ( $language !== null ) {
- $entityIdentifiers['term_language'] = $language;
+
+ if ( $languageCodes !== null ) {
+ $entityIdentifiers['term_language'] = $languageCodes;
+ }
+
+ if ( $termTypes !== null ) {
+ $entityIdentifiers['term_type'] = $termTypes;
}
$numericIds = array();
diff --git a/lib/tests/phpunit/store/MockTermIndex.php
b/lib/tests/phpunit/store/MockTermIndex.php
index 64d5280..c4d3dc8 100644
--- a/lib/tests/phpunit/store/MockTermIndex.php
+++ b/lib/tests/phpunit/store/MockTermIndex.php
@@ -219,8 +219,13 @@
/**
* @throws Exception always
*/
- public function getTermsOfEntities( array $ids, $entityType, $language
= null ) {
- throw new Exception( 'not implemented by mock class ' );
+ public function getTermsOfEntities( array $ids, $entityType, array
$termTypes = null, array $languageCodes = null ) {
+ $terms = array();
+ foreach ( $ids as $id ) {
+ $terms = array_merge( $terms, $this->getTermsOfEntity(
$id, $termTypes, $languageCodes ) );
+ }
+
+ return $terms;
}
/**
diff --git a/lib/tests/phpunit/store/TermIndexTest.php
b/lib/tests/phpunit/store/TermIndexTest.php
index c613b4d..455a1fa 100644
--- a/lib/tests/phpunit/store/TermIndexTest.php
+++ b/lib/tests/phpunit/store/TermIndexTest.php
@@ -584,4 +584,74 @@
"expected to find $k in terms for item" );
}
+ public function testGetTermsOfEntities() {
+ $lookup = $this->getTermIndex();
+
+ $item1 = Item::newEmpty();
+ $item1->setId( 568234314 );
+
+ $item1->setLabel( 'en', 'abc' );
+ $item1->setLabel( 'de', 'def' );
+ $item1->setLabel( 'nl', 'ghi' );
+ $item1->setDescription( 'en', 'one description' );
+ $item1->setAliases( 'fr', array( 'o', '_', 'O' ) );
+
+ $item2 = Item::newEmpty();
+ $item2->setId( 87236423 );
+
+ $item2->setLabel( 'en', 'xyz' );
+ $item2->setLabel( 'de', 'uvw' );
+ $item2->setLabel( 'nl', 'rst' );
+ $item2->setDescription( 'en', 'another description' );
+ $item2->setAliases( 'fr', array( 'X', '~', 'x' ) );
+
+ $this->assertTrue( $lookup->saveTermsOfEntity( $item1 ) );
+ $this->assertTrue( $lookup->saveTermsOfEntity( $item2 ) );
+
+ $ids = array( $item1->getId(), $item2->getId() );
+
+ $labelTerms = $lookup->getTermsOfEntities( $ids,
Item::ENTITY_TYPE, array( Term::TYPE_LABEL ) );
+ $this->assertEquals( 6, count( $labelTerms ), "expected 3
labels" );
+
+ $englishTerms = $lookup->getTermsOfEntities( $ids,
Item::ENTITY_TYPE, null, array( 'en' ) );
+ $this->assertEquals( 4, count( $englishTerms ), "expected 2
English terms" );
+
+ $englishTerms = $lookup->getTermsOfEntities( array(
$item1->getId() ), Item::ENTITY_TYPE, null, array( 'en' ) );
+ $this->assertEquals( 2, count( $englishTerms ), "expected 2
English terms" );
+
+ $germanLabelTerms = $lookup->getTermsOfEntities( $ids,
Item::ENTITY_TYPE, array( Term::TYPE_LABEL ), array( 'de' ) );
+ $this->assertEquals( 2, count( $germanLabelTerms ), "expected 1
German label" );
+
+ $noTerms = $lookup->getTermsOfEntities( $ids,
Item::ENTITY_TYPE, array( Term::TYPE_LABEL ), array() );
+ $this->assertEmpty( $noTerms, "expected no labels" );
+
+ $noTerms = $lookup->getTermsOfEntities( $ids,
Item::ENTITY_TYPE, array(), array( 'de' ) );
+ $this->assertEmpty( $noTerms, "expected no labels" );
+
+ $terms = $lookup->getTermsOfEntities( $ids, Item::ENTITY_TYPE );
+ $this->assertEquals( 14, count( $terms ), "expected 7 terms for
item" );
+
+ // make list of strings for easy checking
+ $term_keys = array();
+ foreach ( $terms as $t ) {
+ $term_keys[] = $t->getType() . '/' . $t->getLanguage()
. '/' . $t->getText();
+ }
+
+ $k = Term::TYPE_LABEL . '/en/abc';
+ $this->assertContains( $k, $term_keys,
+ "expected to find $k in terms for item" );
+
+ $k = Term::TYPE_LABEL . '/en/xyz';
+ $this->assertContains( $k, $term_keys,
+ "expected to find $k in terms for item" );
+
+ $k = Term::TYPE_DESCRIPTION . '/en/another description';
+ $this->assertContains( $k, $term_keys,
+ "expected to find $k in terms for item" );
+
+ $k = Term::TYPE_ALIAS . '/fr/x';
+ $this->assertContains( $k, $term_keys,
+ "expected to find $k in terms for item" );
+ }
+
}
diff --git a/repo/includes/api/SearchEntities.php
b/repo/includes/api/SearchEntities.php
index 2a464d4..904126e 100644
--- a/repo/includes/api/SearchEntities.php
+++ b/repo/includes/api/SearchEntities.php
@@ -220,8 +220,8 @@
}
// Find all the remaining terms for the given entities
- $terms =
WikibaseRepo::getDefaultInstance()->getStore()->getTermIndex()->getTermsOfEntities(
$ids, $entityType,
- $language );
+ $terms =
WikibaseRepo::getDefaultInstance()->getStore()->getTermIndex()->getTermsOfEntities(
+ $ids, $entityType, null, array( $language ) );
// TODO: This needs to be rethought when a different search
engine is used
$aliasPattern = '/^' . preg_quote( $search, '/' ) . '/i';
--
To view, visit https://gerrit.wikimedia.org/r/179527
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie0d92ae917fe6a9ebe112a3a37466c94250334db
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Daniel Kinzler <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
