From 973cd64489bdbb54b93c4930b02a9ab40df7728f Mon Sep 17 00:00:00 2001
From: =?utf8?q?Bartosz=20Dziewo=C5=84ski?= <matma.rex@gmail.com>
Date: Tue, 24 Dec 2013 12:40:42 +0100
Subject: [PATCH] ChangesListSpecialPage: Implement buildMainQueryConds()

This also involved adding some default options to getDefaultOptions()
and a new method getDB().

Change-Id: I7389a72bfcf176480bfc36f9d6efc467e1a5e76a
---
 .../specialpage/ChangesListSpecialPage.php    | 90 ++++++++++++++++++-
 includes/specials/SpecialRecentchanges.php    | 77 ++--------------
 includes/specials/SpecialWatchlist.php        | 62 +++----------
 .../specials/SpecialRecentchangesTest.php     | 12 +--
 4 files changed, 111 insertions(+), 130 deletions(-)

diff --git a/includes/specialpage/ChangesListSpecialPage.php b/includes/specialpage/ChangesListSpecialPage.php
index 1d17394d5d..c088adb5ad 100644
--- a/includes/specialpage/ChangesListSpecialPage.php
+++ b/includes/specialpage/ChangesListSpecialPage.php
@@ -127,13 +127,20 @@ abstract class ChangesListSpecialPage extends SpecialPage {
 
 	/**
 	 * Get a FormOptions object containing the default options. By default returns some basic options,
-	 * you might not call parent method and discard them.
+	 * you might want to not call parent method and discard them, or to override default values.
 	 *
 	 * @return FormOptions
 	 */
 	public function getDefaultOptions() {
 		$opts = new FormOptions();
 
+		$opts->add( 'hideminor', false );
+		$opts->add( 'hidebots', false );
+		$opts->add( 'hideanons', false );
+		$opts->add( 'hideliu', false );
+		$opts->add( 'hidepatrolled', false );
+		$opts->add( 'hidemyself', false );
+
 		$opts->add( 'namespace', '', FormOptions::INTNULL );
 		$opts->add( 'invert', false );
 		$opts->add( 'associated', false );
@@ -185,12 +192,80 @@ abstract class ChangesListSpecialPage extends SpecialPage {
 
 	/**
 	 * Return an array of conditions depending of options set in $opts
-	 * @todo This should build some basic conditions here…
+	 * @todo Whyyyy is this mutating $opts…
 	 *
 	 * @param FormOptions $opts
 	 * @return array
 	 */
-	abstract public function buildMainQueryConds( FormOptions $opts );
+	public function buildMainQueryConds( FormOptions $opts ) {
+		$dbr = $this->getDB();
+		$user = $this->getUser();
+		$conds = array();
+
+		// It makes no sense to hide both anons and logged-in users
+		// Where this occurs, force anons to be shown
+		$botsOnly = false;
+		if ( $opts['hideanons'] && $opts['hideliu'] ) {
+			// Check if the user wants to show bots only
+			if ( $opts['hidebots'] ) {
+				$opts['hideanons'] = false;
+			} else {
+				$botsOnly = true;
+			}
+		}
+
+		// Toggles
+		if ( $opts['hideminor'] ) {
+			$conds['rc_minor'] = 0;
+		}
+		if ( $opts['hidebots'] ) {
+			$conds['rc_bot'] = 0;
+		}
+		if ( $user->useRCPatrol() && $opts['hidepatrolled'] ) {
+			$conds['rc_patrolled'] = 0;
+		}
+		if ( $botsOnly ) {
+			$conds['rc_bot'] = 1;
+		} else {
+			if ( $opts['hideliu'] ) {
+				$conds[] = 'rc_user = 0';
+			}
+			if ( $opts['hideanons'] ) {
+				$conds[] = 'rc_user != 0';
+			}
+		}
+		if ( $opts['hidemyself'] ) {
+			if ( $user->getId() ) {
+				$conds[] = 'rc_user != ' . $dbr->addQuotes( $user->getId() );
+			} else {
+				$conds[] = 'rc_user_text != ' . $dbr->addQuotes( $user->getName() );
+			}
+		}
+
+		// Namespace filtering
+		if ( $opts['namespace'] !== '' ) {
+			$selectedNS = $dbr->addQuotes( $opts['namespace'] );
+			$operator = $opts['invert'] ? '!=' : '=';
+			$boolean = $opts['invert'] ? 'AND' : 'OR';
+
+			// Namespace association (bug 2429)
+			if ( !$opts['associated'] ) {
+				$condition = "rc_namespace $operator $selectedNS";
+			} else {
+				// Also add the associated namespace
+				$associatedNS = $dbr->addQuotes(
+					MWNamespace::getAssociated( $opts['namespace'] )
+				);
+				$condition = "(rc_namespace $operator $selectedNS "
+					. $boolean
+					. " rc_namespace $operator $associatedNS)";
+			}
+
+			$conds[] = $condition;
+		}
+
+		return $conds;
+	}
 
 	/**
 	 * Process the query
@@ -202,6 +277,15 @@ abstract class ChangesListSpecialPage extends SpecialPage {
 	 */
 	abstract public function doMainQuery( $conds, $opts );
 
+	/**
+	 * Return a DatabaseBase object for reading
+	 *
+	 * @return DatabaseBase
+	 */
+	protected function getDB() {
+		return wfGetDB( DB_SLAVE );
+	}
+
 	/**
 	 * Send output to the OutputPage object, only called if not used feeds
 	 * @todo This should do most, if not all, of the outputting now done by subclasses
diff --git a/includes/specials/SpecialRecentchanges.php b/includes/specials/SpecialRecentchanges.php
index d0e6171faf..fdf8dcbfbd 100644
--- a/includes/specials/SpecialRecentchanges.php
+++ b/includes/specials/SpecialRecentchanges.php
@@ -148,26 +148,14 @@ class SpecialRecentChanges extends ChangesListSpecialPage {
 
 	/**
 	 * Return an array of conditions depending of options set in $opts
+	 * @todo Whyyyy is this mutating $opts…
 	 *
 	 * @param FormOptions $opts
 	 * @return array
 	 */
 	public function buildMainQueryConds( FormOptions $opts ) {
-		$dbr = wfGetDB( DB_SLAVE );
-		$conds = array();
-
-		# It makes no sense to hide both anons and logged-in users
-		# Where this occurs, force anons to be shown
-		$forcebot = false;
-		if ( $opts['hideanons'] && $opts['hideliu'] ) {
-			# Check if the user wants to show bots only
-			if ( $opts['hidebots'] ) {
-				$opts['hideanons'] = false;
-			} else {
-				$forcebot = true;
-				$opts['hidebots'] = false;
-			}
-		}
+		$dbr = $this->getDB();
+		$conds = parent::buildMainQueryConds( $opts );
 
 		// Calculate cutoff
 		$cutoff_unixtime = time() - ( $opts['days'] * 86400 );
@@ -183,59 +171,6 @@ class SpecialRecentChanges extends ChangesListSpecialPage {
 
 		$conds[] = 'rc_timestamp >= ' . $dbr->addQuotes( $cutoff );
 
-		$hidePatrol = $this->getUser()->useRCPatrol() && $opts['hidepatrolled'];
-		$hideLoggedInUsers = $opts['hideliu'] && !$forcebot;
-		$hideAnonymousUsers = $opts['hideanons'] && !$forcebot;
-
-		if ( $opts['hideminor'] ) {
-			$conds['rc_minor'] = 0;
-		}
-		if ( $opts['hidebots'] ) {
-			$conds['rc_bot'] = 0;
-		}
-		if ( $hidePatrol ) {
-			$conds['rc_patrolled'] = 0;
-		}
-		if ( $forcebot ) {
-			$conds['rc_bot'] = 1;
-		}
-		if ( $hideLoggedInUsers ) {
-			$conds[] = 'rc_user = 0';
-		}
-		if ( $hideAnonymousUsers ) {
-			$conds[] = 'rc_user != 0';
-		}
-
-		if ( $opts['hidemyself'] ) {
-			if ( $this->getUser()->getId() ) {
-				$conds[] = 'rc_user != ' . $dbr->addQuotes( $this->getUser()->getId() );
-			} else {
-				$conds[] = 'rc_user_text != ' . $dbr->addQuotes( $this->getUser()->getName() );
-			}
-		}
-
-		# Namespace filtering
-		if ( $opts['namespace'] !== '' ) {
-			$selectedNS = $dbr->addQuotes( $opts['namespace'] );
-			$operator = $opts['invert'] ? '!=' : '=';
-			$boolean = $opts['invert'] ? 'AND' : 'OR';
-
-			# namespace association (bug 2429)
-			if ( !$opts['associated'] ) {
-				$condition = "rc_namespace $operator $selectedNS";
-			} else {
-				# Also add the associated namespace
-				$associatedNS = $dbr->addQuotes(
-					MWNamespace::getAssociated( $opts['namespace'] )
-				);
-				$condition = "(rc_namespace $operator $selectedNS "
-					. $boolean
-					. " rc_namespace $operator $associatedNS)";
-			}
-
-			$conds[] = $condition;
-		}
-
 		return $conds;
 	}
 
@@ -252,7 +187,7 @@ class SpecialRecentChanges extends ChangesListSpecialPage {
 		$query_options = array();
 
 		$uid = $this->getUser()->getId();
-		$dbr = wfGetDB( DB_SLAVE );
+		$dbr = $this->getDB();
 		$limit = $opts['limit'];
 		$namespace = $opts['namespace'];
 		$invert = $opts['invert'];
@@ -323,7 +258,7 @@ class SpecialRecentChanges extends ChangesListSpecialPage {
 		$showWatcherCount = $wgRCShowWatchingUsers && $this->getUser()->getOption( 'shownumberswatching' );
 		$watcherCache = array();
 
-		$dbr = wfGetDB( DB_SLAVE );
+		$dbr = $this->getDB();
 
 		$counter = 1;
 		$list = ChangesList::newFromContext( $this->getContext() );
@@ -537,7 +472,7 @@ class SpecialRecentChanges extends ChangesListSpecialPage {
 	 * @return string|bool
 	 */
 	public function checkLastModified( $feedFormat ) {
-		$dbr = wfGetDB( DB_SLAVE );
+		$dbr = $this->getDB();
 		$lastmod = $dbr->selectField( 'recentchanges', 'MAX(rc_timestamp)', false, __METHOD__ );
 		if ( $feedFormat || !$this->getUser()->useRCPatrol() ) {
 			if ( $lastmod && $this->getOutput()->checkLastModified( $lastmod ) ) {
diff --git a/includes/specials/SpecialWatchlist.php b/includes/specials/SpecialWatchlist.php
index 519b6c2653..a98447be9a 100644
--- a/includes/specials/SpecialWatchlist.php
+++ b/includes/specials/SpecialWatchlist.php
@@ -155,57 +155,14 @@ class SpecialWatchlist extends ChangesListSpecialPage {
 	 * @return array
 	 */
 	public function buildMainQueryConds( FormOptions $opts ) {
-		$dbr = wfGetDB( DB_SLAVE, 'watchlist' );
-		$conds = array();
-		$user = $this->getUser();
+		$dbr = $this->getDB();
+		$conds = parent::buildMainQueryConds( $opts );
 
 		// Calculate cutoff
 		if ( $opts['days'] > 0 ) {
 			$conds[] = 'rc_timestamp > ' . $dbr->addQuotes( $dbr->timestamp( time() - intval( $opts['days'] * 86400 ) ) );
 		}
 
-		# Toggles
-		if ( $opts['hidemyself'] ) {
-			$conds[] = 'rc_user != ' . $user->getId();
-		}
-		if ( $opts['hidebots'] ) {
-			$conds[] = 'rc_bot = 0';
-		}
-		if ( $opts['hideminor'] ) {
-			$conds[] = 'rc_minor = 0';
-		}
-		if ( $opts['hideliu'] ) {
-			$conds[] = 'rc_user = 0';
-		}
-		if ( $opts['hideanons'] ) {
-			$conds[] = 'rc_user != 0';
-		}
-		if ( $user->useRCPatrol() && $opts['hidepatrolled'] ) {
-			$conds[] = 'rc_patrolled != 1';
-		}
-
-		# Namespace filtering
-		if ( $opts['namespace'] !== '' ) {
-			$selectedNS = $dbr->addQuotes( $opts['namespace'] );
-			$operator = $opts['invert'] ? '!=' : '=';
-			$boolean = $opts['invert'] ? 'AND' : 'OR';
-
-			# namespace association (bug 2429)
-			if ( !$opts['associated'] ) {
-				$condition = "rc_namespace $operator $selectedNS";
-			} else {
-				# Also add the associated namespace
-				$associatedNS = $dbr->addQuotes(
-					MWNamespace::getAssociated( $opts['namespace'] )
-				);
-				$condition = "(rc_namespace $operator $selectedNS "
-					. $boolean
-					. " rc_namespace $operator $associatedNS)";
-			}
-
-			$conds[] = $condition;
-		}
-
 		return $conds;
 	}
 
@@ -219,7 +176,7 @@ class SpecialWatchlist extends ChangesListSpecialPage {
 	public function doMainQuery( $conds, $opts ) {
 		global $wgShowUpdatedMarker;
 
-		$dbr = wfGetDB( DB_SLAVE, 'watchlist' );
+		$dbr = $this->getDB();
 		$user = $this->getUser();
 		# Toggle watchlist content (all recent edits or just the latest)
 		if ( $opts['extended'] ) {
@@ -294,6 +251,15 @@ class SpecialWatchlist extends ChangesListSpecialPage {
 		return $dbr->select( $tables, $fields, $conds, __METHOD__, $options, $join_conds );
 	}
 
+	/**
+	 * Return a DatabaseBase object for reading
+	 *
+	 * @return DatabaseBase
+	 */
+	protected function getDB() {
+		return wfGetDB( DB_SLAVE, 'watchlist' );
+	}
+
 	/**
 	 * Send output to the OutputPage object, only called if not used feeds
 	 *
@@ -303,7 +269,7 @@ class SpecialWatchlist extends ChangesListSpecialPage {
 	public function webOutput( $rows, $opts ) {
 		global $wgShowUpdatedMarker, $wgRCShowWatchingUsers;
 
-		$dbr = wfGetDB( DB_SLAVE, 'watchlist' );
+		$dbr = $this->getDB();
 		$user = $this->getUser();
 
 		$dbr->dataSeek( $rows, 0 );
@@ -481,7 +447,7 @@ class SpecialWatchlist extends ChangesListSpecialPage {
 		$form = "";
 		$user = $this->getUser();
 
-		$dbr = wfGetDB( DB_SLAVE, 'watchlist' );
+		$dbr = $this->getDB();
 		$numItems = $this->countItems( $dbr );
 
 		// Show watchlist header
diff --git a/tests/phpunit/includes/specials/SpecialRecentchangesTest.php b/tests/phpunit/includes/specials/SpecialRecentchangesTest.php
index b1ba152df8..c3d75aa58c 100644
--- a/tests/phpunit/includes/specials/SpecialRecentchangesTest.php
+++ b/tests/phpunit/includes/specials/SpecialRecentchangesTest.php
@@ -50,8 +50,7 @@ class SpecialRecentchangesTest extends MediaWikiTestCase {
 		$this->assertConditions(
 			array( # expected
 				'rc_bot' => 0,
-				#0 => "rc_timestamp >= '20110223000000'",
-				1 => "rc_namespace = '0'",
+				0 => "rc_namespace = '0'",
 			),
 			array(
 				'namespace' => NS_MAIN,
@@ -63,9 +62,8 @@ class SpecialRecentchangesTest extends MediaWikiTestCase {
 	public function testRcNsFilterInversion() {
 		$this->assertConditions(
 			array( # expected
-				#0 => "rc_timestamp >= '20110223000000'",
 				'rc_bot' => 0,
-				1 => sprintf( "rc_namespace != '%s'", NS_MAIN ),
+				0 => sprintf( "rc_namespace != '%s'", NS_MAIN ),
 			),
 			array(
 				'namespace' => NS_MAIN,
@@ -82,9 +80,8 @@ class SpecialRecentchangesTest extends MediaWikiTestCase {
 	public function testRcNsFilterAssociation( $ns1, $ns2 ) {
 		$this->assertConditions(
 			array( # expected
-				#0 => "rc_timestamp >= '20110223000000'",
 				'rc_bot' => 0,
-				1 => sprintf( "(rc_namespace = '%s' OR rc_namespace = '%s')", $ns1, $ns2 ),
+				0 => sprintf( "(rc_namespace = '%s' OR rc_namespace = '%s')", $ns1, $ns2 ),
 			),
 			array(
 				'namespace' => $ns1,
@@ -101,9 +98,8 @@ class SpecialRecentchangesTest extends MediaWikiTestCase {
 	public function testRcNsFilterAssociationWithInversion( $ns1, $ns2 ) {
 		$this->assertConditions(
 			array( # expected
-				#0 => "rc_timestamp >= '20110223000000'",
 				'rc_bot' => 0,
-				1 => sprintf( "(rc_namespace != '%s' AND rc_namespace != '%s')", $ns1, $ns2 ),
+				0 => sprintf( "(rc_namespace != '%s' AND rc_namespace != '%s')", $ns1, $ns2 ),
 			),
 			array(
 				'namespace' => $ns1,
-- 
2.20.1