Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

LinkCache: soft deprecate addGoodLinkObj()

Browse source 
addGoodLinkObj() has many optional arguments, but omitting them actually
means corrupting the cache.

Nearly all existing callers are in tests.
So LinkCacheTestTrait::addGoodLinkObject() was created only
for testing. It is better to have this method in the
trait, because building the row directly in each test
would make these tests brittle against schema changes.

The only usage in WMF production code was in WikiPage and has been
fixed.

Bug: T284955
Change-Id: I03a2bd9ed64fcc0281ee29a286c8db395a9e03d9
This commit is contained in:
daniel 2021-06-09 23:17:21 +02:00
parent 6e99ddcef6
commit 855988fd0e
11 changed files with 100 additions and 70 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
RELEASE-NOTES-1.37
View file

@ -580,6 +580,9 @@ because of Phabricator reports.
MediaWikiServices::getJobQueueGroup instead.
* JobQueueGroup::destroySingletons was deprecated. JobQueueGroups are now
automatically destroyed after tests.
* LinkCache::addGoodLinkObj() has been soft deprecated, since it is prone to
corrupting the cache with invalid information. Use addGoodLinkObjFromRow()
instead. PHPUnit tests must use LinkCacheTestTrait::addGoodLinkObject().
* SessionProvider ::setLogger(), ::setManager(), ::setConfig(),
::setHookContainer() were hard deprecated. Use ::init() to inject
dependencies or override ::postInitSetup() to do any custom

3
includes/cache/LinkCache.php vendored
View file

