Merge "Introduce an EditRevUpdater service"

includes/AbuseFilter.php

@@ -15,12 +15,6 @@ use User;
  */
 class AbuseFilter {
 
-	/**
-	 * @var array IDs of logged filters like [ page title => [ 'local' => [ids], 'global' => [ids] ] ].
-	 * @fixme avoid global state
-	 */
-	public static $logIds = [];
-
 	/**
 	 * @deprecated
 	 * @todo Phase out

includes/AbuseFilterHooks.php

@@ -21,14 +21,10 @@ use Status;
 use Title;
 use UploadBase;
 use User;
-use WikiMap;
 use WikiPage;
 
 class AbuseFilterHooks {
 
-	/** @var WikiPage|null Make sure edit filter & edit save hooks match */
-	private static $lastEditPage = null;
-
 	/**
 	 * Called right after configuration has been loaded.
 	 * @codeCoverageIgnore Mainly deprecation warnings and other things that can be tested by running the updater
@@ -200,7 +196,8 @@ class AbuseFilterHooks {
 		Content $content,
 		$summary, $slot = SlotRecord::MAIN
 	) : Status {
-		self::$lastEditPage = null;
+		$revUpdater = AbuseFilterServices::getEditRevUpdater();
+		$revUpdater->clearLastEditPage();
 
 		// @todo is there any real point in passing this in?
 		$text = AbuseFilterServices::getTextExtractor()->contentToString( $content );
@@ -234,7 +231,7 @@ class AbuseFilterHooks {
 			return $filterResult;
 		}
 
-		self::$lastEditPage = $page;
+		$revUpdater->setLastEditPage( $page );
 
 		return Status::newGood();
 	}
@@ -281,49 +278,7 @@ class AbuseFilterHooks {
 		int $flags,
 		RevisionRecord $revisionRecord
 	) {
-		$curTitle = $wikiPage->getTitle()->getPrefixedText();
-		if ( !isset( AbuseFilter::$logIds[ $curTitle ] ) ||
-			$wikiPage !== self::$lastEditPage
-		) {
-			// This isn't the edit AbuseFilter::$logIds was set for
-			AbuseFilter::$logIds = [];
-			return;
-		}
-
-		// Ignore null edit.
-		$parentRevId = $revisionRecord->getParentId();
-		if ( $parentRevId !== null ) {
-			$parentRev = MediaWikiServices::getInstance()
-				->getRevisionLookup()
-				->getRevisionById( $parentRevId );
-			if ( $parentRev && $revisionRecord->hasSameContent( $parentRev ) ) {
-				AbuseFilter::$logIds = [];
-				return;
-			}
-		}
-
-		self::$lastEditPage = null;
-
-		$logs = AbuseFilter::$logIds[ $curTitle ];
-		if ( $logs[ 'local' ] ) {
-			// Now actually do our storage
-			$dbw = wfGetDB( DB_MASTER );
-
-			$dbw->update( 'abuse_filter_log',
-				[ 'afl_rev_id' => $revisionRecord->getId() ],
-				[ 'afl_id' => $logs['local'] ],
-				__METHOD__
-			);
-		}
-
-		if ( $logs[ 'global' ] ) {
-			$fdb = AbuseFilterServices::getCentralDBManager()->getConnection( DB_MASTER );
-			$fdb->update( 'abuse_filter_log',
-				[ 'afl_rev_id' => $revisionRecord->getId() ],
-				[ 'afl_id' => $logs['global'], 'afl_wiki' => WikiMap::getCurrentWikiDbDomain()->getId() ],
-				__METHOD__
-			);
-		}
+		AbuseFilterServices::getEditRevUpdater()->updateRev( $wikiPage, $revisionRecord );
 	}
 
 	/**

includes/AbuseFilterServices.php

@@ -244,4 +244,11 @@ class AbuseFilterServices {
 	public static function getVariableGeneratorFactory() : VariableGeneratorFactory {
 		return MediaWikiServices::getInstance()->getService( VariableGeneratorFactory::SERVICE_NAME );
 	}
+
+	/**
+	 * @return EditRevUpdater
+	 */
+	public static function getEditRevUpdater() : EditRevUpdater {
+		return MediaWikiServices::getInstance()->getService( EditRevUpdater::SERVICE_NAME );
+	}
 }

includes/AbuseLogger.php

