From 5119236d4df5b71fbb9fafba21b2603d79a33341 Mon Sep 17 00:00:00 2001 From: Kunal Mehta Date: Fri, 27 May 2016 09:11:58 -0700 Subject: [PATCH] Move Linker::getLinkColour() into LinkRenderer * Rename to getLinkClasses() since it's not really returning colours, but CSS classes. * Dependency inject LinkCache into LinkRenderer * Update all callers of Linker::getLinkColour(), and mark it as deprecated (no other uses in Gerrit) * Update a bunch of tests for new dependency Change-Id: Id178e2dcc60b833ce2dbad4920896b93cabba1bf --- includes/DummyLinker.php | 4 ++ includes/Linker.php | 27 +++----- includes/ServiceWiring.php | 3 +- includes/linker/LinkRenderer.php | 41 ++++++++++- includes/linker/LinkRendererFactory.php | 12 +++- includes/parser/LinkHolderArray.php | 9 ++- tests/phpunit/includes/LinkerTest.php | 1 + .../changes/RCCacheEntryFactoryTest.php | 4 +- .../changes/TestRecentChangesHelper.php | 4 +- .../linker/LinkRendererFactoryTest.php | 13 +++- .../includes/linker/LinkRendererTest.php | 69 +++++++++++++++++-- 11 files changed, 143 insertions(+), 44 deletions(-) diff --git a/includes/DummyLinker.php b/includes/DummyLinker.php index d9330eebc2..ba24799818 100644 --- a/includes/DummyLinker.php +++ b/includes/DummyLinker.php @@ -47,7 +47,11 @@ class DummyLinker { ); } + /** + * @deprecated since 1.28, use LinkRenderer::getLinkClasses() instead + */ public function getLinkColour( $t, $threshold ) { + wfDeprecated( __METHOD__, '1.28' ); return Linker::getLinkColour( $t, $threshold ); } diff --git a/includes/Linker.php b/includes/Linker.php index 71cb4e48d8..424c18d13d 100644 --- a/includes/Linker.php +++ b/includes/Linker.php @@ -137,31 +137,24 @@ class Linker { /** * Return the CSS colour of a known link * + * @deprecated since 1.28, use LinkRenderer::getLinkClasses() instead + * * @since 1.16.3 * @param LinkTarget $t * @param int $threshold User defined threshold * @return string CSS class */ public static function getLinkColour( LinkTarget $t, $threshold ) { - $linkCache = MediaWikiServices::getInstance()->getLinkCache(); - // Make sure the target is in the cache - $id = $linkCache->addLinkObj( $t ); - if ( $id == 0 ) { - // Doesn't exist - return ''; - } - - if ( $linkCache->getGoodLinkFieldObj( $t, 'redirect' ) ) { - # Page is a redirect - return 'mw-redirect'; - } elseif ( $threshold > 0 && MWNamespace::isContent( $t->getNamespace() ) - && $linkCache->getGoodLinkFieldObj( $t, 'length' ) < $threshold - ) { - # Page is a stub - return 'stub'; + wfDeprecated( __METHOD__, '1.28' ); + $services = MediaWikiServices::getInstance(); + $linkRenderer = $services->getLinkRenderer(); + if ( $threshold !== $linkRenderer->getStubThreshold() ) { + // Need to create a new instance with the right stub threshold... + $linkRenderer = $services->getLinkRendererFactory()->create(); + $linkRenderer->setStubThreshold( $threshold ); } - return ''; + return $linkRenderer->getLinkClasses( $t ); } /** diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index b076d07ef6..3edfbc0e38 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -162,7 +162,8 @@ return [ 'LinkRendererFactory' => function( MediaWikiServices $services ) { return new LinkRendererFactory( - $services->getTitleFormatter() + $services->getTitleFormatter(), + $services->getLinkCache() ); }, diff --git a/includes/linker/LinkRenderer.php b/includes/linker/LinkRenderer.php index 432dcb23dc..7ab9cd3c27 100644 --- a/includes/linker/LinkRenderer.php +++ b/includes/linker/LinkRenderer.php @@ -25,8 +25,10 @@ use DummyLinker; use Hooks; use Html; use HtmlArmor; +use LinkCache; use Linker; use MediaWiki\MediaWikiServices; +use MWNamespace; use Sanitizer; use Title; use TitleFormatter; @@ -62,6 +64,11 @@ class LinkRenderer { */ private $titleFormatter; + /** + * @var LinkCache + */ + private $linkCache; + /** * Whether to run the legacy Linker hooks * @@ -71,9 +78,11 @@ class LinkRenderer { /** * @param TitleFormatter $titleFormatter + * @param LinkCache $linkCache */ - public function __construct( TitleFormatter $titleFormatter ) { + public function __construct( TitleFormatter $titleFormatter, LinkCache $linkCache ) { $this->titleFormatter = $titleFormatter; + $this->linkCache = $linkCache; } /** @@ -225,7 +234,7 @@ class LinkRenderer { } /** - * If you have already looked up the proper CSS classes using Linker::getLinkColour() + * If you have already looked up the proper CSS classes using LinkRenderer::getLinkClasses() * or some other method, use this to avoid looking it up again. * * @param LinkTarget $target @@ -276,7 +285,7 @@ class LinkRenderer { if ( $target->isExternal() ) { $classes[] = 'extiw'; } - $colour = Linker::getLinkColour( $target, $this->stubThreshold ); + $colour = $this->getLinkClasses( $target ); if ( $colour !== '' ) { $classes[] = $colour; } @@ -445,4 +454,30 @@ class LinkRenderer { return $ret; } + /** + * Return the CSS classes of a known link + * + * @param LinkTarget $target + * @return string CSS class + */ + public function getLinkClasses( LinkTarget $target ) { + // Make sure the target is in the cache + $id = $this->linkCache->addLinkObj( $target ); + if ( $id == 0 ) { + // Doesn't exist + return ''; + } + + if ( $this->linkCache->getGoodLinkFieldObj( $target, 'redirect' ) ) { + # Page is a redirect + return 'mw-redirect'; + } elseif ( $this->stubThreshold > 0 && MWNamespace::isContent( $target->getNamespace() ) + && $this->linkCache->getGoodLinkFieldObj( $target, 'length' ) < $this->stubThreshold + ) { + # Page is a stub + return 'stub'; + } + + return ''; + } } diff --git a/includes/linker/LinkRendererFactory.php b/includes/linker/LinkRendererFactory.php index 7124be1e99..b7c05c2fe8 100644 --- a/includes/linker/LinkRendererFactory.php +++ b/includes/linker/LinkRendererFactory.php @@ -21,6 +21,7 @@ */ namespace MediaWiki\Linker; +use LinkCache; use TitleFormatter; use User; @@ -35,18 +36,25 @@ class LinkRendererFactory { */ private $titleFormatter; + /** + * @var LinkCache + */ + private $linkCache; + /** * @param TitleFormatter $titleFormatter + * @param LinkCache $linkCache */ - public function __construct( TitleFormatter $titleFormatter ) { + public function __construct( TitleFormatter $titleFormatter, LinkCache $linkCache ) { $this->titleFormatter = $titleFormatter; + $this->linkCache = $linkCache; } /** * @return LinkRenderer */ public function create() { - return new LinkRenderer( $this->titleFormatter ); + return new LinkRenderer( $this->titleFormatter, $this->linkCache ); } /** diff --git a/includes/parser/LinkHolderArray.php b/includes/parser/LinkHolderArray.php index 1c6f404576..b34ac1ff46 100644 --- a/includes/parser/LinkHolderArray.php +++ b/includes/parser/LinkHolderArray.php @@ -288,7 +288,6 @@ class LinkHolderArray { $linkCache = LinkCache::singleton(); $output = $this->parent->getOutput(); $linkRenderer = $this->parent->getLinkRenderer(); - $threshold = $linkRenderer->getStubThreshold(); $dbr = wfGetDB( DB_SLAVE ); @@ -321,7 +320,7 @@ class LinkHolderArray { } else { $id = $linkCache->getGoodLinkID( $pdbk ); if ( $id != 0 ) { - $colours[$pdbk] = Linker::getLinkColour( $title, $threshold ); + $colours[$pdbk] = $linkRenderer->getLinkClasses( $title ); $output->addLink( $title, $id ); $linkcolour_ids[$id] = $pdbk; } elseif ( $linkCache->isBadLink( $pdbk ) ) { @@ -353,7 +352,7 @@ class LinkHolderArray { $pdbk = $title->getPrefixedDBkey(); $linkCache->addGoodLinkObjFromRow( $title, $s ); $output->addLink( $title, $s->page_id ); - $colours[$pdbk] = Linker::getLinkColour( $title, $threshold ); + $colours[$pdbk] = $linkRenderer->getLinkClasses( $title ); // add id to the extension todolist $linkcolour_ids[$s->page_id] = $pdbk; } @@ -456,7 +455,6 @@ class LinkHolderArray { $variantMap = []; // maps $pdbkey_Variant => $keys (of link holders) $output = $this->parent->getOutput(); $linkCache = LinkCache::singleton(); - $threshold = $this->parent->getOptions()->getStubThreshold(); $titlesToBeConverted = ''; $titlesAttrs = []; @@ -549,6 +547,7 @@ class LinkHolderArray { ); $linkcolour_ids = []; + $linkRenderer = $this->parent->getLinkRenderer(); // for each found variants, figure out link holders and replace foreach ( $varRes as $s ) { @@ -575,7 +574,7 @@ class LinkHolderArray { $entry['pdbk'] = $varPdbk; // set pdbk and colour - $colours[$varPdbk] = Linker::getLinkColour( $variantTitle, $threshold ); + $colours[$varPdbk] = $linkRenderer->getLinkClasses( $variantTitle ); $linkcolour_ids[$s->page_id] = $pdbk; } } diff --git a/tests/phpunit/includes/LinkerTest.php b/tests/phpunit/includes/LinkerTest.php index 0dc12c7707..1126afec02 100644 --- a/tests/phpunit/includes/LinkerTest.php +++ b/tests/phpunit/includes/LinkerTest.php @@ -427,6 +427,7 @@ class LinkerTest extends MediaWikiLangTestCase { * @covers Linker::getLinkColour */ public function testGetLinkColour() { + $this->hideDeprecated( 'Linker::getLinkColour' ); $linkCache = MediaWikiServices::getInstance()->getLinkCache(); $foobarTitle = Title::makeTitle( NS_MAIN, 'FooBar' ); $redirectTitle = Title::makeTitle( NS_MAIN, 'Redirect' ); diff --git a/tests/phpunit/includes/changes/RCCacheEntryFactoryTest.php b/tests/phpunit/includes/changes/RCCacheEntryFactoryTest.php index 16f210b717..ccabab68a9 100644 --- a/tests/phpunit/includes/changes/RCCacheEntryFactoryTest.php +++ b/tests/phpunit/includes/changes/RCCacheEntryFactoryTest.php @@ -35,9 +35,7 @@ class RCCacheEntryFactoryTest extends MediaWikiLangTestCase { 'wgArticlePath' => '/wiki/$1' ] ); - $this->linkRenderer = new LinkRenderer( - MediaWikiServices::getInstance()->getTitleFormatter() - ); + $this->linkRenderer = MediaWikiServices::getInstance()->getLinkRenderer(); } public function testNewFromRecentChange() { diff --git a/tests/phpunit/includes/changes/TestRecentChangesHelper.php b/tests/phpunit/includes/changes/TestRecentChangesHelper.php index cac3d436b3..ae51a6c717 100644 --- a/tests/phpunit/includes/changes/TestRecentChangesHelper.php +++ b/tests/phpunit/includes/changes/TestRecentChangesHelper.php @@ -103,9 +103,7 @@ class TestRecentChangesHelper { $rcCacheFactory = new RCCacheEntryFactory( new RequestContext(), [ 'diff' => 'diff', 'cur' => 'cur', 'last' => 'last' ], - new LinkRenderer( - MediaWikiServices::getInstance()->getTitleFormatter() - ) + MediaWikiServices::getInstance()->getLinkRenderer() ); return $rcCacheFactory->newFromRecentChange( $recentChange, false ); } diff --git a/tests/phpunit/includes/linker/LinkRendererFactoryTest.php b/tests/phpunit/includes/linker/LinkRendererFactoryTest.php index 20e010812e..1f7beb3152 100644 --- a/tests/phpunit/includes/linker/LinkRendererFactoryTest.php +++ b/tests/phpunit/includes/linker/LinkRendererFactoryTest.php @@ -14,9 +14,15 @@ class LinkRendererFactoryTest extends MediaWikiLangTestCase { */ private $titleFormatter; + /** + * @var LinkCache + */ + private $linkCache; + public function setUp() { parent::setUp(); $this->titleFormatter = MediaWikiServices::getInstance()->getTitleFormatter(); + $this->linkCache = MediaWikiServices::getInstance()->getLinkCache(); } public static function provideCreateFromLegacyOptions() { @@ -48,7 +54,7 @@ class LinkRendererFactoryTest extends MediaWikiLangTestCase { * @dataProvider provideCreateFromLegacyOptions */ public function testCreateFromLegacyOptions( $options, $func, $val ) { - $factory = new LinkRendererFactory( $this->titleFormatter ); + $factory = new LinkRendererFactory( $this->titleFormatter, $this->linkCache ); $linkRenderer = $factory->createFromLegacyOptions( $options ); @@ -57,16 +63,17 @@ class LinkRendererFactoryTest extends MediaWikiLangTestCase { } public function testCreate() { - $factory = new LinkRendererFactory( $this->titleFormatter ); + $factory = new LinkRendererFactory( $this->titleFormatter, $this->linkCache ); $this->assertInstanceOf( LinkRenderer::class, $factory->create() ); } public function testCreateForUser() { + /** @var PHPUnit_Framework_MockObject_MockObject|User $user */ $user = $this->getMock( User::class, [ 'getStubThreshold' ] ); $user->expects( $this->once() ) ->method( 'getStubThreshold' ) ->willReturn( 15 ); - $factory = new LinkRendererFactory( $this->titleFormatter ); + $factory = new LinkRendererFactory( $this->titleFormatter, $this->linkCache ); $linkRenderer = $factory->createForUser( $user ); $this->assertInstanceOf( LinkRenderer::class, $linkRenderer ); $this->assertEquals( 15, $linkRenderer->getStubThreshold() ); diff --git a/tests/phpunit/includes/linker/LinkRendererTest.php b/tests/phpunit/includes/linker/LinkRendererTest.php index a74db20ede..48949b6e54 100644 --- a/tests/phpunit/includes/linker/LinkRendererTest.php +++ b/tests/phpunit/includes/linker/LinkRendererTest.php @@ -1,6 +1,7 @@ '/w', 'wgScript' => '/w/index.php', ] ); - $this->titleFormatter = MediaWikiServices::getInstance()->getTitleFormatter(); + $this->factory = MediaWikiServices::getInstance()->getLinkRendererFactory(); + } public function testMergeAttribs() { $target = new TitleValue( NS_SPECIAL, 'Blankpage' ); - $linkRenderer = new LinkRenderer( $this->titleFormatter ); + $linkRenderer = $this->factory->create(); $link = $linkRenderer->makeBrokenLink( $target, null, [ // Appended to class 'class' => 'foobar', @@ -46,7 +48,7 @@ class LinkRendererTest extends MediaWikiLangTestCase { public function testMakeKnownLink() { $target = new TitleValue( NS_MAIN, 'Foobar' ); - $linkRenderer = new LinkRenderer( $this->titleFormatter ); + $linkRenderer = $this->factory->create(); // Query added $this->assertEquals( @@ -73,7 +75,7 @@ class LinkRendererTest extends MediaWikiLangTestCase { public function testMakeBrokenLink() { $target = new TitleValue( NS_MAIN, 'Foobar' ); $special = new TitleValue( NS_SPECIAL, 'Foobar' ); - $linkRenderer = new LinkRenderer( $this->titleFormatter ); + $linkRenderer = $this->factory->create(); // action=edit&redlink=1 added $this->assertEquals( @@ -105,7 +107,7 @@ class LinkRendererTest extends MediaWikiLangTestCase { } public function testMakeLink() { - $linkRenderer = new LinkRenderer( $this->titleFormatter ); + $linkRenderer = $this->factory->create(); $foobar = new TitleValue( NS_SPECIAL, 'Foobar' ); $blankpage = new TitleValue( NS_SPECIAL, 'Blankpage' ); $this->assertEquals( @@ -131,4 +133,57 @@ class LinkRendererTest extends MediaWikiLangTestCase { $linkRenderer->makeLink( $foobar, new HtmlArmor( '' ) ) ); } + + public function testGetLinkClasses() { + $titleFormatter = MediaWikiServices::getInstance()->getTitleFormatter(); + $linkCache = new LinkCache( $titleFormatter ); + $foobarTitle = new TitleValue( NS_MAIN, 'FooBar' ); + $redirectTitle = new TitleValue( NS_MAIN, 'Redirect' ); + $userTitle = new TitleValue( NS_USER, 'Someuser' ); + $linkCache->addGoodLinkObj( + 1, // id + $foobarTitle, + 10, // len + 0 // redir + ); + $linkCache->addGoodLinkObj( + 2, // id + $redirectTitle, + 10, // len + 1 // redir + ); + + $linkCache->addGoodLinkObj( + 3, // id + $userTitle, + 10, // len + 0 // redir + ); + + $linkRenderer = new LinkRenderer( $titleFormatter, $linkCache ); + $linkRenderer->setStubThreshold( 0 ); + $this->assertEquals( + '', + $linkRenderer->getLinkClasses( $foobarTitle ) + ); + + $linkRenderer->setStubThreshold( 20 ); + $this->assertEquals( + 'stub', + $linkRenderer->getLinkClasses( $foobarTitle ) + ); + + $linkRenderer->setStubThreshold( 0 ); + $this->assertEquals( + 'mw-redirect', + $linkRenderer->getLinkClasses( $redirectTitle ) + ); + + $linkRenderer->setStubThreshold( 20 ); + $this->assertEquals( + '', + $linkRenderer->getLinkClasses( $userTitle ) + ); + } + } -- 2.20.1