jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/351641 )
Change subject: Two wb_terms reading modes in TermSqlIndex
......................................................................
Two wb_terms reading modes in TermSqlIndex
Bug: T162673
Change-Id: I958152bc099fd6a68f2fc6ea006417c548d83dd4
---
M client/includes/Store/Sql/DirectSqlStore.php
M data-access/src/RepositoryServiceWiring.php
M lib/includes/Store/Sql/TermSqlIndex.php
M lib/tests/phpunit/Store/Sql/TermSqlIndexTest.php
M repo/includes/Store/Sql/SqlStore.php
5 files changed, 238 insertions(+), 34 deletions(-)
Approvals:
WMDE-leszek: Looks good to me, approved
jenkins-bot: Verified
diff --git a/client/includes/Store/Sql/DirectSqlStore.php
b/client/includes/Store/Sql/DirectSqlStore.php
index b24a9e1..4dcb73d 100644
--- a/client/includes/Store/Sql/DirectSqlStore.php
+++ b/client/includes/Store/Sql/DirectSqlStore.php
@@ -370,6 +370,7 @@
$this->termIndex = new TermSqlIndex(
new StringNormalizer(),
$this->entityIdComposer,
+ $this->entityIdParser,
$this->repoWiki,
'',
$this->writeFullEntityIdColumn
diff --git a/data-access/src/RepositoryServiceWiring.php
b/data-access/src/RepositoryServiceWiring.php
index 5083db6..50b0b8a 100644
--- a/data-access/src/RepositoryServiceWiring.php
+++ b/data-access/src/RepositoryServiceWiring.php
@@ -100,6 +100,7 @@
return new TermSqlIndex(
$client->getStringNormalizer(),
$client->getEntityIdComposer(),
+ $client->getEntityIdParser(),
$services->getDatabaseName(),
$services->getRepositoryName()
);
diff --git a/lib/includes/Store/Sql/TermSqlIndex.php
b/lib/includes/Store/Sql/TermSqlIndex.php
index fd5649d..3b9df4e 100644
--- a/lib/includes/Store/Sql/TermSqlIndex.php
+++ b/lib/includes/Store/Sql/TermSqlIndex.php
@@ -9,6 +9,7 @@
use Wikibase\DataModel\Assert\RepositoryNameAssert;
use Wikibase\DataModel\Entity\EntityDocument;
use Wikibase\DataModel\Entity\EntityId;
+use Wikibase\DataModel\Entity\EntityIdParser;
use Wikibase\DataModel\Entity\Int32EntityId;
use Wikibase\DataModel\Entity\Item;
use Wikibase\DataModel\Term\AliasesProvider;
@@ -50,9 +51,19 @@
private $entityIdComposer;
/**
+ * @var EntityIdParser
+ */
+ private $entityIdParser;
+
+ /**
* @var bool
*/
private $writeFullEntityIdColumn;
+
+ /**
+ * @var bool
+ */
+ private $readFullEntityIdColumn = false;
/**
* @var int
@@ -62,6 +73,7 @@
/**
* @param StringNormalizer $stringNormalizer
* @param EntityIdComposer $entityIdComposer
+ * @param EntityIdParser $entityIdParser
* @param string|bool $wikiDb
* @param string $repositoryName
* @param bool $writeFullEntityIdColumn Allow writing to the column.
@@ -69,6 +81,7 @@
public function __construct(
StringNormalizer $stringNormalizer,
EntityIdComposer $entityIdComposer,
+ EntityIdParser $entityIdParser,
$wikiDb = false,
$repositoryName = '',
$writeFullEntityIdColumn = true
@@ -82,7 +95,7 @@
$this->stringNormalizer = $stringNormalizer;
$this->entityIdComposer = $entityIdComposer;
$this->writeFullEntityIdColumn = $writeFullEntityIdColumn;
-
+ $this->entityIdParser = $entityIdParser;
$this->tableName = 'wb_terms';
}
@@ -312,12 +325,18 @@
$this->assertIsNumericEntityId( $entityId );
/** @var EntityId|Int32EntityId $entityId */
- $entityIdentifiers = array(
- 'term_entity_id' => $entityId->getNumericId(),
- 'term_entity_type' => $entityId->getEntityType()
- );
+ $entityIdentifiers = [];
+ $uniqueKeyFields = [ 'term_language', 'term_type', 'term_text'
];
- $uniqueKeyFields = array( 'term_entity_type', 'term_entity_id',
'term_language', 'term_type', 'term_text' );
+ if ( $this->readFullEntityIdColumn ) {
+ $entityIdentifiers['term_full_entity_id'] =
$entityId->getSerialization();
+ $uniqueKeyFields[] = 'term_full_entity_id';
+ } else {
+ $entityIdentifiers['term_entity_id'] =
$entityId->getNumericId();
+ $entityIdentifiers['term_entity_type'] =
$entityId->getEntityType();
+ $uniqueKeyFields[] = 'term_entity_type';
+ $uniqueKeyFields[] = 'term_entity_id';
+ }
wfDebugLog( __CLASS__, __FUNCTION__ . ': deleting terms for ' .
$entityId->getSerialization() );
@@ -400,12 +419,16 @@
$dbw = $this->getConnection( DB_MASTER );
+ $conditions = [ 'term_entity_type' =>
$entityId->getEntityType() ];
+ if ( $this->readFullEntityIdColumn ) {
+ $conditions['term_full_entity_id'] =
$entityId->getSerialization();
+ } else {
+ $conditions['term_entity_id'] =
$entityId->getNumericId();
+ }
+
$success = $dbw->delete(
$this->tableName,
- array(
- 'term_entity_id' => $entityId->getNumericId(),
- 'term_entity_type' => $entityId->getEntityType()
- ),
+ $conditions,
__METHOD__
);
@@ -496,7 +519,8 @@
}
$entityType = null;
- $numericIds = array();
+ $numericIds = [];
+ $fullIds = [];
foreach ( $entityIds as $id ) {
if ( $entityType === null ) {
@@ -508,12 +532,17 @@
$this->assertIsNumericEntityId( $id );
/** @var Int32EntityId $id */
$numericIds[] = $id->getNumericId();
+ $fullIds[] = $id->getLocalPart();
+
}
- $conditions = array(
- 'term_entity_type' => $entityType,
- 'term_entity_id' => $numericIds,
- );
+ $conditions = [ 'term_entity_type' => $entityType ];
+
+ if ( $this->readFullEntityIdColumn ) {
+ $conditions['term_full_entity_id'] = $fullIds;
+ } else {
+ $conditions['term_entity_id'] = $numericIds;
+ }
if ( $languageCodes !== null ) {
$conditions['term_language'] = $languageCodes;
@@ -523,11 +552,19 @@
$conditions['term_type'] = $termTypes;
}
+ $fields = [ 'term_type', 'term_language', 'term_text' ];
+ if ( $this->readFullEntityIdColumn ) {
+ $fields[] = 'term_full_entity_id';
+ } else {
+ $fields[] = 'term_entity_id';
+ $fields[] = 'term_entity_type';
+ }
+
$dbr = $this->getReadDb();
$res = $dbr->select(
$this->tableName,
- [ 'term_entity_type', 'term_type', 'term_language',
'term_text', 'term_entity_id' ],
+ $fields,
$conditions,
__METHOD__
);
@@ -586,21 +623,19 @@
$queryOptions['LIMIT'] = $options['LIMIT'];
}
+ $fields = [ 'term_entity_type', 'term_type', 'term_language',
'term_text', 'term_weight' ];
+ if ( $this->readFullEntityIdColumn ) {
+ $fields[] = 'term_full_entity_id';
+ } else {
+ $fields[] = 'term_entity_id';
+ }
$rows = $dbr->select(
$this->tableName,
- [
- 'term_entity_type',
- 'term_type',
- 'term_language',
- 'term_text',
- 'term_entity_id',
- 'term_weight'
- ],
+ $fields,
array( $dbr->makeList( $termConditions, LIST_OR ) ),
__METHOD__,
$queryOptions
);
-
if ( array_key_exists( 'orderByWeight', $options ) &&
$options['orderByWeight'] ) {
$rows = $this->getRowsOrderedByWeight( $rows );
}
@@ -676,8 +711,14 @@
$row->term_text .
$row->term_type .
$row->term_language .
- $row->term_entity_type .
- $row->term_entity_id;
+ $row->term_entity_type;
+
+ if ( $this->readFullEntityIdColumn ) {
+ $sortData[$key]['string'] .=
$row->term_full_entity_id;
+ } else {
+ $sortData[$key]['string'] .=
$row->term_entity_id;
+ }
+
$rowMap[$key] = $row;
}
@@ -830,9 +871,13 @@
$termRow->term_entity_type,
$termRow->term_entity_id
);
+ } elseif ( isset( $termRow->term_full_entity_id ) ) {
+ return $this->entityIdParser->parse(
+ $termRow->term_full_entity_id
+ );
+ } else {
+ return null;
}
-
- return null;
}
/**
@@ -929,8 +974,12 @@
$where['L.term_type'] = TermIndexEntry::TYPE_LABEL;
$where['D.term_type'] = TermIndexEntry::TYPE_DESCRIPTION;
- $where[] = 'D.term_entity_id=' . 'L.term_entity_id';
- $where[] = 'D.term_entity_type=' . 'L.term_entity_type';
+ if ( $this->readFullEntityIdColumn ) {
+ $where[] = 'D.term_full_entity_id=' .
'L.term_full_entity_id';
+ } else {
+ $where[] = 'D.term_entity_id=' . 'L.term_entity_id';
+ $where[] = 'D.term_entity_type=' . 'L.term_entity_type';
+ }
$termConditions = array();
@@ -1045,4 +1094,11 @@
return $normalized;
}
+ /**
+ * @param bool $readFullEntityIdColumn
+ */
+ public function setReadFullEntityIdColumn( $readFullEntityIdColumn ) {
+ $this->readFullEntityIdColumn = $readFullEntityIdColumn;
+ }
+
}
diff --git a/lib/tests/phpunit/Store/Sql/TermSqlIndexTest.php
b/lib/tests/phpunit/Store/Sql/TermSqlIndexTest.php
index adf4c7c..f21d6d8 100644
--- a/lib/tests/phpunit/Store/Sql/TermSqlIndexTest.php
+++ b/lib/tests/phpunit/Store/Sql/TermSqlIndexTest.php
@@ -3,10 +3,12 @@
namespace Wikibase\Lib\Tests\Store\Sql;
use MWException;
+use Wikibase\DataModel\Entity\BasicEntityIdParser;
use Wikibase\DataModel\Entity\EntityDocument;
use Wikibase\DataModel\Entity\Item;
use Wikibase\DataModel\Entity\ItemId;
use Wikibase\DataModel\Entity\PropertyId;
+use Wikibase\DataModel\Services\EntityId\PrefixMappingEntityIdParser;
use Wikibase\DataModel\Term\AliasGroupList;
use Wikibase\DataModel\Term\Fingerprint;
use Wikibase\DataModel\Term\Term;
@@ -59,6 +61,7 @@
new TermSqlIndex(
new StringNormalizer(),
new EntityIdComposer( [] ),
+ new BasicEntityIdParser(),
false,
$repositoryName
);
@@ -72,12 +75,13 @@
new StringNormalizer(),
new EntityIdComposer( [
'item' => function( $repositoryName,
$uniquePart ) {
- return new ItemId( 'Q' . $uniquePart );
+ return
ItemId::newFromRepositoryAndNumber( $repositoryName, $uniquePart );
},
'property' => function( $repositoryName,
$uniquePart ) {
- return new PropertyId( 'P' .
$uniquePart );
+ return
PropertyId::newFromRepositoryAndNumber( $repositoryName, $uniquePart );
},
- ] )
+ ] ),
+ new BasicEntityIdParser()
);
}
@@ -122,6 +126,43 @@
}
/**
+ * @dataProvider termProvider
+ */
+ public function testGetMatchingTerms2_withFullEntityId(
+ $languageCode,
+ $termText,
+ $searchText,
+ $matches
+ ) {
+ $termIndex = $this->getTermIndex();
+ $termIndex->clear();
+ $termIndex->setReadFullEntityIdColumn( true );
+
+ $item = new Item( new ItemId( 'Q42' ) );
+ $item->setLabel( $languageCode, $termText );
+
+ $termIndex->saveTermsOfEntity( $item );
+
+ $term = new TermIndexSearchCriteria( [ 'termLanguage' =>
$languageCode, 'termText' => $searchText ] );
+
+ //FIXME: test with arrays for term types and entity types!
+ $obtainedTerms = $termIndex->getMatchingTerms(
+ [ $term ],
+ TermIndexEntry::TYPE_LABEL,
+ Item::ENTITY_TYPE,
+ [ 'caseSensitive' => false ]
+ );
+
+ $this->assertEquals( $matches ? 1 : 0, count( $obtainedTerms )
);
+
+ if ( $matches ) {
+ $obtainedTerm = array_shift( $obtainedTerms );
+
+ $this->assertEquals( $termText,
$obtainedTerm->getText() );
+ }
+ }
+
+ /**
* Returns a fake term index configured for the given repository which
uses the local database.
*
* @param string $repository
@@ -138,6 +179,7 @@
return
PropertyId::newFromRepositoryAndNumber( $repositoryName, $uniquePart );
},
] ),
+ new PrefixMappingEntityIdParser( [ '' => $repository ],
new BasicEntityIdParser() ),
false,
$repository
);
@@ -152,6 +194,28 @@
$localTermIndex->saveTermsOfEntity( $item );
$fooTermIndex = $this->getTermIndexForRepository( 'foo' );
+
+ $results = $fooTermIndex->getMatchingTerms( [ new
TermIndexSearchCriteria( [ 'termText' => 'Foo' ] ) ] );
+
+ $this->assertCount( 1, $results );
+
+ $termIndexEntry = $results[0];
+
+ $this->assertTrue( $termIndexEntry->getEntityId()->equals( new
ItemId( 'foo:Q300' ) ) );
+ $this->assertEquals( 'Foo', $termIndexEntry->getText() );
+ }
+
+ public function
testGivenForeignRepositoryName_getMatchingTermsReturnsEntityIdWithTheRepositoryPrefixFullEntityId()
{
+ $localTermIndex = $this->getTermIndex();
+ $localTermIndex->setReadFullEntityIdColumn( true );
+
+ $item = new Item( new ItemId( 'Q300' ) );
+ $item->setLabel( 'en', 'Foo' );
+
+ $localTermIndex->saveTermsOfEntity( $item );
+
+ $fooTermIndex = $this->getTermIndexForRepository( 'foo' );
+ $fooTermIndex->setReadFullEntityIdColumn( true );
$results = $fooTermIndex->getMatchingTerms( [ new
TermIndexSearchCriteria( [ 'termText' => 'Foo' ] ) ] );
@@ -256,6 +320,44 @@
}
}
+ /**
+ * @dataProvider getMatchingTermsOptionsProvider
+ *
+ * @param Fingerprint $fingerprint
+ * @param TermIndexEntry[] $queryTerms
+ * @param array $options
+ * @param TermIndexEntry[] $expected
+ */
+ public function testGetMatchingTerms_options_withFullEntityId(
+ Fingerprint $fingerprint,
+ array $queryTerms,
+ array $options,
+ array $expected
+ ) {
+ $termIndex = $this->getTermIndex();
+ $termIndex->clear();
+ $termIndex->setReadFullEntityIdColumn( true );
+
+ $item = new Item( new ItemId( 'Q42' ) );
+ $item->setFingerprint( $fingerprint );
+
+ $termIndex->saveTermsOfEntity( $item );
+
+ $actual = $termIndex->getMatchingTerms( $queryTerms, null,
null, $options );
+
+ $this->assertSameSize( $expected, $actual );
+
+ foreach ( $expected as $key => $expectedTerm ) {
+ $this->assertArrayHasKey( $key, $actual );
+ if ( $expectedTerm instanceof TermIndexEntry ) {
+ $actualTerm = $actual[$key];
+ $this->assertEquals(
$expectedTerm->getTermType(), $actualTerm->getTermType(), 'termType' );
+ $this->assertEquals(
$expectedTerm->getLanguage(), $actualTerm->getLanguage(), 'termLanguage' );
+ $this->assertEquals( $expectedTerm->getText(),
$actualTerm->getText(), 'termText' );
+ }
+ }
+ }
+
public function provideGetSearchKey() {
return [
'basic' => [
@@ -314,10 +416,32 @@
}
/**
+ * @dataProvider provideGetSearchKey
+ */
+ public function testGetSearchKey_withFullEntityId( $raw, $normalized ) {
+ $index = $this->getTermIndex();
+ $index->setReadFullEntityIdColumn( true );
+
+ $key = $index->getSearchKey( $raw );
+ $this->assertEquals( $normalized, $key );
+ }
+
+ /**
* @dataProvider getEntityTermsProvider
*/
public function testGetEntityTerms( $expectedTerms, EntityDocument
$entity ) {
$termIndex = $this->getTermIndex();
+ $wikibaseTerms = $termIndex->getEntityTerms( $entity );
+
+ $this->assertEquals( $expectedTerms, $wikibaseTerms );
+ }
+
+ /**
+ * @dataProvider getEntityTermsProvider
+ */
+ public function testGetEntityTerms_withFullEntityId( $expectedTerms,
EntityDocument $entity ) {
+ $termIndex = $this->getTermIndex();
+ $termIndex->setReadFullEntityIdColumn( true );
$wikibaseTerms = $termIndex->getEntityTerms( $entity );
$this->assertEquals( $expectedTerms, $wikibaseTerms );
@@ -451,6 +575,7 @@
return new PropertyId( 'P' .
$uniquePart );
},
] ),
+ new BasicEntityIdParser(),
false,
'',
false
@@ -488,4 +613,24 @@
$this->assertSame( 'Q1112362', $row->term_full_entity_id );
}
+ public function testSaveTermsOfEntity_withReadFullEntityId() {
+ $item = new Item( new ItemId( 'Q1112362' ) );
+ $item->setLabel( 'en', 'kitten-Q1112362' );
+
+ $termIndex = $this->getTermIndex();
+ $termIndex->setReadFullEntityIdColumn( true );
+
+ $result = $termIndex->saveTermsOfEntity( $item );
+ $this->assertTrue( $result );
+
+ $row = $this->db->selectRow(
+ 'wb_terms',
+ [ 'term_entity_id', 'term_entity_type',
'term_full_entity_id' ],
+ [ 'term_entity_id' => '1112362', 'term_entity_type' =>
'item' ],
+ __METHOD__
+ );
+
+ $this->assertSame( 'Q1112362', $row->term_full_entity_id );
+ }
+
}
diff --git a/repo/includes/Store/Sql/SqlStore.php
b/repo/includes/Store/Sql/SqlStore.php
index 45ea480..1ea5956 100644
--- a/repo/includes/Store/Sql/SqlStore.php
+++ b/repo/includes/Store/Sql/SqlStore.php
@@ -254,6 +254,7 @@
return new TermSqlIndex(
$stringNormalizer,
$this->entityIdComposer,
+ $this->entityIdParser,
false,
'',
$this->writeFullEntityIdColumn
--
To view, visit https://gerrit.wikimedia.org/r/351641
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I958152bc099fd6a68f2fc6ea006417c548d83dd4
Gerrit-PatchSet: 10
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Ladsgroup <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Ladsgroup <[email protected]>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <[email protected]>
Gerrit-Reviewer: WMDE-leszek <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
