Remove deprecated $wgAbuseFilterCustomActionsHandlers

Extensions should now specify custom actions using the
AbuseFilterCustomActions hook.

Change-Id: Id21640d406b18c627eedff39d3f246cf21e042b3

extension.json

@@ -418,10 +418,6 @@
 			"value": 5,
 			"description": "Duration, in days, for which users' autopromotion is blocked by filters."
 		},
-		"AbuseFilterCustomActionsHandlers": {
-			"value": [],
-			"description": "Callback functions for custom actions. DEPRECATED since 1.36! Use the AbuseFilterCustomActions hook instead."
-		},
 		"AbuseFilterValidGroups": {
 			"value": [ "default" ],
 			"description": "The list of 'groups' filters can be divided into – used for applying edit filters to certain types of actions. By default there is only one group."

includes/Consequences/Consequence/BCConsequence.php

@@ -1,70 +0,0 @@
-<?php
-
-namespace MediaWiki\Extension\AbuseFilter\Consequences\Consequence;
-
-use LogicException;
-use MediaWiki\Extension\AbuseFilter\Consequences\Parameters;
-use MediaWiki\Extension\AbuseFilter\Variables\VariableHolder;
-use Title;
-
-/**
- * BC class for custom consequences specified via $wgAbuseFilterCustomActionsHandlers
- * @internal Temporary class
- * @codeCoverageIgnore
- */
-class BCConsequence extends Consequence implements HookAborterConsequence {
-	/** @var array */
-	private $rawParams;
-	/** @var VariableHolder */
-	private $vars;
-	/** @var callable */
-	private $callback;
-
-	/** @var string|null */
-	private $message;
-
-	/**
-	 * @param Parameters $parameters
-	 * @param array $rawParams Parameters as stored in the DB
-	 * @param VariableHolder $vars
-	 * @param callable $cb
-	 */
-	public function __construct(
-		Parameters $parameters,
-		array $rawParams,
-		VariableHolder $vars,
-		callable $cb
-	) {
-		parent::__construct( $parameters );
-		$this->rawParams = $rawParams;
-		$this->vars = $vars;
-		$this->callback = $cb;
-	}
-
-	/**
-	 * @inheritDoc
-	 */
-	public function execute() : bool {
-		$msg = call_user_func(
-			$this->callback,
-			$this->parameters->getAction(),
-			$this->rawParams,
-			Title::castFromLinkTarget( $this->parameters->getTarget() ),
-			$this->vars,
-			$this->parameters->getFilter()->getName(),
-			$this->parameters->getFilter()->getID()
-		);
-		$this->message = $msg;
-		return true;
-	}
-
-	/**
-	 * @inheritDoc
-	 */
-	public function getMessage(): array {
-		if ( $this->message === null ) {
-			throw new LogicException( 'No message, did you call execute()?' );
-		}
-		return [ $this->message ];
-	}
-}

includes/Consequences/ConsequencesExecutor.php

@@ -4,7 +4,6 @@ namespace MediaWiki\Extension\AbuseFilter\Consequences;
 
 use MediaWiki\Block\BlockUser;
 use MediaWiki\Config\ServiceOptions;
-use MediaWiki\Extension\AbuseFilter\Consequences\Consequence\BCConsequence;
 use MediaWiki\Extension\AbuseFilter\Consequences\Consequence\Consequence;
 use MediaWiki\Extension\AbuseFilter\Consequences\Consequence\ConsequencesDisablerConsequence;
 use MediaWiki\Extension\AbuseFilter\Consequences\Consequence\HookAborterConsequence;
@@ -23,7 +22,6 @@ class ConsequencesExecutor {
 		'AbuseFilterBlockDuration',
 		'AbuseFilterAnonBlockDuration',
 		'AbuseFilterBlockAutopromoteDuration',
-		'AbuseFilterCustomActionsHandlers',
 	];
 
 	/** @var ConsequencesLookup */
@@ -265,17 +263,9 @@ class ConsequencesExecutor {
 				}
 				return $this->consFactory->newTag( $baseConsParams, $accountName, $rawParams );
 			default:
-				$customHandlers = $this->options->get( 'AbuseFilterCustomActionsHandlers' );
 				if ( array_key_exists( $actionName, $this->consRegistry->getCustomActions() ) ) {
 					$callback = $this->consRegistry->getCustomActions()[$actionName];
 					return $callback( $baseConsParams, $rawParams );
-				} elseif ( isset( $customHandlers[$actionName] ) ) {
-					wfDeprecated(
-						'$wgAbuseFilterCustomActionsHandlers; use the AbuseFilterCustomActions hook instead',
-						'1.36'
-					);
-					$customFunction = $customHandlers[$actionName];
-					return new BCConsequence( $baseConsParams, $rawParams, $this->vars, $customFunction );
 				} else {
 					$this->logger->warning( "Unrecognised action $actionName" );
 					return null;
@@ -324,18 +314,6 @@ class ConsequencesExecutor {
 	 * @todo Improve return value
 	 */
 	private function takeConsequenceAction( Consequence $consequence ) : array {
-		// Special case
-		if ( $consequence instanceof BCConsequence ) {
-			$consequence->execute();
-			try {
-				$message = $consequence->getMessage();
-			} catch ( \LogicException $_ ) {
-				// Swallow. Sigh.
-				$message = null;
-			}
-			return [ true, $message ];
-		}
-
 		$res = $consequence->execute();
 		if ( $res && $consequence instanceof HookAborterConsequence ) {
 			$message = $consequence->getMessage();

includes/Consequences/ConsequencesRegistry.php

@@ -21,8 +21,6 @@ class ConsequencesRegistry {
 	private $hookRunner;
 	/** @var bool[] */
 	private $configActions;
-	/** @var callable[] */
-	private $customHandlers;
 
 	/** @var string[]|null */
 	private $dangerousActionsCache;
@@ -32,16 +30,13 @@ class ConsequencesRegistry {
 	/**
 	 * @param AbuseFilterHookRunner $hookRunner
 	 * @param bool[] $configActions
-	 * @param callable[] $customHandlers
 	 */
 	public function __construct(
 		AbuseFilterHookRunner $hookRunner,
-		array $configActions,
-		array $customHandlers
+		array $configActions
 	) {
 		$this->hookRunner = $hookRunner;
 		$this->configActions = $configActions;
-		$this->customHandlers = $customHandlers;
 	}
 
 	/**
@@ -67,7 +62,6 @@ class ConsequencesRegistry {
 		return array_unique(
 			array_merge(
 				array_keys( $this->configActions ),
-				array_keys( $this->customHandlers ),
 				array_keys( $this->getCustomActions() )
 			)
 		);

includes/ServiceWiring.php

@@ -229,8 +229,7 @@ return [
 	ConsequencesRegistry::SERVICE_NAME => function ( MediaWikiServices $services ): ConsequencesRegistry {
 		return new ConsequencesRegistry(
 			$services->get( AbuseFilterHookRunner::SERVICE_NAME ),
-			$services->getMainConfig()->get( 'AbuseFilterActions' ),
-			$services->getMainConfig()->get( 'AbuseFilterCustomActionsHandlers' )
+			$services->getMainConfig()->get( 'AbuseFilterActions' )
 		);
 	},
 	AbuseLoggerFactory::SERVICE_NAME => function ( MediaWikiServices $services ) : AbuseLoggerFactory {

tests/phpunit/unit/Consequences/ConsequencesRegistryTest.php

@@ -23,7 +23,7 @@ class ConsequencesRegistryTest extends MediaWikiUnitTestCase {
 		$hookRunner = $this->createMock( AbuseFilterHookRunner::class );
 		$this->assertInstanceOf(
 			ConsequencesRegistry::class,
-			new ConsequencesRegistry( $hookRunner, [], [] )
+			new ConsequencesRegistry( $hookRunner, [] )
 		);
 	}
 
@@ -32,7 +32,6 @@ class ConsequencesRegistryTest extends MediaWikiUnitTestCase {
 	 */
 	public function testGetAllActionNames() {
 		$configActions = [ 'nothing' => false, 'rickroll' => true ];
-		$customHandlers = [ 'blahblah' => 'strlen' ];
 		$customActionName = 'spell';
 		$hookRunner = $this->createMock( AbuseFilterHookRunner::class );
 		$hookRunner->method( 'onAbuseFilterCustomActions' )->willReturnCallback(
@@ -40,12 +39,8 @@ class ConsequencesRegistryTest extends MediaWikiUnitTestCase {
 				$actions[$customActionName] = 'strlen';
 			}
 		);
-		$expected = [ 'nothing', 'rickroll', 'blahblah', 'spell' ];
-		$registry = new ConsequencesRegistry(
-			$hookRunner,
-			$configActions,
-			$customHandlers
-		);
+		$expected = [ 'nothing', 'rickroll', 'spell' ];
+		$registry = new ConsequencesRegistry( $hookRunner, $configActions );
 		$this->assertSame( $expected, $registry->getAllActionNames() );
 	}
 
@@ -54,7 +49,6 @@ class ConsequencesRegistryTest extends MediaWikiUnitTestCase {
 	 */
 	public function testGetAllEnabledActionNames() {
 		$configActions = [ 'nothing' => false, 'rickroll' => true ];
-		$customHandlers = [ 'blahblah' => 'strlen' ];
 		$customActionName = 'spell';
 		$hookRunner = $this->createMock( AbuseFilterHookRunner::class );
 		$hookRunner->method( 'onAbuseFilterCustomActions' )->willReturnCallback(
@@ -62,12 +56,8 @@ class ConsequencesRegistryTest extends MediaWikiUnitTestCase {
 				$actions[$customActionName] = 'strlen';
 			}
 		);
-		$expected = [ 'rickroll', 'blahblah', 'spell' ];
-		$registry = new ConsequencesRegistry(
-			$hookRunner,
-			$configActions,
-			$customHandlers
-		);
+		$expected = [ 'rickroll', 'spell' ];
+		$registry = new ConsequencesRegistry( $hookRunner, $configActions );
 		$this->assertSame( $expected, $registry->getAllEnabledActionNames() );
 	}
 
@@ -79,7 +69,7 @@ class ConsequencesRegistryTest extends MediaWikiUnitTestCase {
 		$regReflection = new ReflectionClass( ConsequencesRegistry::class );
 		$expected = $regReflection->getConstant( 'DANGEROUS_ACTIONS' );
 
-		$registry = new ConsequencesRegistry( $this->createMock( AbuseFilterHookRunner::class ), [], [] );
+		$registry = new ConsequencesRegistry( $this->createMock( AbuseFilterHookRunner::class ), [] );
 		$this->assertSame( $expected, $registry->getDangerousActionNames() );
 	}
 
@@ -94,7 +84,7 @@ class ConsequencesRegistryTest extends MediaWikiUnitTestCase {
 				$array[] = $extraDangerous;
 			}
 		);
-		$registry = new ConsequencesRegistry( $hookRunner, [], [] );
+		$registry = new ConsequencesRegistry( $hookRunner, [] );
 		$this->assertContains( $extraDangerous, $registry->getDangerousActionNames() );
 	}
 
@@ -111,7 +101,7 @@ class ConsequencesRegistryTest extends MediaWikiUnitTestCase {
 				$actions[$customActionName] = $customAction;
 			}
 		);
-		$registry = new ConsequencesRegistry( $hookRunner, [], [] );
+		$registry = new ConsequencesRegistry( $hookRunner, [] );
 		$this->assertSame( [ $customActionName => $customAction ], $registry->getCustomActions() );
 	}
 
@@ -127,7 +117,7 @@ class ConsequencesRegistryTest extends MediaWikiUnitTestCase {
 				$actions[$invalidKey] = $this->createMock( Consequence::class );
 			}
 		);
-		$registry = new ConsequencesRegistry( $hookRunner, [], [] );
+		$registry = new ConsequencesRegistry( $hookRunner, [] );
 		$this->expectException( RuntimeException::class );
 		$registry->getCustomActions();
 	}
@@ -144,7 +134,7 @@ class ConsequencesRegistryTest extends MediaWikiUnitTestCase {
 				$actions['myaction'] = $invalidValue;
 			}
 		);
-		$registry = new ConsequencesRegistry( $hookRunner, [], [] );
+		$registry = new ConsequencesRegistry( $hookRunner, [] );
 		$this->expectException( RuntimeException::class );
 		$registry->getCustomActions();
 	}

tests/phpunit/unit/FilterCompareTest.php

@@ -33,7 +33,7 @@ class FilterCompareTest extends MediaWikiUnitTestCase {
 			'throttle', 'warn', 'disallow', 'blockautopromote', 'block', 'rangeblock', 'degroup', 'tag'
 		];
 		$allActions = array_fill_keys( $allActions, true );
-		$registry = new ConsequencesRegistry( $this->createMock( AbuseFilterHookRunner::class ), $allActions, [] );
+		$registry = new ConsequencesRegistry( $this->createMock( AbuseFilterHookRunner::class ), $allActions );
 		$compare = new FilterCompare( $registry );
 
 		$this->assertSame( $expected, $compare->compareVersions( $firstVersion, $secondVersion ) );

tests/phpunit/unit/FilterImporterTest.php

@@ -49,8 +49,7 @@ class FilterImporterTest extends MediaWikiUnitTestCase {
 		$actions = array_fill_keys( $actions ?? [ 'warn', 'disallow', 'block' ], true );
 		$registry = new ConsequencesRegistry(
 			$this->createMock( AbuseFilterHookRunner::class ),
-			$actions,
-			[]
+			$actions
 		);
 		return new FilterImporter(
 			new ServiceOptions(