Replace deprecated ApiPageSet::getGoodTitles

Remove Title objects from the data provider and use PageReferences

Bug: T339384
Change-Id: I3ff14424c5caa9e4436dfe62052a6c95d30ac89f
This commit is contained in:
Umherirrender 2024-03-18 22:54:02 +01:00
parent 5817881b6d
commit beef358191
2 changed files with 46 additions and 30 deletions

View file

@ -5,7 +5,8 @@ namespace PageImages;
use ApiBase;
use ApiQuery;
use ApiQueryBase;
use MediaWiki\Title\Title;
use MediaWiki\Page\PageReference;
use MediaWiki\Page\PageReferenceValue;
use RepoGroup;
use Wikimedia\ParamValidator\ParamValidator;
use Wikimedia\ParamValidator\TypeDef\IntegerDef;
@ -43,17 +44,17 @@ class ApiQueryPageImages extends ApiQueryBase {
* Gets the set of titles to get page images for.
*
* Note well that the set of titles comprises the set of "good" titles
* (see {@see ApiPageSet::getGoodTitles}) union the set of "missing"
* (see {@see ApiPageSet::getGoodPages}) union the set of "missing"
* titles in the File namespace that might correspond to foreign files.
* The latter are included because titles in the File namespace are
* expected to be found with {@see \RepoGroup::findFile}.
*
* @return Title[] A map of page ID, which will be negative in the case
* of missing titles in the File namespace, to Title object
* @return PageReference[] A map of page ID, which will be negative in the case
* of missing titles in the File namespace, to PageReference object
*/
protected function getTitles() {
$pageSet = $this->getPageSet();
$titles = $pageSet->getGoodTitles();
$titles = $pageSet->getGoodPages();
// T98791: We want foreign files to be treated like local files
// in #execute, so include the set of missing filespace pages,
@ -65,7 +66,7 @@ class ApiQueryPageImages extends ApiQueryBase {
// whereas $missingFileTitles is a map of title text to ID.
// Do not use array_merge here as it doesn't preserve keys.
foreach ( $missingFileTitles as $dbkey => $id ) {
$titles[$id] = Title::makeTitle( NS_FILE, $dbkey );
$titles[$id] = PageReferenceValue::localReference( NS_FILE, $dbkey );
}
return $titles;
@ -114,7 +115,7 @@ class ApiQueryPageImages extends ApiQueryBase {
// Find any titles in the file namespace so we can handle those separately
$filePageTitles = [];
foreach ( $titles as $id => $title ) {
if ( $title->inNamespace( NS_FILE ) ) {
if ( $title->getNamespace() === NS_FILE ) {
$filePageTitles[$id] = $title;
unset( $titles[$id] );
}

View file

@ -3,11 +3,10 @@
namespace PageImages\Tests;
use MediaWiki\Config\HashConfig;
use MediaWiki\MediaWikiServices;
use MediaWiki\Title\Title;
use MediaWiki\Page\PageReferenceValue;
use MediaWikiIntegrationTestCase;
use PageImages\ApiQueryPageImages;
use PageImages\PageImages;
use PHPUnit\Framework\TestCase;
use Wikimedia\ParamValidator\ParamValidator;
use Wikimedia\ParamValidator\TypeDef\IntegerDef;
use Wikimedia\Rdbms\FakeResultWrapper;
@ -22,7 +21,7 @@ use Wikimedia\TestingAccessWrapper;
* @author Sam Smith
* @author Thiemo Kreuz
*/
class ApiQueryPageImagesTest extends TestCase {
class ApiQueryPageImagesTest extends MediaWikiIntegrationTestCase {
private function newInstance() {
$config = new HashConfig( [
@ -44,7 +43,7 @@ class ApiQueryPageImagesTest extends TestCase {
->method( 'getMain' )
->willReturn( $main );
return new ApiQueryPageImages( $query, '', MediaWikiServices::getInstance()->getRepoGroup() );
return new ApiQueryPageImages( $query, '', $this->getServiceContainer()->getRepoGroup() );
}
public function testConstructor() {
@ -79,9 +78,21 @@ class ApiQueryPageImagesTest extends TestCase {
/**
* @dataProvider provideGetTitles
*/
public function testGetTitles( $titles, $missingTitlesByNamespace, $expected ) {
public function testGetTitles( $pageNames, $missingTitlesByNamespace, $expectedNames ) {
$titleParser = $this->getServiceContainer()->getTitleParser();
$titles = [];
foreach ( $pageNames as $pageName ) {
$titleValue = $titleParser->parseTitle( $pageName );
$titles[] = PageReferenceValue::localReference( $titleValue->getNamespace(), $titleValue->getDBkey() );
}
$expected = [];
foreach ( $expectedNames as $id => $expectedName ) {
$titleValue = $titleParser->parseTitle( $expectedName );
$expected[$id] = PageReferenceValue::localReference( $titleValue->getNamespace(), $titleValue->getDBkey() );
}
$pageSet = $this->createMock( \ApiPageSet::class );
$pageSet->method( 'getGoodTitles' )
$pageSet->method( 'getGoodPages' )
->willReturn( $titles );
$pageSet->method( 'getMissingTitlesByNamespace' )
->willReturn( $missingTitlesByNamespace );
@ -93,29 +104,29 @@ class ApiQueryPageImagesTest extends TestCase {
public static function provideGetTitles() {
return [
[
[ Title::makeTitle( NS_MAIN, 'Foo' ) ],
[ 'Foo' ],
[],
[ Title::makeTitle( NS_MAIN, 'Foo' ) ],
[ 'Foo' ],
],
[
[ Title::makeTitle( NS_MAIN, 'Foo' ) ],
[ 'Foo' ],
[
NS_TALK => [
'Bar' => -1,
],
],
[ Title::makeTitle( NS_MAIN, 'Foo' ) ],
[ 'Foo' ],
],
[
[ Title::makeTitle( NS_MAIN, 'Foo' ) ],
[ 'Foo' ],
[
NS_FILE => [
'Bar' => -1,
],
],
[
0 => Title::makeTitle( NS_MAIN, 'Foo' ),
-1 => Title::makeTitle( NS_FILE, 'Bar' ),
0 => 'Foo',
-1 => 'File:Bar',
],
],
];
@ -124,14 +135,18 @@ class ApiQueryPageImagesTest extends TestCase {
/**
* @dataProvider provideExecute
* @param array $requestParams Request parameters to the API
* @param array $titles Page titles passed to the API
* @param array $pageNames Page titles passed to the API
* @param array $queryPageIds Page IDs that will be used for querying the DB.
* @param array $queryResults Results of the DB select query
* @param int $setResultValueCount The number results the API returned
*/
public function testExecute( $requestParams, $titles, $queryPageIds,
public function testExecute( $requestParams, $pageNames, $queryPageIds,
$queryResults, $setResultValueCount
) {
$titles = [];
foreach ( $pageNames as $pageName ) {
$titles[] = PageReferenceValue::localReference( NS_MAIN, $pageName );
}
$mock = TestingAccessWrapper::newFromObject(
$this->getMockBuilder( ApiQueryPageImages::class )
->disableOriginalConstructor()
@ -178,7 +193,7 @@ class ApiQueryPageImagesTest extends TestCase {
[
[ 'prop' => [ 'thumbnail' ], 'thumbsize' => 100, 'limit' => 10,
'license' => 'any', 'langcode' => null ],
[ Title::newFromText( 'Page 1' ), Title::newFromText( 'Page 2' ) ],
[ 'Page 1', 'Page 2' ],
[ 0, 1 ],
[
(object)[ 'pp_page' => 0, 'pp_value' => 'A_Free.jpg',
@ -200,7 +215,7 @@ class ApiQueryPageImagesTest extends TestCase {
[
[ 'prop' => [ 'thumbnail' ], 'continue' => 1, 'thumbsize' => 400,
'limit' => 10, 'license' => 'any', 'langcode' => null ],
[ Title::newFromText( 'Page 1' ), Title::newFromText( 'Page 2' ) ],
[ 'Page 1', 'Page 2' ],
[ 1 ],
[
(object)[ 'pp_page' => 1, 'pp_value' => 'B_Free.jpg',
@ -213,7 +228,7 @@ class ApiQueryPageImagesTest extends TestCase {
[
[ 'prop' => [ 'thumbnail' ], 'thumbsize' => 500, 'limit' => 10,
'license' => 'any', 'langcode' => 'en' ],
[ Title::newFromText( 'Page 1' ), Title::newFromText( 'Page 2' ) ],
[ 'Page 1', 'Page 2' ],
[ 0, 1 ],
[
(object)[ 'pp_page' => 1, 'pp_value' => 'B_Free.jpg',
@ -224,7 +239,7 @@ class ApiQueryPageImagesTest extends TestCase {
[
[ 'prop' => [ 'thumbnail' ], 'continue' => 1, 'thumbsize' => 500,
'limit' => 10, 'license' => 'any', 'langcode' => 'de' ],
[ Title::newFromText( 'Page 1' ), Title::newFromText( 'Page 2' ) ],
[ 'Page 1', 'Page 2' ],
[ 1 ],
[
(object)[ 'pp_page' => 1, 'pp_value' => 'B_Free.jpg',
@ -235,7 +250,7 @@ class ApiQueryPageImagesTest extends TestCase {
[
[ 'prop' => [ 'thumbnail' ], 'thumbsize' => 510, 'limit' => 10,
'license' => 'free', 'langcode' => 'de' ],
[ Title::newFromText( 'Page 1' ), Title::newFromText( 'Page 2' ) ],
[ 'Page 1', 'Page 2' ],
[ 0, 1 ],
[],
0
@ -243,7 +258,7 @@ class ApiQueryPageImagesTest extends TestCase {
[
[ 'prop' => [ 'thumbnail' ], 'thumbsize' => 510, 'limit' => 10,
'license' => 'free', 'langcode' => 'en' ],
[ Title::newFromText( 'Page 1' ), Title::newFromText( 'Page 2' ) ],
[ 'Page 1', 'Page 2' ],
[ 0, 1 ],
[
(object)[ 'pp_page' => 0, 'pp_value' => 'A_Free.jpg',
@ -256,7 +271,7 @@ class ApiQueryPageImagesTest extends TestCase {
[
[ 'prop' => [ 'thumbnail', 'original' ], 'thumbsize' => 510,
'limit' => 10, 'license' => 'free', 'langcode' => 'en' ],
[ Title::newFromText( 'Page 1' ), Title::newFromText( 'Page 2' ) ],
[ 'Page 1', 'Page 2' ],
[ 0, 1 ],
[
(object)[