Jeroen De Dauw has uploaded a new change for review.
https://gerrit.wikimedia.org/r/240723
Change subject: Make WikiPageEntityRedirectLookup adhere to its interfaces
contract
......................................................................
Make WikiPageEntityRedirectLookup adhere to its interfaces contract
Change-Id: I9c1ea664a37dcc2293550dce52154385550bfd01
---
M lib/tests/phpunit/MockRepositoryTest.php
M repo/includes/Interactors/RedirectCreationInteractor.php
M repo/includes/LinkedData/EntityDataRequestHandler.php
M repo/includes/store/sql/WikiPageEntityRedirectLookup.php
M repo/tests/phpunit/includes/store/sql/WikiPageEntityRedirectLookupTest.php
M repo/tests/phpunit/includes/store/sql/WikiPageEntityStoreTest.php
6 files changed, 72 insertions(+), 32 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/23/240723/1
diff --git a/lib/tests/phpunit/MockRepositoryTest.php
b/lib/tests/phpunit/MockRepositoryTest.php
index d6297ba..233fad7 100644
--- a/lib/tests/phpunit/MockRepositoryTest.php
+++ b/lib/tests/phpunit/MockRepositoryTest.php
@@ -729,9 +729,14 @@
$mock->putEntity( new Item( $q5 ) );
$mock->putRedirect( new EntityRedirect( $q55, $q5 ) );
- $this->assertFalse( $mock->getRedirectForEntityId( $q77 ),
'unknown id' );
$this->assertNull( $mock->getRedirectForEntityId( $q5 ), 'not a
redirect' );
$this->assertEquals( $q5, $mock->getRedirectForEntityId( $q55 )
);
+
+ $this->setExpectedException(
+
'Wikibase\DataModel\Services\Lookup\EntityRedirectLookupException',
+ 'unknown id'
+ );
+ $mock->getRedirectForEntityId( $q77 );
}
}
diff --git a/repo/includes/Interactors/RedirectCreationInteractor.php
b/repo/includes/Interactors/RedirectCreationInteractor.php
index df7d710..7418b25 100644
--- a/repo/includes/Interactors/RedirectCreationInteractor.php
+++ b/repo/includes/Interactors/RedirectCreationInteractor.php
@@ -6,6 +6,7 @@
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Entity\EntityRedirect;
use Wikibase\DataModel\Services\Lookup\EntityRedirectLookup;
+use Wikibase\DataModel\Services\Lookup\EntityRedirectLookupException;
use Wikibase\Lib\Store\EntityRevisionLookup;
use Wikibase\Lib\Store\EntityStore;
use Wikibase\Lib\Store\StorageException;
@@ -192,17 +193,21 @@
* @throws RedirectCreationException
*/
private function checkExistsNoRedirect( EntityId $entityId ) {
- $redirect = $this->entityRedirectLookup->getRedirectForEntityId(
- $entityId,
- 'for update'
- );
-
- if ( $redirect === false ) {
- throw new RedirectCreationException(
- "Entity $entityId not found",
- 'no-such-entity'
+ try {
+ $redirect =
$this->entityRedirectLookup->getRedirectForEntityId(
+ $entityId,
+ 'for update'
);
- } elseif ( $redirect !== null ) {
+ }
+ catch ( EntityRedirectLookupException $ex ) {
+ throw new RedirectCreationException(
+ $ex->getMessage(),
+ 'no-such-entity',
+ $ex
+ );
+ }
+
+ if ( $redirect !== null ) {
throw new RedirectCreationException(
"Entity $entityId is a redirect",
'target-is-redirect'
diff --git a/repo/includes/LinkedData/EntityDataRequestHandler.php
b/repo/includes/LinkedData/EntityDataRequestHandler.php
index 4d1eac8..2bbb89d 100644
--- a/repo/includes/LinkedData/EntityDataRequestHandler.php
+++ b/repo/includes/LinkedData/EntityDataRequestHandler.php
@@ -12,6 +12,7 @@
use Wikibase\DataModel\Entity\EntityIdParser;
use Wikibase\DataModel\Entity\EntityIdParsingException;
use Wikibase\DataModel\Services\Lookup\EntityRedirectLookup;
+use Wikibase\DataModel\Services\Lookup\EntityRedirectLookupException;
use Wikibase\EntityRevision;
use Wikibase\Lib\Store\BadRevisionException;
use Wikibase\Lib\Store\EntityRevisionLookup;
@@ -399,7 +400,7 @@
private function getIncomingRedirects( EntityId $id ) {
try {
return $this->entityRedirectLookup->getRedirectIds( $id
);
- } catch ( StorageException $ex ) {
+ } catch ( EntityRedirectLookupException $ex ) {
$prefixedId = $id->getSerialization();
wfDebugLog( __CLASS__, __FUNCTION__ . ": failed to load
incoming redirects of $prefixedId: $ex" );
return array();
diff --git a/repo/includes/store/sql/WikiPageEntityRedirectLookup.php
b/repo/includes/store/sql/WikiPageEntityRedirectLookup.php
index 85c9e6a..5507795 100644
--- a/repo/includes/store/sql/WikiPageEntityRedirectLookup.php
+++ b/repo/includes/store/sql/WikiPageEntityRedirectLookup.php
@@ -6,6 +6,7 @@
use Title;
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Services\Lookup\EntityRedirectLookup;
+use Wikibase\DataModel\Services\Lookup\EntityRedirectLookupException;
use Wikibase\Lib\Store\EntityTitleLookup;
use Wikibase\Store\EntityIdLookup;
@@ -55,11 +56,21 @@
* @param EntityId $targetId
*
* @return EntityId[]
+ * @throws EntityRedirectLookupException
*/
public function getRedirectIds( EntityId $targetId ) {
- $title = $this->entityTitleLookup->getTitleForId( $targetId );
+ try {
+ $title = $this->entityTitleLookup->getTitleForId(
$targetId );
+ } catch ( \Exception $ex ) {
+ // TODO: catch more specific type of exception once
EntityTitleLookup contract is clarified
+ throw new EntityRedirectLookupException( $targetId,
null, $ex );
+ }
- $dbr = $this->loadBalancer->getConnection( DB_SLAVE );
+ try {
+ $dbr = $this->loadBalancer->getConnection( DB_SLAVE );
+ } catch ( \MWException $ex ) {
+ throw new EntityRedirectLookupException( $targetId,
null, $ex );
+ }
$res = $dbr->select(
array( 'page', 'redirect' ),
@@ -75,7 +86,11 @@
)
);
- $this->loadBalancer->reuseConnection( $dbr );
+ try {
+ $this->loadBalancer->reuseConnection( $dbr );
+ } catch ( \MWException $ex ) {
+ throw new EntityRedirectLookupException( $targetId,
null, $ex );
+ }
if ( !$res ) {
return array();
@@ -99,14 +114,25 @@
* @param EntityId $entityId
* @param string $forUpdate
*
- * @return EntityId|null|false The ID of the redirect target, or null
if $entityId
- * does not refer to a redirect, or false if $entityId is not
known.
+ * @return EntityId|null The ID of the redirect target, or null if
$entityId
+ * does not refer to a redirect
+ * @throws EntityRedirectLookupException
*/
public function getRedirectForEntityId( EntityId $entityId, $forUpdate
= '' ) {
- $title = $this->entityTitleLookup->getTitleForId( $entityId );
+ try {
+ $title = $this->entityTitleLookup->getTitleForId(
$entityId );
+ } catch ( \Exception $ex ) {
+ // TODO: catch more specific type of exception once
EntityTitleLookup contract is clarified
+ throw new EntityRedirectLookupException( $entityId,
null, $ex );
+ }
- $forUpdate === 'for update' ? DB_MASTER : DB_SLAVE;
- $db = $this->loadBalancer->getConnection( $forUpdate );
+ $forUpdate = $forUpdate === 'for update' ? DB_MASTER : DB_SLAVE;
+
+ try {
+ $db = $this->loadBalancer->getConnection( $forUpdate );
+ } catch ( \MWException $ex ) {
+ throw new EntityRedirectLookupException( $entityId,
null, $ex );
+ }
$row = $db->selectRow(
array( 'page', 'redirect' ),
@@ -122,7 +148,11 @@
)
);
- $this->loadBalancer->reuseConnection( $db );
+ try {
+ $this->loadBalancer->reuseConnection( $db );
+ } catch ( \MWException $ex ) {
+ throw new EntityRedirectLookupException( $entityId,
null, $ex );
+ }
if ( !$row ) {
return false;
diff --git
a/repo/tests/phpunit/includes/store/sql/WikiPageEntityRedirectLookupTest.php
b/repo/tests/phpunit/includes/store/sql/WikiPageEntityRedirectLookupTest.php
index 585bda0..c4df74d 100644
--- a/repo/tests/phpunit/includes/store/sql/WikiPageEntityRedirectLookupTest.php
+++ b/repo/tests/phpunit/includes/store/sql/WikiPageEntityRedirectLookupTest.php
@@ -72,9 +72,8 @@
}
public function testGetRedirectForEntityId_entityDoesNotExist() {
- $res =
$this->getWikiPageEntityRedirectLookup()->getRedirectForEntityId( new ItemId(
'Q48758903' ) );
-
- $this->assertFalse( $res );
+ $this->setExpectedException(
'Wikibase\DataModel\Services\Lookup\EntityRedirectLookupException' );
+
$this->getWikiPageEntityRedirectLookup()->getRedirectForEntityId( new ItemId(
'Q48758903' ) );
}
public function testGetRedirectForEntityId_entityNotARedirect() {
diff --git a/repo/tests/phpunit/includes/store/sql/WikiPageEntityStoreTest.php
b/repo/tests/phpunit/includes/store/sql/WikiPageEntityStoreTest.php
index e56db07..27d5c5c 100644
--- a/repo/tests/phpunit/includes/store/sql/WikiPageEntityStoreTest.php
+++ b/repo/tests/phpunit/includes/store/sql/WikiPageEntityStoreTest.php
@@ -256,6 +256,14 @@
$this->assertEntityPerPage( true, $oneId );
}
+ private function assertRedirectPerPage( EntityId $expected, EntityId
$entityId ) {
+ $entityRedirectLookup =
WikibaseRepo::getDefaultInstance()->getStore()->getEntityRedirectLookup();
+
+ $targetId = $entityRedirectLookup->getRedirectForEntityId(
$entityId );
+
+ $this->assertEquals( $expected, $targetId );
+ }
+
public function unsupportedRedirectProvider() {
$p1 = new PropertyId( 'P1' );
$p2 = new PropertyId( 'P2' );
@@ -571,14 +579,6 @@
}
return $pageId = (int)$row->epp_page_id;
- }
-
- private function assertRedirectPerPage( EntityId $expected, EntityId
$entityId ) {
- $entityRedirectLookup =
WikibaseRepo::getDefaultInstance()->getStore()->getEntityRedirectLookup();
-
- $targetId = $entityRedirectLookup->getRedirectForEntityId(
$entityId );
-
- $this->assertEquals( $expected, $targetId );
}
}
--
To view, visit https://gerrit.wikimedia.org/r/240723
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9c1ea664a37dcc2293550dce52154385550bfd01
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Jeroen De Dauw <jeroended...@gmail.com>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