@@ -40,6 +40,8 @@ class AbuseLogger {
 	private $varBlobStore;
 	/** @var VariablesManager */
 	private $varManager;
+	/** @var EditRevUpdater */
+	private $editRevUpdater;
 	/** @var ILoadBalancer */
 	private $loadBalancer;
 	/** @var ServiceOptions */
@@ -54,6 +56,7 @@ class AbuseLogger {
 	 * @param FilterLookup $filterLookup
 	 * @param VariablesBlobStore $varBlobStore
 	 * @param VariablesManager $varManager
+	 * @param EditRevUpdater $editRevUpdater
 	 * @param ILoadBalancer $loadBalancer
 	 * @param ServiceOptions $options
 	 * @param string $wikiID
@@ -67,6 +70,7 @@ class AbuseLogger {
 		FilterLookup $filterLookup,
 		VariablesBlobStore $varBlobStore,
 		VariablesManager $varManager,
+		EditRevUpdater $editRevUpdater,
 		ILoadBalancer $loadBalancer,
 		ServiceOptions $options,
 		string $wikiID,
@@ -82,6 +86,7 @@ class AbuseLogger {
 		$this->filterLookup = $filterLookup;
 		$this->varBlobStore = $varBlobStore;
 		$this->varManager = $varManager;
+		$this->editRevUpdater = $editRevUpdater;
 		$this->loadBalancer = $loadBalancer;
 		$options->assertRequiredOptions( self::CONSTRUCTOR_OPTIONS );
 		$this->options = $options;
@@ -166,10 +171,10 @@ class AbuseLogger {
 			$globalLogIDs = $this->insertGlobalLogEntries( $centralLogRows, $fdb );
 		}
 
-		AbuseFilter::$logIds[ $this->title->getPrefixedText() ] = [
-			'local' => $localLogIDs,
-			'global' => $globalLogIDs
-		];
+		$this->editRevUpdater->setLogIdsForTarget(
+			$this->title,
+			[ 'local' => $localLogIDs, 'global' => $globalLogIDs ]
+		);
 
 		return [ 'local' => $loggedLocalFilters, 'global' => $loggedGlobalFilters ];
 	}
@@ -203,7 +208,7 @@ class AbuseLogger {
 	/**
 	 * @param array[] $logRows
 	 * @param IDatabase $dbw
-	 * @return array
+	 * @return int[]
 	 */
 	private function insertLocalLogEntries( array $logRows, IDatabase $dbw ) : array {
 		global $wgAbuseFilterAflFilterMigrationStage;
@@ -275,7 +280,7 @@ class AbuseLogger {
 	/**
 	 * @param array[] $centralLogRows
 	 * @param IDatabase $fdb
-	 * @return array
+	 * @return int[]
 	 */
 	private function insertGlobalLogEntries( array $centralLogRows, IDatabase $fdb ) : array {
 		$this->varManager->computeDBVars( $this->vars );

includes/AbuseLoggerFactory.php

@@ -21,6 +21,8 @@ class AbuseLoggerFactory {
 	private $varBlobStore;
 	/** @var VariablesManager */
 	private $varManager;
+	/** @var EditRevUpdater */
+	private $editRevUpdater;
 	/** @var ILoadBalancer */
 	private $loadBalancer;
 	/** @var ServiceOptions */
@@ -35,6 +37,7 @@ class AbuseLoggerFactory {
 	 * @param FilterLookup $filterLookup
 	 * @param VariablesBlobStore $varBlobStore
 	 * @param VariablesManager $varManager
+	 * @param EditRevUpdater $editRevUpdater
 	 * @param ILoadBalancer $loadBalancer
 	 * @param ServiceOptions $options
 	 * @param string $wikiID
@@ -45,6 +48,7 @@ class AbuseLoggerFactory {
 		FilterLookup $filterLookup,
 		VariablesBlobStore $varBlobStore,
 		VariablesManager $varManager,
+		EditRevUpdater $editRevUpdater,
 		ILoadBalancer $loadBalancer,
 		ServiceOptions $options,
 		string $wikiID,
@@ -54,6 +58,7 @@ class AbuseLoggerFactory {
 		$this->filterLookup = $filterLookup;
 		$this->varBlobStore = $varBlobStore;
 		$this->varManager = $varManager;
+		$this->editRevUpdater = $editRevUpdater;
 		$this->loadBalancer = $loadBalancer;
 		$this->options = $options;
 		$this->wikiID = $wikiID;
@@ -76,6 +81,7 @@ class AbuseLoggerFactory {
 			$this->filterLookup,
 			$this->varBlobStore,
 			$this->varManager,
+			$this->editRevUpdater,
 			$this->loadBalancer,
 			$this->options,
 			$this->wikiID,

includes/EditRevUpdater.php

@@ -0,0 +1,138 @@
+<?php
+
+namespace MediaWiki\Extension\AbuseFilter;
+
+use InvalidArgumentException;
+use MediaWiki\Linker\LinkTarget;
+use MediaWiki\Revision\RevisionLookup;
+use MediaWiki\Revision\RevisionRecord;
+use Wikimedia\Rdbms\ILoadBalancer;
+use WikiPage;
+
+/**
+ * This service allows "linking" the edit filter hook and the page save hook
+ */
+class EditRevUpdater {
+	public const SERVICE_NAME = 'AbuseFilterEditRevUpdater';
+
+	/** @var CentralDBManager */
+	private $centralDBManager;
+	/** @var RevisionLookup */
+	private $revisionLookup;
+	/** @var ILoadBalancer */
+	private $loadBalancer;
+	/** @var string */
+	private $wikiID;
+
+	/** @var WikiPage|null */
+	private $wikiPage;
+	/**
+	 * @var int[][][] IDs of logged filters like [ page title => [ 'local' => [ids], 'global' => [ids] ] ].
+	 * @phan-var array<string,array{local:int[],global:int[]}>
+	 */
+	private $logIds = [];
+
+	/**
+	 * @param CentralDBManager $centralDBManager
+	 * @param RevisionLookup $revisionLookup
+	 * @param ILoadBalancer $loadBalancer
+	 * @param string $wikiID
+	 */
+	public function __construct(
+		CentralDBManager $centralDBManager,
+		RevisionLookup $revisionLookup,
+		ILoadBalancer $loadBalancer,
+		string $wikiID
+	) {
+		$this->centralDBManager = $centralDBManager;
+		$this->revisionLookup = $revisionLookup;
+		$this->loadBalancer = $loadBalancer;
+		$this->wikiID = $wikiID;
+	}
+
+	/**
+	 * Set the WikiPage object used for the ongoing edit
+	 *
+	 * @param WikiPage $page
+	 */
+	public function setLastEditPage( WikiPage $page ) : void {
+		$this->wikiPage = $page;
+	}
+
+	/**
+	 * Clear the WikiPage object used for the ongoing edit
+	 */
+	public function clearLastEditPage() : void {
+		$this->wikiPage = null;
+	}
+
+	/**
+	 * @param LinkTarget $target
+	 * @param int[][] $logIds
+	 * @phan-param array{local:int[],global:int[]} $logIds
+	 */
+	public function setLogIdsForTarget( LinkTarget $target, array $logIds ) : void {
+		if ( count( $logIds ) !== 2 || array_diff( array_keys( $logIds ), [ 'local', 'global' ] ) ) {
+			throw new InvalidArgumentException( 'Wrong keys; got: ' . implode( ', ', array_keys( $logIds ) ) );
+		}
+		$key = $this->getCacheKey( $target );
+		$this->logIds[$key] = $logIds;
+	}
+
+	/**
+	 * @param WikiPage $wikiPage
+	 * @param RevisionRecord $revisionRecord
+	 * @return bool Whether the DB was updated
+	 */
+	public function updateRev( WikiPage $wikiPage, RevisionRecord $revisionRecord ) : bool {
+		$key = $this->getCacheKey( $wikiPage->getTitle() );
+		if ( !isset( $this->logIds[ $key ] ) || $wikiPage !== $this->wikiPage ) {
+			// This isn't the edit $this->logIds was set for
+			$this->logIds = [];
+			return false;
+		}
+
+		// Ignore null edit.
+		$parentRevId = $revisionRecord->getParentId();
+		if ( $parentRevId !== null ) {
+			$parentRev = $this->revisionLookup->getRevisionById( $parentRevId );
+			if ( $parentRev && $revisionRecord->hasSameContent( $parentRev ) ) {
+				$this->logIds = [];
+				return false;
+			}
+		}
+
+		$this->clearLastEditPage();
+
+		$ret = false;
+		$logs = $this->logIds[ $key ];
+		if ( $logs[ 'local' ] ) {
+			$dbw = $this->loadBalancer->getConnectionRef( DB_MASTER );
+			$dbw->update( 'abuse_filter_log',
+				[ 'afl_rev_id' => $revisionRecord->getId() ],
+				[ 'afl_id' => $logs['local'] ],
+				__METHOD__
+			);
+			$ret = true;
+		}
+
+		if ( $logs[ 'global' ] ) {
+			$fdb = $this->centralDBManager->getConnection( DB_MASTER );
+			$fdb->update( 'abuse_filter_log',
+				[ 'afl_rev_id' => $revisionRecord->getId() ],
+				[ 'afl_id' => $logs['global'], 'afl_wiki' => $this->wikiID ],
+				__METHOD__
+			);
+			$ret = true;
+		}
+		return $ret;
+	}
+
+	/**
+	 * @param LinkTarget $target
+	 * @return string
+	 */
+	private function getCacheKey( LinkTarget $target ) : string {
+		return $target->getNamespace() . '|' . $target->getText();
+	}
+}

includes/ServiceWiring.php

@@ -17,6 +17,7 @@ use MediaWiki\Extension\AbuseFilter\Consequences\ConsequencesLookup;
 use MediaWiki\Extension\AbuseFilter\Consequences\ConsequencesRegistry;
 use MediaWiki\Extension\AbuseFilter\EchoNotifier;
 use MediaWiki\Extension\AbuseFilter\EditBoxBuilderFactory;
+use MediaWiki\Extension\AbuseFilter\EditRevUpdater;
 use MediaWiki\Extension\AbuseFilter\FilterCompare;
 use MediaWiki\Extension\AbuseFilter\FilterImporter;
 use MediaWiki\Extension\AbuseFilter\FilterLookup;
@@ -228,6 +229,7 @@ return [
 			$services->get( FilterLookup::SERVICE_NAME ),
 			$services->get( VariablesBlobStore::SERVICE_NAME ),
 			$services->get( VariablesManager::SERVICE_NAME ),
+			$services->get( EditRevUpdater::SERVICE_NAME ),
 			$services->getDBLoadBalancer(),
 			new ServiceOptions(
 				AbuseLogger::CONSTRUCTOR_OPTIONS,
@@ -331,6 +333,14 @@ return [
 			$services->getRepoGroup()
 		);
 	},
+	EditRevUpdater::SERVICE_NAME => function ( MediaWikiServices $services ): EditRevUpdater {
+		return new EditRevUpdater(
+			$services->get( CentralDBManager::SERVICE_NAME ),
+			$services->getRevisionLookup(),
+			$services->getDBLoadBalancer(),
+			WikiMap::getCurrentWikiDbDomain()->getId()
+		);
+	},
 ];
 
 // @codeCoverageIgnoreEnd

tests/phpunit/unit/AbuseLoggerFactoryTest.php

@@ -7,6 +7,7 @@ use MediaWiki\Config\ServiceOptions;
 use MediaWiki\Extension\AbuseFilter\AbuseLogger;
 use MediaWiki\Extension\AbuseFilter\AbuseLoggerFactory;
 use MediaWiki\Extension\AbuseFilter\CentralDBManager;
+use MediaWiki\Extension\AbuseFilter\EditRevUpdater;
 use MediaWiki\Extension\AbuseFilter\FilterLookup;
 use MediaWiki\Extension\AbuseFilter\Variables\VariableHolder;
 use MediaWiki\Extension\AbuseFilter\Variables\VariablesBlobStore;
@@ -32,6 +33,7 @@ class AbuseLoggerFactoryTest extends MediaWikiUnitTestCase {
 			$this->createMock( FilterLookup::class ),
 			$this->createMock( VariablesBlobStore::class ),
 			$this->createMock( VariablesManager::class ),
+			$this->createMock( EditRevUpdater::class ),
 			$this->createMock( ILoadBalancer::class ),
 			new ServiceOptions(
 				AbuseLogger::CONSTRUCTOR_OPTIONS,

tests/phpunit/unit/EditRevUpdaterTest.php

@@ -0,0 +1,207 @@
+<?php
+
+namespace MediaWiki\Extension\AbuseFilter\Tests\Unit;
+
+use InvalidArgumentException;
+use MediaWiki\Extension\AbuseFilter\CentralDBManager;
+use MediaWiki\Extension\AbuseFilter\EditRevUpdater;
+use MediaWiki\Linker\LinkTarget;
+use MediaWiki\Revision\MutableRevisionRecord;
+use MediaWiki\Revision\RevisionLookup;
+use MediaWikiUnitTestCase;
+use Title;
+use TitleValue;
+use Wikimedia\Rdbms\IDatabase;
+use Wikimedia\Rdbms\ILoadBalancer;
+use WikiPage;
+
+/**
+ * @group Test
+ * @group AbuseFilter
+ * @coversDefaultClass \MediaWiki\Extension\AbuseFilter\EditRevUpdater
+ */
+class EditRevUpdaterTest extends MediaWikiUnitTestCase {
+
+	/**
+	 * @covers ::__construct
+	 */
+	public function testConstruct() {
+		$this->assertInstanceOf(
+			EditRevUpdater::class,
+			new EditRevUpdater(
+				$this->createMock( CentralDBManager::class ),
+				$this->createMock( RevisionLookup::class ),
+				$this->createMock( ILoadBalancer::class ),
+				''
+			)
+		);
+	}
+
+	/**
+	 * @param IDatabase|null $localDB
+	 * @param IDatabase|null $centralDB
+	 * @param RevisionLookup|null $revLookup
+	 * @return EditRevUpdater
+	 */
+	private function getUpdater(
+		IDatabase $localDB = null,
+		IDatabase $centralDB = null,
+		RevisionLookup $revLookup = null
+	) : EditRevUpdater {
+		$localDB = $localDB ?? $this->createMock( IDatabase::class );
+		$lb = $this->createMock( ILoadBalancer::class );
+		$lb->method( 'getConnectionRef' )->willReturn( $localDB );
+		$centralDB = $centralDB ?? $this->createMock( IDatabase::class );
+		$dbManager = $this->createMock( CentralDBManager::class );
+		$dbManager->method( 'getConnection' )->willReturn( $centralDB );
+		return new EditRevUpdater(
+			$dbManager,
+			$revLookup ?? $this->createMock( RevisionLookup::class ),
+			$lb,
+			'fake-wiki-id'
+		);
+	}
+
+	/**
+	 * @param LinkTarget $target
+	 * @return array
+	 */
+	private function getPageAndRev( LinkTarget $target ) : array {
+		$title = Title::newFromLinkTarget( $target );
+		// Legacy code. Yay.
+		$title->mArticleID = 123456;
+		return [ new WikiPage( $title ), new MutableRevisionRecord( $title ) ];
+	}
+
+	/**
+	 * @covers ::updateRev
+	 * @covers ::getCacheKey
+	 */
+	public function testUpdateRev_noIDs() {
+		$titleValue = new TitleValue( NS_PROJECT, 'EditRevUpdater' );
+		$this->assertFalse( $this->getUpdater()->updateRev( ...$this->getPageAndRev( $titleValue ) ) );
+	}
+
+	/**
+	 * @covers ::setLastEditPage
+	 * @covers ::setLogIdsForTarget
+	 * @covers ::updateRev
+	 * @covers ::getCacheKey
+	 */
+	public function testUpdateRev_differentPages() {
+		$titleValue = new TitleValue( NS_PROJECT, 'EditRevUpdater' );
+		$updater = $this->getUpdater();
+		$title = Title::makeTitle( NS_HELP, 'Foobar' );
+		// Legacy code. Yay.
+		$title->mArticleID = 123456;
+		$updater->setLastEditPage( new WikiPage( $title ) );
+		$updater->setLogIdsForTarget( $titleValue, [ 'local' => [ 1, 2 ], 'global' => [] ] );
+		$this->assertFalse( $updater->updateRev( ...$this->getPageAndRev( $titleValue ) ) );
+	}
+
+	/**
+	 * @covers ::setLastEditPage
+	 * @covers ::setLogIdsForTarget
+	 * @covers ::updateRev
+	 * @covers ::getCacheKey
+	 */
+	public function testUpdateRev_nullEdit() {
+		$titleValue = new TitleValue( NS_PROJECT, 'EditRevUpdater' );
+		[ $page, $rev ] = $this->getPageAndRev( $titleValue );
+		$rev->setParentId( 42 );
+		$revLookup = $this->createMock( RevisionLookup::class );
+		$revLookup->expects( $this->once() )->method( 'getRevisionById' )->with( 42 )->willReturn( $rev );
+		$updater = $this->getUpdater( null, null, $revLookup );
+		$updater->setLastEditPage( $page );
+		$updater->setLogIdsForTarget( $titleValue, [ 'local' => [ 1 ], 'global' => [ 1 ] ] );
+
+		$this->assertFalse( $updater->updateRev( $page, $rev ) );
+	}
+
+	/**
+	 * @param array $ids
+	 * @covers ::setLastEditPage
+	 * @covers ::setLogIdsForTarget
+	 * @covers ::updateRev
+	 * @covers ::getCacheKey
+	 * @dataProvider provideIDsSuccess
+	 */
+	public function testUpdateRev_success( array $ids ) {
+		$titleValue = new TitleValue( NS_PROJECT, 'EditRevUpdater' );
+		[ $page, $rev ] = $this->getPageAndRev( $titleValue );
+		$localDB = $this->createMock( IDatabase::class );
+		$localDB->expects( $ids['local'] ? $this->once() : $this->never() )->method( 'update' );
+		$centralDB = $this->createMock( IDatabase::class );
+		$centralDB->expects( $ids['global'] ? $this->once() : $this->never() )->method( 'update' );
+		$updater = $this->getUpdater( $localDB, $centralDB );
+		$updater->setLastEditPage( $page );
+		$updater->setLogIdsForTarget( $titleValue, $ids );
+
+		$this->assertTrue( $updater->updateRev( $page, $rev ) );
+	}
+
+	public function provideIDsSuccess() : array {
+		return [
+			'local only' => [ [ 'local' => [ 1, 2 ], 'global' => [] ] ],
+			'global only' => [ [ 'local' => [], 'global' => [ 1, 2 ] ] ],
+			'local and global' => [ [ 'local' => [ 1, 2 ], 'global' => [ 1, 2 ] ] ],
+		];
+	}
+
+	/**
+	 * @covers ::setLastEditPage
+	 * @covers ::setLogIdsForTarget
+	 * @covers ::updateRev
+	 * @covers ::getCacheKey
+	 */
+	public function testUpdateRev_multipleTitles() {
+		$goodTitleValue = new TitleValue( NS_PROJECT, 'EditRevUpdater' );
+		$badTitleValue = new TitleValue( NS_PROJECT, 'These should not be used' );
+		$goodIDs = [ 'local' => [ 1, 2 ], 'global' => [] ];
+		$badIDs = [ 'local' => [], 'global' => [ 1, 2 ] ];
+		[ $page, $rev ] = $this->getPageAndRev( $goodTitleValue );
+		$localDB = $this->createMock( IDatabase::class );
+		$localDB->expects( $this->once() )->method( 'update' );
+		$centralDB = $this->createMock( IDatabase::class );
+		$centralDB->expects( $this->never() )->method( 'update' );
+		$updater = $this->getUpdater( $localDB, $centralDB );
+		$updater->setLastEditPage( $page );
+		$updater->setLogIdsForTarget( $goodTitleValue, $goodIDs );
+		$updater->setLogIdsForTarget( $badTitleValue, $badIDs );
+
+		$this->assertTrue( $updater->updateRev( $page, $rev ) );
+	}
+
+	/**
+	 * @covers ::clearLastEditPage
+	 * @covers ::updateRev
+	 */
+	public function testClearLastEditPage() {
+		$titleValue = new TitleValue( NS_PROJECT, 'EditRevUpdater-clear' );
+		[ $page, $revisionRecord ] = $this->getPageAndRev( $titleValue );
+		$updater = $this->getUpdater();
+		$updater->setLastEditPage( $page );
+		$updater->setLogIdsForTarget( $titleValue, [ 'local' => [ 1, 2 ], 'global' => [] ] );
+		$updater->clearLastEditPage();
+		$this->assertFalse( $updater->updateRev( $page, $revisionRecord ) );
+	}
+
+	/**
+	 * @param array $ids
+	 * @covers ::setLogIdsForTarget
+	 * @dataProvider provideInvalidIDs
+	 */
+	public function testSetLogIdsForTarget_invalid( array $ids ) {
+		$updater = $this->getUpdater();
+		$this->expectException( InvalidArgumentException::class );
+		$updater->setLogIdsForTarget( new TitleValue( NS_MAIN, 'x' ), $ids );
+	}
+
+	public function provideInvalidIDs() : array {
+		return [
+			'empty' => [ [] ],
+			'missing key' => [ [ 'local' => [ 1 ] ] ],
+			'extra key' => [ [ 'local' => [ 1 ], 'global' => [ 1 ], 'foo' => [ 1 ] ] ],
+		];
+	}
+}