1 files changed, 195 insertions, 210 deletions

diff --git a/engine/lib/database.php b/engine/lib/database.php
index 16efb5874..a7949788d 100644
--- a/engine/lib/database.php
+++ b/engine/lib/database.php
@@ -12,14 +12,19 @@
 /**
  * Query cache for all queries.
  *
- * Each query and its results are stored in this array as:
+ * Each query and its results are stored in this cache as:
  * <code>
- * $DB_QUERY_CACHE[$query] => array(result1, result2, ... resultN)
+ * $DB_QUERY_CACHE[query hash] => array(result1, result2, ... resultN)
  * </code>
+ * @see elgg_query_runner() for details on the hash.
  *
- * @global array $DB_QUERY_CACHE
+ * @warning Elgg used to set this as an empty array to turn off the cache
+ *
+ * @global ElggLRUCache|null $DB_QUERY_CACHE
+ * @access private
  */
-$DB_QUERY_CACHE = array();
+global $DB_QUERY_CACHE;
+$DB_QUERY_CACHE = null;
 
 /**
  * Queries to be executed upon shutdown.
@@ -37,7 +42,9 @@ $DB_QUERY_CACHE = array();
  * </code>
  *
  * @global array $DB_DELAYED_QUERIES
+ * @access private
  */
+global $DB_DELAYED_QUERIES;
 $DB_DELAYED_QUERIES = array();
 
 /**
@@ -46,8 +53,10 @@ $DB_DELAYED_QUERIES = array();
  * Each database link created with establish_db_link($name) is stored in
  * $dblink as $dblink[$name] => resource.  Use get_db_link($name) to retrieve it.
  *
- * @global array $dblink
+ * @global resource[] $dblink
+ * @access private
  */
+global $dblink;
 $dblink = array();
 
 /**
@@ -56,7 +65,9 @@ $dblink = array();
  * Each call to the database increments this counter.
  *
  * @global integer $dbcalls
+ * @access private
  */