@ -279,7 +279,8 @@ class LinkCache implements LoggerAwareInterface {
/**
* Add information about an existing page to the cache.
*
* @see addGoodLinkObjFromRow()
* @deprecated since 1.37, use addGoodLinkObjFromRow() instead. PHPUnit tests
* must use LinkCacheTestTrait::addGoodLinkObject().
*
* @param int $id Page's ID
* @param LinkTarget|PageReference $page The page to set cached info for.

18
includes/page/WikiPage.php
View file

@ -1455,7 +1455,13 @@ class WikiPage implements Page, IDBAccessObject, PageRecord {
$result = $dbw->affectedRows() > 0;
if ( $result ) {
$this->mTitle->loadFromRow( (object)$row );
$insertedRow = $this->pageData( $dbw, [ 'page_id' => $this->getId() ] );
if ( !$insertedRow ) {
throw new MWException( 'Failed to load freshly inserted row' );
}
$this->mTitle->loadFromRow( $insertedRow );
$this->updateRedirectOn( $dbw, $rt, $lastRevIsRedirect );
$this->setLastEdit( $revision );
$this->mRedirectTarget = null;
@ -1463,16 +1469,12 @@ class WikiPage implements Page, IDBAccessObject, PageRecord {
$this->mPageIsRedirectField = (bool)$rt;
$this->mIsNew = (bool)$isNew;
$this->mIsRedirect = (bool)$isRedirect;
// Update the LinkCache.
$linkCache = MediaWikiServices::getInstance()->getLinkCache();
$linkCache->addGoodLinkObj(
$this->getId(),
$linkCache->addGoodLinkObjFromRow(
$this->mTitle,
$len,
$this->mPageIsRedirectField,
$this->mLatest,
$model,
$this->mLanguage
$insertedRow
);
}

3
tests/common/TestsAutoLoader.php
View file

@ -97,6 +97,9 @@ $wgAutoloadClasses += [
# tests/phpunit/includes/block
'MediaWiki\\Tests\\Block\\Restriction\\RestrictionTestCase' => "$testDir/phpunit/includes/block/Restriction/RestrictionTestCase.php",
# tests/phpunit/includes/cache
'LinkCacheTestTrait' => "$testDir/phpunit/includes/cache/LinkCacheTestTrait.php",
# tests/phpunit/includes/changes
'TestRecentChangesHelper' => "$testDir/phpunit/includes/changes/TestRecentChangesHelper.php",

43
tests/phpunit/includes/cache/LinkCacheTest.php vendored
View file

@ -9,6 +9,7 @@ use MediaWiki\Page\PageReferenceValue;
* @covers LinkCache
*/
class LinkCacheTest extends MediaWikiIntegrationTestCase {
use LinkCacheTestTrait;
private function newLinkCache( WANObjectCache $wanCache = null ) {
if ( !$wanCache ) {
@ -123,25 +124,16 @@ class LinkCacheTest extends MediaWikiIntegrationTestCase {
}
/**
* @dataProvider providePageAndLink
* @covers LinkCache::addGoodLinkObj()
* @covers LinkCache::addGoodLinkObjFromRow()
* @covers LinkCache::getGoodLinkRow()
* @covers LinkCache::getGoodLinkID()
* @covers LinkCache::getGoodLinkFieldObj()
*/
public function testAddGoodLinkObjWithAllParameters( $page ) {
$linkCache = $this->newLinkCache();
public function testAddGoodLinkObjWithAllParameters() {
$linkCache = $this->getServiceContainer()->getLinkCache();
$page = new PageReferenceValue( NS_USER, __METHOD__, PageReference::LOCAL );
$linkCache->addGoodLinkObj(
8,
$page,
18,
0,
118,
CONTENT_MODEL_TEXT,
'xyz'
);
$this->addGoodLinkObject( 8, $page, 18, 0, 118, CONTENT_MODEL_TEXT, 'xyz' );
$row = $linkCache->getGoodLinkRow( $page->getNamespace(), $page->getDBkey() );
$this->assertEquals( 8, (int)$row->page_id );
@ -171,19 +163,17 @@ class LinkCacheTest extends MediaWikiIntegrationTestCase {
}
/**
* @covers LinkCache::addGoodLinkObj()
* @covers LinkCache::addGoodLinkObjFromRow()
* @covers LinkCache::getGoodLinkRow()
* @covers LinkCache::getGoodLinkID()
* @covers LinkCache::getGoodLinkFieldObj()
*/
public function testAddGoodLinkObjWithMinimalParameters() {
$linkCache = $this->newLinkCache();
public function testAddGoodLinkObjFromRowWithMinimalParameters() {
$linkCache = $this->getServiceContainer()->getLinkCache();
$page = new PageReferenceValue( NS_USER, __METHOD__, PageReference::LOCAL );
$linkCache->addGoodLinkObj(
8,
$page
);
$this->addGoodLinkObject( 8, $page );
$expectedRow = [
'page_id' => 8,
'page_len' => -1,
@ -226,13 +216,14 @@ class LinkCacheTest extends MediaWikiIntegrationTestCase {
}
/**
* @covers LinkCache::addGoodLinkObj()
* @covers LinkCache::addGoodLinkObjFromRow()
*/
public function testAddGoodLinkObjWithInterwikiLink() {
$linkCache = $this->newLinkCache();
public function testAddGoodLinkObjFromRowWithInterwikiLink() {
$linkCache = $this->getServiceContainer()->getLinkCache();
$page = new TitleValue( NS_USER, __METHOD__, '', 'acme' );
$linkCache->addGoodLinkObj( 8, $page );
$this->addGoodLinkObject( 8, $page );
$this->assertSame( 0, $linkCache->getGoodLinkID( $page ) );
}
@ -244,10 +235,10 @@ class LinkCacheTest extends MediaWikiIntegrationTestCase {
* @covers LinkCache::clearLink()
*/
public function testAddBadLinkObj( $key ) {
$linkCache = $this->newLinkCache();
$linkCache = $this->getServiceContainer()->getLinkCache();
$this->assertFalse( $linkCache->isBadLink( $key ) );
$linkCache->addGoodLinkObj( 17, $key );
$this->addGoodLinkObject( 17, $key );
$linkCache->addBadLinkObj( $key );
$this->assertTrue( $linkCache->isBadLink( $key ) );

47
tests/phpunit/includes/cache/LinkCacheTestTrait.php vendored Normal file
View file

@ -0,0 +1,47 @@
<?php
use MediaWiki\Linker\LinkTarget;
use MediaWiki\MediaWikiServices;
use MediaWiki\Page\PageReference;
/**
* A trait providing utility functions for testing LinkCache.
* This trait is intended to be used on subclasses of
* MediaWikiIntegrationTestCase.
*
* @stable to use
* @since 1.37
*/
trait LinkCacheTestTrait {
/**
* Force information about a page into the cache, pretending it exists.
*
* @param int $id Page's ID
* @param LinkTarget|PageReference $page The page to set cached info for.
* @param int $len Text's length
* @param int|null $redir Whether the page is a redirect
* @param int $revision Latest revision's ID
* @param string|null $model Latest revision's content model ID
* @param string|null $lang Language code of the page, if not the content language
*/
public function addGoodLinkObject(
$id, $page, $len = -1, $redir = null, $revision = 0, $model = null, $lang = null
) {
MediaWikiServices::getInstance()->getLinkCache()->addGoodLinkObjFromRow( $page, (object)[
'page_id' => (int)$id,
'page_namespace' => $page->getNamespace(),
'page_title' => $page->getDBkey(),
'page_len' => (int)$len,
'page_is_redirect' => (int)$redir,
'page_latest' => (int)$revision,
'page_content_model' => $model ? (string)$model : null,
'page_lang' => $lang ? (string)$lang : null,
'page_restrictions' => null,
'page_is_new' => 0,
'page_touched' => '',
] );
}
}

26
tests/phpunit/includes/linker/LinkRendererTest.php
View file

@ -10,6 +10,7 @@ use MediaWiki\Page\PageReferenceValue;
* @covers MediaWiki\Linker\LinkRenderer
*/
class LinkRendererTest extends MediaWikiLangTestCase {
use LinkCacheTestTrait;
/**
* @var LinkRendererFactory
@ -202,47 +203,30 @@ class LinkRendererTest extends MediaWikiLangTestCase {
*/
public function testGetLinkClasses( $foobarTitle, $redirectTitle, $userTitle ) {
$services = MediaWikiServices::getInstance();
$wanCache = $services->getMainWANObjectCache();
$titleFormatter = $services->getTitleFormatter();
$nsInfo = $services->getNamespaceInfo();
$specialPageFactory = $services->getSpecialPageFactory();
$hookContainer = $services->getHookContainer();
$loadBalancer = $services->getDBLoadBalancer();
$linkCache = new LinkCache( $titleFormatter, $wanCache, $nsInfo, $loadBalancer );
$linkCache = $services->getLinkCache();
if ( $foobarTitle instanceof PageReference ) {
$cacheTitle = Title::castFromPageReference( $foobarTitle );
} else {
$cacheTitle = $foobarTitle;
}
$linkCache->addGoodLinkObj(
1, // id
$cacheTitle,
10, // len
0 // redir
);
$this->addGoodLinkObject( 1, $cacheTitle, 10, 0 );
if ( $redirectTitle instanceof PageReference ) {
$cacheTitle = Title::castFromPageReference( $redirectTitle );
} else {
$cacheTitle = $redirectTitle;
}
$linkCache->addGoodLinkObj(
2, // id
$cacheTitle,
10, // len
1 // redir
);
$this->addGoodLinkObject( 2, $cacheTitle, 10, 1 );
if ( $userTitle instanceof PageReference ) {
$cacheTitle = Title::castFromPageReference( $userTitle );
} else {
$cacheTitle = $userTitle;
}
$linkCache->addGoodLinkObj(
3, // id
$cacheTitle,
10, // len
0 // redir
);
$this->addGoodLinkObject( 3, $cacheTitle, 10, 0 );
$linkRenderer = new LinkRenderer( $titleFormatter, $linkCache,
$nsInfo, $specialPageFactory, $hookContainer );

4
tests/phpunit/includes/page/PageStoreTest.php
View file

@ -4,6 +4,7 @@ namespace MediaWiki\Tests\Page;
use Exception;
use IDatabase;
use InvalidArgumentException;
use LinkCacheTestTrait;
use LoadBalancer;
use MediaWiki\Config\ServiceOptions;
use MediaWiki\DAO\WikiAwareEntity;
@ -23,6 +24,7 @@ use Wikimedia\Rdbms\DBConnRef;
class PageStoreTest extends MediaWikiIntegrationTestCase {
use MockTitleTrait;
use LinkCacheTestTrait;
protected function setUp(): void {
parent::setUp();
@ -332,7 +334,7 @@ class PageStoreTest extends MediaWikiIntegrationTestCase {
// Has all fields needed by LinkCache, but not all fields needed by PageStore.
// This may happen when legacy code injects rows directly into LinkCache.
$linkCache->addGoodLinkObj( 8, $nonexistingPage );
$this->addGoodLinkObject( 8, $nonexistingPage );
$pageStore = $this->getPageStore();
$page = $pageStore->getPageByName( $ns, $dbkey );

9
tests/phpunit/includes/resourceloader/ResourceLoaderWikiModuleTest.php
View file

@ -1,6 +1,5 @@
<?php
use MediaWiki\MediaWikiServices;
use MediaWiki\Page\PageIdentity;
use MediaWiki\Page\PageIdentityValue;
use MediaWiki\Page\PageRecord;
@ -12,6 +11,7 @@ use Wikimedia\TestingAccessWrapper;
* @covers ResourceLoaderWikiModule
*/
class ResourceLoaderWikiModuleTest extends ResourceLoaderTestCase {
use LinkCacheTestTrait;
/**
* @dataProvider provideConstructor
@ -438,12 +438,7 @@ class ResourceLoaderWikiModuleTest extends ResourceLoaderTestCase {
} );
// Mock away Title's db queries with LinkCache
MediaWikiServices::getInstance()->getLinkCache()->addGoodLinkObj(
1, // id
new TitleValue( NS_MEDIAWIKI, 'Redirect.js' ),
1, // len
1 // redirect
);
$this->addGoodLinkObject( 1, new TitleValue( NS_MEDIAWIKI, 'Redirect.js' ), 1, 1 );
$this->assertSame(
"/*\nMediaWiki:Redirect.js\n*/\ntarget;\n",

6
tests/phpunit/includes/search/SearchNearMatcherTest.php
View file

@ -6,6 +6,8 @@ use MediaWiki\MediaWikiServices;
* @covers SearchNearMatcher
*/
class SearchNearMatcherTest extends MediaWikiIntegrationTestCase {
use LinkCacheTestTrait;
public function nearMatchProvider() {
return [
'empty request returns nothing' => [ null, 'en', '', 'Near Match Test' ],
@ -74,7 +76,7 @@ class SearchNearMatcherTest extends MediaWikiIntegrationTestCase {
$enableSearchContributorsByIP = false
) {
$services = MediaWikiServices::getInstance();
$services->getLinkCache()->addGoodLinkObj( 42, Title::newFromText( $titleText ) );
$this->addGoodLinkObject( 42, Title::newFromText( $titleText ) );
$config = new HashConfig( [
'EnableSearchContributorsByIP' => $enableSearchContributorsByIP,
] );
@ -126,7 +128,7 @@ class SearchNearMatcherTest extends MediaWikiIntegrationTestCase {
*/
public function testGetNearMatchResultSet() {
$services = MediaWikiServices::getInstance();
$services->getLinkCache()->addGoodLinkObj( 42, Title::newFromText( "Test Link" ) );
$this->addGoodLinkObject( 42, Title::newFromText( "Test Link" ) );
$config = new HashConfig( [
'EnableSearchContributorsByIP' => false,

8
tests/phpunit/mocks/search/MockSearchEngine.php
View file

@ -1,8 +1,8 @@
<?php
use MediaWiki\MediaWikiServices;
class MockSearchEngine extends SearchEngine {
use LinkCacheTestTrait;
/** @var SearchResult[][] */
private static $results = [];
/** @var ISearchResultSet[][] */
@ -19,7 +19,7 @@ class MockSearchEngine extends SearchEngine {
* @param SearchResult[] $results The results to return for $query
*/
public static function addMockResults( $query, array $results ) {
$lc = MediaWikiServices::getInstance()->getLinkCache();
$mockSearchEngine = new self();
foreach ( $results as &$result ) {
// Resolve deferred results; needed to work around T203279
if ( is_callable( $result ) ) {
@ -27,7 +27,7 @@ class MockSearchEngine extends SearchEngine {
}
// TODO: better page ids? Does it matter?
$lc->addGoodLinkObj( mt_rand(), $result->getTitle() );
$mockSearchEngine->addGoodLinkObject( mt_rand(), $result->getTitle() );
}
self::$results[$query] = $results;
}