Always pass a User object to SpamBlacklist::filter

There are some usages outside of SpamBlacklist that must be fixed. After
doing that, the signature should be updated to make the user
non-optional.

Note: I've changed the signature because external callers only pass the
first two parameters. Hence, it's easier to change it now, add a
User parameter to the callers and then make it non-optional, than having
to pass $preserveLog and $mode in all callers and then swapping the
order (as that would break the world).

Change-Id: I0714eb9dbc6af3c775ab7a81cb4b59e687183f77

includes/ApiSpamBlacklist.php

@@ -31,7 +31,12 @@
 class ApiSpamBlacklist extends ApiBase {
 	public function execute() {
 		$params = $this->extractRequestParams();
-		$matches = BaseBlacklist::getInstance( 'spam' )->filter( $params['url'], null, true );
+		$matches = BaseBlacklist::getSpamBlacklist()->filter(
+			$params['url'],
+			null,
+			$this->getUser(),
+			true
+		);
 		$res = $this->getResult();
 
 		if ( $matches !== false ) {

includes/BaseBlacklist.php

@@ -69,10 +69,16 @@ abstract class BaseBlacklist {
 	/**
 	 * @param array $links
 	 * @param ?Title $title
+	 * @param User|null $user
 	 * @param bool $preventLog
 	 * @return mixed
 	 */
-	abstract public function filter( array $links, ?Title $title, $preventLog = false );
+	abstract public function filter(
+		array $links,
+		?Title $title,
+		User $user = null,
+		$preventLog = false
+	);
 
 	/**
 	 * Adds a blacklist class to the registry
@@ -431,8 +437,9 @@ abstract class BaseBlacklist {
 	/**
 	 * @param Title $title
 	 * @param string[] $entries
+	 * @param User $user
 	 */
-	public function warmCachesForFilter( Title $title, array $entries ) {
+	public function warmCachesForFilter( Title $title, array $entries, User $user ) {
 		// subclass this
 	}
 }

includes/EmailBlacklist.php

@@ -5,12 +5,9 @@
  */
 class EmailBlacklist extends BaseBlacklist {
 	/**
-	 * @param array $links
-	 * @param ?Title $title
-	 * @param bool $preventLog
-	 * @return mixed
+	 * @inheritDoc
 	 */
-	public function filter( array $links, ?Title $title, $preventLog = false ) {
+	public function filter( array $links, ?Title $title, User $user = null, $preventLog = false ) {
 		throw new LogicException( __CLASS__ . ' cannot be used to filter links.' );
 	}
 

includes/SpamBlacklist.php

@@ -35,20 +35,20 @@ class SpamBlacklist extends BaseBlacklist {
 	 *               This is used to load the old links already on the page, so
 	 *               the filter is only applied to links that got added. If not given,
 	 *               the filter is applied to all $links.
+	 * @param User|null $user Relevant user, only optional for backwards compatibility
 	 * @param bool $preventLog Whether to prevent logging of hits. Set to true when
 	 *               the action is testing the links rather than attempting to save them
 	 *               (e.g. the API spamblacklist action)
 	 * @param string $mode Either 'check' or 'stash'
-	 * @param User|null $user Relevant user, only optional for backwards compatibility
 	 *
 	 * @return string[]|bool Matched text(s) if the edit should not be allowed; false otherwise
 	 */
 	public function filter(
 		array $links,
 		?Title $title,
+		User $user = null,
 		$preventLog = false,
-		$mode = 'check',
-		User $user = null
+		$mode = 'check'
 	) {
 		$statsd = MediaWikiServices::getInstance()->getStatsdDataFactory();
 		$cache = ObjectCache::getLocalClusterInstance();
@@ -196,8 +196,14 @@ class SpamBlacklist extends BaseBlacklist {
 		);
 	}
 
-	public function warmCachesForFilter( Title $title, array $entries ) {
-		$this->filter( $entries, $title, true /* no logging */, 'stash' );
+	public function warmCachesForFilter( Title $title, array $entries, User $user ) {
+		$this->filter(
+			$entries,
+			$title,
+			$user,
+			true /* no logging */,
+			'stash'
+		);
 	}
 
 	/**

includes/SpamBlacklistHooks.php

@@ -42,7 +42,7 @@ class SpamBlacklistHooks {
 		}
 
 		$spamObj = BaseBlacklist::getSpamBlacklist();
-		$matches = $spamObj->filter( $links, $title );
+		$matches = $spamObj->filter( $links, $title, $user );
 
 		if ( $matches !== false ) {
 			$error = new ApiMessage(
@@ -62,11 +62,13 @@ class SpamBlacklistHooks {
 	public static function onParserOutputStashForEdit(
 		WikiPage $page,
 		Content $content,
-		ParserOutput $output
+		ParserOutput $output,
+		$summary,
+		User $user
 	) {
 		$links = array_keys( $output->getExternalLinks() );
 		$spamObj = BaseBlacklist::getSpamBlacklist();
-		$spamObj->warmCachesForFilter( $page->getTitle(), $links );
+		$spamObj->warmCachesForFilter( $page->getTitle(), $links, $user );
 	}
 
 	/**
@@ -211,7 +213,7 @@ class SpamBlacklistHooks {
 		}
 
 		$spamObj = BaseBlacklist::getSpamBlacklist();
-		$matches = $spamObj->filter( $links, $title );
+		$matches = $spamObj->filter( $links, $title, $user );
 
 		if ( $matches !== false ) {
 			$error = new ApiMessage(

tests/phpunit/SpamBlacklistTest.php

@@ -63,7 +63,11 @@ class SpamBlacklistTest extends MediaWikiTestCase {
 	 * @dataProvider spamProvider
 	 */
 	public function testSpam( $links, $expected ) {
-		$returnValue = $this->spamFilter->filter( $links, Title::newMainPage() );
+		$returnValue = $this->spamFilter->filter(
+			$links,
+			Title::newMainPage(),
+			$this->createMock( User::class )
+		);
 		$this->assertEquals( $expected, $returnValue );
 	}