Hoo man has uploaded a new change for review. https://gerrit.wikimedia.org/r/181718
Change subject: Add EntityRevisionLookup::getEntityRevisions/getLatestRevisionIds ...................................................................... Add EntityRevisionLookup::getEntityRevisions/getLatestRevisionIds To reduce database load when we need to load multiple entities. That is significant for wbgetentities requests with many ids, json dump generation and possibly other things where we need to access multiple entities at once. Change-Id: I90380428fe9f31677458bb43ceb5cdac24a81ef5 --- M lib/includes/store/CachingEntityRevisionLookup.php M lib/includes/store/EntityRevisionLookup.php M lib/includes/store/sql/WikiPageEntityRevisionLookup.php M lib/tests/phpunit/EntityRevisionLookupTest.php M lib/tests/phpunit/MockRepository.php M repo/includes/api/GetEntities.php 6 files changed, 568 insertions(+), 108 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase refs/changes/18/181718/1 diff --git a/lib/includes/store/CachingEntityRevisionLookup.php b/lib/includes/store/CachingEntityRevisionLookup.php index c4404ba..2553119 100644 --- a/lib/includes/store/CachingEntityRevisionLookup.php +++ b/lib/includes/store/CachingEntityRevisionLookup.php @@ -4,6 +4,7 @@ use BagOStuff; use Wikibase\DataModel\Entity\EntityId; +use InvalidArgumentException; use Wikibase\EntityRevision; /** @@ -13,6 +14,7 @@ * * @licence GNU GPL v2+ * @author Daniel Kinzler + * @author Marius Hoch < [email protected] > */ class CachingEntityRevisionLookup implements EntityRevisionLookup, EntityStoreWatcher { @@ -88,7 +90,8 @@ } /** - * @see EntityLookup::getEntity + * @see EntityLookup::getEntity + * @see EntityRevisionLookup::getEntityRevision * * @note: If this lookup is configured to verify revisions, getLatestRevisionId() * will be called on the underlying lookup to check whether the cached revision is @@ -137,6 +140,80 @@ } /** + * @see EntityLookup::getEntityRevisions + * + * @since 0.5 + * + * @param EntityId[] $entityIds + * @param string $from LATEST_FROM_SLAVE or LATEST_FROM_MASTER. LATEST_FROM_MASTER would + * force the revision to be determined from the canonical master database. + * + * @throws StorageException + * @throws InvalidArgumentException + * @return array entityid -> EntityRevision, EntityRedirect or false (if not found) + */ + public function getEntityRevisions( array $entityIds, $mode = self::LATEST_FROM_SLAVE ) { + $cacheKeys = array(); + + foreach ( $entityIds as $entityId ) { + if ( !$entityId instanceof EntityId ) { + throw new InvalidArgumentException( '$entityIds needs to be an array of EntityIds' ); + } + + $cacheKeys[$entityId->getSerialization()] = $this->getCacheKey( $entityId ); + } + $entityIds = array_combine( array_keys( $cacheKeys ), $entityIds ); + + $cachedEntityRevisions = $this->cache->getMulti( $cacheKeys ); + $entityRevisions = array(); + $latestRevisionsToLoad = array(); + + foreach( $cacheKeys as $id => $key ) { + if ( !isset( $cachedEntityRevisions[$key] ) || !$cachedEntityRevisions[$key] ) { + $entityRevisions[$id] = false; + continue; + } + + $latestRevisionsToLoad[] = $entityIds[$id]; + $entityRevisions[$id] = $cachedEntityRevisions[$key]; + } + + $latestRevisions = array(); + if ( $this->shouldVerifyRevision && $latestRevisionsToLoad ) { + $latestRevisions = $this->lookup->getLatestRevisionIds( $latestRevisionsToLoad, $mode ); + } + + $entityRevisionsToLoad = array(); + + /** @var EntityRevision $entityRevision */ + foreach( $entityRevisions as $id => &$entityRevision ) { + if ( $entityRevision !== false && $this->shouldVerifyRevision ) { + $latestRevision = $latestRevisions[$id]; + + if ( $latestRevision === false ) { + // entity no longer exists! + $entityRevision = null; + } elseif ( $entityRevision && $entityRevision->getRevisionId() !== $latestRevision ) { + $entityRevision = false; + } + } + + if ( $entityRevision === false ) { + $entityRevisionsToLoad[] = $entityIds[$id]; + } + } + + if ( $entityRevisionsToLoad ) { + $entityRevisions = array_merge( + $entityRevisions, + $this->lookup->getEntityRevisions( $entityRevisionsToLoad, $mode ) + ); + } + + return $entityRevisions; + } + + /** * Fetches the EntityRevision and updates the cache accordingly. * * @param EntityId $entityId @@ -176,21 +253,56 @@ * @return int|false */ public function getLatestRevisionId( EntityId $entityId, $mode = self::LATEST_FROM_SLAVE ) { + $result = $this->getLatestRevisionIds( array( $entityId ), $mode ); + return $result[$entityId->getSerialization()]; + } + + /** + * @see EntityRevisionLookup::getLatestRevisionIds + * + * @param EntityId[] $entityIds + * @param string $mode + * + * @throws InvalidArgumentException + * + * @return array + */ + public function getLatestRevisionIds( array $entityIds, $mode = self::LATEST_FROM_SLAVE ) { + $cacheKeys = array(); + + foreach ( $entityIds as $entityId ) { + if ( !$entityId instanceof EntityId ) { + throw new InvalidArgumentException( '$entityIds needs to be an array of EntityIds' ); + } + + $cacheKeys[$entityId->getSerialization()] = $this->getCacheKey( $entityId ); + } + $entityIds = array_combine( array_keys( $cacheKeys ), $entityIds ); // If we do not need to verify the revision, and the revision isn't // needed for an update, we can get the revision from the cached object. // XXX: whether this is actually quicker depends on the cache. if ( ! ( $this->shouldVerifyRevision || $mode === self::LATEST_FROM_MASTER ) ) { - $key = $this->getCacheKey( $entityId ); - /** @var EntityRevision $entityRevision */ - $entityRevision = $this->cache->get( $key ); + $cachedEntityRevisions = $this->cache->getMulti( $cacheKeys ); + $latestRevisions = array(); + $latestRevisionsToLoad = array(); - if ( $entityRevision ) { - return $entityRevision->getRevisionId(); + foreach( $cacheKeys as $id => $key ) { + if ( !isset( $cachedEntityRevisions[$key] ) || !$cachedEntityRevisions[$key] ) { + $latestRevisionsToLoad[] = $entityIds[$id]; + continue; + } else { + $latestRevisions[$id] = $cachedEntityRevisions[$key]->getRevisionId(); + } } + + return array_merge( + $latestRevisions, + $this->lookup->getLatestRevisionIds( $latestRevisionsToLoad, $mode ) + ); } - return $this->lookup->getLatestRevisionId( $entityId, $mode ); + return $this->lookup->getLatestRevisionIds( $entityIds, $mode ); } /** diff --git a/lib/includes/store/EntityRevisionLookup.php b/lib/includes/store/EntityRevisionLookup.php index b43051b..f058f27 100644 --- a/lib/includes/store/EntityRevisionLookup.php +++ b/lib/includes/store/EntityRevisionLookup.php @@ -12,6 +12,7 @@ * * @licence GNU GPL v2+ * @author Daniel Kinzler + * @author Marius Hoch < [email protected] > */ interface EntityRevisionLookup { @@ -51,6 +52,24 @@ public function getEntityRevision( EntityId $entityId, $revisionId = self::LATEST_FROM_SLAVE ); /** + * Returns an array entityid -> EntityRevision, EntityRedirect or false (if not found). + * Please note that this doesn't throw UnresolvedRedirectExceptions but rather returns + * EntityRedirect objects for entities that are infact redirects. + * + * Implementations of this method must not silently resolve redirects. + * + * @since 0.5 + * + * @param EntityId[] $entityIds + * @param string $mode LATEST_FROM_SLAVE or LATEST_FROM_MASTER. LATEST_FROM_MASTER would + * force the revision to be determined from the canonical master database. + * + * @throws StorageException + * @return array entityid -> EntityRevision, EntityRedirect or false (if not found) + */ + public function getEntityRevisions( array $entityIds, $mode = self::LATEST_FROM_SLAVE ); + + /** * Returns the id of the latest revision of the given entity, or false if there is no such entity. * * Implementations of this method must not silently resolve redirects. @@ -63,4 +82,17 @@ */ public function getLatestRevisionId( EntityId $entityId, $mode = self::LATEST_FROM_SLAVE ); + /** + * Returns an array with entityid => the id of the latest revisions of the given entities, or + * false if there is no such entity. + * + * Implementations of this method must not silently resolve redirects. + * + * @param EntityId[] $entityIds + * @param string $mode LATEST_FROM_SLAVE or LATEST_FROM_MASTER. LATEST_FROM_MASTER would force the + * revision to be determined from the canonical master database. + * + * @return array + */ + public function getLatestRevisionIds( array $entityIds, $mode = self::LATEST_FROM_SLAVE ); } diff --git a/lib/includes/store/sql/WikiPageEntityRevisionLookup.php b/lib/includes/store/sql/WikiPageEntityRevisionLookup.php index 98fc076..4055502 100644 --- a/lib/includes/store/sql/WikiPageEntityRevisionLookup.php +++ b/lib/includes/store/sql/WikiPageEntityRevisionLookup.php @@ -3,12 +3,14 @@ namespace Wikibase\Lib\Store; use DBAccessBase; +use DatabaseBase; use DBQueryError; use MWContentSerializationException; use Revision; use Wikibase\DataModel\Entity\EntityId; use Wikibase\DataModel\Entity\EntityIdParser; use Wikibase\EntityRevision; +use Wikibase\Lib\Store\EntityRedirect; /** * Implements an entity repo based on blobs stored in wiki pages on a locally reachable @@ -19,6 +21,7 @@ * * @licence GNU GPL v2+ * @author Daniel Kinzler + * @author Marius Hoch < [email protected] > */ class WikiPageEntityRevisionLookup extends DBAccessBase implements EntityRevisionLookup { @@ -75,7 +78,12 @@ /** @var EntityRevision $entityRevision */ $entityRevision = null; - $row = $this->loadRevisionRow( $entityId, $revisionId ); + if ( is_int( $revisionId ) ) { + $row = $this->loadRevisionInformationById( $entityId, $revisionId ); + } else { + $rows = $this->loadRevisionInformation( array( $entityId ), $revisionId ); + $row = $rows[$entityId->getSerialization()]; + } if ( $row ) { /** @var EntityRedirect $redirect */ @@ -111,6 +119,49 @@ } /** + * Returns an array like entityid -> EntityRevision, EntityRedirect or null (if not found) + * Please note that this doesn't throw UnresolvedRedirectExceptions but rather returns + * EntityRedirect objects for entities that are infact redirects. + * + * @since 0.5 + * @see EntityRevisionLookup::getEntityRevisions + * + * @param EntityId[] $entityIds + * @param string $mode LATEST_FROM_SLAVE or LATEST_FROM_MASTER. LATEST_FROM_MASTER would + * force the revision to be determined from the canonical master database. + * + * @throws InvalidArgumentException + * @throws StorageException + * @return array entityid -> EntityRevision, EntityRedirect or false (if not found) + */ + public function getEntityRevisions( array $entityIds, $mode = self::LATEST_FROM_SLAVE ) { + $entityIds = array_map( array( $this, 'getProperEntityId' ), $entityIds ); + + $rows = $this->loadRevisionInformation( $entityIds, $mode ); + + $entityRevisions = array(); + + foreach ( $rows as $id => $row ) { + if ( !$row ) { + $entityRevisions[$id] = false; + continue; + } + + list( $entityRevision, $redirect ) = $this->loadEntity( $row ); + + if ( $redirect !== null ) { + $entityRevisions[$id] = $redirect; + } elseif( $entityRevision ) { + $entityRevisions[$id] = $entityRevision; + } else { + $entityRevisions[$id] = false; + } + } + + return $entityRevisions; + } + + /** * @see EntityRevisionLookup::getLatestRevisionId * * @since 0.5 @@ -121,43 +172,94 @@ * @return int|false */ public function getLatestRevisionId( EntityId $entityId, $mode = self::LATEST_FROM_SLAVE ) { - $row = null; + $revisions = $this->getLatestRevisionIds( array( $entityId ), $mode ); + return $revisions[$entityId->getSerialization()]; + } + + /** + * @since 0.5 + * @see EntityRevisionLookup::getLatestRevisionIds + * + * @param EntityId[] $entityIds + * @param string $mode LATEST_FROM_SLAVE or LATEST_FROM_MASTER. LATEST_FROM_MASTER would force the + * revision to be determined from the canonical master database. + * + * @throws StorageException + * @return array entityid -> page_latest or false (if not found) + */ + public function getLatestRevisionIds( array $entityIds, $mode = self::LATEST_FROM_SLAVE ) { + $entityIds = array_map( array( $this, 'getProperEntityId' ), $entityIds ); + + $revisions = array(); if ( $mode !== self::LATEST_FROM_MASTER ) { - $row = $this->selectPageLatest( $entityId, DB_SLAVE ); + $revisions = $this->selectPageLatest( $entityIds, DB_SLAVE ); } - if ( !$row ) { - $row = $this->selectPageLatest( $entityId, DB_MASTER ); + $loadFromMaster = array(); + foreach ( $entityIds as $entityId ) { + if ( !isset( $revisions[$entityId->getSerialization()] ) || !is_int( $revisions[$entityId->getSerialization()] ) ) { + $loadFromMaster[] = $entityId; + } } - if ( $row ) { - return (int)$row->page_latest; + if ( $loadFromMaster ) { + $revisions = array_merge( + $revisions, + $this->selectPageLatest( $loadFromMaster, DB_MASTER ) + ); } - return false; + return $revisions; + } + + /** + * @param EntityId[] $entityIds + * @param string $mode + * + * @throws DBQueryError + * @return array entityid -> object or false (if not found) + */ + private function loadRevisionInformation( array $entityIds, $mode ) { + $rows = array(); + + if ( $mode !== self::LATEST_FROM_MASTER ) { + $rows = $this->selectRevisionInformation( $entityIds, DB_SLAVE ); + } + + $loadFromMaster = array(); + foreach ( $entityIds as $entityId ) { + if ( !isset( $rows[$entityId->getSerialization()] ) || !$rows[$entityId->getSerialization()] ) { + $loadFromMaster[] = $entityId; + } + } + + if ( $loadFromMaster ) { + $rows = array_merge( + $rows, + $this->selectRevisionInformation( $loadFromMaster, DB_MASTER ) + ); + } + + return $rows; } /** * @param EntityId $entityId - * @param int|string $revisionId + * @param int $revisionId * * @throws DBQueryError - * @return object|null + * @return object|bool */ - private function loadRevisionRow( EntityId $entityId, $revisionId ) { - $row = null; - - if ( $revisionId !== self::LATEST_FROM_MASTER ) { - $row = $this->selectRevisionRow( $entityId, $revisionId, DB_SLAVE ); - } + private function loadRevisionInformationById( EntityId $entityId, $revisionId ) { + $row = $this->selectRevisionInformationById( $entityId, $revisionId, DB_SLAVE ); if ( !$row ) { - // try loading from master + // Try loading from master wfDebugLog( __CLASS__, __FUNCTION__ . ': try to load ' . $entityId . " with $revisionId from DB_MASTER." ); - $row = $this->selectRevisionRow( $entityId, $revisionId, DB_MASTER ); + $row = $this->selectRevisionInformationById( $entityId, $revisionId, DB_MASTER ); } return $row; @@ -183,51 +285,68 @@ * Selects revision information from the page, revision, and text tables. * * @param EntityId $entityId The entity to query the DB for. - * @param int|string $revisionId The desired revision id, or LATEST_FROM_SLAVE or LATEST_FROM_MASTER. + * @param int $revisionId The desired revision id * @param int $connType DB_SLAVE or DB_MASTER * * @throws DBQueryError If the query fails. - * @return object|null a raw database row object, or null if no such entity revision exists. + * @return object|bool a raw database row object, or false if no such entity revision exists. */ - private function selectRevisionRow( EntityId $entityId, $revisionId, $connType ) { + private function selectRevisionInformationById( EntityId $entityId, $revisionId, $connType ) { wfProfileIn( __METHOD__ ); $db = $this->getConnection( $connType ); - $where = array(); + // pick text via rev_text_id + $join = array( 'text' => array( 'INNER JOIN', 'old_id=rev_text_id' ) ); + + wfDebugLog( __CLASS__, __FUNCTION__ . ": Looking up revision $revisionId of " . $entityId ); + + $row = $db->selectRow( + array( 'revision', 'text' ), + $this->selectFields(), + array( 'rev_id' => $revisionId ), + __METHOD__, + array(), + $join + ); + + $this->releaseConnection( $db ); + + wfProfileOut( __METHOD__ ); + return $row; + } + + /** + * Selects revision information from the page, revision, and text tables. + * Returns an array like entityid -> object or false (if not found) + * + * @param EntityId $entityId The entity to query the DB for. + * @param int $connType DB_SLAVE or DB_MASTER + * + * @throws DBQueryError If the query fails. + * @return array + */ + private function selectRevisionInformation( array $entityIds, $connType ) { + wfProfileIn( __METHOD__ ); + $db = $this->getConnection( $connType ); + $join = array(); + $fields = $this->selectFields(); + // To be able to link rows with entity ids + $fields[] = 'epp_entity_id'; + $fields[] = 'epp_entity_type'; - if ( is_int( $revisionId ) ) { - $tables = array( 'revision', 'text' ); + $tables = array( 'page', 'revision', 'text', 'wb_entity_per_page' ); - // pick revision by id - $where['rev_id'] = $revisionId; + // pick page via epp_page_id + $join['page'] = array( 'INNER JOIN', 'epp_page_id=page_id' ); - // pick text via rev_text_id - $join['text'] = array( 'INNER JOIN', 'old_id=rev_text_id' ); + // pick latest revision via page_latest + $join['revision'] = array( 'INNER JOIN', 'page_latest=rev_id' ); - wfDebugLog( __CLASS__, __FUNCTION__ . ": Looking up revision $revisionId of " . $entityId ); - } else { - $tables = array( 'page', 'revision', 'text', 'wb_entity_per_page' ); + // pick text via rev_text_id + $join['text'] = array( 'INNER JOIN', 'old_id=rev_text_id' ); - $entityId = $this->getProperEntityId( $entityId ); - - // pick entity by id - $where['epp_entity_id'] = $entityId->getNumericId(); - $where['epp_entity_type'] = $entityId->getEntityType(); - - // pick page via epp_page_id - $join['page'] = array( 'INNER JOIN', 'epp_page_id=page_id' ); - - // pick latest revision via page_latest - $join['revision'] = array( 'INNER JOIN', 'page_latest=rev_id' ); - - // pick text via rev_text_id - $join['text'] = array( 'INNER JOIN', 'old_id=rev_text_id' ); - - wfDebugLog( __CLASS__, __FUNCTION__ . ': Looking up latest revision of ' . $entityId ); - } - - $res = $db->select( $tables, $this->selectFields(), $where, __METHOD__, array(), $join ); + $res = $db->select( $tables, $fields, $this->getEppWhere( $entityIds, $db ), __METHOD__, array(), $join ); if ( !$res ) { // this can only happen if the DB is set to ignore errors, which shouldn't be the case... @@ -239,26 +358,37 @@ $this->releaseConnection( $db ); - if ( !( $row = $res->fetchObject() ) ) { - $row = null; + $rows = array(); + foreach ( $entityIds as $entityId ) { + $rows[$entityId->getSerialization()] = false; + + // This sucks, but is the sanest option given that we will only have few rows in here + foreach ( $res as $row ) { + if ( (int)$row->epp_entity_id === $entityId->getNumericId() && + $row->epp_entity_type === $entityId->getEntityType() ) { + $rows[$entityId->getSerialization()] = $row; + break; + } + } } wfProfileOut( __METHOD__ ); - return $row; + return $rows; } /** - * Selects page information from the page table. + * Selects page_latest from the page table for the given entity ids. + * Returns an array like entityid -> page_latest or false (if not found) * * @since 0.4 * - * @param EntityId $entityId The entity to query the DB for. + * @param EntityId[] $entityIds * @param int $connType DB_SLAVE or DB_MASTER * * @throws DBQueryError If the query fails. - * @return object|null a raw database row object, or null if no such entity revision exists. + * @return array */ - protected function selectPageLatest( EntityId $entityId, $connType = DB_SLAVE ) { + private function selectPageLatest( array $entityIds, $connType = DB_SLAVE ) { wfProfileIn( __METHOD__ ); $db = $this->getConnection( $connType ); @@ -267,20 +397,13 @@ 'wb_entity_per_page', ); - $where = array(); $join = array(); - - $entityId = $this->getProperEntityId( $entityId ); - - // pick entity by id - $where['epp_entity_id'] = $entityId->getNumericId(); - $where['epp_entity_type'] = $entityId->getEntityType(); // pick page via epp_page_id $join['page'] = array( 'INNER JOIN', 'epp_page_id=page_id' ); + $fields = array( 'page_latest', 'epp_entity_id', 'epp_entity_type' ); - $res = $db->select( $tables, 'page_latest', $where, __METHOD__, array(), $join ); - + $res = $db->select( $tables, $fields, $this->getEppWhere( $entityIds, $db ), __METHOD__, array(), $join ); if ( !$res ) { // this can only happen if the DB is set to ignore errors, which shouldn't be the case... $error = $db->lastError(); @@ -292,22 +415,51 @@ $this->releaseConnection( $db ); - if ( !( $row = $res->fetchObject() ) ) { - $row = null; + $pageLatest = array(); + foreach ( $entityIds as $entityId ) { + $pageLatest[$entityId->getSerialization()] = false; + + // This sucks, but is the sanest option given that we will only have few rows in here + foreach ( $res as $row ) { + if ( (int)$row->epp_entity_id === $entityId->getNumericId() && + $row->epp_entity_type === $entityId->getEntityType() ) { + $pageLatest[$entityId->getSerialization()] = (int)$row->page_latest; + break; + } + } } wfProfileOut( __METHOD__ ); - return $row; + return $pageLatest; + } + + /** + * @param EntityId[] $entityIds + * @param DatabaseBase $db + * @return string + */ + private function getEppWhere( array $entityIds, DatabaseBase $db ) { + foreach ( $entityIds as &$entityId ) { + // pick entity by id + $where[] = $db->makeList( array( + 'epp_entity_id' => $entityId->getNumericId(), + 'epp_entity_type' => $entityId->getEntityType() + ), LIST_AND ); + } + + return $db->makeList( $where, LIST_OR ); } /** * @todo: migrate table away from using a numeric field & get rid of this function + * @private * * @param EntityId $id * * @return mixed */ - protected function getProperEntityId( EntityId $id ) { + public function getProperEntityId( EntityId $id ) { + // Needs to be public for PHP 5.3 compat return $this->entityIdParser->parse( $id->getSerialization() ); } diff --git a/lib/tests/phpunit/EntityRevisionLookupTest.php b/lib/tests/phpunit/EntityRevisionLookupTest.php index 84f5fc1..8b0a50e 100644 --- a/lib/tests/phpunit/EntityRevisionLookupTest.php +++ b/lib/tests/phpunit/EntityRevisionLookupTest.php @@ -147,6 +147,68 @@ } } + /** + * @dataProvider provideGetEntityRevisions + * + * @param EntityId[] $ids + * @param string $mode LATEST_FROM_SLAVE or LATEST_FROM_MASTER + * @param bool[] $exist + * @param array $redirectTo + */ + public function testGetEntityRevisions( array $ids, $mode, array $exist, array $redirectTo ) { + $lookup = $this->getEntityRevisionLookup(); + $result = $lookup->getEntityRevisions( $ids, $mode ); + + $i = 0; + foreach ( $result as $entityRevision ) { + $this->assertSame( + $exist[$i], + $entityRevision instanceof EntityRevision, + "Failed for " . $ids[$i]->getSerialization() + ); + + if ( $redirectTo[$i] instanceof EntityId ) { + $this->assertSame( + $entityRevision->getTargetId()->getSerialization(), + $redirectTo[$i]->getSerialization() + ); + } + + $i++; + } + } + + public static function provideGetEntityRevisions() { + $cases = array( + array( + array( new ItemId( 'q42' ) ), + EntityRevisionLookup::LATEST_FROM_SLAVE, + array( true ), + array( false ), + ), + array( + array( new ItemId( 'q42' ), new PropertyId( 'p753' ) ), + EntityRevisionLookup::LATEST_FROM_SLAVE, + array( true, true ), + array( false, false ), + ), + array( + array( new ItemId( 'q42' ), new PropertyId( 'p75555553' ) ), + EntityRevisionLookup::LATEST_FROM_SLAVE, + array( true, false ), + array( false, false ), + ), + array( + array( new PropertyId( 'p753' ), new ItemId( 'q23' ) ), + EntityRevisionLookup::LATEST_FROM_SLAVE, + array( true, false ), + array( false, new ItemId( 'q42' ) ), + ), + ); + + return $cases; + } + public function provideGetEntityRevision_redirect() { $redirects = $this->getTestRedirects(); $cases = array(); @@ -189,6 +251,39 @@ return $cases; } + public function provideGetLatestRevisionIds() { + $cases = array( + array( + array( new ItemId( 'q42' ) ), + array( 'Q42' => 12 ), + ), + array( + array( + new PropertyId( 'p753' ), + new ItemId( 'q42' ) + ), + array( + 'P753' => 13, + 'Q42' => 12 + ), + ), + array( + array( + new PropertyId( 'p753' ), + new PropertyId( 'p453904583095843095' ), + new ItemId( 'q42' ) + ), + array( + 'P753' => 13, + 'P453904583095843095' => false, + 'Q42' => 12 + ), + ) + ); + + return $cases; + } + /** * @dataProvider provideGetLatestRevisionId * @@ -208,6 +303,23 @@ $this->assertInstanceOf( 'Wikibase\EntityRevision', $entityRev ); } + /** + * @dataProvider provideGetLatestRevisionIds + * + * @param EntityId[] $id + * @param mixed $expected + */ + public function testGetLatestRevisionIds( array $ids, array $expected ) { + $lookup = $this->getEntityRevisionLookup(); + $result = $lookup->getLatestRevisionIds( $ids ); + + foreach ( $expected as &$foo ) { + $foo = $this->resolveLogicalRevision( $foo ); + } + + $this->assertSame( $expected, $result ); + } + public function testGetLatestRevisionForMissing() { $lookup = $this->getEntityRevisionLookup(); $itemId = new ItemId( 'Q753' ); diff --git a/lib/tests/phpunit/MockRepository.php b/lib/tests/phpunit/MockRepository.php index ffd0a29..34bc024 100644 --- a/lib/tests/phpunit/MockRepository.php +++ b/lib/tests/phpunit/MockRepository.php @@ -156,6 +156,37 @@ } /** + * @since 0.5 + * @see EntityRevisionLookup::getEntityRevisions + * + * @param EntityId[] $entityIds + * @param string $mode LATEST_FROM_SLAVE or LATEST_FROM_MASTER. LATEST_FROM_MASTER would + * force the revision to be determined from the canonical master database. + * + * @throws InvalidArgumentException + * @throws StorageException + * @return array + */ + public function getEntityRevisions( array $entityIds, $mode = self::LATEST_FROM_SLAVE ) { + $entityRevisions = array(); + + foreach ( $entityIds as $entityId ) { + if ( !$entityId instanceof EntityId ) { + throw new InvalidArgumentException( '$entityIds needs to be an array of EntityIds' ); + } + + $key = $entityId->getSerialization(); + try { + $entityRevisions[$key] = $this->getEntityRevision( $entityId, $mode ); + } catch ( UnresolvedRedirectException $e ) { + $entityRevisions[$key] = $this->redirects[$key]; + } + } + + return $entityRevisions; + } + + /** * See EntityLookup::hasEntity() * * @since 0.4 @@ -590,6 +621,37 @@ } /** + * @since 0.5 + * @see EntityRevisionLookup::getLatestRevisionIds + * + * @param EntityId[] $entityIds + * @param string $mode LATEST_FROM_SLAVE or LATEST_FROM_MASTER. LATEST_FROM_MASTER would force the + * revision to be determined from the canonical master database. + * + * @throws InvalidArgumentException + * @throws StorageException + * @return array + */ + public function getLatestRevisionIds( array $entityIds, $mode = self::LATEST_FROM_SLAVE ) { + $revisions = array(); + + foreach ( $entityIds as $entityId ) { + if ( !$entityId instanceof EntityId ) { + throw new InvalidArgumentException( '$entityIds needs to be an array of EntityIds' ); + } + + $revision = $this->getEntityRevision( $entityId, $mode ); + if ( $revision ) { + $revisions[$entityId->getSerialization()] = $revision->getRevisionId(); + } else { + $revisions[$entityId->getSerialization()] = false; + } + } + + return $revisions; + } + + /** * Stores the given Entity. * * @param Entity $entity the entity to save. diff --git a/repo/includes/api/GetEntities.php b/repo/includes/api/GetEntities.php index eb7b1ba..10480af 100644 --- a/repo/includes/api/GetEntities.php +++ b/repo/includes/api/GetEntities.php @@ -4,13 +4,14 @@ use ApiBase; use ApiMain; +use InvalidArgumentException; use Wikibase\DataModel\Entity\EntityId; use Wikibase\DataModel\Entity\EntityIdParsingException; use Wikibase\EntityRevision; use Wikibase\LanguageFallbackChainFactory; use Wikibase\Lib\Serializers\EntitySerializer; use Wikibase\Lib\Serializers\SerializationOptions; -use Wikibase\Lib\Store\UnresolvedRedirectException; +use Wikibase\Lib\Store\EntityRedirect; use Wikibase\Repo\SiteLinkTargetProvider; use Wikibase\Repo\WikibaseRepo; use Wikibase\StringNormalizer; @@ -201,57 +202,46 @@ * @return EntityRevision[] */ protected function getEntityRevisionsFromEntityIds( $entityIds, $resolveRedirects = false ) { - $revisionArray = array(); + $revisionArray = $this->getEntityRevisionLookup()->getEntityRevisions($entityIds); - foreach ( $entityIds as $entityId ) { - $key = $entityId->getSerialization(); - $entityRevision = $this->getEntityRevision( $entityId, $resolveRedirects ); + // Resolve/ throw away eventual EntityRedirects + foreach ( $revisionArray as &$entityRevision ) { + if ( !( $entityRevision instanceof EntityRedirect ) ) { + continue; + } - $revisionArray[$key] = $entityRevision; + $entityRevision = null; + if ( $resolveRedirects ) { + $entityId = $entityRevision->getTargetId(); + $entityRevision = $this->getEntityRevisionLookup()->getEntityRevision( $entityId ); + } } return $revisionArray; } /** - * @param EntityId $entityId - * @param bool $resolveRedirects - * - * @return null|EntityRevision - */ - private function getEntityRevision( EntityId $entityId, $resolveRedirects = false ) { - $entityRevision = null; - - try { - $entityRevision = $this->getEntityRevisionLookup()->getEntityRevision( $entityId ); - } catch ( UnresolvedRedirectException $ex ) { - if ( $resolveRedirects ) { - $entityId = $ex->getRedirectTargetId(); - $entityRevision = $this->getEntityRevision( $entityId, false ); - } - } - - return $entityRevision; - } - - /** * Adds the given EntityRevision to the API result. * + * @throws InvalidArgumentException + * * @param string|null $key - * @param EntityRevision|null $entityRevision + * @param EntityRevision|bool $entityRevision * @param array $params */ - protected function handleEntity( $key, EntityRevision $entityRevision = null, array $params = array() ) { + protected function handleEntity( $key, $entityRevision, array $params = array() ) { wfProfileIn( __METHOD__ ); - if ( $entityRevision === null ) { + if ( $entityRevision === false ) { $this->getResultBuilder()->addMissingEntity( $key, array( 'id' => $key ) ); - } else { + } elseif ( $entityRevision instanceof EntityRevision ) { $props = $this->getPropsFromParams( $params ); $options = $this->getSerializationOptions( $params, $props ); $siteFilterIds = $params['sitefilter']; $this->getResultBuilder()->addEntityRevision( $key, $entityRevision, $options, $props, $siteFilterIds ); + } else { + throw new InvalidArgumentException( '$entityRevision must be either EntityRevision or false' ); } wfProfileOut( __METHOD__ ); -- To view, visit https://gerrit.wikimedia.org/r/181718 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I90380428fe9f31677458bb43ceb5cdac24a81ef5 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Wikibase Gerrit-Branch: master Gerrit-Owner: Hoo man <[email protected]> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
