Clean up (Global)AbuseFilterPager

- Inject dependencies.
- Make class variables private or protected and rename them.

Untangling the circular dependency is left for a future patch.

Change-Id: I5d625e30171bfbf60d9f5a94fa50475fdfe853dd

includes/Pager/AbuseFilterPager.php

@@ -5,7 +5,8 @@ namespace MediaWiki\Extension\AbuseFilter\Pager;
 use FakeResultWrapper;
 use Linker;
 use LogicException;
-use MediaWiki\Extension\AbuseFilter\AbuseFilterServices;
+use MediaWiki\Extension\AbuseFilter\AbuseFilterPermissionManager;
+use MediaWiki\Extension\AbuseFilter\SpecsFormatter;
 use MediaWiki\Extension\AbuseFilter\View\AbuseFilterViewList;
 use MediaWiki\Linker\LinkRenderer;
 use MWException;
@@ -19,42 +20,55 @@ use Wikimedia\AtEase\AtEase;
  */
 class AbuseFilterPager extends TablePager {
 
+	/** @var AbuseFilterPermissionManager */
+	protected $afPermManager;
+
+	/** @var SpecsFormatter */
+	protected $specsFormatter;
+
 	/**
 	 * @var AbuseFilterViewList The associated page
 	 */
-	public $mPage;
+	protected $mPage;
 	/**
 	 * @var array Query WHERE conditions
 	 */
-	public $mConds;
+	protected $conds;
 	/**
 	 * @var string|null The pattern being searched
 	 */
-	private $mSearchPattern;
+	private $searchPattern;
 	/**
 	 * @var string|null The pattern search mode (LIKE, RLIKE or IRLIKE)
 	 */
-	private $mSearchMode;
+	private $searchMode;
 
 	/**
 	 * @param AbuseFilterViewList $page
-	 * @param array $conds
 	 * @param LinkRenderer $linkRenderer
+	 * @param AbuseFilterPermissionManager $afPermManager
+	 * @param SpecsFormatter $specsFormatter
+	 * @param array $conds
 	 * @param ?string $searchPattern Null if no pattern was specified
 	 * @param ?string $searchMode
 	 */
 	public function __construct(
 		AbuseFilterViewList $page,
-		$conds,
 		LinkRenderer $linkRenderer,
+		AbuseFilterPermissionManager $afPermManager,
+		SpecsFormatter $specsFormatter,
+		array $conds,
 		?string $searchPattern,
 		?string $searchMode
 	) {
+		// needed by parent's constructor call
+		$this->afPermManager = $afPermManager;
+		$this->specsFormatter = $specsFormatter;
 		parent::__construct( $page->getContext(), $linkRenderer );
 		$this->mPage = $page;
-		$this->mConds = $conds;
-		$this->mSearchPattern = $searchPattern;
-		$this->mSearchMode = $searchMode;
+		$this->conds = $conds;
+		$this->searchPattern = $searchPattern;
+		$this->searchMode = $searchMode;
 	}
 
 	/**
@@ -80,7 +94,7 @@ class AbuseFilterPager extends TablePager {
 				'af_group',
 				'af_throttled'
 			],
-			'conds' => $this->mConds,
+			'conds' => $this->conds,
 		];
 	}
 
@@ -90,7 +104,7 @@ class AbuseFilterPager extends TablePager {
 	 * Otherwise, it does a query with no limit and then slices the results à la ContribsPager.
 	 */
 	public function reallyDoQuery( $offset, $limit, $order ) {
-		if ( $this->mSearchMode === null ) {
+		if ( $this->searchMode === null ) {
 			return parent::reallyDoQuery( $offset, $limit, $order );
 		}
 
@@ -126,8 +140,8 @@ class AbuseFilterPager extends TablePager {
 	 * @throws LogicException
 	 */
 	private function matchesPattern( $subject ) {
-		$pattern = $this->mSearchPattern;
-		switch ( $this->mSearchMode ) {
+		$pattern = $this->searchPattern;
+		switch ( $this->searchMode ) {
 			case 'RLIKE':
 				return (bool)preg_match( "/$pattern/u", $subject );
 			case 'IRLIKE':
@@ -135,7 +149,7 @@ class AbuseFilterPager extends TablePager {
 			case 'LIKE':
 				return mb_stripos( $subject, $pattern ) !== false;
 			default:
-				throw new LogicException( "Unknown search type {$this->mSearchMode}" );
+				throw new LogicException( "Unknown search type {$this->searchMode}" );
 		}
 	}
 
@@ -145,8 +159,6 @@ class AbuseFilterPager extends TablePager {
 	 * @see Pager::getFieldNames()
 	 */
 	public function getFieldNames() {
-		$afPermManager = AbuseFilterServices::getPermissionManager();
-
 		$headers = [
 			'af_id' => 'abusefilter-list-id',
 			'af_public_comments' => 'abusefilter-list-public',
@@ -157,11 +169,11 @@ class AbuseFilterPager extends TablePager {
 		];
 
 		$user = $this->getUser();
-		if ( $afPermManager->canSeeLogDetails( $user ) ) {
+		if ( $this->afPermManager->canSeeLogDetails( $user ) ) {
 			$headers['af_hit_count'] = 'abusefilter-list-hitcount';
 		}
 
-		if ( $afPermManager->canViewPrivateFilters( $user ) && $this->mSearchMode !== null ) {
+		if ( $this->afPermManager->canViewPrivateFilters( $user ) && $this->searchMode !== null ) {
 			// This is also excluded in the default view
 			$headers['af_pattern'] = 'abusefilter-list-pattern';
 		}
@@ -186,8 +198,6 @@ class AbuseFilterPager extends TablePager {
 		$lang = $this->getLanguage();
 		$user = $this->getUser();
 		$linkRenderer = $this->getLinkRenderer();
-		$specsFormatter = AbuseFilterServices::getSpecsFormatter();
-		$specsFormatter->setMessageLocalizer( $this->getContext() );
 		$row = $this->mCurrentRow;
 
 		switch ( $name ) {
@@ -207,7 +217,7 @@ class AbuseFilterPager extends TablePager {
 				$actions = explode( ',', $value );
 				$displayActions = [];
 				foreach ( $actions as $action ) {
-					$displayActions[] = $specsFormatter->getActionDisplay( $action );
+					$displayActions[] = $this->specsFormatter->getActionDisplay( $action );
 				}
 				return $lang->commaList( $displayActions );
 			case 'af_enabled':
@@ -232,8 +242,7 @@ class AbuseFilterPager extends TablePager {
 				$msg = $value ? 'abusefilter-hidden' : 'abusefilter-unhidden';
 				return $this->msg( $msg )->parse();
 			case 'af_hit_count':
-				$afPermManager = AbuseFilterServices::getPermissionManager();
-				if ( $afPermManager->canSeeLogDetailsForFilter( $user, $row->af_hidden ) ) {
+				if ( $this->afPermManager->canSeeLogDetailsForFilter( $user, $row->af_hidden ) ) {
 					$count_display = $this->msg( 'abusefilter-hitcount' )
 						->numParams( $value )->text();
 					$link = $linkRenderer->makeKnownLink(
@@ -276,7 +285,7 @@ class AbuseFilterPager extends TablePager {
 						wfEscapeWikiText( $row->af_user_text )
 					)->parse();
 			case 'af_group':
-				return $specsFormatter->nameGroup( $value );
+				return $this->specsFormatter->nameGroup( $value );
 			default:
 				throw new MWException( "Unknown row type $name!" );
 		}
@@ -289,16 +298,16 @@ class AbuseFilterPager extends TablePager {
 	 * @return string
 	 */
 	private function getHighlightedPattern( stdClass $row ) {
-		if ( $this->mSearchMode === null ) {
+		if ( $this->searchMode === null ) {
 			throw new LogicException( 'Cannot search without a mode.' );
 		}
 		$maxLen = 50;
-		if ( $this->mSearchMode === 'LIKE' ) {
-			$position = mb_stripos( $row->af_pattern, $this->mSearchPattern );
-			$length = mb_strlen( $this->mSearchPattern );
+		if ( $this->searchMode === 'LIKE' ) {
+			$position = mb_stripos( $row->af_pattern, $this->searchPattern );
+			$length = mb_strlen( $this->searchPattern );
 		} else {
-			$regex = '/' . $this->mSearchPattern . '/u';
-			if ( $this->mSearchMode === 'IRLIKE' ) {
+			$regex = '/' . $this->searchPattern . '/u';
+			if ( $this->searchMode === 'IRLIKE' ) {
 				$regex .= 'i';
 			}
 
@@ -380,8 +389,6 @@ class AbuseFilterPager extends TablePager {
 	 * @return bool
 	 */
 	public function isFieldSortable( $name ) {
-		$afPermManager = AbuseFilterServices::getPermissionManager();
-
 		$sortable_fields = [
 			'af_id',
 			'af_enabled',
@@ -389,7 +396,7 @@ class AbuseFilterPager extends TablePager {
 			'af_hidden',
 			'af_group',
 		];
-		if ( $afPermManager->canSeeLogDetails( $this->getUser() ) ) {
+		if ( $this->afPermManager->canSeeLogDetails( $this->getUser() ) ) {
 			$sortable_fields[] = 'af_hit_count';
 			$sortable_fields[] = 'af_public_comments';
 		}

includes/Pager/GlobalAbuseFilterPager.php

@@ -2,7 +2,9 @@
 
 namespace MediaWiki\Extension\AbuseFilter\Pager;
 
-use MediaWiki\Extension\AbuseFilter\AbuseFilterServices;
+use MediaWiki\Extension\AbuseFilter\AbuseFilterPermissionManager;
+use MediaWiki\Extension\AbuseFilter\CentralDBManager;
+use MediaWiki\Extension\AbuseFilter\SpecsFormatter;
 use MediaWiki\Extension\AbuseFilter\View\AbuseFilterViewList;
 use MediaWiki\Linker\LinkRenderer;
 
@@ -10,15 +12,25 @@ use MediaWiki\Linker\LinkRenderer;
  * Class to build paginated filter list for wikis using global abuse filters
  */
 class GlobalAbuseFilterPager extends AbuseFilterPager {
+
 	/**
 	 * @param AbuseFilterViewList $page
-	 * @param array $conds
 	 * @param LinkRenderer $linkRenderer
+	 * @param AbuseFilterPermissionManager $afPermManager
+	 * @param SpecsFormatter $specsFormatter
+	 * @param CentralDBManager $centralDBManager
+	 * @param array $conds
 	 */
-	public function __construct( AbuseFilterViewList $page, $conds, LinkRenderer $linkRenderer ) {
-		parent::__construct( $page, $conds, $linkRenderer, null, null );
-		$this->mDb = wfGetDB(
-			DB_REPLICA, [], $this->getConfig()->get( 'AbuseFilterCentralDB' ) );
+	public function __construct(
+		AbuseFilterViewList $page,
+		LinkRenderer $linkRenderer,
+		AbuseFilterPermissionManager $afPermManager,
+		SpecsFormatter $specsFormatter,
+		CentralDBManager $centralDBManager,
+		array $conds
+	) {
+		parent::__construct( $page, $linkRenderer, $afPermManager, $specsFormatter, $conds, null, null );
+		$this->mDb = $centralDBManager->getConnection( DB_REPLICA );
 	}
 
 	/**
@@ -28,8 +40,6 @@ class GlobalAbuseFilterPager extends AbuseFilterPager {
 	 */
 	public function formatValue( $name, $value ) {
 		$lang = $this->getLanguage();
-		$specsFormatter = AbuseFilterServices::getSpecsFormatter();
-		$specsFormatter->setMessageLocalizer( $this->getContext() );
 		$row = $this->mCurrentRow;
 
 		switch ( $name ) {
@@ -69,7 +79,7 @@ class GlobalAbuseFilterPager extends AbuseFilterPager {
 				)->parse();
 			case 'af_group':
 				// If this is global, local name probably doesn't exist, but try
-				return $specsFormatter->nameGroup( $value );
+				return $this->specsFormatter->nameGroup( $value );
 			default:
 				return parent::formatValue( $name, $value );
 		}

includes/Special/SpecialAbuseFilter.php

@@ -3,6 +3,7 @@
 namespace MediaWiki\Extension\AbuseFilter\Special;
 
 use MediaWiki\Extension\AbuseFilter\AbuseFilterPermissionManager;
+use MediaWiki\Extension\AbuseFilter\CentralDBManager;
 use MediaWiki\Extension\AbuseFilter\Consequences\ConsequencesFactory;
 use MediaWiki\Extension\AbuseFilter\Consequences\ConsequencesRegistry;
 use MediaWiki\Extension\AbuseFilter\EditBox\EditBoxBuilderFactory;
@@ -76,6 +77,8 @@ class SpecialAbuseFilter extends AbuseFilterSpecialPage {
 		AbuseFilterViewList::class => [
 			AbuseFilterPermissionManager::SERVICE_NAME,
 			FilterProfiler::SERVICE_NAME,
+			SpecsFormatter::SERVICE_NAME,
+			CentralDBManager::SERVICE_NAME,
 		],
 		AbuseFilterViewRevert::class => [
 			'UserFactory',

includes/View/AbuseFilterViewList.php

@@ -6,9 +6,11 @@ use Html;
 use HTMLForm;
 use IContextSource;
 use MediaWiki\Extension\AbuseFilter\AbuseFilterPermissionManager;
+use MediaWiki\Extension\AbuseFilter\CentralDBManager;
 use MediaWiki\Extension\AbuseFilter\FilterProfiler;
 use MediaWiki\Extension\AbuseFilter\Pager\AbuseFilterPager;
 use MediaWiki\Extension\AbuseFilter\Pager\GlobalAbuseFilterPager;
+use MediaWiki\Extension\AbuseFilter\SpecsFormatter;
 use MediaWiki\Linker\LinkRenderer;
 use OOUI;
 use StringUtils;
@@ -19,14 +21,20 @@ use Xml;
  */
 class AbuseFilterViewList extends AbuseFilterView {
 
-	/**
-	 * @var FilterProfiler
-	 */
+	/** @var FilterProfiler */
 	private $filterProfiler;
 
+	/** @var SpecsFormatter */
+	private $specsFormatter;
+
+	/** @var CentralDBManager */
+	private $centralDBManager;
+
 	/**
 	 * @param AbuseFilterPermissionManager $afPermManager
 	 * @param FilterProfiler $filterProfiler
+	 * @param SpecsFormatter $specsFormatter
+	 * @param CentralDBManager $centralDBManager
 	 * @param IContextSource $context
 	 * @param LinkRenderer $linkRenderer
 	 * @param string $basePageName
@@ -35,6 +43,8 @@ class AbuseFilterViewList extends AbuseFilterView {
 	public function __construct(
 		AbuseFilterPermissionManager $afPermManager,
 		FilterProfiler $filterProfiler,
+		SpecsFormatter $specsFormatter,
+		CentralDBManager $centralDBManager,
 		IContextSource $context,
 		LinkRenderer $linkRenderer,
 		string $basePageName,
@@ -42,6 +52,9 @@ class AbuseFilterViewList extends AbuseFilterView {
 	) {
 		parent::__construct( $afPermManager, $context, $linkRenderer, $basePageName, $params );
 		$this->filterProfiler = $filterProfiler;
+		$this->specsFormatter = $specsFormatter;
+		$this->specsFormatter->setMessageLocalizer( $context );
+		$this->centralDBManager = $centralDBManager;
 	}
 
 	/**
@@ -192,16 +205,22 @@ class AbuseFilterViewList extends AbuseFilterView {
 		$searchmode = $optarray['searchmode'];
 
 		if ( $centralDB !== null && !$dbIsCentral && $scope === 'global' ) {
+			// TODO: remove the circular dependency
 			$pager = new GlobalAbuseFilterPager(
 				$this,
-				$conds,
-				$this->linkRenderer
+				$this->linkRenderer,
+				$this->afPermManager,
+				$this->specsFormatter,
+				$this->centralDBManager,
+				$conds
 			);
 		} else {
 			$pager = new AbuseFilterPager(
 				$this,
-				$conds,
 				$this->linkRenderer,
+				$this->afPermManager,
+				$this->specsFormatter,
+				$conds,
 				$querypattern,
 				$searchmode
 			);