Clean up AbuseFilterViewExamine and AbuseFilterExaminePager

Move most stuff from the pager to the view class to untangle
circular dependency. Declare class properties as private.
Leave input validation to the form.

Change-Id: Ia8b1a9d08af9c0cac23b34f6bbbe2c44d01f6c8c

includes/Pager/AbuseFilterExaminePager.php

@@ -3,34 +3,51 @@
 namespace MediaWiki\Extension\AbuseFilter\Pager;
 
 use MediaWiki\Extension\AbuseFilter\AbuseFilterChangesList;
-use MediaWiki\Extension\AbuseFilter\View\AbuseFilterViewExamine;
+use MediaWiki\Linker\LinkRenderer;
 use RecentChange;
 use ReverseChronologicalPager;
 use stdClass;
 use Title;
+use Wikimedia\Rdbms\IDatabase;
 
 class AbuseFilterExaminePager extends ReverseChronologicalPager {
 	/**
 	 * @var AbuseFilterChangesList Our changes list
 	 */
-	public $mChangesList;
+	private $changesList;
 	/**
-	 * @var AbuseFilterViewExamine The associated view
+	 * @var Title
 	 */
-	public $mPage;
+	private $title;
+	/**
+	 * @var array Query conditions
+	 */
+	private $conds;
 	/**
 	 * @var int Line number of the row, see RecentChange::$counter
 	 */
-	public $rcCounter;
+	private $rcCounter;
 
 	/**
-	 * @param AbuseFilterViewExamine $page
 	 * @param AbuseFilterChangesList $changesList
+	 * @param LinkRenderer $linkRenderer
+	 * @param IDatabase $dbr
+	 * @param Title $title
+	 * @param array $conds
 	 */
-	public function __construct( AbuseFilterViewExamine $page, AbuseFilterChangesList $changesList ) {
-		parent::__construct();
-		$this->mChangesList = $changesList;
-		$this->mPage = $page;
+	public function __construct(
+		AbuseFilterChangesList $changesList,
+		LinkRenderer $linkRenderer,
+		IDatabase $dbr,
+		Title $title,
+		array $conds
+	) {
+		// Set database before parent constructor to avoid setting it there with wfGetDB
+		$this->mDb = $dbr;
+		parent::__construct( $changesList, $linkRenderer );
+		$this->changesList = $changesList;
+		$this->title = $title;
+		$this->conds = $conds;
 		$this->rcCounter = 1;
 	}
 
@@ -38,30 +55,11 @@ class AbuseFilterExaminePager extends ReverseChronologicalPager {
 	 * @return array
 	 */
 	public function getQueryInfo() {
-		$dbr = wfGetDB( DB_REPLICA );
-		$conds = [];
 		$rcQuery = RecentChange::getQueryInfo();
-
-		if ( (string)$this->mPage->mSearchUser !== '' ) {
-			$conds[$rcQuery['fields']['rc_user_text']] = $this->mPage->mSearchUser;
-		}
-
-		$startTS = strtotime( $this->mPage->mSearchPeriodStart );
-		if ( $startTS ) {
-			$conds[] = 'rc_timestamp>=' . $dbr->addQuotes( $dbr->timestamp( $startTS ) );
-		}
-		$endTS = strtotime( $this->mPage->mSearchPeriodEnd );
-		if ( $endTS ) {
-			$conds[] = 'rc_timestamp<=' . $dbr->addQuotes( $dbr->timestamp( $endTS ) );
-		}
-
-		$conds[] = $this->mPage->buildTestConditions( $dbr );
-		$conds = array_merge( $conds, $this->mPage->buildVisibilityConditions( $dbr, $this->getAuthority() ) );
-
 		$info = [
 			'tables' => $rcQuery['tables'],
 			'fields' => $rcQuery['fields'],
-			'conds' => $conds,
+			'conds' => $this->conds,
 			'join_conds' => $rcQuery['joins'],
 		];
 
@@ -75,7 +73,7 @@ class AbuseFilterExaminePager extends ReverseChronologicalPager {
 	public function formatRow( $row ) {
 		$rc = RecentChange::newFromRow( $row );
 		$rc->counter = $this->rcCounter++;
-		return $this->mChangesList->recentChangesLine( $rc, false );
+		return $this->changesList->recentChangesLine( $rc, false );
 	}
 
 	/**
@@ -87,10 +85,11 @@ class AbuseFilterExaminePager extends ReverseChronologicalPager {
 	}
 
 	/**
+	 * @codeCoverageIgnore Merely declarative
 	 * @return Title
 	 */
 	public function getTitle() {
-		return $this->mPage->getTitle( 'examine' );
+		return $this->title;
 	}
 
 	/**

includes/Special/SpecialAbuseFilter.php

@@ -58,6 +58,7 @@ class SpecialAbuseFilter extends AbuseFilterSpecialPage {
 			SpecsFormatter::SERVICE_NAME,
 		],
 		AbuseFilterViewExamine::class => [
+			'DBLoadBalancer',
 			AbuseFilterPermissionManager::SERVICE_NAME,
 			FilterLookup::SERVICE_NAME,
 			EditBoxBuilderFactory::SERVICE_NAME,

includes/View/AbuseFilterViewExamine.php

@@ -23,25 +23,18 @@ use MediaWiki\Revision\RevisionRecord;
 use OOUI;
 use RecentChange;
 use Title;
+use Wikimedia\Rdbms\ILoadBalancer;
 use Xml;
 
 class AbuseFilterViewExamine extends AbuseFilterView {
 	/**
-	 * @var string The user whose entries we're examinating
+	 * @var string The rules of the filter we're examining
 	 */
-	public $mSearchUser;
+	private $testFilter;
 	/**
-	 * @var string The start time of the search period
+	 * @var ILoadBalancer
 	 */
-	public $mSearchPeriodStart;
-	/**
-	 * @var string The end time of the search period
-	 */
-	public $mSearchPeriodEnd;
-	/**
-	 * @var string The ID of the filter we're examinating
-	 */
-	public $mTestFilter;
+	private $loadBalancer;
 	/**
 	 * @var FilterLookup
 	 */
@@ -68,6 +61,7 @@ class AbuseFilterViewExamine extends AbuseFilterView {
 	private $varGeneratorFactory;
 
 	/**
+	 * @param ILoadBalancer $loadBalancer
 	 * @param AbuseFilterPermissionManager $afPermManager
 	 * @param FilterLookup $filterLookup
 	 * @param EditBoxBuilderFactory $boxBuilderFactory
@@ -81,6 +75,7 @@ class AbuseFilterViewExamine extends AbuseFilterView {
 	 * @param array $params
 	 */
 	public function __construct(
+		ILoadBalancer $loadBalancer,
 		AbuseFilterPermissionManager $afPermManager,
 		FilterLookup $filterLookup,
 		EditBoxBuilderFactory $boxBuilderFactory,
@@ -94,6 +89,7 @@ class AbuseFilterViewExamine extends AbuseFilterView {
 		array $params
 	) {
 		parent::__construct( $afPermManager, $context, $linkRenderer, $basePageName, $params );
+		$this->loadBalancer = $loadBalancer;
 		$this->filterLookup = $filterLookup;
 		$this->boxBuilderFactory = $boxBuilderFactory;
 		$this->varBlobStore = $varBlobStore;
@@ -116,7 +112,7 @@ class AbuseFilterViewExamine extends AbuseFilterView {
 			$out->addWikiMsg( 'abusefilter-examine-intro-examine-only' );
 		}
 
-		$this->loadParameters();
+		$this->testFilter = $this->getRequest()->getText( 'testfilter' );
 
 		// Check if we've got a subpage
 		if ( count( $this->mParams ) > 1 && is_numeric( $this->mParams[1] ) ) {
@@ -143,27 +139,24 @@ class AbuseFilterViewExamine extends AbuseFilterView {
 				'label-message' => 'abusefilter-test-user',
 				'type' => 'user',
 				'ipallowed' => true,
-				'default' => $this->mSearchUser,
 			],
 			'SearchPeriodStart' => [
 				'label-message' => 'abusefilter-test-period-start',
 				'type' => 'datetime',
-				'default' => $this->mSearchPeriodStart,
 				'min' => $min,
 				'max' => $max,
 			],
 			'SearchPeriodEnd' => [
 				'label-message' => 'abusefilter-test-period-end',
 				'type' => 'datetime',
-				'default' => $this->mSearchPeriodEnd,
 				'min' => $min,
 				'max' => $max,
 			],
 		];
-		$htmlForm = HTMLForm::factory( 'ooui', $formDescriptor, $this->getContext() );
-		$htmlForm->setWrapperLegendMsg( 'abusefilter-examine-legend' )
+		HTMLForm::factory( 'ooui', $formDescriptor, $this->getContext() )
+			->addHiddenField( 'testfilter', $this->testFilter )
+			->setWrapperLegendMsg( 'abusefilter-examine-legend' )
 			->setSubmitTextMsg( 'abusefilter-examine-submit' )
-			->setFormIdentifier( 'examine-select-date' )
 			->setSubmitCallback( [ $this, 'showResults' ] )
 			->showAlways();
 	}
@@ -175,8 +168,36 @@ class AbuseFilterViewExamine extends AbuseFilterView {
 	 * @return bool
 	 */
 	public function showResults( array $formData, HTMLForm $form ): bool {
-		$changesList = new AbuseFilterChangesList( $this->getContext(), $this->mTestFilter );
-		$pager = new AbuseFilterExaminePager( $this, $changesList );
+		$changesList = new AbuseFilterChangesList( $this->getContext(), $this->testFilter );
+
+		$dbr = $this->loadBalancer->getConnection( DB_REPLICA );
+		$conds = $this->buildVisibilityConditions( $dbr, $this->getAuthority() );
+		$conds[] = $this->buildTestConditions( $dbr );
+
+		// Normalise username
+		$userTitle = Title::newFromText( $formData['SearchUser'], NS_USER );
+		$userName = $userTitle ? $userTitle->getText() : '';
+
+		if ( $userName !== '' ) {
+			$rcQuery = RecentChange::getQueryInfo();
+			$conds[$rcQuery['fields']['rc_user_text']] = $userName;
+		}
+
+		$startTS = strtotime( $formData['SearchPeriodStart'] );
+		if ( $startTS ) {
+			$conds[] = 'rc_timestamp>=' . $dbr->addQuotes( $dbr->timestamp( $startTS ) );
+		}
+		$endTS = strtotime( $formData['SearchPeriodEnd'] );
+		if ( $endTS ) {
+			$conds[] = 'rc_timestamp<=' . $dbr->addQuotes( $dbr->timestamp( $endTS ) );
+		}
+		$pager = new AbuseFilterExaminePager(
+			$changesList,
+			$this->linkRenderer,
+			$dbr,
+			$this->getTitle( 'examine' ),
+			$conds
+		);
 
 		$output = $changesList->beginRecentChangesList()
 			. $pager->getNavigationBar()
@@ -220,7 +241,7 @@ class AbuseFilterViewExamine extends AbuseFilterView {
 	 */
 	public function showExaminerForLogEntry( $logid ) {
 		// Get data
-		$dbr = wfGetDB( DB_REPLICA );
+		$dbr = $this->loadBalancer->getConnection( DB_REPLICA );
 		$user = $this->getUser();
 		$out = $this->getOutput();
 
@@ -299,7 +320,7 @@ class AbuseFilterViewExamine extends AbuseFilterView {
 			);
 
 			$tester = Xml::tags( 'h2', null, $this->msg( 'abusefilter-examine-test' )->parse() );
-			$tester .= $boxBuilder->buildEditBox( $this->mTestFilter, false, false, false );
+			$tester .= $boxBuilder->buildEditBox( $this->testFilter, false, false, false );
 			$tester .= $this->buildFilterLoader();
 			$html .= Xml::tags( 'div', [ 'id' => 'mw-abusefilter-examine-editor' ], $tester );
 			$html .= Xml::tags( 'p',
@@ -331,18 +352,4 @@ class AbuseFilterViewExamine extends AbuseFilterView {
 		$output->addHTML( $html );
 	}
 
-	/**
-	 * Loads parameters from request
-	 */
-	public function loadParameters() {
-		$request = $this->getRequest();
-		$this->mSearchPeriodStart = $request->getText( 'wpSearchPeriodStart' );
-		$this->mSearchPeriodEnd = $request->getText( 'wpSearchPeriodEnd' );
-		$this->mTestFilter = $request->getText( 'testfilter' );
-
-		// Normalise username
-		$searchUsername = $request->getText( 'wpSearchUser' );
-		$userTitle = Title::newFromText( $searchUsername, NS_USER );
-		$this->mSearchUser = $userTitle ? $userTitle->getText() : '';
-	}
 }