jenkins-bot has submitted this change and it was merged.
Change subject: Runtime optimizations and more narrow interfaces in TermSqlIndex
......................................................................
Runtime optimizations and more narrow interfaces in TermSqlIndex
This is a small set of trivial, non-controversial refactorings, split
from I0f7e2e3 to make it easier to review.
Key changes:
* Calculate weight not from the fingerprint but from the more narrow
LabelsProvider.
* Do not calculate weight in the loop. This does nothing but wasting time.
* Nothing this class does makes any sense when no ID is set (term table
entries with no ID are useless), so fail as early as possible. Note
that it would fail anyway two lines below when assuming getId returns
an EntityId.
In case you wonder: saveTermsOfEntity is called in EntityHandler,
everything else is class private, even the methods that are declared
public for technical reasons.
Bug: T134735
Change-Id: I6e888ce3bf63660789ebbaac131a954e4850acef
---
M lib/includes/Store/Sql/TermSqlIndex.php
M lib/includes/Store/TermIndex.php
M lib/tests/phpunit/Store/Sql/TermSqlIndexTest.php
3 files changed, 41 insertions(+), 30 deletions(-)
Approvals:
Hoo man: Looks good to me, approved
jenkins-bot: Verified
diff --git a/lib/includes/Store/Sql/TermSqlIndex.php
b/lib/includes/Store/Sql/TermSqlIndex.php
index b831872..fe98528 100644
--- a/lib/includes/Store/Sql/TermSqlIndex.php
+++ b/lib/includes/Store/Sql/TermSqlIndex.php
@@ -10,9 +10,9 @@
use Wikibase\DataModel\Entity\EntityDocument;
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Entity\Item;
-use Wikibase\DataModel\Term\AliasGroup;
use Wikibase\DataModel\Term\Fingerprint;
use Wikibase\DataModel\Term\FingerprintProvider;
+use Wikibase\DataModel\Term\LabelsProvider;
use Wikibase\Lib\Store\LabelConflictFinder;
/**
@@ -86,11 +86,17 @@
*
* @since 0.1
*
- * @param EntityDocument $entity
+ * @param EntityDocument $entity Must have an ID, and optionally any
combination of terms as
+ * declared by the TermIndexEntry::TYPE_... constants.
*
+ * @throws InvalidArgumentException when $entity does not have an ID.
* @return bool Success indicator
*/
public function saveTermsOfEntity( EntityDocument $entity ) {
+ if ( $entity->getId() === null ) {
+ throw new InvalidArgumentException( '$entity must have
an ID' );
+ }
+
//First check whether there's anything to update
$newTerms = $this->getEntityTerms( $entity );
$oldTerms = $this->getTermsOfEntity( $entity->getId() );
@@ -139,7 +145,8 @@
$entityIdentifiers = array(
// FIXME: this will fail for IDs that do not have a
numeric form
'term_entity_id' => $entity->getId()->getNumericId(),
- 'term_entity_type' => $entity->getType()
+ 'term_entity_type' => $entity->getType(),
+ 'term_weight' => $this->getWeight( $entity ),
);
wfDebugLog( __CLASS__, __FUNCTION__ . ': inserting terms for '
. $entity->getId()->getSerialization() );
@@ -150,8 +157,7 @@
$this->tableName,
array_merge(
$this->getTermFields( $term ),
- $entityIdentifiers,
- array( 'term_weight' =>
$this->getWeight( $entity ) )
+ $entityIdentifiers
),
__METHOD__,
array( 'IGNORE' )
@@ -173,19 +179,13 @@
public function getEntityTerms( EntityDocument $entity ) {
// FIXME: OCP violation. No support for new types of entities
can be registered
+ $extraFields = [
+ 'entityType' => $entity->getType(),
+ 'entityId' => $entity->getId()->getNumericId(),
+ ];
+
if ( $entity instanceof FingerprintProvider ) {
$fingerprint = $entity->getFingerprint();
-
- /** @var EntityDocument $entity */
- $extraFields = array(
- 'entityType' => $entity->getType(),
- );
-
- $entityId = $entity->getId();
- if ( $entityId !== null ) {
- $extraFields['entityId'] =
$entityId->getNumericId();
- }
-
return $this->getFingerprintTerms( $fingerprint,
$extraFields );
}
@@ -198,7 +198,7 @@
*
* @return TermIndexEntry[]
*/
- private function getFingerprintTerms( Fingerprint $fingerprint, array
$extraFields = array() ) {
+ private function getFingerprintTerms( Fingerprint $fingerprint, array
$extraFields ) {
$terms = array();
foreach ( $fingerprint->getDescriptions()->toTextArray() as
$languageCode => $description ) {
@@ -221,12 +221,13 @@
$terms[] = $term;
}
- /** @var AliasGroup $aliasGroup */
- foreach ( $fingerprint->getAliasGroups() as $aliasGroup ) {
+ foreach ( $fingerprint->getAliasGroups()->toArray() as
$aliasGroup ) {
+ $languageCode = $aliasGroup->getLanguageCode();
+
foreach ( $aliasGroup->getAliases() as $alias ) {
$term = new TermIndexEntry( $extraFields );
- $term->setLanguage(
$aliasGroup->getLanguageCode() );
+ $term->setLanguage( $languageCode );
$term->setType( TermIndexEntry::TYPE_ALIAS );
$term->setText( $alias );
@@ -296,15 +297,15 @@
*
* @param EntityDocument $entity
*
- * @return float weight
+ * @return float
*/
private function getWeight( EntityDocument $entity ) {
// FIXME: OCP violation. No support for new types of entities
can be registered
$weight = 0.0;
- if ( $entity instanceof FingerprintProvider ) {
- $weight = max( $weight,
$entity->getFingerprint()->getLabels()->count() / 1000.0 );
+ if ( $entity instanceof LabelsProvider ) {
+ $weight = max( $weight, $entity->getLabels()->count() /
1000.0 );
}
if ( $entity instanceof Item ) {
diff --git a/lib/includes/Store/TermIndex.php b/lib/includes/Store/TermIndex.php
index dc97c2c..13161ea 100644
--- a/lib/includes/Store/TermIndex.php
+++ b/lib/includes/Store/TermIndex.php
@@ -2,6 +2,7 @@
namespace Wikibase;
+use InvalidArgumentException;
use Wikibase\DataModel\Entity\EntityDocument;
use Wikibase\DataModel\Entity\EntityId;
@@ -20,8 +21,10 @@
*
* @since 0.1
*
- * @param EntityDocument $entity
+ * @param EntityDocument $entity Must have an ID, and optionally any
combination of terms as
+ * declared by the TermIndexEntry::TYPE_... constants.
*
+ * @throws InvalidArgumentException when $entity does not have an ID.
* @return boolean Success indicator
*/
public function saveTermsOfEntity( EntityDocument $entity );
diff --git a/lib/tests/phpunit/Store/Sql/TermSqlIndexTest.php
b/lib/tests/phpunit/Store/Sql/TermSqlIndexTest.php
index 6f340d8..5425868 100644
--- a/lib/tests/phpunit/Store/Sql/TermSqlIndexTest.php
+++ b/lib/tests/phpunit/Store/Sql/TermSqlIndexTest.php
@@ -252,12 +252,14 @@
}
public function getEntityTermsProvider() {
+ $id = new ItemId( 'Q999' );
+
$fingerprint = new Fingerprint();
$fingerprint->setLabel( 'en', 'kittens!!!:)' );
$fingerprint->setDescription( 'es', 'es un gato!' );
$fingerprint->setAliasGroup( 'en', array( 'kitten-alias' ) );
- $item = new Item( new ItemId( 'Q999' ) );
+ $item = new Item( $id );
$item->setFingerprint( $fingerprint );
$expectedTerms = array(
@@ -284,11 +286,16 @@
) )
);
- return array(
- array( $expectedTerms, $item ),
- array( array(), new Item() ),
- array( array(), $this->getMock( EntityDocument::class )
)
- );
+ $entityWithoutTerms = $this->getMock( EntityDocument::class );
+ $entityWithoutTerms->expects( $this->any() )
+ ->method( 'getId' )
+ ->will( $this->returnValue( $id ) );
+
+ return [
+ [ $expectedTerms, $item ],
+ [ [], new Item( $id ) ],
+ [ [], $entityWithoutTerms ]
+ ];
}
/**
--
To view, visit https://gerrit.wikimedia.org/r/290900
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I6e888ce3bf63660789ebbaac131a954e4850acef
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Thiemo Mättig (WMDE) <[email protected]>
Gerrit-Reviewer: Aude <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Hoo man <[email protected]>
Gerrit-Reviewer: JanZerebecki <[email protected]>
Gerrit-Reviewer: Jonas Kress (WMDE) <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
