From fdb926bca485186d20423cf5bd0a018df2dfcbe1 Mon Sep 17 00:00:00 2001
From: Roan Kattouw <catrope@users.mediawiki.org>
Date: Wed, 11 Feb 2009 19:25:25 +0000
Subject: [PATCH] * API: Add documentation to important API classes * Move
 page_is_redirect up in ApiPageSet::getPageTableFields()

---
 includes/api/ApiBase.php      | 176 +++++++++++++++++++++++-----------
 includes/api/ApiPageSet.php   | 139 ++++++++++++++++++++-------
 includes/api/ApiQueryInfo.php |  38 ++++----
 includes/api/ApiResult.php    |  42 +++++---
 4 files changed, 269 insertions(+), 126 deletions(-)

diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php
index 2be4c54d69..7956005b5f 100644
--- a/includes/api/ApiBase.php
+++ b/includes/api/ApiBase.php
@@ -24,15 +24,17 @@
  */
 
 /**
- * This abstract class implements many basic API functions, and is the base of all API classes.
+ * This abstract class implements many basic API functions, and is the base of
+ * all API classes.
  * The class functions are divided into several areas of functionality:
  *
- * Module parameters: Derived classes can define getAllowedParams() to specify which parameters to expect,
- * 	how to parse and validate them.
+ * Module parameters: Derived classes can define getAllowedParams() to specify
+ * 	which parameters to expect,h ow to parse and validate them.
  *
- * Profiling: various methods to allow keeping tabs on various tasks and their time costs
+ * Profiling: various methods to allow keeping tabs on various tasks and their
+ * 	time costs
  *
- * Self-documentation: code to allow api to document its own state.
+ * Self-documentation: code to allow the API to document its own state
  *
  * @ingroup API
  */
@@ -56,8 +58,11 @@ abstract class ApiBase {
 	private $mMainModule, $mModuleName, $mModulePrefix;
 
 	/**
-	* Constructor
-	*/
+	 * Constructor
+	 * @param $mainModule ApiMain object
+	 * @param $moduleName string Name of this module
+	 * @param $modulePrefix string Prefix to use for parameter names
+	 */
 	public function __construct($mainModule, $moduleName, $modulePrefix = '') {
 		$this->mMainModule = $mainModule;
 		$this->mModuleName = $moduleName;
@@ -69,32 +74,34 @@ abstract class ApiBase {
 	 *****************************************************************************/
 
 	/**
-	 * Evaluates the parameters, performs the requested query, and sets up the
-	 * result. Concrete implementations of ApiBase must override this method to
-	 * provide whatever functionality their module offers. Implementations must
-	 * not produce any output on their own and are not expected to handle any
-	 * errors.
+	 * Evaluates the parameters, performs the requested query, and sets up
+	 * the result. Concrete implementations of ApiBase must override this
+	 * method to provide whatever functionality their module offers.
+	 * Implementations must not produce any output on their own and are not
+	 * expected to handle any errors.
 	 *
-	 * The execute method will be invoked directly by ApiMain immediately before
-	 * the result of the module is output. Aside from the constructor, implementations
-	 * should assume that no other methods will be called externally on the module
-	 * before the result is processed.
+	 * The execute() method will be invoked directly by ApiMain immediately
+	 * before the result of the module is output. Aside from the
+	 * constructor, implementations should assume that no other methods
+	 * will be called externally on the module before the result is
+	 * processed.
 	 *
-	 * The result data should be stored in the result object referred to by
-	 * "getResult()". Refer to ApiResult.php for details on populating a result
-	 * object.
+	 * The result data should be stored in the ApiResult object available
+	 * through getResult().
 	 */
 	public abstract function execute();
 
 	/**
-	 * Returns a String that identifies the version of the extending class. Typically
-	 * includes the class name, the svn revision, timestamp, and last author. May
-	 * be severely incorrect in many implementations!
+	 * Returns a string that identifies the version of the extending class.
+	 * Typically includes the class name, the svn revision, timestamp, and
+	 * last author. Usually done with SVN's Id keyword
+	 * @return string
 	 */
 	public abstract function getVersion();
 
 	/**
 	 * Get the name of the module being executed by this instance
+	 * @return string
 	 */
 	public function getModuleName() {
 		return $this->mModuleName;
@@ -102,6 +109,7 @@ abstract class ApiBase {
 
 	/**
 	 * Get parameter prefix (usually two letters or an empty string).
+	 * @return string
 	 */
 	public function getModulePrefix() {
 		return $this->mModulePrefix;
@@ -109,6 +117,7 @@ abstract class ApiBase {
 
 	/**
 	 * Get the name of the module as shown in the profiler log
+	 * @return string
 	 */
 	public function getModuleProfileName($db = false) {
 		if ($db)
@@ -118,7 +127,8 @@ abstract class ApiBase {
 	}
 
 	/**
-	 * Get main module
+	 * Get the main module
+	 * @return ApiMain object
 	 */
 	public function getMain() {
 		return $this->mMainModule;
@@ -127,14 +137,15 @@ abstract class ApiBase {
 	/**
 	 * Returns true if this module is the main module ($this === $this->mMainModule),
 	 * false otherwise.
+	 * @return bool
 	 */
 	public function isMain() {
 		return $this === $this->mMainModule;
 	}
 
 	/**
-	 * Get the result object. Please refer to the documentation in ApiResult.php
-	 * for details on populating and accessing data in a result object.
+	 * Get the result object
+	 * @return ApiResult
 	 */
 	public function getResult() {
 		// Main module has getResult() method overriden
@@ -145,15 +156,19 @@ abstract class ApiBase {
 	}
 
 	/**
-	 * Get the result data array
+	 * Get the result data array (read-only)
+	 * @return array
 	 */
 	public function getResultData() {
 		return $this->getResult()->getData();
 	}
 
 	/**
-	 * Set warning section for this module. Users should monitor this section to
-	 * notice any changes in API.
+	 * Set warning section for this module. Users should monitor this
+	 * section to notice any changes in API. Multiple calls to this
+	 * function will result in the warning messages being separated by
+	 * newlines
+	 * @param $warning string Warning message
 	 */
 	public function setWarning($warning) {
 		$data = $this->getResult()->getData();
@@ -179,6 +194,7 @@ abstract class ApiBase {
 	 * If the module may only be used with a certain format module,
 	 * it should override this method to return an instance of that formatter.
 	 * A value of null means the default format will be used.
+	 * @return mixed instance of a derived class of ApiFormatBase, or null
 	 */
 	public function getCustomPrinter() {
 		return null;
@@ -186,6 +202,7 @@ abstract class ApiBase {
 
 	/**
 	 * Generates help message for this module, or false if there is no description
+	 * @return mixed string or false
 	 */
 	public function makeHelpMsg() {
 
@@ -244,6 +261,7 @@ abstract class ApiBase {
 	/**
 	 * Generates the parameter descriptions for this module, to be displayed in the
 	 * module's help.
+	 * @return string
 	 */
 	public function makeHelpMsgParameters() {
 		$params = $this->getFinalParams();
@@ -314,6 +332,7 @@ abstract class ApiBase {
 
 	/**
 	 * Returns the description string for this module
+	 * @return mixed string or array of strings
 	 */
 	protected function getDescription() {
 		return false;
@@ -321,15 +340,18 @@ abstract class ApiBase {
 
 	/**
 	 * Returns usage examples for this module. Return null if no examples are available.
+	 * @return mixed string or array of strings
 	 */
 	protected function getExamples() {
 		return false;
 	}
 
 	/**
-	 * Returns an array of allowed parameters (keys) => default value for that parameter.
-	 * Don't call this function directly: use getFinalParams() to allow hooks
-	 * to modify parameters as needed.
+	 * Returns an array of allowed parameters (parameter name) => (default
+	 * value) or (parameter name) => (array with PARAM_* constants as keys)
+	 * Don't call this function directly: use getFinalParams() to allow
+	 * hooks to modify parameters as needed.
+	 * @return array
 	 */
 	protected function getAllowedParams() {
 		return false;
@@ -337,24 +359,30 @@ abstract class ApiBase {
 
 	/**
 	 * Returns an array of parameter descriptions.
-	 * Don't call this functon directly: use getFinalParamDescription() to allow
-	 * hooks to modify descriptions as needed.
+	 * Don't call this functon directly: use getFinalParamDescription() to
+	 * allow hooks to modify descriptions as needed.
+	 * @return array
 	 */
 	protected function getParamDescription() {
 		return false;
 	}
 	
 	/**
-	 * Get final list of parameters, after hooks have had
-	 * a chance to tweak it as needed.
+	 * Get final list of parameters, after hooks have had a chance to
+	 * tweak it as needed.
+	 * @return array
 	 */
 	public function getFinalParams() {
 		$params = $this->getAllowedParams();
 		wfRunHooks('APIGetAllowedParams', array(&$this, &$params));
 		return $params;
 	}
-	
-	
+
+	/**
+	 * Get final description, after hooks have had a chance to tweak it as
+	 * needed.
+	 * @return array
+	 */
 	public function getFinalParamDescription() {
 		$desc = $this->getParamDescription();
 		wfRunHooks('APIGetParamDescription', array(&$this, &$desc));
@@ -364,16 +392,21 @@ abstract class ApiBase {
 	/**
 	 * This method mangles parameter name based on the prefix supplied to the constructor.
 	 * Override this method to change parameter name during runtime
+	 * @param $paramName string Parameter name
+	 * @return string Prefixed parameter name
 	 */
 	public function encodeParamName($paramName) {
 		return $this->mModulePrefix . $paramName;
 	}
 
 	/**
-	* Using getAllowedParams(), makes an array of the values provided by the user,
-	* with key being the name of the variable, and value - validated value from user or default.
-	* limit=max will not be parsed if $parseMaxLimit is set to false; use this
-	* when the max limit is not definite, e.g. when getting revisions.
+	* Using getAllowedParams(), this function makes an array of the values
+	* provided by the user, with key being the name of the variable, and
+	* value - validated value from user or default. limit=max will not be
+	* parsed if $parseMaxLimit is set to false; use this when the max
+	* limit is not definitive yet, e.g. when getting revisions.
+	* @param $parseMaxLimit bool
+	* @return array
 	*/
 	public function extractRequestParams($parseMaxLimit = true) {
 		$params = $this->getFinalParams();
@@ -387,6 +420,9 @@ abstract class ApiBase {
 
 	/**
 	 * Get a value for the given parameter
+	 * @param $paramName string Parameter name
+	 * @param $parseMaxLimit bool see extractRequestParams()
+	 * @return mixed Parameter value
 	 */
 	protected function getParameter($paramName, $parseMaxLimit = true) {
 		$params = $this->getFinalParams();
@@ -396,6 +432,7 @@ abstract class ApiBase {
 	
 	/**
 	 * Die if none or more than one of a certain set of parameters is set
+	 * @param $params array of parameter names
 	 */
 	public function requireOnlyOneParameter($params) {
 		$required = func_get_args();
@@ -414,6 +451,7 @@ abstract class ApiBase {
 	/**
 	 * Returns an array of the namespaces (by integer id) that exist on the
 	 * wiki. Used primarily in help documentation.
+	 * @return array
 	 */
 	public static function getValidNamespaces() {
 		static $mValidNamespaces = null;
@@ -433,8 +471,10 @@ abstract class ApiBase {
 	 * Using the settings determine the value for the given parameter
 	 *
 	 * @param $paramName String: parameter name
-	 * @param $paramSettings Mixed: default value or an array of settings using PARAM_* constants.
+	 * @param $paramSettings Mixed: default value or an array of settings
+	 *  using PARAM_* constants.
 	 * @param $parseMaxLimit Boolean: parse limit when max is given?
+	 * @return mixed Parameter value
 	 */
 	protected function getParameterFromSettings($paramName, $paramSettings, $parseMaxLimit) {
 
@@ -553,11 +593,14 @@ abstract class ApiBase {
 	* Return an array of values that were given in a 'a|b|c' notation,
 	* after it optionally validates them against the list allowed values.
 	*
-	* @param valueName - The name of the parameter (for error reporting)
-	* @param value - The value being parsed
-	* @param allowMultiple - Can $value contain more than one value separated by '|'?
-	* @param allowedValues - An array of values to check against. If null, all values are accepted.
-	* @return (allowMultiple ? an_array_of_values : a_single_value)
+	* @param $valueName string The name of the parameter (for error
+	*  reporting)
+	* @param $value mixed The value being parsed
+	* @param $allowMultiple bool Can $value contain more than one value
+	*  separated by '|'?
+	* @param $allowedValues mixed An array of values to check against. If
+	*  null, all values are accepted.
+	* @return mixed (allowMultiple ? an_array_of_values : a_single_value)
 	*/
 	protected function parseMultiValue($valueName, $value, $allowMultiple, $allowedValues) {
 		if( trim($value) === "" )
@@ -593,8 +636,14 @@ abstract class ApiBase {
 	}
 
 	/**
-	* Validate the value against the minimum and user/bot maximum limits. Prints usage info on failure.
-	*/
+	 * Validate the value against the minimum and user/bot maximum limits.
+	 * Prints usage info on failure.
+	 * @param $paramName string Parameter name
+	 * @param $value int Parameter value
+	 * @param $min int Minimum value
+	 * @param $max int Maximum value for users
+	 * @param $botMax int Maximum value for sysops/bots
+	 */
 	function validateLimit($paramName, $value, $min, $max, $botMax = null) {
 		if (!is_null($min) && $value < $min) {
 			$this->dieUsage($this->encodeParamName($paramName) . " may not be less than $min (set to $value)", $paramName);
@@ -635,7 +684,10 @@ abstract class ApiBase {
 	}
 
 	/**
-	 * Call main module's error handler
+	 * Call the main module's error handler
+	 * @param $description string Error text
+	 * @param $errorCode string Error code
+	 * @param $httpRespCode int HTTP response code
 	 */
 	public function dieUsage($description, $errorCode, $httpRespCode = 0) {
 		wfProfileClose();
@@ -761,7 +813,7 @@ abstract class ApiBase {
 
 	/**
 	 * Output the error message related to a certain array
-	 * @param array $error Element of a getUserPermissionsErrors()-style array
+	 * @param $error array Element of a getUserPermissionsErrors()-style array
 	 */
 	public function dieUsageMsg($error) {
 		$parsed = $this->parseMsg($error);
@@ -770,7 +822,7 @@ abstract class ApiBase {
 	
 	/**
 	 * Return the error message related to a certain array
-	 * @param array $error Element of a getUserPermissionsErrors()-style array
+	 * @param $error array Element of a getUserPermissionsErrors()-style array
 	 * @return array('code' => code, 'info' => info)
 	 */
 	public function parseMsg($error) {
@@ -787,13 +839,16 @@ abstract class ApiBase {
 
 	/**
 	 * Internal code errors should be reported with this method
+	 * @param $method string Method or function name
+	 * @param $message string Error message
 	 */
 	protected static function dieDebug($method, $message) {
 		wfDebugDieBacktrace("Internal error in $method: $message");
 	}
 
 	/**
-	 * Indicates if API needs to check maxlag
+	 * Indicates if this module needs maxlag to be checked
+	 * @return bool
 	 */
 	public function shouldCheckMaxlag() {
 		return true;
@@ -801,6 +856,7 @@ abstract class ApiBase {
 
 	/**
 	 * Indicates if this module requires edit mode
+	 * @return bool
 	 */
 	public function isEditMode() {
 		return false;
@@ -808,6 +864,7 @@ abstract class ApiBase {
 
 	/**
 	 * Indicates whether this module must be called with a POST request
+	 * @return bool
 	 */
 	public function mustBePosted() {
 		return false;
@@ -857,6 +914,7 @@ abstract class ApiBase {
 
 	/**
 	 * Total time the module was executed
+	 * @return float
 	 */
 	public function getProfileTime() {
 		if ($this->mTimeIn !== 0)
@@ -900,6 +958,7 @@ abstract class ApiBase {
 
 	/**
 	 * Total time the module used the database
+	 * @return float
 	 */
 	public function getProfileDBTime() {
 		if ($this->mDBTimeIn !== 0)
@@ -907,8 +966,14 @@ abstract class ApiBase {
 		return $this->mDBTime;
 	}
 
+	/**
+	 * Debugging function that prints a value and an optional backtrace
+	 * @param $value mixed Value to print
+	 * @param $name string Description of the printed value
+	 * @param $backtrace bool If true, print a backtrace
+	 */
 	public static function debugPrint($value, $name = 'unknown', $backtrace = false) {
-		print "\n\n<pre><b>Debuging value '$name':</b>\n\n";
+		print "\n\n<pre><b>Debugging value '$name':</b>\n\n";
 		var_export($value);
 		if ($backtrace)
 			print "\n" . wfBacktrace();
@@ -917,7 +982,8 @@ abstract class ApiBase {
 
 
 	/**
-	 * Returns a String that identifies the version of this class.
+	 * Returns a string that identifies the version of this class.
+	 * @return string
 	 */
 	public static function getBaseVersion() {
 		return __CLASS__ . ': $Id$';
diff --git a/includes/api/ApiPageSet.php b/includes/api/ApiPageSet.php
index a9724d3902..445c69ef85 100644
--- a/includes/api/ApiPageSet.php
+++ b/includes/api/ApiPageSet.php
@@ -30,13 +30,14 @@ if (!defined('MEDIAWIKI')) {
 
 /**
  * This class contains a list of pages that the client has requested.
- * Initially, when the client passes in titles=, pageids=, or revisions= parameter,
- * an instance of the ApiPageSet class will normalize titles,
- * determine if the pages/revisions exist, and prefetch any additional data page data requested.
+ * Initially, when the client passes in titles=, pageids=, or revisions=
+ * parameter, an instance of the ApiPageSet class will normalize titles,
+ * determine if the pages/revisions exist, and prefetch any additional page
+ * data requested.
  *
- * When generator is used, the result of the generator will become the input for the
- * second instance of this class, and all subsequent actions will go use the second instance
- * for all their work.
+ * When a generator is used, the result of the generator will become the input
+ * for the second instance of this class, and all subsequent actions will use
+ * the second instance for all their work.
  *
  * @ingroup API
  */
@@ -52,6 +53,11 @@ class ApiPageSet extends ApiQueryBase {
 
 	private $mRequestedPageFields;
 
+	/**
+	 * Constructor
+	 * @param $query ApiQuery
+	 * @param $resolveRedirects bool Whether redirects should be resolved
+	 */
 	public function __construct($query, $resolveRedirects = false) {
 		parent :: __construct($query, 'query');
 
@@ -75,20 +81,38 @@ class ApiPageSet extends ApiQueryBase {
 		$this->mFakePageId = -1;
 	}
 
+	/**
+	 * Check whether this PageSet is resolving redirects
+	 * @return bool
+	 */
 	public function isResolvingRedirects() {
 		return $this->mResolveRedirects;
 	}
 
+	/**
+	 * Request an additional field from the page table. Must be called
+	 * before execute()
+	 * @param $fieldName string Field name
+	 */
 	public function requestField($fieldName) {
 		$this->mRequestedPageFields[$fieldName] = null;
 	}
 
+	/**
+	 * Get the value of a custom field previously requested through
+	 * requestField()
+	 * @param $fieldName string Field name
+	 * @return mixed Field value
+	 */
 	public function getCustomField($fieldName) {
 		return $this->mRequestedPageFields[$fieldName];
 	}
 
 	/**
-	 * Get fields that modules have requested from the page table
+	 * Get the fields that have to be queried from the page table:
+	 * the ones requested through requestField() and a few basic ones
+	 * we always need
+	 * @return array of field names
 	 */
 	public function getPageTableFields() {
 		// Ensure we get minimum required fields
@@ -99,12 +123,12 @@ class ApiPageSet extends ApiQueryBase {
 			'page_id' => null,
 		);
 
-		// only store non-default fields
-		$this->mRequestedPageFields = array_diff_key($this->mRequestedPageFields, $pageFlds);
-
 		if ($this->mResolveRedirects)
 			$pageFlds['page_is_redirect'] = null;
 
+		// only store non-default fields
+		$this->mRequestedPageFields = array_diff_key($this->mRequestedPageFields, $pageFlds);
+
 		$pageFlds = array_merge($pageFlds, $this->mRequestedPageFields);
 		return array_keys($pageFlds);
 	}
@@ -113,6 +137,7 @@ class ApiPageSet extends ApiQueryBase {
 	 * Returns an array [ns][dbkey] => page_id for all requested titles.
 	 * page_id is a unique negative number in case title was not found.
 	 * Invalid titles will also have negative page IDs and will be in namespace 0
+	 * @return array
 	 */
 	public function getAllTitlesByNamespace() {
 		return $this->mAllPages;
@@ -128,6 +153,7 @@ class ApiPageSet extends ApiQueryBase {
 
 	/**
 	 * Returns the number of unique pages (not revisions) in the set.
+	 * @return int
 	 */
 	public function getTitleCount() {
 		return count($this->mTitles);
@@ -143,6 +169,7 @@ class ApiPageSet extends ApiQueryBase {
 
 	/**
 	 * Returns the number of found unique pages (not revisions) in the set.
+	 * @return int
 	 */
 	public function getGoodTitleCount() {
 		return count($this->mGoodTitles);
@@ -175,7 +202,8 @@ class ApiPageSet extends ApiQueryBase {
 	}
 
 	/**
-	 * Get a list of redirects when doing redirect resolution
+	 * Get a list of redirect resolutions - maps a title to its redirect
+	 * target.
 	 * @return array prefixed_title (string) => prefixed_title (string)
 	 */
 	public function getRedirectTitles() {
@@ -183,8 +211,8 @@ class ApiPageSet extends ApiQueryBase {
 	}
 
 	/**
-	 * Get a list of title normalizations - maps the title given
-	 * with its normalized version.
+	 * Get a list of title normalizations - maps a title to its normalized
+	 * version.
 	 * @return array raw_prefixed_title (string) => prefixed_title (string)
 	 */
 	public function getNormalizedTitles() {
@@ -192,8 +220,8 @@ class ApiPageSet extends ApiQueryBase {
 	}
 
 	/**
-	 * Get a list of interwiki titles - maps the title given
-	 * with to the interwiki prefix.
+	 * Get a list of interwiki titles - maps a title to its interwiki
+	 * prefix.
 	 * @return array raw_prefixed_title (string) => interwiki_prefix (string)
 	 */
 	public function getInterwikiTitles() {
@@ -201,7 +229,7 @@ class ApiPageSet extends ApiQueryBase {
 	}
 
 	/**
-	 * Get the list of revision IDs (requested with revids= parameter)
+	 * Get the list of revision IDs (requested with the revids= parameter)
 	 * @return array revID (int) => pageID (int)
 	 */
 	public function getRevisionIDs() {
@@ -217,14 +245,15 @@ class ApiPageSet extends ApiQueryBase {
 	}
 
 	/**
-	 * Returns the number of revisions (requested with revids= parameter)
+	 * Returns the number of revisions (requested with revids= parameter)\
+	 * @return int
 	 */
 	public function getRevisionCount() {
 		return count($this->getRevisionIDs());
 	}
 
 	/**
-	 * Populate from the request parameters
+	 * Populate the PageSet from the request parameters.
 	 */
 	public function execute() {
 		$this->profileIn();
@@ -267,7 +296,8 @@ class ApiPageSet extends ApiQueryBase {
 	}
 
 	/**
-	 * Initialize PageSet from a list of Titles
+	 * Populate this PageSet from a list of Titles
+	 * @param $titles array of Title objects
 	 */
 	public function populateFromTitles($titles) {
 		$this->profileIn();
@@ -276,7 +306,8 @@ class ApiPageSet extends ApiQueryBase {
 	}
 
 	/**
-	 * Initialize PageSet from a list of Page IDs
+	 * Populate this PageSet from a list of page IDs
+	 * @param $pageIDs array of page IDs
 	 */
 	public function populateFromPageIDs($pageIDs) {
 		$this->profileIn();
@@ -285,7 +316,9 @@ class ApiPageSet extends ApiQueryBase {
 	}
 
 	/**
-	 * Initialize PageSet from a rowset returned from the database
+	 * Populate this PageSet from a rowset returned from the database
+	 * @param $db Database object
+	 * @param $queryResult Query result object
 	 */
 	public function populateFromQueryResult($db, $queryResult) {
 		$this->profileIn();
@@ -294,17 +327,18 @@ class ApiPageSet extends ApiQueryBase {
 	}
 
 	/**
-	 * Initialize PageSet from a list of Revision IDs
+	 * Populate this PageSet from a list of revision IDs
+	 * @param $revIDs array of revision IDs
 	 */
 	public function populateFromRevisionIDs($revIDs) {
 		$this->profileIn();
-		$revIDs = array_map('intval', $revIDs); // paranoia
 		$this->initFromRevIDs($revIDs);
 		$this->profileOut();
 	}
 
 	/**
 	 * Extract all requested fields from the row received from the database
+	 * @param $row Result row
 	 */
 	public function processDbRow($row) {
 
@@ -325,6 +359,9 @@ class ApiPageSet extends ApiQueryBase {
 			$fieldValues[$pageId] = $row-> $fieldName;
 	}
 
+	/**
+	 * Resolve redirects, if applicable
+	 */
 	public function finishPageSetGeneration() {
 		$this->profileIn();
 		$this->resolvePendingRedirects();
@@ -341,9 +378,11 @@ class ApiPageSet extends ApiQueryBase {
 	 *
 	 * Additionally, when resolving redirects:
 	 * #3 If no more redirects left, stop.
-	 * #4 For each redirect, get its links from `pagelinks` table.
+	 * #4 For each redirect, get its target from the `redirect` table.
 	 * #5 Substitute the original LinkBatch object with the new list
 	 * #6 Repeat from step #1
+	 *
+	 * @param $titles array of Title objects or strings
 	 */
 	private function initFromTitles($titles) {
 
@@ -357,7 +396,8 @@ class ApiPageSet extends ApiQueryBase {
 
 		// Get pageIDs data from the `page` table
 		$this->profileDBIn();
-		$res = $db->select('page', $this->getPageTableFields(), $set, __METHOD__);
+		$res = $db->select('page', $this->getPageTableFields(), $set,
+					__METHOD__);
 		$this->profileDBOut();
 
 		// Hack: get the ns:titles stored in array(ns => array(titles)) format
@@ -367,6 +407,10 @@ class ApiPageSet extends ApiQueryBase {
 		$this->resolvePendingRedirects();
 	}
 
+	/**
+	 * Does the same as initFromTitles(), but is based on page IDs instead
+	 * @param $pageids array of page IDs
+	 */
 	private function initFromPageIds($pageids) {
 		if(!count($pageids))
 			return;
@@ -375,12 +419,12 @@ class ApiPageSet extends ApiQueryBase {
 		$set = array (
 			'page_id' => $pageids
 		);
-
 		$db = $this->getDB();
 
 		// Get pageIDs data from the `page` table
 		$this->profileDBIn();
-		$res = $db->select('page', $this->getPageTableFields(), $set, __METHOD__);
+		$res = $db->select('page', $this->getPageTableFields(), $set,
+					__METHOD__);
 		$this->profileDBOut();
 
 		$remaining = array_flip($pageids);
@@ -395,12 +439,11 @@ class ApiPageSet extends ApiQueryBase {
 	 * and for each row create and store title object and save any extra fields requested.
 	 * @param $db Database
 	 * @param $res DB Query result
-	 * @param $remaining Array of either pageID or ns/title elements (optional).
+	 * @param $remaining array of either pageID or ns/title elements (optional).
 	 *        If given, any missing items will go to $mMissingPageIDs and $mMissingTitles
 	 * @param $processTitles bool Must be provided together with $remaining.
 	 *        If true, treat $remaining as an array of [ns][title]
 	 *        If false, treat it as an array of [pageIDs]
-	 * @return Array of redirect IDs (only when resolving redirects)
 	 */
 	private function initFromQueryResult($db, $res, &$remaining = null, $processTitles = null) {
 		if (!is_null($remaining) && is_null($processTitles))
@@ -448,11 +491,17 @@ class ApiPageSet extends ApiQueryBase {
 		}
 	}
 
+	/**
+	 * Does the same as initFromTitles(), but is based on revision IDs
+	 * instead
+	 * @param $revids array of revision IDs
+	 */
 	private function initFromRevIDs($revids) {
 
 		if(!count($revids))
 			return;
 
+		$revIDs = array_map('intval', $revIDs); // paranoia
 		$db = $this->getDB();
 		$pageids = array();
 		$remaining = array_flip($revids);
@@ -460,18 +509,19 @@ class ApiPageSet extends ApiQueryBase {
 		$tables = array('revision', 'page');
 		$fields = array('rev_id', 'rev_page');
 		$where = array('rev_id' => $revids, 'rev_page = page_id');
+		$options = array();
 
 		// Get pageIDs data from the `page` table
 		$this->profileDBIn();
-		$res = $db->select( $tables, $fields, $where,  __METHOD__ );
-		while ( $row = $db->fetchObject( $res ) ) {
+		$res = $db->select($tables, $fields, $where,  __METHOD__);
+		while ($row = $db->fetchObject($res)) {
 			$revid = intval($row->rev_id);
 			$pageid = intval($row->rev_page);
 			$this->mGoodRevIDs[$revid] = $pageid;
 			$pageids[$pageid] = '';
 			unset($remaining[$revid]);
 		}
-		$db->freeResult( $res );
+		$db->freeResult($res);
 		$this->profileDBOut();
 
 		$this->mMissingRevIDs = array_keys($remaining);
@@ -480,6 +530,11 @@ class ApiPageSet extends ApiQueryBase {
 		$this->initFromPageIds(array_keys($pageids));
 	}
 
+	/**
+	 * Resolve any redirects in the result if redirect resolution was
+	 * requested. This function is called repeatedly until all redirects
+	 * have been resolved.
+	 */
 	private function resolvePendingRedirects() {
 
 		if($this->mResolveRedirects) {
@@ -498,7 +553,7 @@ class ApiPageSet extends ApiQueryBase {
 					break;
 
 				$set = $linkBatch->constructSet('page', $db);
-				if(false === $set)
+				if($set === false)
 					break;
 
 				// Get pageIDs data from the `page` table
@@ -512,6 +567,13 @@ class ApiPageSet extends ApiQueryBase {
 		}
 	}
 
+	/**
+	 * Get the targets of the pending redirects from the database
+	 *
+	 * Also creates entries in the redirect table for redirects that don't
+	 * have one.
+	 * @return LinkBatch
+	 */
 	private function getRedirectTargets() {
 		$lb = new LinkBatch();
 		$db = $this->getDB();
@@ -562,7 +624,8 @@ class ApiPageSet extends ApiQueryBase {
 	 * This method validates access rights for the title,
 	 * and appends normalization values to the output.
 	 *
-	 * @return LinkBatch of title objects.
+	 * @param $titles array of Title objects or strings
+	 * @return LinkBatch
 	 */
 	private function processTitlesArray($titles) {
 
@@ -592,9 +655,11 @@ class ApiPageSet extends ApiQueryBase {
 					$linkBatch->addObj($titleObj);
 			}
 
-			// Make sure we remember the original title that was given to us
-			// This way the caller can correlate new titles with the originally requested,
-			// i.e. namespace is localized or capitalization is different
+			// Make sure we remember the original title that was
+			// given to us. This way the caller can correlate new
+			// titles with the originally requested when e.g. the
+			// namespace is localized or the capitalization is
+			// different
 			if (is_string($title) && $title !== $titleObj->getPrefixedText()) {
 				$this->mNormalizedTitles[$title] = $titleObj->getPrefixedText();
 			}
diff --git a/includes/api/ApiQueryInfo.php b/includes/api/ApiQueryInfo.php
index 202cc15a11..0ffa1ae0d2 100644
--- a/includes/api/ApiQueryInfo.php
+++ b/includes/api/ApiQueryInfo.php
@@ -435,6 +435,7 @@ class ApiQueryInfo extends ApiQueryBase {
 		}
 
 		$count = 0;
+		ksort($titles); // Ensure they're always in the same order
 		foreach ( $titles as $pageid => $title ) {
 			$count++;
 			if(!is_null($params['continue']))
@@ -505,7 +506,7 @@ class ApiQueryInfo extends ApiQueryBase {
 				if(!is_null($params['continue']))
 					if($count < $params['continue'])
 						continue;
-				$fit = true;
+				$r = array();
 				if(!is_null($params['token'])) 
 				{
 					$tokenFunctions = $this->getTokenFunctions();
@@ -516,12 +517,10 @@ class ApiQueryInfo extends ApiQueryBase {
 						if($val === false)
 							$this->setWarning("Action '$t' is not allowed for the current user");
 						else
-							$fit = $result->addValue(
-								array('query', 'pages', $pageid),
-								$t . 'token', $val);
+							$r[$t . 'token'] = $val;
 					}
 				}
-				if($fld_protection && $fit)
+				if($fld_protection)
 				{
 					// Apparently the XML formatting code doesn't like array(null)
 					// This is painful to fix, so we'll just work around it
@@ -530,26 +529,21 @@ class ApiQueryInfo extends ApiQueryBase {
 					else
 						$val = array();
 					$result->setIndexedTagName($val, 'pr');
-					$fit = $result->addValue(
-							array('query', 'pages', $pageid),
-							'protection', $val);
+					$r['protection'] = $val;
 				}
-				if($fld_talkid && isset($talkids[$title->getNamespace()][$title->getDbKey()]) && $fit)
-					$fit = $result->addValue(array('query', 'pages', $pageid), 'talkid',
-						$talkids[$title->getNamespace()][$title->getDbKey()]);
-				if($fld_subjectid && isset($subjectids[$title->getNamespace()][$title->getDbKey()]) && $fit)
-					$fit = $result->addValue(array('query', 'pages', $pageid), 'subjectid',
-						$subjectids[$title->getNamespace()][$title->getDbKey()]);
-				if($fld_url && $fit) {
-					$fit = $result->addValue(array('query', 'pages', $pageid), 'fullurl',
-						$title->getFullURL());
-					if($fit)
-						$fit = $result->addValue(array('query', 'pages', $pageid), 'editurl',
-							$title->getFullURL('action=edit'));
+				if($fld_talkid && isset($talkids[$title->getNamespace()][$title->getDbKey()]))
+					$r['talkid'] = $talkids[$title->getNamespace()][$title->getDbKey()];
+				if($fld_subjectid && isset($subjectids[$title->getNamespace()][$title->getDbKey()]))
+					$r['subjectid'] = $subjectids[$title->getNamespace()][$title->getDbKey()];
+				if($fld_url)
+				{
+					$r['fullurl'] = $title->getFullURL();
+					$r['editurl'] = $title->getFullURL('action=edit');
 				}
-				if($fld_readable && $fit)
+				if($fld_readable)
 					if($title->userCanRead())
-						$fit = $result->addValue(array('query', 'pages', $pageid), 'readable', '');
+						$r['readable'] = '';
+				$fit = $result->addValue(array('query', 'pages'), $pageid, $r);
 				if(!$fit)
 				{
 					$this->setContinueEnumParameter('continue', $count);
diff --git a/includes/api/ApiResult.php b/includes/api/ApiResult.php
index 90156bdaa3..a796d74c71 100644
--- a/includes/api/ApiResult.php
+++ b/includes/api/ApiResult.php
@@ -50,8 +50,9 @@ class ApiResult extends ApiBase {
 	private $mData, $mIsRawMode, $mSize, $mCheckingSize;
 
 	/**
-	* Constructor
-	*/
+	 * Constructor
+	 * @param $main ApiMain object
+	 */
 	public function __construct($main) {
 		parent :: __construct($main, 'result');
 		$this->mIsRawMode = false;
@@ -76,7 +77,8 @@ class ApiResult extends ApiBase {
 	}
 
 	/**
-	 * Returns true if the result is being created for the formatter that requested raw data.
+	 * Returns true whether the formatter requested raw data.
+	 * @return bool
 	 */
 	public function getIsRawMode() {
 		return $this->mIsRawMode;
@@ -84,6 +86,7 @@ class ApiResult extends ApiBase {
 
 	/**
 	 * Get the result's internal data array (read-only)
+	 * @return array
 	 */
 	public function getData() {
 		return $this->mData;
@@ -92,7 +95,7 @@ class ApiResult extends ApiBase {
 	/**
 	 * Get the 'real' size of a result item. This means the strlen() of the item,
 	 * or the sum of the strlen()s of the elements if the item is an array.
-	 * @param mixed $value
+	 * @param $value mixed
 	 * @return int
 	 */
 	public static function size($value) {
@@ -133,6 +136,9 @@ class ApiResult extends ApiBase {
 	/**
 	 * Add an output value to the array by name.
 	 * Verifies that value with the same name has not been added before.
+	 * @param $arr array to add $value to
+	 * @param $name string Index of $arr to add $value at
+	 * @param $value mixed
 	 */
 	public static function setElement(& $arr, $name, $value) {
 		if ($arr === null || $name === null || $value === null || !is_array($arr) || is_array($name))
@@ -152,10 +158,12 @@ class ApiResult extends ApiBase {
 	}
 
 	/**
-	 * Adds the content element to the array.
+	 * Adds a content element to an array.
 	 * Use this function instead of hardcoding the '*' element.
-	 * @param string $subElemName when present, content element is created as a sub item of the arr.
-	 *  Use this parameter to create elements in format <elem>text</elem> without attributes
+	 * @param $arr array to add the content element to
+	 * @param $subElemName string when present, content element is created
+	 *  as a sub item of $arr. Use this parameter to create elements in
+	 *  format <elem>text</elem> without attributes
 	 */
 	public static function setContent(& $arr, $value, $subElemName = null) {
 		if (is_array($value))
@@ -171,7 +179,10 @@ class ApiResult extends ApiBase {
 
 	/**
 	 * In case the array contains indexed values (in addition to named),
-	 * all indexed values will have the given tag name.
+	 * give all indexed values the given tag name. This function MUST be
+	 * called on every arrray that has numerical indexes.
+	 * @param $arr array
+	 * @param $tag string Tag name
 	 */
 	public function setIndexedTagName(& $arr, $tag) {
 		// In raw mode, add the '_element', otherwise just ignore
@@ -184,7 +195,9 @@ class ApiResult extends ApiBase {
 	}
 
 	/**
-	 * Calls setIndexedTagName() on $arr and each sub-array
+	 * Calls setIndexedTagName() on each sub-array of $arr
+	 * @param $arr array
+	 * @param $tag string Tag name
 	 */
 	public function setIndexedTagName_recursive(&$arr, $tag)
 	{
@@ -203,8 +216,8 @@ class ApiResult extends ApiBase {
 	 * Calls setIndexedTagName() on an array already in the result.
 	 * Don't specify a path to a value that's not in the result, or
 	 * you'll get nasty errors.
-	 * @param array $path Path to the array, like addValue()'s path
-	 * @param string $tag
+	 * @param $path array Path to the array, like addValue()'s $path
+	 * @param $tag string
 	 */
 	public function setIndexedTagName_internal( $path, $tag ) {
 		$data = & $this->mData;
@@ -257,6 +270,8 @@ class ApiResult extends ApiBase {
 	 * Unset a value previously added to the result set.
 	 * Fails silently if the value isn't found.
 	 * For parameters, see addValue()
+	 * @param $path array
+	 * @param $name string
 	 */
 	public function unsetValue($path, $name) {
 		$data = & $this->mData;
@@ -277,7 +292,10 @@ class ApiResult extends ApiBase {
 	{
 		array_walk_recursive($this->mData, array('ApiResult', 'cleanUp_helper'));
 	}
-	
+
+	/**
+	 * Callback function for cleanUpUTF8()
+	 */
 	private static function cleanUp_helper(&$s)
 	{
 		if(!is_string($s))
-- 
2.20.1