+global $dbcalls;
 $dbcalls = 0;
 
 /**
@@ -68,10 +79,12 @@ $dbcalls = 0;
  * resource. eg "read", "write", or "readwrite".
  *
  * @return void
+ * @throws DatabaseException
+ * @access private
  */
 function establish_db_link($dblinkname = "readwrite") {
 	// Get configuration, and globalise database link
-	global $CONFIG, $dblink, $DB_QUERY_CACHE, $dbcalls;
+	global $CONFIG, $dblink, $DB_QUERY_CACHE;
 
 	if ($dblinkname != "readwrite" && isset($CONFIG->db[$dblinkname])) {
 		if (is_array($CONFIG->db[$dblinkname])) {
@@ -95,13 +108,13 @@ function establish_db_link($dblinkname = "readwrite") {
 
 	// Connect to database
 	if (!$dblink[$dblinkname] = mysql_connect($dbhost, $dbuser, $dbpass, true)) {
-		$msg = sprintf(elgg_echo('DatabaseException:WrongCredentials'),
-				$dbuser, $dbhost, "****");
+		$msg = elgg_echo('DatabaseException:WrongCredentials',
+				array($dbuser, $dbhost, "****"));
 		throw new DatabaseException($msg);
 	}
 
 	if (!mysql_select_db($dbname, $dblink[$dblinkname])) {
-		$msg = sprintf(elgg_echo('DatabaseException:NoConnect'), $dbname);
+		$msg = elgg_echo('DatabaseException:NoConnect', array($dbname));
 		throw new DatabaseException($msg);
 	}
 
@@ -115,7 +128,8 @@ function establish_db_link($dblinkname = "readwrite") {
 
 	// Set up cache if global not initialized and query cache not turned off
 	if ((!$DB_QUERY_CACHE) && (!$db_cache_off)) {
-		$DB_QUERY_CACHE = new ElggStaticVariableCache('db_query_cache');
+		// @todo if we keep this cache in 1.9, expose the size as a config parameter
+		$DB_QUERY_CACHE = new ElggLRUCache(200);
 	}
 }
 
@@ -126,9 +140,10 @@ function establish_db_link($dblinkname = "readwrite") {
  * links up separately; otherwise just create the one database link.
  *
  * @return void
+ * @access private
  */
 function setup_db_connections() {
-	global $CONFIG, $dblink;
+	global $CONFIG;
 
 	if (!empty($CONFIG->db->split)) {
 		establish_db_link('read');
@@ -142,6 +157,7 @@ function setup_db_connections() {
  * Display profiling information about db at NOTICE debug level upon shutdown.
  *
  * @return void
+ * @access private
  */
 function db_profiling_shutdown_hook() {
 	global $dbcalls;
@@ -154,15 +170,23 @@ function db_profiling_shutdown_hook() {
  * Execute any delayed queries upon shutdown.
  *
  * @return void
+ * @access private
  */
 function db_delayedexecution_shutdown_hook() {
-	global $DB_DELAYED_QUERIES, $CONFIG;
+	global $DB_DELAYED_QUERIES;
 
 	foreach ($DB_DELAYED_QUERIES as $query_details) {
-		// use one of our db functions so it is included in profiling.
-		$result = execute_query($query_details['q'], $query_details['l']);
-
 		try {
+			$link = $query_details['l'];
+
+			if ($link == 'read' || $link == 'write') {
+				$link = get_db_link($link);
+			} elseif (!is_resource($link)) {
+				elgg_log("Link for delayed query not valid resource or db_link type. Query: {$query_details['q']}", 'WARNING');
+			}
+			
+			$result = execute_query($query_details['q'], $link);
+			
 			if ((isset($query_details['h'])) && (is_callable($query_details['h']))) {
 				$query_details['h']($result);
 			}
@@ -174,21 +198,6 @@ function db_delayedexecution_shutdown_hook() {
 }
 
 /**
- * Registers shutdown functions for database profiling and delayed queries.
- *
- * @note Database connections are established upon first call to database.
- *
- * @return true
- * @elgg_event_handler boot system
- */
-function init_db() {
-	register_shutdown_function('db_delayedexecution_shutdown_hook');
-	register_shutdown_function('db_profiling_shutdown_hook');
-
-	return true;
-}
-
-/**
  * Returns (if required, also creates) a database link resource.
  *
  * Database link resources are stored in the {@link $dblink} global.  These
@@ -197,7 +206,8 @@ function init_db() {
  *
  * @param string $dblinktype The type of link we want: "read", "write" or "readwrite".
  *
- * @return object Database link
+ * @return resource Database link
+ * @access private
  */
 function get_db_link($dblinktype) {
 	global $dblink;
@@ -215,10 +225,11 @@ function get_db_link($dblinktype) {
 /**
  * Execute an EXPLAIN for $query.
  *
- * @param str   $query The query to explain
+ * @param string $query The query to explain
  * @param mixed $link  The database link resource to user.
  *
  * @return mixed An object of the query's result, or FALSE
+ * @access private
  */
 function explain_query($query, $link) {
 	if ($result = execute_query("explain " . $query, $link)) {
@@ -238,20 +249,26 @@ function explain_query($query, $link) {
  * {@link $dbcalls} is incremented and the query is saved into the {@link $DB_QUERY_CACHE}.
  *
  * @param string $query  The query
- * @param link   $dblink The DB link
+ * @param resource   $dblink The DB link
  *
- * @return The result of mysql_query()
+ * @return resource result of mysql_query()
  * @throws DatabaseException
+ * @access private
  */
 function execute_query($query, $dblink) {
-	global $CONFIG, $dbcalls, $DB_QUERY_CACHE;
+	global $dbcalls;
+
+	if ($query == NULL) {
+		throw new DatabaseException(elgg_echo('DatabaseException:InvalidQuery'));
+	}
+
+	if (!is_resource($dblink)) {
+		throw new DatabaseException(elgg_echo('DatabaseException:InvalidDBLink'));
+	}
 
 	$dbcalls++;
 
 	$result = mysql_query($query, $dblink);
-	if ($DB_QUERY_CACHE) {
-		$DB_QUERY_CACHE[$query] = -1; // Set initial cache to -1
-	}
 
 	if (mysql_errno($dblink)) {
 		throw new DatabaseException(mysql_error($dblink) . "\n\n QUERY: " . $query);
@@ -267,10 +284,11 @@ function execute_query($query, $dblink) {
  * the raw result from {@link mysql_query()}.
  *
  * @param string   $query   The query to execute
- * @param resource $dblink  The database link to use
+ * @param resource|string $dblink  The database link to use or the link type (read | write)
  * @param string   $handler A callback function to pass the results array to
  *
  * @return true
+ * @access private
  */
 function execute_delayed_query($query, $dblink, $handler = "") {
 	global $DB_DELAYED_QUERIES;
@@ -279,6 +297,10 @@ function execute_delayed_query($query, $dblink, $handler = "") {
 		$DB_DELAYED_QUERIES = array();
 	}
 
+	if (!is_resource($dblink) && $dblink != 'read' && $dblink != 'write') {
+		return false;
+	}
+
 	// Construct delayed query
 	$delayed_query = array();
 	$delayed_query['q'] = $query;
@@ -299,9 +321,10 @@ function execute_delayed_query($query, $dblink, $handler = "") {
  * @return true
  * @uses execute_delayed_query()
  * @uses get_db_link()
+ * @access private
  */
 function execute_delayed_write_query($query, $handler = "") {
-	return execute_delayed_query($query, get_db_link('write'), $handler);
+	return execute_delayed_query($query, 'write', $handler);
 }
 
 /**
@@ -313,9 +336,10 @@ function execute_delayed_write_query($query, $handler = "") {
  * @return true
  * @uses execute_delayed_query()
  * @uses get_db_link()
+ * @access private
  */
 function execute_delayed_read_query($query, $handler = "") {
-	return execute_delayed_query($query, get_db_link('read'), $handler);
+	return execute_delayed_query($query, 'read', $handler);
 }
 
 /**
@@ -327,59 +351,15 @@ function execute_delayed_read_query($query, $handler = "") {
  * argument to $callback.  If no callback function is defined, the
  * entire result set is returned as an array.
  *
- * If no results are matched, FALSE is returned.
- *
  * @param mixed  $query    The query being passed.
  * @param string $callback Optionally, the name of a function to call back to on each row
  *
- * @return array|false An array of database result objects or callback function results or false
+ * @return array An array of database result objects or callback function results. If the query
+ *               returned nothing, an empty array.
+ * @access private
  */
 function get_data($query, $callback = "") {
-	global $CONFIG, $DB_QUERY_CACHE;
-
-	// Is cached?
-	if ($DB_QUERY_CACHE) {
-		$cached_query = $DB_QUERY_CACHE[$query];
-	}
-
-	if ((isset($cached_query)) && ($cached_query)) {
-		elgg_log("$query results returned from cache");
-
-		if ($cached_query === -1) {
-			// Last time this query returned nothing, so return an empty array
-			return array();
-		}
-
-		return $cached_query;
-	}
-
-	$dblink = get_db_link('read');
-	$resultarray = array();
-
-	if ($result = execute_query("$query", $dblink)) {
-		while ($row = mysql_fetch_object($result)) {
-			if (!empty($callback) && is_callable($callback)) {
-				$row = $callback($row);
-			}
-			if ($row) {
-				$resultarray[] = $row;
-			}
-		}
-	}
-
-	if (empty($resultarray)) {
-		elgg_log("DB query \"$query\" returned no results.");
-		// @todo consider changing this to return empty array #1242
-		return FALSE;
-	}
-
-	// Cache result
-	if ($DB_QUERY_CACHE) {
-		$DB_QUERY_CACHE[$query] = $resultarray;
-		elgg_log("$query results cached");
-	}
-
-	return $resultarray;
+	return elgg_query_runner($query, $callback, false);
 }
 
 /**
@@ -393,49 +373,77 @@ function get_data($query, $callback = "") {
  * @param string $callback A callback function
  *
  * @return mixed A single database result object or the result of the callback function.
+ * @access private
  */
 function get_data_row($query, $callback = "") {
-	global $CONFIG, $DB_QUERY_CACHE;
+	return elgg_query_runner($query, $callback, true);
+}
 
-	// Is cached
-	if ($DB_QUERY_CACHE) {
-		$cached_query = $DB_QUERY_CACHE[$query];
-	}
+/**
+ * Handles returning data from a query, running it through a callback function,
+ * and caching the results. This is for R queries (from CRUD).
+ *
+ * @access private
+ *
+ * @param string $query    The query to execute
+ * @param string $callback An optional callback function to run on each row
+ * @param bool   $single   Return only a single result?
+ *
+ * @return array An array of database result objects or callback function results. If the query
+ *               returned nothing, an empty array.
+ * @since 1.8.0
+ * @access private
+ */
+function elgg_query_runner($query, $callback = null, $single = false) {
+	global $DB_QUERY_CACHE;
 
-	if ((isset($cached_query)) && ($cached_query)) {
-		elgg_log("$query results returned from cache");
+	// Since we want to cache results of running the callback, we need to
+	// need to namespace the query with the callback and single result request.
+	// https://github.com/elgg/elgg/issues/4049
+	$hash = (string)$callback . (int)$single . $query;
 
-		if ($cached_query === -1) {
-			// Last time this query returned nothing, so return false
-			//@todo fix me this should return array().
-			return FALSE;
+	// Is cached?
+	if ($DB_QUERY_CACHE) {
+		if (isset($DB_QUERY_CACHE[$hash])) {
+			elgg_log("DB query $query results returned from cache (hash: $hash)", 'NOTICE');
+			return $DB_QUERY_CACHE[$hash];
 		}
-
-		return $cached_query;
 	}
 
 	$dblink = get_db_link('read');
+	$return = array();
 
 	if ($result = execute_query("$query", $dblink)) {
-		$row = mysql_fetch_object($result);
-
-		// Cache result (even if query returned no data)
-		if ($DB_QUERY_CACHE) {
-			$DB_QUERY_CACHE[$query] = $row;
-			elgg_log("$query results cached");
-		}
 
-		if (!empty($callback) && is_callable($callback)) {
+		// test for callback once instead of on each iteration.
+		// @todo check profiling to see if this needs to be broken out into
+		// explicit cases instead of checking in the iteration.
+		$is_callable = is_callable($callback);
+		while ($row = mysql_fetch_object($result)) {
+			if ($is_callable) {
 				$row = $callback($row);
-		}
+			}
 
-		if ($row) {
-			return $row;
+			if ($single) {
+				$return = $row;
+				break;
+			} else {
+				$return[] = $row;
+			}
 		}
 	}
 
-	elgg_log("$query returned no results.");
-	return FALSE;
+	if (empty($return)) {
+		elgg_log("DB query $query returned no results.", 'NOTICE');
+	}
+
+	// Cache result
+	if ($DB_QUERY_CACHE) {
+		$DB_QUERY_CACHE[$hash] = $return;
+		elgg_log("DB query $query results cached (hash: $hash)", 'NOTICE');
+	}
+
+	return $return;
 }
 
 /**
@@ -447,18 +455,15 @@ function get_data_row($query, $callback = "") {
  *
  * @return int|false The database id of the inserted row if a AUTO_INCREMENT field is
  *                   defined, 0 if not, and false on failure.
+ * @access private
  */
 function insert_data($query) {
-	global $CONFIG, $DB_QUERY_CACHE;
 
+	elgg_log("DB query $query", 'NOTICE');
+	
 	$dblink = get_db_link('write');
 
-	// Invalidate query cache
-	if ($DB_QUERY_CACHE) {
-		$DB_QUERY_CACHE->clear();
-	}
-
-	elgg_log("Query cache invalidated");
+	_elgg_invalidate_query_cache();
 
 	if (execute_query("$query", $dblink)) {
 		return mysql_insert_id($dblink);
@@ -468,24 +473,22 @@ function insert_data($query) {
 }
 
 /**
- * Update a row in the database.
+ * Update the database.
  *
  * @note Altering the DB invalidates all queries in {@link $DB_QUERY_CACHE}.
  *
  * @param string $query The query to run.
  *
- * @return Bool
+ * @return bool
+ * @access private
  */
 function update_data($query) {
-	global $CONFIG, $DB_QUERY_CACHE;
+
+	elgg_log("DB query $query", 'NOTICE');
 
 	$dblink = get_db_link('write');
 
-	// Invalidate query cache
-	if ($DB_QUERY_CACHE) {
-		$DB_QUERY_CACHE->clear();
-		elgg_log("Query cache invalidated");
-	}
+	_elgg_invalidate_query_cache();
 
 	if (execute_query("$query", $dblink)) {
 		return TRUE;
@@ -495,24 +498,22 @@ function update_data($query) {
 }
 
 /**
- * Remove a row from the database.
+ * Remove data from the database.
  *
  * @note Altering the DB invalidates all queries in {@link $DB_QUERY_CACHE}.
  *
  * @param string $query The SQL query to run
  *
  * @return int|false The number of affected rows or false on failure
+ * @access private
  */
 function delete_data($query) {
-	global $CONFIG, $DB_QUERY_CACHE;
+
+	elgg_log("DB query $query", 'NOTICE');
 
 	$dblink = get_db_link('write');
 
-	// Invalidate query cache
-	if ($DB_QUERY_CACHE) {
-		$DB_QUERY_CACHE->clear();
-		elgg_log("Query cache invalidated");
-	}
+	_elgg_invalidate_query_cache();
 
 	if (execute_query("$query", $dblink)) {
 		return mysql_affected_rows($dblink);
@@ -521,6 +522,22 @@ function delete_data($query) {
 	return FALSE;
 }
 
+/**
+ * Invalidate the query cache
+ *
+ * @access private
+ */
+function _elgg_invalidate_query_cache() {
+	global $DB_QUERY_CACHE;
+	if ($DB_QUERY_CACHE instanceof ElggLRUCache) {
+		$DB_QUERY_CACHE->clear();
+		elgg_log("Query cache invalidated", 'NOTICE');
+	} elseif ($DB_QUERY_CACHE) {
+		// In case someone sets the cache to an array and primes it with data
+		$DB_QUERY_CACHE = array();
+		elgg_log("Query cache invalidated", 'NOTICE');
+	}
+}
 
 /**
  * Return tables matching the database prefix {@link $CONFIG->dbprefix}% in the currently
@@ -528,6 +545,7 @@ function delete_data($query) {
  *
  * @return array|false List of tables or false on failure
  * @static array $tables Tables found matching the database prefix
+ * @access private
  */
 function get_db_tables() {
 	global $CONFIG;
@@ -570,6 +588,7 @@ function get_db_tables() {
  * @param string $table The name of the table to optimise
  *
  * @return bool
+ * @access private
  */
 function optimize_table($table) {
 	$table = sanitise_string($table);
@@ -582,6 +601,7 @@ function optimize_table($table) {
  * @param resource $dblink The DB link
  *
  * @return string Database error message
+ * @access private
  */
 function get_db_error($dblink) {
 	return mysql_error($dblink);
@@ -606,6 +626,7 @@ function get_db_error($dblink) {
  *
  * @return void
  * @throws DatabaseException
+ * @access private
  */
 function run_sql_script($scriptlocation) {
 	if ($script = file_get_contents($scriptlocation)) {
@@ -624,7 +645,7 @@ function run_sql_script($scriptlocation) {
 			$statement = str_replace("prefix_", $CONFIG->dbprefix, $statement);
 			if (!empty($statement)) {
 				try {
-					$result = update_data($statement);
+					update_data($statement);
 				} catch (DatabaseException $e) {
 					$errors[] = $e->getMessage();
 				}
@@ -640,73 +661,21 @@ function run_sql_script($scriptlocation) {
 			throw new DatabaseException($msg);
 		}
 	} else {
-		$msg = sprintf(elgg_echo('DatabaseException:ScriptNotFound'), $scriptlocation);
+		$msg = elgg_echo('DatabaseException:ScriptNotFound', array($scriptlocation));
 		throw new DatabaseException($msg);
 	}
 }
 
 /**
- * Upgrade the database schema in an ordered sequence.
- *
- * Executes all upgrade files in elgg/engine/schema/upgrades/ in sequential order.
- * Upgrade files must be in the standard Elgg release format of YYYYMMDDII.sql
- * where II is an incrementor starting from 01.
- *
- * Files that are < $version will be ignored.
- *
- * @warning Plugin authors should not call this function directly.
- *
- * @param int    $version The version you are upgrading from in the format YYYYMMDDII.
- * @param string $fromdir Optional directory to load upgrades from. default: engine/schema/upgrades/
- * @param bool   $quiet   If true, suppress all error messages. Only use for the upgrade from <=1.6.
+ * Format a query string for logging
  *
- * @return bool
- * @see upgrade.php
- * @see version.php
+ * @param string $query Query string
+ * @return string
+ * @access private
  */
-function db_upgrade($version, $fromdir = "", $quiet = FALSE) {
-	global $CONFIG;
-
-	$version = (int) $version;
-
-	if (!$fromdir) {
-		$fromdir = $CONFIG->path . 'engine/schema/upgrades/';
-	}
-
-	if ($handle = opendir($fromdir)) {
-		$sqlupgrades = array();
-
-		while ($sqlfile = readdir($handle)) {
-			if (!is_dir($fromdir . $sqlfile)) {
-				if (preg_match('/^([0-9]{10})\.(sql)$/', $sqlfile, $matches)) {
-					$sql_version = (int) $matches[1];
-					if ($sql_version > $version) {
-						$sqlupgrades[] = $sqlfile;
-					}
-				}
-			}
-		}
-
-		asort($sqlupgrades);
-
-		if (sizeof($sqlupgrades) > 0) {
-			foreach ($sqlupgrades as $sqlfile) {
-
-				// hide all errors.
-				if ($quiet) {
-					try {
-						run_sql_script($fromdir . $sqlfile);
-					} catch (DatabaseException $e) {
-						error_log($e->getmessage());
-					}
-				} else {
-					run_sql_script($fromdir . $sqlfile);
-				}
-			}
-		}
-	}
-
-	return TRUE;
+function elgg_format_query($query) {
+	// remove newlines and extra spaces so logs are easier to read
+	return preg_replace('/\s\s+/', ' ', $query);
 }
 
 /**
@@ -754,26 +723,42 @@ function sanitize_string($string) {
 /**
  * Sanitises an integer for database use.
  *
- * @param int $int Integer
- *
- * @return int Sanitised integer
+ * @param int  $int    Value to be sanitized
+ * @param bool $signed Whether negative values should be allowed (true)
+ * @return int
  */
-function sanitise_int($int) {
+function sanitise_int($int, $signed = true) {
+	$int = (int) $int;
+
+	if ($signed === false) {
+		if ($int < 0) {
+			$int = 0;
+		}
+	}
+
 	return (int) $int;
 }
 
 /**
- * Wrapper function for alternate English spelling
- *
- * @param int $int Integer
+ * Sanitizes an integer for database use.
+ * Wrapper function for alternate English spelling (@see sanitise_int)
  *
- * @return int Sanitised integer
+ * @param int  $int    Value to be sanitized
+ * @param bool $signed Whether negative values should be allowed (true)
+ * @return int
  */
-function sanitize_int($int) {
-	return (int) $int;
+function sanitize_int($int, $signed = true) {
+	return sanitise_int($int, $signed);
 }
 
 /**
- * @elgg_register_event boot system init_db
+ * Registers shutdown functions for database profiling and delayed queries.
+ *
+ * @access private
  */
-register_elgg_event_handler('boot', 'system', 'init_db', 0);
\ No newline at end of file
+function init_db() {
+	register_shutdown_function('db_delayedexecution_shutdown_hook');
+	register_shutdown_function('db_profiling_shutdown_hook');
+}
+
+elgg_register_event_handler('init', 'system', 'init_db');