MaxSem has uploaded a new change for review. https://gerrit.wikimedia.org/r/324826
Change subject: Revert "Allow querying non-free images too" ...................................................................... Revert "Allow querying non-free images too" Most images are gone. This reverts commit 382c70f981831c16ba11a5dd5e9679368fd5282a. Bug: T152155 Change-Id: I3fefa06103481f48dbd62b9a1b7658301a341be9 --- M extension.json M i18n/en.json M i18n/qqq.json M includes/ApiQueryPageImages.php M includes/LinksUpdateHookHandler.php M includes/PageImages.php M tests/phpunit/ApiQueryPageImagesTest.php M tests/phpunit/LinksUpdateHookHandlerTest.php M tests/phpunit/PageImagesTest.php 9 files changed, 98 insertions(+), 315 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/PageImages refs/changes/26/324826/1 diff --git a/extension.json b/extension.json index 4e74d66..7c9bc0a 100644 --- a/extension.json +++ b/extension.json @@ -56,6 +56,10 @@ "20": 5, "30": 0, "31": -100 + }, + "rights": { + "@doc": "don't show nonfree images", + "nonfree": -100 } } }, diff --git a/i18n/en.json b/i18n/en.json index a0b48e8..5ce3e6f 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -9,8 +9,5 @@ "apihelp-query+pageimages-example-1": "Get name and <kbd>100</kbd>-pixel thumbnail of an image on the <kbd>Albert Einstein</kbd> page.", "apihelp-query+pageimages-param-prop": "Which information to return:\n;thumbnail:URL and dimensions of image associated with page, if any.\n;name:Image title.", "apihelp-query+pageimages-param-thumbsize": "Maximum thumbnail dimension.", - "apihelp-query+pageimages-param-limit": "Properties of how many pages to return.", - "apihelp-query+pageimages-param-license": "Limit page images to a certain license type", - "apihelp-query+pageimages-paramvalue-license-free": "only free images", - "apihelp-query+pageimages-paramvalue-license-any": "best image, whether free or non-free." + "apihelp-query+pageimages-param-limit": "Properties of how many pages to return." } diff --git a/i18n/qqq.json b/i18n/qqq.json index 4aa4243..05a7c28 100644 --- a/i18n/qqq.json +++ b/i18n/qqq.json @@ -12,8 +12,5 @@ "apihelp-query+pageimages-example-1": "{{doc-apihelp-example|query+pageimages}}", "apihelp-query+pageimages-param-prop": "{{doc-apihelp-param|query+pageimages|prop}}", "apihelp-query+pageimages-param-thumbsize": "{{doc-apihelp-param|query+pageimages|thumbsize}}", - "apihelp-query+pageimages-param-limit": "{{doc-apihelp-param|query+pageimages|limit}}", - "apihelp-query+pageimages-param-license": "{{doc-apihelp-param|query+pageimages|license}}", - "apihelp-query+pageimages-paramvalue-license-free": "{{doc-apihelp-paramvalue|query+pageimages|license|free}}", - "apihelp-query+pageimages-paramvalue-license-any": "{{doc-apihelp-paramvalue|query+pageimages|license|any}}" + "apihelp-query+pageimages-param-limit": "{{doc-apihelp-param|query+pageimages|limit}}" } diff --git a/includes/ApiQueryPageImages.php b/includes/ApiQueryPageImages.php index 0f6437a..098b765 100644 --- a/includes/ApiQueryPageImages.php +++ b/includes/ApiQueryPageImages.php @@ -14,16 +14,6 @@ class ApiQueryPageImages extends ApiQueryBase { /** - * @const API parameter value for free images - */ - const PARAM_LICENSE_FREE = 'free'; - - /** - * @const API parameter value for images with any type of license - */ - const PARAM_LICENSE_ANY = 'any'; - - /** * @param ApiQuery $query * @param string $moduleName */ @@ -76,6 +66,8 @@ if ( !count( $prop ) ) { $this->dieUsage( 'No properties selected', '_noprop' ); } + $size = $params['thumbsize']; + $limit = $params['limit']; $allTitles = $this->getTitles(); @@ -96,7 +88,6 @@ } } - $limit = $params['limit']; // Slice the part of the array we want to find images for $titles = array_slice( $allTitles, $offset, $limit, true ); @@ -115,30 +106,19 @@ } } - $size = $params['thumbsize']; // Extract page images from the page_props table if ( count( $titles ) > 0 ) { $this->addTables( 'page_props' ); $this->addFields( array( 'pp_page', 'pp_propname', 'pp_value' ) ); - $this->addWhere( array( 'pp_page' => array_keys( $titles ), - 'pp_propname' => self::getPropNames( $params['license'] ) ) ); + $this->addWhere( array( 'pp_page' => array_keys( $titles ), 'pp_propname' => PageImages::PROP_NAME ) ); $res = $this->select( __METHOD__ ); - $buffer = []; - $propNameAny = PageImages::getPropName( false ); foreach ( $res as $row ) { $pageId = $row->pp_page; - if ( !array_key_exists( $pageId, $buffer ) || $row->pp_propname === $propNameAny ) { - $buffer[$pageId] = $row; - } - } - - foreach ( $buffer as $pageId => $row ) { $fileName = $row->pp_value; $this->setResultValues( $prop, $pageId, $fileName, $size ); } - } // End page props image extraction // Extract images from file namespace pages. In this case we just use @@ -147,23 +127,6 @@ $fileName = $title->getDBkey(); $this->setResultValues( $prop, $pageId, $fileName, $size ); } - } - - /** - * Get property names used in page_props table - * - * If the license is free, then only the free property name will be returned, - * otherwise both free and non-free property names will be returned. That's - * because we save the image name only once if it's free and the best image. - * - * @param string $license - * @return string|array - */ - protected static function getPropNames( $license ) { - if ( $license === self::PARAM_LICENSE_FREE ) { - return PageImages::getPropName( true ); - } - return [ PageImages::getPropName( true ), PageImages::getPropName( false ) ]; } public function getCacheMode( $params ) { @@ -234,7 +197,7 @@ ), 'thumbsize' => array( ApiBase::PARAM_TYPE => 'integer', - ApiBase::PARAM_DFLT => 50, + APiBase::PARAM_DFLT => 50, ), 'limit' => array( ApiBase::PARAM_DFLT => 1, @@ -243,11 +206,6 @@ ApiBase::PARAM_MAX => 50, ApiBase::PARAM_MAX2 => 100, ), - 'license' => [ - ApiBase::PARAM_TYPE => [ self::PARAM_LICENSE_FREE, self::PARAM_LICENSE_ANY ], - ApiBase::PARAM_ISMULTI => false, - ApiBase::PARAM_DFLT => self::PARAM_LICENSE_FREE, - ], 'continue' => array( ApiBase::PARAM_TYPE => 'integer', /** @todo Once support for MediaWiki < 1.25 is dropped, just use ApiBase::PARAM_HELP_MSG directly */ diff --git a/includes/LinksUpdateHookHandler.php b/includes/LinksUpdateHookHandler.php index d63e879..4e1bdfc 100644 --- a/includes/LinksUpdateHookHandler.php +++ b/includes/LinksUpdateHookHandler.php @@ -56,26 +56,15 @@ } $image = false; - $free_image = false; foreach ( $scores as $name => $score ) { - if ( $score > 0 ) { - if ( !$image || $score > $scores[$image] ) { - $image = $name; - } - if ( ( !$free_image || $score > $scores[$free_image] ) && $this->isImageFree( $name ) ) { - $free_image = $name; - } + if ( $score > 0 && ( !$image || $score > $scores[$image] ) ) { + $image = $name; } } - if ( $free_image ) { - $linksUpdate->mProperties[PageImages::getPropName( true )] = $free_image; - } - - // Only store the image if it's not free. Free image (if any) has already been stored above. - if ( $image && $image !== $free_image ) { - $linksUpdate->mProperties[PageImages::getPropName( false )] = $image; + if ( $image ) { + $linksUpdate->mProperties[PageImages::PROP_NAME] = $image; } } @@ -91,6 +80,11 @@ protected function getScore( array $image, $position ) { global $wgPageImagesScores; + $file = wfFindFile( $image['filename'] ); + if ( $file ) { + $image += $this->getMetadata( $file ); + } + if ( isset( $image['handler'] ) ) { // Standalone image $score = $this->scoreFromTable( $image['handler']['width'], $wgPageImagesScores['width'] ); @@ -105,6 +99,10 @@ $ratio = intval( $this->getRatio( $image ) * 10 ); $score += $this->scoreFromTable( $ratio, $wgPageImagesScores['ratio'] ); + + if ( isset( $image['rights'] ) && isset( $wgPageImagesScores['rights'][$image['rights']] ) ) { + $score += $wgPageImagesScores['rights'][$image['rights']]; + } $blacklist = $this->getBlacklist(); if ( isset( $blacklist[$image['filename']] ) ) { @@ -137,34 +135,27 @@ } /** - * Check whether image's copyright allows it to be used freely. - * - * @param string $fileName Name of the image file - * @return bool - */ - protected function isImageFree( $fileName ) { - $file = wfFindFile( $fileName ); - if ( $file ) { - // Process copyright metadata from CommonsMetadata, if present. - // Image is considered free if the value is '0' or unset. - return empty( $this->fetchFileMetadata( $file )['NonFree']['value'] ); - } - return true; - } - - /** - * Fetch file metadata + * Return some file metadata (only what's relevant to page image scores). * * @param File $file - * @return array + * + * @return string[] */ - protected function fetchFileMetadata( $file ) { + protected function getMetadata( File $file ) { $format = new FormatMetadata; $context = new DerivativeContext( $format->getContext() ); $format->setSingleLanguage( true ); // we don't care and it's slightly faster $context->setLanguage( 'en' ); // we don't care so avoid splitting the cache $format->setContext( $context ); - return $format->fetchExtendedMetadata( $file ); + $extmetadata = $format->fetchExtendedMetadata( $file ); + $processedMetadata = array(); + + // process copyright metadata from CommonsMetadata, if present + if ( !empty( $extmetadata['NonFree']['value'] ) ) { // not '0' or unset + $processedMetadata['rights'] = 'nonfree'; + } + + return $processedMetadata; } /** diff --git a/includes/PageImages.php b/includes/PageImages.php index d84df6b..f652d6a 100644 --- a/includes/PageImages.php +++ b/includes/PageImages.php @@ -9,26 +9,9 @@ class PageImages { /** - * Page property used to store the best page image information. - * If the best image is the same as the best image with free license, - * then nothing is stored under this property. - * @see PageImages::PROP_NAME_FREE + * Page property used to store the page image information */ const PROP_NAME = 'page_image'; - /** - * Page property used to store the best free page image information - */ - const PROP_NAME_FREE = 'page_image_free'; - - /** - * Get property name used in page_props table - * - * @param bool $isFree Whether the image is a free-license image - * @return string - */ - public static function getPropName( $isFree ) { - return $isFree ? self::PROP_NAME_FREE : self::PROP_NAME; - } /** * Returns page image for a given title diff --git a/tests/phpunit/ApiQueryPageImagesTest.php b/tests/phpunit/ApiQueryPageImagesTest.php index 9eab2fb..29daca3 100644 --- a/tests/phpunit/ApiQueryPageImagesTest.php +++ b/tests/phpunit/ApiQueryPageImagesTest.php @@ -7,7 +7,6 @@ use FakeResultWrapper; use PageImages; use PHPUnit_Framework_TestCase; -use TestingAccessWrapper; use Title; class ApiPageSetStub extends ApiPageSet { @@ -41,10 +40,6 @@ return parent::getTitles(); } - /** inheritdoc */ - public static function getPropNames( $license ) { - return parent::getPropNames( $license ); - } } /** @@ -103,10 +98,6 @@ $this->assertInternalType( 'array', $params ); $this->assertNotEmpty( $params ); $this->assertContainsOnly( 'array', $params ); - $this->assertArrayHasKey( 'license', $params ); - $this->assertEquals( $params['license'][\ApiBase::PARAM_TYPE], ['free', 'any'] ); - $this->assertEquals( $params['license'][\ApiBase::PARAM_DFLT], 'free' ); - $this->assertEquals( $params['license'][\ApiBase::PARAM_ISMULTI], false ); } public function testGetParamDescription() { @@ -166,13 +157,11 @@ * @param int $setResultValueCount The number results the API returned */ public function testExecute( $requestParams, $titles, $queryPageIds, $queryResults, $setResultValueCount ) { - $mock = TestingAccessWrapper::newFromObject( - $this->getMockBuilder( ApiQueryPageImages::class ) - ->disableOriginalConstructor() - -> setMethods( ['extractRequestParams', 'getTitles', 'setContinueParameter', 'dieUsage', - 'addTables', 'addFields', 'addWhere', 'select', 'setResultValues'] ) - ->getMock() - ); + $mock = $this->getMockBuilder( ApiQueryPageImages::class ) + ->disableOriginalConstructor() + -> setMethods( ['extractRequestParams', 'getTitles', 'setContinueParameter', 'dieUsage', + 'addTables', 'addFields', 'addWhere', 'select', 'setResultValues'] ) + ->getMock(); $mock->expects( $this->any() ) ->method( 'extractRequestParams' ) ->willReturn( $requestParams ); @@ -189,15 +178,9 @@ ->method( 'dieUsage' ); } - $license = isset( $requestParams['license'] ) ? $requestParams['license'] : 'free'; - if ( $license == ApiQueryPageImages::PARAM_LICENSE_ANY ) { - $propName = [PageImages::getPropName( true ), PageImages::getPropName( false )]; - } else { - $propName = PageImages::getPropName( true ); - } $mock->expects( $this->exactly( count ( $queryPageIds ) > 0 ? 1 : 0 ) ) ->method( 'addWhere' ) - ->with( ['pp_page' => $queryPageIds, 'pp_propname' => $propName] ); + ->with( ['pp_page' => $queryPageIds, 'pp_propname' => PageImages::PROP_NAME] ); $mock->expects( $this->exactly( $setResultValueCount ) ) ->method( 'setResultValues' ); @@ -208,13 +191,12 @@ public function provideExecute() { return [ [ - ['prop' => ['thumbnail'], 'thumbsize' => 100, 'limit' => 10, 'license' => 'any'], + ['prop' => ['thumbnail'], 'thumbsize' => 100, 'limit' => 10], [Title::newFromText( 'Page 1' ), Title::newFromText( 'Page 2' )], [0, 1], [ - (object) ['pp_page' => 0, 'pp_value' => 'A_Free.jpg', 'pp_propname' => PageImages::PROP_NAME_FREE], - (object) ['pp_page' => 0, 'pp_value' => 'A.jpg', 'pp_propname' => PageImages::PROP_NAME], - (object) ['pp_page' => 1, 'pp_value' => 'B.jpg', 'pp_propname' => PageImages::PROP_NAME], + (object) ['pp_page' => 0, 'pp_value' => 'A.jpg'], + (object) ['pp_page' => 1, 'pp_value' => 'B.jpg'], ], 2 ], @@ -226,67 +208,14 @@ 0 ], [ - ['prop' => ['thumbnail'], 'continue' => 1, 'thumbsize' => 400, 'limit' => 10, 'license' => 'any'], + ['prop' => ['thumbnail'], 'continue' => 1, 'thumbsize' => 400, 'limit' => 10], [Title::newFromText( 'Page 1' ), Title::newFromText( 'Page 2' )], [1], [ - (object) ['pp_page' => 1, 'pp_value' => 'B_Free.jpg', 'pp_propname' => PageImages::PROP_NAME_FREE], - (object) ['pp_page' => 1, 'pp_value' => 'B.jpg', 'pp_propname' => PageImages::PROP_NAME], + (object) ['pp_page' => 1, 'pp_value' => 'B.jpg'], ], 1 ], - [ - ['prop' => ['thumbnail'], 'thumbsize' => 500, 'limit' => 10, 'license' => 'any'], - [Title::newFromText( 'Page 1' ), Title::newFromText( 'Page 2' )], - [0, 1], - [ - (object) ['pp_page' => 1, 'pp_value' => 'B_Free.jpg', 'pp_propname' => PageImages::PROP_NAME], - ], - 1 - ], - [ - ['prop' => ['thumbnail'], 'continue' => 1, 'thumbsize' => 500, 'limit' => 10, 'license' => 'any'], - [Title::newFromText( 'Page 1' ), Title::newFromText( 'Page 2' )], - [1], - [ - (object) ['pp_page' => 1, 'pp_value' => 'B_Free.jpg', 'pp_propname' => PageImages::PROP_NAME_FREE], - ], - 1 - ], - [ - ['prop' => ['thumbnail'], 'thumbsize' => 510, 'limit' => 10, 'license' => 'free'], - [Title::newFromText( 'Page 1' ), Title::newFromText( 'Page 2' )], - [0, 1], - [], - 0 - ], - [ - ['prop' => ['thumbnail'], 'thumbsize' => 510, 'limit' => 10, 'license' => 'free'], - [Title::newFromText( 'Page 1' ), Title::newFromText( 'Page 2' )], - [0, 1], - [ - (object) ['pp_page' => 0, 'pp_value' => 'A_Free.jpg', 'pp_propname' => PageImages::PROP_NAME_FREE], - (object) ['pp_page' => 1, 'pp_value' => 'B_Free.jpg', 'pp_propname' => PageImages::PROP_NAME_FREE], - ], - 2 - ], - ]; - } - - /** - * @dataProvider provideGetPropName - * @param string $license - * @param string $expected - */ - public function testGetPropName( $license, $expected ) { - $this->assertEquals( $expected, ApiQueryPageImagesProxy::getPropNames( $license ) ); - } - - public function provideGetPropName() - { - return [ - [ 'free', \PageImages::PROP_NAME_FREE ], - [ 'any', [ \PageImages::PROP_NAME_FREE, \PageImages::PROP_NAME ] ] ]; } } diff --git a/tests/phpunit/LinksUpdateHookHandlerTest.php b/tests/phpunit/LinksUpdateHookHandlerTest.php index 5655633..afe91c7 100644 --- a/tests/phpunit/LinksUpdateHookHandlerTest.php +++ b/tests/phpunit/LinksUpdateHookHandlerTest.php @@ -3,13 +3,31 @@ namespace PageImages\Tests\Hooks; use LinksUpdate; -use PageImages; use PageImages\Hooks\LinksUpdateHookHandler; +use PageImages; use ParserOutput; use PHPUnit_Framework_TestCase; use RepoGroup; -use TestingAccessWrapper; + +class LinksUpdateHookHandlerProxy extends PageImages\Hooks\LinksUpdateHookHandler { + + public function getScore( array $image, $position ) { + return parent::getScore( $image, $position ); + } + + public function scoreFromTable( $value, array $scores ) { + return parent::scoreFromTable( $value, $scores ); + } + + public function getRatio( array $image ) { + return parent::getRatio( $image ); + } + + public function getBlacklist() { + return parent::getBlacklist(); + } +} /** * @covers PageImages\Hooks\LinksUpdateHookHandler @@ -28,12 +46,13 @@ } /** - * @param array $images * @return LinksUpdate */ - private function getLinksUpdate( array $images ) { + private function getLinksUpdate() { $parserOutput = new ParserOutput(); - $parserOutput->setExtensionData( 'pageImages', $images ); + $parserOutput->setExtensionData( 'pageImages', array( + array( 'filename' => 'A.jpg', 'fullwidth' => 100, 'fullheight' => 50 ), + ) ); $linksUpdate = $this->getMockBuilder( 'LinksUpdate' ) ->disableOriginalConstructor() @@ -46,7 +65,6 @@ } /** - * Required to make wfFindFile in LinksUpdateHookHandler::getScore return something. * @return RepoGroup */ private function getRepoGroup() { @@ -68,104 +86,28 @@ return $repoGroup; } - /** - * @dataProvider provideDoLinksUpdate - * @param $images - * @param $expectedFreeFileName - * @param $expectedNonFreeFileName - */ - public function testDoLinksUpdate( $images, $expectedFreeFileName, $expectedNonFreeFileName ) { - $linksUpdate = $this->getLinksUpdate( $images ); - $mock = TestingAccessWrapper::newFromObject( - $this->getMockBuilder( LinksUpdateHookHandler::class ) - ->setMethods( ['getScore', 'isImageFree'] ) - ->getMock() - ); + public function testOnLinksUpdate() { + $linksUpdate = $this->getLinksUpdate(); - $scoreMap = []; - $isFreeMap = []; - $counter = 0; - foreach ( $images as $image ) { - array_push( $scoreMap, [$image, $counter++, $image['score']] ); - array_push( $isFreeMap, [$image['filename'], $image['isFree']] ); - } - - $mock->expects( $this->any() ) - ->method( 'getScore' ) - ->will( $this->returnValueMap( $scoreMap ) ); - - $mock->expects( $this->any() ) - ->method( 'isImageFree' ) - ->will( $this->returnValueMap( $isFreeMap ) ); - - $mock->doLinksUpdate( $linksUpdate ); + LinksUpdateHookHandler::onLinksUpdate( $linksUpdate ); $this->assertTrue( property_exists( $linksUpdate, 'mProperties' ), 'precondition' ); - if ( is_null( $expectedFreeFileName ) ) { - $this->assertFalse( isset( $linksUpdate->mProperties[PageImages::PROP_NAME_FREE] ) ); - } else { - $this->assertSame( $expectedFreeFileName, $linksUpdate->mProperties[PageImages::PROP_NAME_FREE] ); - } - if ( is_null( $expectedNonFreeFileName ) ) { - $this->assertFalse( isset( $linksUpdate->mProperties[PageImages::PROP_NAME] ) ); - } else { - $this->assertSame( $expectedNonFreeFileName, $linksUpdate->mProperties[PageImages::PROP_NAME] ); - } - } - - public function provideDoLinksUpdate() { - return [ - // both images are non-free - [ - [ - [ 'filename' => 'A.jpg', 'score' => 100, 'isFree' => false ], - [ 'filename' => 'B.jpg', 'score' => 90, 'isFree' => false ], - ], - null, - 'A.jpg' - ], - // both images are free - [ - [ - [ 'filename' => 'A.jpg', 'score' => 100, 'isFree' => true ], - [ 'filename' => 'B.jpg', 'score' => 90, 'isFree' => true ], - ], - 'A.jpg', - null - ], - // one free (with a higher score), one non-free image - [ - [ - [ 'filename' => 'A.jpg', 'score' => 100, 'isFree' => true ], - [ 'filename' => 'B.jpg', 'score' => 90, 'isFree' => false ], - ], - 'A.jpg', - null - ], - // one non-free (with a higher score), one free image - [ - [ - [ 'filename' => 'A.jpg', 'score' => 100, 'isFree' => false ], - [ 'filename' => 'B.jpg', 'score' => 90, 'isFree' => true ], - ], - 'B.jpg', - 'A.jpg' - ] - ]; + $this->assertSame( 'A.jpg', $linksUpdate->mProperties[PageImages::PROP_NAME] ); } /** * @dataProvider provideGetScore */ - public function testGetScore( $image, $scoreFromTable, $position, $expected ) { - $mock = TestingAccessWrapper::newFromObject( - $this->getMockBuilder( LinksUpdateHookHandler::class ) - ->setMethods( ['scoreFromTable', 'getMetadata', 'getRatio', 'getBlacklist'] ) - ->getMock() - ); + public function testGetScore( $image, $scoreFromTable, $position, $metadata, $expected ) { + $mock = $this->getMockBuilder( LinksUpdateHookHandlerProxy::class ) + ->setMethods( ['scoreFromTable', 'getMetadata', 'getRatio', 'getBlacklist'] ) + ->getMock(); $mock->expects( $this->any() ) ->method( 'scoreFromTable' ) ->will( $this->returnValue( $scoreFromTable ) ); + $mock->expects( $this->any() ) + ->method( 'getMetadata' ) + ->will( $this->returnValue( $metadata ) ); $mock->expects( $this->any() ) ->method( 'getRatio' ) ->will( $this->returnValue( 0 ) ); @@ -183,6 +125,7 @@ ['filename' => 'A.jpg', 'handler' => ['width' => 100]], 100, 0, + [], // width score + ratio score + position score 100 + 100 + 8 ], @@ -190,6 +133,7 @@ ['filename' => 'A.jpg', 'fullwidth' => 100], 50, 1, + [], // width score + ratio score + position score 106 ], @@ -197,6 +141,7 @@ ['filename' => 'A.jpg', 'fullwidth' => 100], 50, 2, + [], // width score + ratio score + position score 104 ], @@ -204,6 +149,7 @@ ['filename' => 'A.jpg', 'fullwidth' => 100], 50, 3, + [], // width score + ratio score + position score 103 ], @@ -211,6 +157,7 @@ ['filename' => 'blacklisted.jpg', 'fullwidth' => 100], 50, 3, + [], // blacklist score -1000 ], @@ -223,9 +170,9 @@ public function testScoreFromTable( $type, $value, $expected ) { global $wgPageImagesScores; - $handlerWrapper = TestingAccessWrapper::newFromObject( new LinksUpdateHookHandler ); + $proxy = new LinksUpdateHookHandlerProxy; - $score = $handlerWrapper->scoreFromTable( $value, $wgPageImagesScores[$type] ); + $score = $proxy->scoreFromTable( $value, $wgPageImagesScores[$type] ); $this->assertEquals( $expected, $score ); } @@ -255,32 +202,14 @@ ]; } - /** - * @dataProvider provideIsFreeImage - * @param $fileName - * @param $metadata - */ - public function testIsFreeImage( $fileName, $metadata, $expected ) { + public function testFetchingExtendedMetadataFromFile() { + // Required to make wfFindFile in LinksUpdateHookHandler::getScore return something. RepoGroup::setSingleton( $this->getRepoGroup() ); - $mock = TestingAccessWrapper::newFromObject( - $this->getMockBuilder( LinksUpdateHookHandler::class ) - ->setMethods( ['fetchFileMetadata'] ) - ->getMock() - ); - $mock->expects( $this->any() ) - ->method( 'fetchFileMetadata' ) - ->will( $this->returnValue( $metadata ) ); - $this->assertEquals( $expected, $mock->isImageFree( $fileName )); + $linksUpdate = $this->getLinksUpdate(); + + LinksUpdateHookHandler::onLinksUpdate( $linksUpdate ); + + $this->assertTrue( true, 'no errors in getMetadata' ); } - public function provideIsFreeImage() { - return [ - ['A.jpg', [], true], - ['A.jpg', ['NonFree' => ['value' => '0']], true], - ['A.jpg', ['NonFree' => ['value' => 0]], true], - ['A.jpg', ['NonFree' => ['value' => false]], true], - ['A.jpg', ['NonFree' => ['value' => 'something']], false], - ['A.jpg', ['something' => ['value' => 'something']], true], - ]; - } } diff --git a/tests/phpunit/PageImagesTest.php b/tests/phpunit/PageImagesTest.php index 20a8562..6fe3e2b 100644 --- a/tests/phpunit/PageImagesTest.php +++ b/tests/phpunit/PageImagesTest.php @@ -17,9 +17,8 @@ */ class PageImagesTest extends MediaWikiTestCase { - public function testPagePropertyNames() { + public function testPagePropertyName() { $this->assertSame( 'page_image', PageImages::PROP_NAME ); - $this->assertSame( 'page_image_free', PageImages::PROP_NAME_FREE ); } public function testConstructor() { @@ -34,8 +33,4 @@ $this->assertFalse( PageImages::getPageImage( $title ) ); } - public function testGetPropName() { - $this->assertSame( 'page_image', PageImages::getPropName( false ) ); - $this->assertSame( 'page_image_free', PageImages::getPropName( true ) ); - } } -- To view, visit https://gerrit.wikimedia.org/r/324826 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I3fefa06103481f48dbd62b9a1b7658301a341be9 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/PageImages Gerrit-Branch: master Gerrit-Owner: MaxSem <maxsem.w...@gmail.com> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits