Merge branch 'master' of git://gitorious.org/lorea/elgg into saravea

19 files changed, 366 insertions, 246 deletions

diff --git a/engine/classes/ElggPAM.php b/engine/classes/ElggPAM.php
index 0681a909b..f07095fc1 100644
--- a/engine/classes/ElggPAM.php
+++ b/engine/classes/ElggPAM.php
@@ -53,11 +53,17 @@ class ElggPAM {
 
 		foreach ($_PAM_HANDLERS[$this->policy] as $k => $v) {
 			$handler = $v->handler;
+			if (!is_callable($handler)) {
+				continue;
+			}
+			/* @var callable $handler */
+
 			$importance = $v->importance;
 
 			try {
 				// Execute the handler
-				$result = $handler($credentials);
+				// @todo don't assume $handler is a global function
+				$result = call_user_func($handler, $credentials);
 				if ($result) {
 					$authenticated = true;
 				} elseif ($result === false) {
diff --git a/engine/classes/ElggPlugin.php b/engine/classes/ElggPlugin.php
index 3e43c8e81..c6ce2905f 100644
--- a/engine/classes/ElggPlugin.php
+++ b/engine/classes/ElggPlugin.php
@@ -36,8 +36,9 @@ class ElggPlugin extends ElggObject {
 	 * @warning Unlike other ElggEntity objects, you cannot null instantiate
 	 *          ElggPlugin. You must point it to an actual plugin GUID or location.
 	 *
-	 * @param mixed $plugin The GUID of the ElggPlugin object or the path of
-	 *                      the plugin to load.
+	 * @param mixed $plugin The GUID of the ElggPlugin object or the path of the plugin to load.
+	 *
+	 * @throws PluginException
 	 */
 	public function __construct($plugin) {
 		if (!$plugin) {
@@ -76,6 +77,8 @@ class ElggPlugin extends ElggObject {
 			// load the rest of the plugin
 			parent::__construct($existing_guid);
 		}
+
+		_elgg_cache_plugin_by_id($this);
 	}
 
 	/**
diff --git a/engine/classes/ElggVolatileMetadataCache.php b/engine/classes/ElggVolatileMetadataCache.php
index 24ae58d42..8a33c198d 100644
--- a/engine/classes/ElggVolatileMetadataCache.php
+++ b/engine/classes/ElggVolatileMetadataCache.php
@@ -279,6 +279,9 @@ class ElggVolatileMetadataCache {
 			),
 			'selects' => array('n.string AS name', 'v.string AS value'),
 			'order_by' => 'n_table.entity_guid, n_table.time_created ASC',
+
+			// @todo don't know why this is necessary
+			'wheres' => array(get_access_sql_suffix('n_table')),
 		);
 		$data = elgg_get_metadata($options);
 
diff --git a/engine/handlers/cache_handler.php b/engine/handlers/cache_handler.php
index b332ec379..7706c2c92 100644
--- a/engine/handlers/cache_handler.php
+++ b/engine/handlers/cache_handler.php
@@ -64,7 +64,7 @@ $ts = $matches[4];
 
 // If is the same ETag, content didn't changed.
 $etag = $ts;
-if (isset($_SERVER['HTTP_IF_NONE_MATCH']) && trim($_SERVER['HTTP_IF_NONE_MATCH']) == $etag) {
+if (isset($_SERVER['HTTP_IF_NONE_MATCH']) && trim($_SERVER['HTTP_IF_NONE_MATCH']) == "\"$etag\"") {
 	header("HTTP/1.1 304 Not Modified");
 	exit;
 }
@@ -80,10 +80,10 @@ switch ($type) {
 		break;
 }
 
-header('Expires: ' . date('r', strtotime("+6 months")), true);
+header('Expires: ' . gmdate('D, d M Y H:i:s \G\M\T', strtotime("+6 months")), true);
 header("Pragma: public", true);
 header("Cache-Control: public", true);
-header("ETag: $etag");
+header("ETag: \"$etag\"");
 
 $filename = $dataroot . 'views_simplecache/' . md5($viewtype . $view);
 
diff --git a/engine/lib/access.php b/engine/lib/access.php
index e8b3b0d52..3b2b7aeaa 100644
--- a/engine/lib/access.php
+++ b/engine/lib/access.php
@@ -88,11 +88,7 @@ function get_access_array($user_id = 0, $site_id = 0, $flush = false) {
 
 	// @todo everything from the db is cached.
 	// this cache might be redundant. But db cache is flushed on every db write.
-	static $access_array;
-
-	if (!isset($access_array)) {
-		$access_array = array();
-	}
+	static $access_array = array();
 
 	if ($user_id == 0) {
 		$user_id = elgg_get_logged_in_user_guid();
@@ -476,7 +472,7 @@ function can_edit_access_collection($collection_id, $user_guid = null) {
 		return false;
 	}
 
-	$write_access = get_write_access_array($user->getGUID(), null, true);
+	$write_access = get_write_access_array($user->getGUID(), 0, true);
 
 	// don't ignore access when checking users.
 	if ($user_guid) {
@@ -560,8 +556,6 @@ function create_access_collection($name, $owner_guid = 0, $site_guid = 0) {
  * @see remove_user_from_access_collection()
  */
 function update_access_collection($collection_id, $members) {
-	global $CONFIG;
-
 	$acl = get_access_collection($collection_id);
 
 	if (!$acl) {
@@ -1018,6 +1012,7 @@ function elgg_override_permissions($hook, $type, $value, $params) {
  */
 function access_test($hook, $type, $value, $params) {
 	global $CONFIG;
+
 	$value[] = $CONFIG->path . 'engine/tests/api/access_collections.php';
 	return $value;
 }
diff --git a/engine/lib/admin.php b/engine/lib/admin.php
index b65d98c95..3f23f079c 100644
--- a/engine/lib/admin.php
+++ b/engine/lib/admin.php
@@ -268,8 +268,9 @@ function admin_init() {
 	// users
 	elgg_register_admin_menu_item('administer', 'users', null, 20);
 	elgg_register_admin_menu_item('administer', 'online', 'users', 10);
-	elgg_register_admin_menu_item('administer', 'newest', 'users', 20);
-	elgg_register_admin_menu_item('administer', 'add', 'users', 30);
+	elgg_register_admin_menu_item('administer', 'admins', 'users', 20);
+	elgg_register_admin_menu_item('administer', 'newest', 'users', 30);
+	elgg_register_admin_menu_item('administer', 'add', 'users', 40);
 
 	// configure
 	// plugins
diff --git a/engine/lib/configuration.php b/engine/lib/configuration.php
index 305aa00b6..b10e51130 100644
--- a/engine/lib/configuration.php
+++ b/engine/lib/configuration.php
@@ -91,23 +91,29 @@ function elgg_get_config($name, $site_guid = 0) {
 		return $CONFIG->$name;
 	}
 
-	if ($site_guid === NULL) {
+	if ($site_guid === null) {
 		// installation wide setting
 		$value = datalist_get($name);
 	} else {
-		// site specific setting
-		if ($site_guid == 0) {
-			$site_guid = (int) $CONFIG->site_id;
+		// hit DB only if we're not sure if value exists or not
+		if (!isset($CONFIG->site_config_loaded)) {
+			// site specific setting
+			if ($site_guid == 0) {
+				$site_guid = (int) $CONFIG->site_id;
+			}
+			$value = get_config($name, $site_guid);
+		} else {
+			$value = null;
 		}
-		$value = get_config($name, $site_guid);
 	}
 
-	if ($value !== false) {
-		$CONFIG->$name = $value;
-		return $value;
+	// @todo document why we don't cache false
+	if ($value === false) {
+		return null;
 	}
 
-	return null;
+	$CONFIG->$name = $value;
+	return $value;
 }
 
 /**
@@ -558,6 +564,8 @@ function _elgg_load_site_config() {
 	$CONFIG->url = $CONFIG->wwwroot;
 
 	get_all_config();
+	// gives hint to elgg_get_config function how to approach missing values
+	$CONFIG->site_config_loaded = true;
 }
 
 /**
diff --git a/engine/lib/elgglib.php b/engine/lib/elgglib.php
index 26c1cccfd..dd3cba25d 100644
--- a/engine/lib/elgglib.php
+++ b/engine/lib/elgglib.php
@@ -671,7 +671,7 @@ function elgg_register_event_handler($event, $object_type, $callback, $priority
 	global $CONFIG;
 
 	if (empty($event) || empty($object_type)) {
-		return FALSE;
+		return false;
 	}
 
 	if (!isset($CONFIG->events)) {
@@ -684,8 +684,8 @@ function elgg_register_event_handler($event, $object_type, $callback, $priority
 		$CONFIG->events[$event][$object_type] = array();
 	}
 
-	if (!is_callable($callback)) {
-		return FALSE;
+	if (!is_callable($callback, true)) {
+		return false;
 	}
 
 	$priority = max((int) $priority, 0);
@@ -695,7 +695,7 @@ function elgg_register_event_handler($event, $object_type, $callback, $priority
 	}
 	$CONFIG->events[$event][$object_type][$priority] = $callback;
 	ksort($CONFIG->events[$event][$object_type]);
-	return TRUE;
+	return true;
 }
 
 /**
@@ -770,14 +770,14 @@ function elgg_trigger_event($event, $object_type, $object = null) {
 	foreach ($events as $callback_list) {
 		if (is_array($callback_list)) {
 			foreach ($callback_list as $callback) {
-				if (call_user_func_array($callback, $args) === FALSE) {
-					return FALSE;
+				if (is_callable($callback) && (call_user_func_array($callback, $args) === false)) {
+					return false;
 				}
 			}
 		}
 	}
 
-	return TRUE;
+	return true;
 }
 
 /**
@@ -850,7 +850,7 @@ function elgg_register_plugin_hook_handler($hook, $type, $callback, $priority =
 	global $CONFIG;
 
 	if (empty($hook) || empty($type)) {
-		return FALSE;
+		return false;
 	}
 
 	if (!isset($CONFIG->hooks)) {
@@ -863,8 +863,8 @@ function elgg_register_plugin_hook_handler($hook, $type, $callback, $priority =
 		$CONFIG->hooks[$hook][$type] = array();
 	}
 
-	if (!is_callable($callback)) {
-		return FALSE;
+	if (!is_callable($callback, true)) {
+		return false;
 	}
 
 	$priority = max((int) $priority, 0);
@@ -874,7 +874,7 @@ function elgg_register_plugin_hook_handler($hook, $type, $callback, $priority =
 	}
 	$CONFIG->hooks[$hook][$type][$priority] = $callback;
 	ksort($CONFIG->hooks[$hook][$type]);
-	return TRUE;
+	return true;
 }
 
 /**
@@ -970,10 +970,12 @@ function elgg_trigger_plugin_hook($hook, $type, $params = null, $returnvalue = n
 	foreach ($hooks as $callback_list) {
 		if (is_array($callback_list)) {
 			foreach ($callback_list as $hookcallback) {
-				$args = array($hook, $type, $returnvalue, $params);
-				$temp_return_value = call_user_func_array($hookcallback, $args);
-				if (!is_null($temp_return_value)) {
-					$returnvalue = $temp_return_value;
+				if (is_callable($hookcallback)) {
+					$args = array($hook, $type, $returnvalue, $params);
+					$temp_return_value = call_user_func_array($hookcallback, $args);
+					if (!is_null($temp_return_value)) {
+						$returnvalue = $temp_return_value;
+					}
 				}
 			}
 		}
@@ -1883,7 +1885,7 @@ function elgg_cacheable_view_page_handler($page, $type) {
 		header("Content-type: $content_type");
 
 		// @todo should js be cached when simple cache turned off
-		//header('Expires: ' . date('r', time() + 864000));
+		//header('Expires: ' . gmdate('D, d M Y H:i:s \G\M\T', strtotime("+10 days")), true);
 		//header("Pragma: public");
 		//header("Cache-Control: public");
 		//header("Content-Length: " . strlen($return));
diff --git a/engine/lib/entities.php b/engine/lib/entities.php
index a50567d9f..fda554388 100644
--- a/engine/lib/entities.php
+++ b/engine/lib/entities.php
@@ -17,13 +17,13 @@ global $ENTITY_CACHE;
 $ENTITY_CACHE = array();
 
 /**
- * Cache subtypes and related class names once loaded.
+ * Cache subtypes and related class names.
  *
- * @global array $SUBTYPE_CACHE
+ * @global array|null $SUBTYPE_CACHE array once populated from DB, initially null
  * @access private
  */
 global $SUBTYPE_CACHE;
-$SUBTYPE_CACHE = NULL;
+$SUBTYPE_CACHE = null;
 
 /**
  * Invalidate this class's entry in the cache.
@@ -59,16 +59,24 @@ function invalidate_cache_for_entity($guid) {
 function cache_entity(ElggEntity $entity) {
 	global $ENTITY_CACHE;
 
-	// Don't cache entities while access control is off, otherwise they could be
+	// Don't cache non-plugin entities while access control is off, otherwise they could be
 	// exposed to users who shouldn't see them when control is re-enabled.
-	if (elgg_get_ignore_access()) {
+	if (!($entity instanceof ElggPlugin) && elgg_get_ignore_access()) {
 		return;
 	}
 
 	// Don't store too many or we'll have memory problems
 	// TODO(evan): Pick a less arbitrary limit
 	if (count($ENTITY_CACHE) > 256) {
-		unset($ENTITY_CACHE[array_rand($ENTITY_CACHE)]);
+		$random_guid = array_rand($ENTITY_CACHE);
+
+		unset($ENTITY_CACHE[$random_guid]);
+
+		// Purge separate metadata cache. Original idea was to do in entity destructor, but that would
+		// have caused a bunch of unnecessary purges at every shutdown. Doing it this way we have no way
+		// to know that the expunged entity will be GCed (might be another reference living), but that's
+		// OK; the metadata will reload if necessary.
+		elgg_get_metadata_cache()->clear($random_guid);
 	}
 
 	$ENTITY_CACHE[$entity->guid] = $entity;
@@ -87,8 +95,6 @@ function cache_entity(ElggEntity $entity) {
 function retrieve_cached_entity($guid) {
 	global $ENTITY_CACHE;
 
-	$guid = (int)$guid;
-
 	if (isset($ENTITY_CACHE[$guid])) {
 		if ($ENTITY_CACHE[$guid]->isFullyLoaded()) {
 			return $ENTITY_CACHE[$guid];
@@ -148,29 +154,23 @@ function retrieve_cached_entity_row($guid) {
  * @access private
  */
 function get_subtype_id($type, $subtype) {
-	global $CONFIG, $SUBTYPE_CACHE;
+	global $SUBTYPE_CACHE;
 
-	$type = sanitise_string($type);
-	$subtype = sanitise_string($subtype);
-
-	if ($subtype == "") {
-		return FALSE;
+	if (!$subtype) {
+		return false;
 	}
 
-	// @todo use the cache before hitting database
-	$result = get_data_row("SELECT * from {$CONFIG->dbprefix}entity_subtypes
-		where type='$type' and subtype='$subtype'");
-
-	if ($result) {
-		if (!$SUBTYPE_CACHE) {
-			$SUBTYPE_CACHE = array();
-		}
+	if ($SUBTYPE_CACHE === null) {
+		_elgg_populate_subtype_cache();
+	}
 
-		$SUBTYPE_CACHE[$result->id] = $result;
+	// use the cache before hitting database
+	$result = _elgg_retrieve_cached_subtype($type, $subtype);
+	if ($result !== null) {
 		return $result->id;
 	}
 
-	return FALSE;
+	return false;
 }
 
 /**
@@ -178,35 +178,67 @@ function get_subtype_id($type, $subtype) {
  *
  * @param int $subtype_id Subtype ID
  *
- * @return string Subtype name
+ * @return string|false Subtype name, false if subtype not found
  * @link http://docs.elgg.org/DataModel/Entities/Subtypes
  * @see get_subtype_from_id()
  * @access private
  */
 function get_subtype_from_id($subtype_id) {
-	global $CONFIG, $SUBTYPE_CACHE;
-
-	$subtype_id = (int)$subtype_id;
+	global $SUBTYPE_CACHE;
 
 	if (!$subtype_id) {
 		return false;
 	}
 
+	if ($SUBTYPE_CACHE === null) {
+		_elgg_populate_subtype_cache();
+	}
+
 	if (isset($SUBTYPE_CACHE[$subtype_id])) {
 		return $SUBTYPE_CACHE[$subtype_id]->subtype;
 	}
 
-	$result = get_data_row("SELECT * from {$CONFIG->dbprefix}entity_subtypes where id=$subtype_id");
-	if ($result) {
-		if (!$SUBTYPE_CACHE) {
-			$SUBTYPE_CACHE = array();
-		}
+	return false;
+}
+
+/**
+ * Retrieve subtype from the cache.
+ *
+ * @param string $type
+ * @param string $subtype
+ * @return stdClass|null
+ *
+ * @access private
+ */
+function _elgg_retrieve_cached_subtype($type, $subtype) {
+	global $SUBTYPE_CACHE;
 
-		$SUBTYPE_CACHE[$subtype_id] = $result;
-		return $result->subtype;
+	if ($SUBTYPE_CACHE === null) {
+		_elgg_populate_subtype_cache();
 	}
 
-	return false;
+	foreach ($SUBTYPE_CACHE as $obj) {
+		if ($obj->type === $type && $obj->subtype === $subtype) {
+			return $obj;
+		}
+	}
+	return null;
+}
+
+/**
+ * Fetch all suptypes from DB to local cache.
+ *
+ * @access private
+ */
+function _elgg_populate_subtype_cache() {
+	global $CONFIG, $SUBTYPE_CACHE;
+	
+	$results = get_data("SELECT * FROM {$CONFIG->dbprefix}entity_subtypes");
+	
+	$SUBTYPE_CACHE = array();
+	foreach ($results as $row) {
+		$SUBTYPE_CACHE[$row->id] = $row;
+	}
 }
 
 /**
@@ -225,25 +257,19 @@ function get_subtype_from_id($subtype_id) {
  * @access private
  */
 function get_subtype_class($type, $subtype) {
-	global $CONFIG, $SUBTYPE_CACHE;
-
-	$type = sanitise_string($type);
-	$subtype = sanitise_string($subtype);
-
-	// @todo use the cache before going to the database
-	$result = get_data_row("SELECT * from {$CONFIG->dbprefix}entity_subtypes
-		where type='$type' and subtype='$subtype'");
+	global $SUBTYPE_CACHE;
 
-	if ($result) {
-		if (!$SUBTYPE_CACHE) {
-			$SUBTYPE_CACHE = array();
-		}
-
-		$SUBTYPE_CACHE[$result->id] = $result;
-		return $result->class;
+	if ($SUBTYPE_CACHE === null) {
+		_elgg_populate_subtype_cache();
+	}
+	
+	// use the cache before going to the database
+	$obj = _elgg_retrieve_cached_subtype($type, $subtype);
+	if ($obj) {
+		return $obj->class;
 	}
 
-	return NULL;
+	return null;
 }
 
 /**
@@ -257,29 +283,21 @@ function get_subtype_class($type, $subtype) {
  * @access private
  */
 function get_subtype_class_from_id($subtype_id) {
-	global $CONFIG, $SUBTYPE_CACHE;
-
-	$subtype_id = (int)$subtype_id;
+	global $SUBTYPE_CACHE;
 
 	if (!$subtype_id) {
-		return false;
+		return null;
 	}
 
+	if ($SUBTYPE_CACHE === null) {
+		_elgg_populate_subtype_cache();
+	}
+	
 	if (isset($SUBTYPE_CACHE[$subtype_id])) {
 		return $SUBTYPE_CACHE[$subtype_id]->class;
 	}
 
-	$result = get_data_row("SELECT * from {$CONFIG->dbprefix}entity_subtypes where id=$subtype_id");
-
-	if ($result) {
-		if (!$SUBTYPE_CACHE) {
-			$SUBTYPE_CACHE = array();
-		}
-		$SUBTYPE_CACHE[$subtype_id] = $result;
-		return $result->class;
-	}
-
-	return NULL;
+	return null;
 }
 
 /**
@@ -305,21 +323,32 @@ function get_subtype_class_from_id($subtype_id) {
  * @see get_entity()
  */
 function add_subtype($type, $subtype, $class = "") {
-	global $CONFIG;
-	$type = sanitise_string($type);
-	$subtype = sanitise_string($subtype);
-	$class = sanitise_string($class);
+	global $CONFIG, $SUBTYPE_CACHE;
 
-	// Short circuit if no subtype is given
-	if ($subtype == "") {
+	if (!$subtype) {
 		return 0;
 	}
 
 	$id = get_subtype_id($type, $subtype);
 
-	if ($id == 0) {
-		return insert_data("insert into {$CONFIG->dbprefix}entity_subtypes"
-			. " (type, subtype, class) values ('$type','$subtype','$class')");
+	if (!$id) {
+		// In cache we store non-SQL-escaped strings because that's what's returned by query
+		$cache_obj = (object) array(
+			'type' => $type,
+			'subtype' => $subtype,
+			'class' => $class,
+		);
+
+		$type = sanitise_string($type);
+		$subtype = sanitise_string($subtype);
+		$class = sanitise_string($class);
+
+		$id = insert_data("INSERT INTO {$CONFIG->dbprefix}entity_subtypes"
+			. " (type, subtype, class) VALUES ('$type', '$subtype', '$class')");
+		
+		// add entry to cache
+		$cache_obj->id = $id;
+		$SUBTYPE_CACHE[$id] = $cache_obj;
 	}
 
 	return $id;
@@ -361,22 +390,31 @@ function remove_subtype($type, $subtype) {
 function update_subtype($type, $subtype, $class = '') {
 	global $CONFIG, $SUBTYPE_CACHE;
 
-	if (!$id = get_subtype_id($type, $subtype)) {
-		return FALSE;
+	$id = get_subtype_id($type, $subtype);
+	if (!$id) {
+		return false;
 	}
+
+	if ($SUBTYPE_CACHE === null) {
+		_elgg_populate_subtype_cache();
+	}
+
+	$unescaped_class = $class;
+
 	$type = sanitise_string($type);
 	$subtype = sanitise_string($subtype);
-
-	$result = update_data("UPDATE {$CONFIG->dbprefix}entity_subtypes
+	$class = sanitise_string($class);
+	
+	$success = update_data("UPDATE {$CONFIG->dbprefix}entity_subtypes
 		SET type = '$type', subtype = '$subtype', class = '$class'
 		WHERE id = $id
 	");
 
-	if ($result && isset($SUBTYPE_CACHE[$id])) {
-		$SUBTYPE_CACHE[$id]->class = $class;
+	if ($success && isset($SUBTYPE_CACHE[$id])) {
+		$SUBTYPE_CACHE[$id]->class = $unescaped_class;
 	}
 
-	return $result;
+	return $success;
 }
 
 /**
@@ -1772,7 +1810,7 @@ function oddentity_to_elggentity(ODDEntity $element) {
 	if (!$tmp) {
 		// Construct new class with owner from session
 		$classname = get_subtype_class($class, $subclass);
-		if ($classname != "") {
+		if ($classname) {
 			if (class_exists($classname)) {
 				$tmp = new $classname();
 
@@ -1838,7 +1876,7 @@ function oddentity_to_elggentity(ODDEntity $element) {
 function import_entity_plugin_hook($hook, $entity_type, $returnvalue, $params) {
 	$element = $params['element'];
 
-	$tmp = NULL;
+	$tmp = null;
 
 	if ($element instanceof ODDEntity) {
 		$tmp = oddentity_to_elggentity($element);
@@ -2011,7 +2049,7 @@ function get_entity_url($entity_guid) {
 function elgg_register_entity_url_handler($entity_type, $entity_subtype, $function_name) {
 	global $CONFIG;
 
-	if (!is_callable($function_name)) {
+	if (!is_callable($function_name, true)) {
 		return false;
 	}
 
diff --git a/engine/lib/extender.php b/engine/lib/extender.php
index 43421342c..538f601e1 100644
--- a/engine/lib/extender.php
+++ b/engine/lib/extender.php
@@ -136,14 +136,15 @@ function can_edit_extender($extender_id, $type, $user_guid = 0) {
 
 	$functionname = "elgg_get_{$type}_from_id";
 	if (is_callable($functionname)) {
-		$extender = $functionname($extender_id);
+		$extender = call_user_func($functionname, $extender_id);
 	} else {
 		return false;
 	}
 
-	if (!is_a($extender, "ElggExtender")) {
+	if (!($extender instanceof ElggExtender)) {
 		return false;
 	}
+	/* @var ElggExtender $extender */
 
 	// If the owner is the specified user, great! They can edit.
 	if ($extender->getOwnerGUID() == $user->getGUID()) {
@@ -175,7 +176,7 @@ function elgg_register_extender_url_handler($extender_type, $extender_name, $fun
 
 	global $CONFIG;
 
-	if (!is_callable($function_name)) {
+	if (!is_callable($function_name, true)) {
 		return false;
 	}
 
@@ -228,7 +229,7 @@ function get_extender_url(ElggExtender $extender) {
 	if ($url == "") {
 		$nameid = $extender->id;
 		if ($type == 'volatile') {
-			$nameid == $extender->name;
+			$nameid = $extender->name;
 		}
 		$url = "export/$view/$guid/$type/$nameid/";
 	}
diff --git a/engine/lib/notification.php b/engine/lib/notification.php
index 18faff27f..9e3c075a8 100644
--- a/engine/lib/notification.php
+++ b/engine/lib/notification.php
@@ -38,7 +38,7 @@ $NOTIFICATION_HANDLERS = array();
 function register_notification_handler($method, $handler, $params = NULL) {
 	global $NOTIFICATION_HANDLERS;
 
-	if (is_callable($handler)) {
+	if (is_callable($handler, true)) {
 		$NOTIFICATION_HANDLERS[$method] = new stdClass;
 
 		$NOTIFICATION_HANDLERS[$method]->handler = $handler;
@@ -131,8 +131,9 @@ function notify_user($to, $from, $subject, $message, array $params = NULL, $meth
 					// Extract method details from list
 					$details = $NOTIFICATION_HANDLERS[$method];
 					$handler = $details->handler;
+					/* @var callable $handler */
 
-					if ((!$NOTIFICATION_HANDLERS[$method]) || (!$handler)) {
+					if ((!$NOTIFICATION_HANDLERS[$method]) || (!$handler) || (!is_callable($handler))) {
 						error_log(elgg_echo('NotificationException:NoHandlerFound', array($method)));
 					}
 
@@ -140,7 +141,7 @@ function notify_user($to, $from, $subject, $message, array $params = NULL, $meth
 
 					// Trigger handler and retrieve result.
 					try {
-						$result[$guid][$method] = $handler(
+						$result[$guid][$method] = call_user_func($handler,
 							$from ? get_entity($from) : NULL, 	// From entity
 							get_entity($guid), 					// To entity
 							$subject,							// The subject
diff --git a/engine/lib/pagehandler.php b/engine/lib/pagehandler.php
index ba7518a77..0cf99b6fe 100644
--- a/engine/lib/pagehandler.php
+++ b/engine/lib/pagehandler.php
@@ -45,7 +45,10 @@ function page_handler($handler, $page) {
 	$page = $request['segments'];
 
 	$result = false;
-	if (isset($CONFIG->pagehandler) && !empty($handler) && isset($CONFIG->pagehandler[$handler])) {
+	if (isset($CONFIG->pagehandler)
+			&& !empty($handler)
+			&& isset($CONFIG->pagehandler[$handler])
+			&& is_callable($CONFIG->pagehandler[$handler])) {
 		$function = $CONFIG->pagehandler[$handler];
 		$result = call_user_func($function, $page, $handler);
 	}
@@ -76,14 +79,15 @@ function page_handler($handler, $page) {
  * @param string $handler  The page type to handle
  * @param string $function Your function name
  *
- * @return true|false Depending on success
+ * @return bool Depending on success
  */
 function elgg_register_page_handler($handler, $function) {
 	global $CONFIG;
+
 	if (!isset($CONFIG->pagehandler)) {
 		$CONFIG->pagehandler = array();
 	}
-	if (is_callable($function)) {
+	if (is_callable($function, true)) {
 		$CONFIG->pagehandler[$handler] = $function;
 		return true;
 	}
diff --git a/engine/lib/pam.php b/engine/lib/pam.php
index 4f9f44278..1c9c3bfe1 100644
--- a/engine/lib/pam.php
+++ b/engine/lib/pam.php
@@ -30,7 +30,9 @@ $_PAM_HANDLERS = array();
  * failure, return false or throw an exception. Returning nothing indicates that
  * the handler wants to be skipped.
  *
- * @param string $handler    The handler function in the format
+ * Note, $handler must be string callback (not an array/Closure).
+ *
+ * @param string $handler    Callable global handler function in the format ()
  * 		                     pam_handler($credentials = NULL);
  * @param string $importance The importance - "sufficient" (default) or "required"
  * @param string $policy     The policy type, default is "user"
@@ -45,7 +47,8 @@ function register_pam_handler($handler, $importance = "sufficient", $policy = "u
 		$_PAM_HANDLERS[$policy] = array();
 	}
 
-	if (is_callable($handler)) {
+	// @todo remove requirement that $handle be a global function
+	if (is_string($handler) && is_callable($handler, true)) {
 		$_PAM_HANDLERS[$policy][$handler] = new stdClass;
 
 		$_PAM_HANDLERS[$policy][$handler]->handler = $handler;
diff --git a/engine/lib/plugins.php b/engine/lib/plugins.php
index d5cd4fe76..ca4a957f4 100644
--- a/engine/lib/plugins.php
+++ b/engine/lib/plugins.php
@@ -176,6 +176,19 @@ function elgg_generate_plugin_entities() {
 }
 
 /**
+ * Cache a reference to this plugin by its ID
+ * 
+ * @param ElggPlugin $plugin
+ * 
+ * @access private
+ */
+function _elgg_cache_plugin_by_id(ElggPlugin $plugin) {
+	$map = (array) elgg_get_config('plugins_by_id_map');
+	$map[$plugin->getID()] = $plugin;
+	elgg_set_config('plugins_by_id_map', $map);
+}
+
+/**
  * Returns an ElggPlugin object with the path $path.
  *
  * @param string $plugin_id The id (dir name) of the plugin. NOT the guid.
@@ -183,6 +196,11 @@ function elgg_generate_plugin_entities() {
  * @since 1.8.0
  */
 function elgg_get_plugin_from_id($plugin_id) {
+	$map = (array) elgg_get_config('plugins_by_id_map');
+	if (isset($map[$plugin_id])) {
+		return $map[$plugin_id];
+	}
+
 	$plugin_id = sanitize_string($plugin_id);
 	$db_prefix = get_config('dbprefix');
 
@@ -512,6 +530,8 @@ function elgg_namespace_plugin_private_setting($type, $name, $id = null) {
  * @return string|false Plugin name, or false if no plugin name was called
  * @since 1.8.0
  * @access private
+ *
+ * @todo get rid of this
  */
 function elgg_get_calling_plugin_id($mainfilename = false) {
 	if (!$mainfilename) {
@@ -920,6 +940,7 @@ function elgg_set_plugin_setting($name, $value, $plugin_id = null) {
  *
  * @return mixed
  * @since 1.8.0
+ * @todo make $plugin_id required in future version
  */
 function elgg_get_plugin_setting($name, $plugin_id = null) {
 	if ($plugin_id) {
diff --git a/engine/lib/relationships.php b/engine/lib/relationships.php
index 09d541e22..01654b1ce 100644
--- a/engine/lib/relationships.php
+++ b/engine/lib/relationships.php
@@ -416,7 +416,7 @@ function elgg_list_entities_from_relationship_count($options) {
 function elgg_register_relationship_url_handler($relationship_type, $function_name) {
 	global $CONFIG;
 
-	if (!is_callable($function_name)) {
+	if (!is_callable($function_name, true)) {
 		return false;
 	}
 
diff --git a/engine/lib/river.php b/engine/lib/river.php
index b717a7756..33f34360e 100644
--- a/engine/lib/river.php
+++ b/engine/lib/river.php
@@ -361,6 +361,7 @@ function elgg_get_river(array $options = array()) {
 		}
 
 		$river_items = get_data($query, 'elgg_row_to_elgg_river_item');
+		_elgg_prefetch_river_entities($river_items);
 
 		return $river_items;
 	} else {
@@ -370,11 +371,56 @@ function elgg_get_river(array $options = array()) {
 }
 
 /**
+ * Prefetch entities that will be displayed in the river.
+ *
+ * @param ElggRiverItem[] $river_items
+ * @access private
+ */
+function _elgg_prefetch_river_entities(array $river_items) {
+	// prefetch objects and subjects
+	$guids = array();
+	foreach ($river_items as $item) {
+		if ($item->subject_guid && !retrieve_cached_entity($item->subject_guid)) {
+			$guids[$item->subject_guid] = true;
+		}
+		if ($item->object_guid && !retrieve_cached_entity($item->object_guid)) {
+			$guids[$item->object_guid] = true;
+		}
+	}
+	if ($guids) {
+		// avoid creating oversized query
+		// @todo how to better handle this?
+		$guids = array_slice($guids, 0, 300, true);
+		// return value unneeded, just priming cache
+		elgg_get_entities(array(
+			'guids' => array_keys($guids),
+			'limit' => 0,
+		));
+	}
+
+	// prefetch object containers
+	$guids = array();
+	foreach ($river_items as $item) {
+		$object = $item->getObjectEntity();
+		if ($object->container_guid && !retrieve_cached_entity($object->container_guid)) {
+			$guids[$object->container_guid] = true;
+		}
+	}
+	if ($guids) {
+		$guids = array_slice($guids, 0, 300, true);
+		elgg_get_entities(array(
+			'guids' => array_keys($guids),
+			'limit' => 0,
+		));
+	}
+}
+
+/**
  * List river items
  *
  * @param array $options Any options from elgg_get_river() plus:
  * 	 pagination => BOOL Display pagination links (true)
-
+ *
  * @return string
  * @since 1.8.0
  */
diff --git a/engine/lib/views.php b/engine/lib/views.php
index b92d9c57a..e24ccb942 100644
--- a/engine/lib/views.php
+++ b/engine/lib/views.php
@@ -101,15 +101,15 @@ function elgg_get_viewtype() {
 		return $CURRENT_SYSTEM_VIEWTYPE;
 	}
 
-	$viewtype = get_input('view', NULL);
-	if ($viewtype) {
+	$viewtype = get_input('view', '', false);
+	if (is_string($viewtype) && $viewtype !== '') {
 		// only word characters allowed.
-		if (!preg_match('[\W]', $viewtype)) {
+		if (!preg_match('/\W/', $viewtype)) {
 			return $viewtype;
 		}
 	}
 
-	if (isset($CONFIG->view) && !empty($CONFIG->view)) {
+	if (!empty($CONFIG->view)) {
 		return $CONFIG->view;
 	}
 
@@ -258,8 +258,6 @@ function elgg_get_view_location($view, $viewtype = '') {
 	} else {
 		return $CONFIG->views->locations[$viewtype][$view];
 	}
-
-	return false;
 }
 
 /**
@@ -329,7 +327,7 @@ function elgg_view_exists($view, $viewtype = '', $recurse = true) {
 		$location = $CONFIG->views->locations[$viewtype][$view];
 	}
 
-	if (file_exists($location . "{$viewtype}/{$view}.php")) {
+	if (file_exists("{$location}{$viewtype}/{$view}.php")) {
 		return true;
 	}
 
@@ -378,7 +376,7 @@ function elgg_view_exists($view, $viewtype = '', $recurse = true) {
  * @param boolean $bypass   If set to true, elgg_view will bypass any specified
  *                          alternative template handler; by default, it will
  *                          hand off to this if requested (see set_template_handler)
- * @param boolean $debug    If set to true, the viewer will complain if it can't find a view
+ * @param boolean $ignored  This argument is ignored and will be removed eventually
  * @param string  $viewtype If set, forces the viewtype for the elgg_view call to be
  *                          this value (default: standard detection)
  *
@@ -386,18 +384,30 @@ function elgg_view_exists($view, $viewtype = '', $recurse = true) {
  * @see set_template_handler()
  * @example views/elgg_view.php
  * @link http://docs.elgg.org/View
- * @todo $debug isn't used.
- * @todo $usercache is redundant.
  */
-function elgg_view($view, $vars = array(), $bypass = false, $debug = false, $viewtype = '') {
+function elgg_view($view, $vars = array(), $bypass = false, $ignored = false, $viewtype = '') {
 	global $CONFIG;
-	static $usercache;
-
-	$view = (string)$view;
 
+	if (!is_string($view) || !is_string($viewtype)) {
+		elgg_log("View and Viewtype in views must be a strings: $view", 'NOTICE');
+		return '';
+	}
 	// basic checking for bad paths
 	if (strpos($view, '..') !== false) {
-		return false;
+		return '';
+	}
+
+	if (!is_array($vars)) {
+		elgg_log("Vars in views must be an array: $view", 'ERROR');
+		$vars = array();
+	}
+
+	// Get the current viewtype
+	if ($viewtype === '') {
+		$viewtype = elgg_get_viewtype();
+	} elseif (preg_match('/\W/', $viewtype)) {
+		// Viewtypes can only be alphanumeric
+		return '';
 	}
 
 	$view_orig = $view;
@@ -408,19 +418,6 @@ function elgg_view($view, $vars = array(), $bypass = false, $debug = false, $vie
 		elgg_trigger_event('pagesetup', 'system');
 	}
 
-	if (!is_array($usercache)) {
-		$usercache = array();
-	}
-
-	if (!is_array($vars)) {
-		elgg_log("Vars in views must be an array: $view", 'ERROR');
-		$vars = array();
-	}
-
-	if (empty($vars)) {
-		$vars = array();
-	}
-
 	// @warning - plugin authors: do not expect user, config, and url to be
 	// set by elgg_view() in the future. Instead, use elgg_get_logged_in_user_entity(),
 	// elgg_get_config(), and elgg_get_site_url() in your views.
@@ -475,16 +472,6 @@ function elgg_view($view, $vars = array(), $bypass = false, $debug = false, $vie
 		}
 	}
 
-	// Get the current viewtype
-	if (empty($viewtype)) {
-		$viewtype = elgg_get_viewtype();
-	}
-
-	// Viewtypes can only be alphanumeric
-	if (preg_match('[\W]', $viewtype)) {
-		return '';
-	}
-
 	// Set up any extensions to the requested view
 	if (isset($CONFIG->views->extensions[$view])) {
 		$viewlist = $CONFIG->views->extensions[$view];
@@ -496,19 +483,21 @@ function elgg_view($view, $vars = array(), $bypass = false, $debug = false, $vie
 	ob_start();
 
 	foreach ($viewlist as $priority => $view) {
+
 		$view_location = elgg_get_view_location($view, $viewtype);
 		$view_file = "$view_location$viewtype/$view.php";
 
-		$default_location = elgg_get_view_location($view, 'default');
-		$default_view_file = "{$default_location}default/$view.php";
-
 		// try to include view
 		if (!file_exists($view_file) || !include($view_file)) {
 			// requested view does not exist
 			$error = "$viewtype/$view view does not exist.";
 
 			// attempt to load default view
-			if ($viewtype != 'default' && elgg_does_viewtype_fallback($viewtype)) {
+			if ($viewtype !== 'default' && elgg_does_viewtype_fallback($viewtype)) {
+
+				$default_location = elgg_get_view_location($view, 'default');
+				$default_view_file = "{$default_location}default/$view.php";
+
 				if (file_exists($default_view_file) && include($default_view_file)) {
 					// default view found
 					$error .= " Using default/$view instead.";
@@ -533,7 +522,7 @@ function elgg_view($view, $vars = array(), $bypass = false, $debug = false, $vie
 	// backward compatibility with less granular hook will be gone in 2.0
 	$content_tmp = elgg_trigger_plugin_hook('display', 'view', $params, $content);
 
-	if ($content_tmp != $content) {
+	if ($content_tmp !== $content) {
 		$content = $content_tmp;
 		elgg_deprecated_notice('The display:view plugin hook is deprecated by view:view_name', 1.8);
 	}
@@ -559,33 +548,32 @@ function elgg_view($view, $vars = array(), $bypass = false, $debug = false, $vie
  * @param string $view_extension This view is added to $view
  * @param int    $priority       The priority, from 0 to 1000,
  *                               to add at (lowest numbers displayed first)
- * @param string $viewtype       Not used
  *
  * @return void
  * @since 1.7.0
  * @link http://docs.elgg.org/Views/Extend
  * @example views/extend.php
  */
-function elgg_extend_view($view, $view_extension, $priority = 501, $viewtype = '') {
+function elgg_extend_view($view, $view_extension, $priority = 501) {
 	global $CONFIG;
 
 	if (!isset($CONFIG->views)) {
-		$CONFIG->views = new stdClass;
-	}
-
-	if (!isset($CONFIG->views->extensions)) {
-		$CONFIG->views->extensions = array();
-	}
-
-	if (!isset($CONFIG->views->extensions[$view])) {
-		$CONFIG->views->extensions[$view][500] = "{$view}";
+		$CONFIG->views = (object) array(
+			'extensions' => array(),
+		);
+		$CONFIG->views->extensions[$view][500] = (string)$view;
+	} else {
+		if (!isset($CONFIG->views->extensions[$view])) {
+			$CONFIG->views->extensions[$view][500] = (string)$view;
+		}
 	}
 
+	// raise priority until it doesn't match one already registered
 	while (isset($CONFIG->views->extensions[$view][$priority])) {
 		$priority++;
 	}
 
-	$CONFIG->views->extensions[$view][$priority] = "{$view_extension}";
+	$CONFIG->views->extensions[$view][$priority] = (string)$view_extension;
 	ksort($CONFIG->views->extensions[$view]);
 }
 
@@ -601,14 +589,6 @@ function elgg_extend_view($view, $view_extension, $priority = 501, $viewtype = '
 function elgg_unextend_view($view, $view_extension) {
 	global $CONFIG;
 
-	if (!isset($CONFIG->views)) {
-		return FALSE;
-	}
-
-	if (!isset($CONFIG->views->extensions)) {
-		return FALSE;
-	}
-
 	if (!isset($CONFIG->views->extensions[$view])) {
 		return FALSE;
 	}
@@ -1105,10 +1085,6 @@ function elgg_view_annotation_list($annotations, array $vars = array()) {
  * @todo Change the hook name.
  */
 function elgg_view_entity_annotations(ElggEntity $entity, $full_view = true) {
-	if (!$entity) {
-		return false;
-	}
-
 	if (!($entity instanceof ElggEntity)) {
 		return false;
 	}
@@ -1131,7 +1107,7 @@ function elgg_view_entity_annotations(ElggEntity $entity, $full_view = true) {
  * This is a shortcut for {@elgg_view page/elements/title}.
  *
  * @param string $title The page title
- * @param string $vars  View variables (was submenu be displayed? (deprecated))
+ * @param array $vars   View variables (was submenu be displayed? (deprecated))
  *
  * @return string The HTML (etc)
  */
@@ -1203,7 +1179,7 @@ function elgg_view_comments($entity, $add_comment = true, array $vars = array())
  *
  * @param string $image The icon and other information
  * @param string $body  Description content
- * @param string $vars  Additional parameters for the view
+ * @param array $vars   Additional parameters for the view
  *
  * @return string
  * @since 1.8.0
@@ -1230,7 +1206,6 @@ function elgg_view_image_block($image, $body, $vars = array()) {
  * @since 1.8.0
  */
 function elgg_view_module($type, $title, $body, array $vars = array()) {
-
 	$vars['class'] = elgg_extract('class', $vars, '') . " elgg-module-$type";
 	$vars['title'] = $title;
 	$vars['body'] = $body;
@@ -1243,11 +1218,15 @@ function elgg_view_module($type, $title, $body, array $vars = array()) {
  * @param ElggRiverItem $item A river item object
  * @param array         $vars An array of variables for the view
  *
- * @return string|false Depending on success
+ * @return string returns empty string if could not be rendered
  */
 function elgg_view_river_item($item, array $vars = array()) {
+	if (!($item instanceof ElggRiverItem)) {
+		return '';
+	}
 	// checking default viewtype since some viewtypes do not have unique views per item (rss)
-	if (!$item || !$item->getView() || !elgg_view_exists($item->getView(), 'default')) {
+	$view = $item->getView();
+	if (!$view || !elgg_view_exists($view, 'default')) {
 		return '';
 	}
 
@@ -1339,7 +1318,7 @@ function elgg_view_list_item($item, array $vars = array()) {
 		return elgg_view_river_item($item, $vars);
 	}
 
-	return false;
+	return '';
 }
 
 /**
@@ -1354,7 +1333,7 @@ function elgg_view_list_item($item, array $vars = array()) {
  */
 function elgg_view_icon($name, $class = '') {
 	// @todo deprecate boolean in Elgg 1.9
-	if (is_bool($class) && $class === true) {
+	if ($class === true) {
 		$class = 'float';
 	}
 	return "<span class=\"elgg-icon elgg-icon-$name $class\"></span>";
@@ -1403,7 +1382,8 @@ function elgg_view_access_collections($owner_guid) {
  */
 function set_template_handler($function_name) {
 	global $CONFIG;
-	if (!empty($function_name) && is_callable($function_name)) {
+
+	if (is_callable($function_name)) {
 		$CONFIG->template_handler = $function_name;
 		return true;
 	}
@@ -1516,17 +1496,13 @@ function elgg_view_tree($view_root, $viewtype = "") {
  * @param string $base_location_path The base views directory to use with elgg_set_view_location()
  * @param string $viewtype           The type of view we're looking at (default, rss, etc)
  *
- * @return void
+ * @return bool returns false if folder can't be read
  * @since 1.7.0
  * @see elgg_set_view_location()
  * @todo This seems overly complicated.
  * @access private
  */
 function autoregister_views($view_base, $folder, $base_location_path, $viewtype) {
-	if (!isset($i)) {
-		$i = 0;
-	}
-
 	if ($handle = opendir($folder)) {
 		while ($view = readdir($handle)) {
 			if (!in_array($view, array('.', '..', '.svn', 'CVS')) && !is_dir($folder . "/" . $view)) {
@@ -1608,16 +1584,15 @@ function elgg_views_handle_deprecated_views() {
 function elgg_views_boot() {
 	global $CONFIG;
 
-	elgg_register_simplecache_view('css/elgg');
 	elgg_register_simplecache_view('css/ie');
 	elgg_register_simplecache_view('css/ie6');
 	elgg_register_simplecache_view('css/ie7');
-	elgg_register_simplecache_view('js/elgg');
 
 	elgg_register_js('jquery', '/vendors/jquery/jquery-1.6.4.min.js', 'head');
 	elgg_register_js('jquery-ui', '/vendors/jquery/jquery-ui-1.8.16.min.js', 'head');
 	elgg_register_js('jquery.form', '/vendors/jquery/jquery.form.js');
-	
+
+	elgg_register_simplecache_view('js/elgg');
 	$elgg_js_url = elgg_get_simplecache_url('js', 'elgg');
 	elgg_register_js('elgg', $elgg_js_url, 'head');
 
@@ -1626,14 +1601,17 @@ function elgg_views_boot() {
 	elgg_load_js('elgg');
 
 	elgg_register_simplecache_view('js/lightbox');
-	elgg_register_simplecache_view('css/lightbox');
 	$lightbox_js_url = elgg_get_simplecache_url('js', 'lightbox');
 	elgg_register_js('lightbox', $lightbox_js_url);
+
+	elgg_register_simplecache_view('css/lightbox');
 	$lightbox_css_url = elgg_get_simplecache_url('css', 'lightbox');
 	elgg_register_css('lightbox', $lightbox_css_url);
 
+	elgg_register_simplecache_view('css/elgg');
 	$elgg_css_url = elgg_get_simplecache_url('css', 'elgg');
 	elgg_register_css('elgg', $elgg_css_url);
+
 	elgg_load_css('elgg');
 
 	elgg_register_ajax_view('js/languages');
@@ -1647,7 +1625,7 @@ function elgg_views_boot() {
 	$views = scandir($view_path);
 
 	foreach ($views as $view) {
-		if ('.' !== substr($view, 0, 1) && is_dir($view_path . $view)) {
+		if ($view[0] !== '.' && is_dir($view_path . $view)) {
 			elgg_register_viewtype($view);
 		}
 	}
diff --git a/engine/lib/web_services.php b/engine/lib/web_services.php
index da3ed76a9..c8e4a13cc 100644
--- a/engine/lib/web_services.php
+++ b/engine/lib/web_services.php
@@ -232,6 +232,7 @@ function execute_method($method) {
 	$function = $API_METHODS[$method]["function"];
 	$serialised_parameters = trim($serialised_parameters, ", ");
 
+	// @todo document why we cannot use call_user_func_array here
 	$result = eval("return $function($serialised_parameters);");
 
 	// Sanity check result
@@ -1194,6 +1195,8 @@ $ERRORS = array();
  *
  * @return void
  * @access private
+ * 
+ * @throws Exception
  */
 function _php_api_error_handler($errno, $errmsg, $filename, $linenum, $vars) {
 	global $ERRORS;
@@ -1278,11 +1281,9 @@ function service_handler($handler, $request) {
 		// no handlers set or bad url
 		header("HTTP/1.0 404 Not Found");
 		exit;
-	} else if (isset($CONFIG->servicehandler[$handler])
-	&& is_callable($CONFIG->servicehandler[$handler])) {
-
+	} else if (isset($CONFIG->servicehandler[$handler]) && is_callable($CONFIG->servicehandler[$handler])) {
 		$function = $CONFIG->servicehandler[$handler];
-		$function($request, $handler);
+		call_user_func($function, $request, $handler);
 	} else {
 		// no handler for this web service
 		header("HTTP/1.0 404 Not Found");
@@ -1301,10 +1302,11 @@ function service_handler($handler, $request) {
  */
 function register_service_handler($handler, $function) {
 	global $CONFIG;
+
 	if (!isset($CONFIG->servicehandler)) {
 		$CONFIG->servicehandler = array();
 	}
-	if (is_callable($function)) {
+	if (is_callable($function, true)) {
 		$CONFIG->servicehandler[$handler] = $function;
 		return true;
 	}
@@ -1319,11 +1321,13 @@ function register_service_handler($handler, $function) {
  *
  * @param string $handler web services type
  *
- * @return 1.7.0
+ * @return void
+ * @since 1.7.0
  */
 function unregister_service_handler($handler) {
 	global $CONFIG;
-	if (isset($CONFIG->servicehandler) && isset($CONFIG->servicehandler[$handler])) {
+
+	if (isset($CONFIG->servicehandler, $CONFIG->servicehandler[$handler])) {
 		unset($CONFIG->servicehandler[$handler]);
 	}
 }
@@ -1333,6 +1337,8 @@ function unregister_service_handler($handler) {
  *
  * @return void
  * @access private
+ *
+ * @throws SecurityException|APIException
  */
 function rest_handler() {
 	global $CONFIG;
@@ -1387,7 +1393,7 @@ function rest_handler() {
 /**
  * Unit tests for API
  *
- * @param sting  $hook   unit_test
+ * @param string  $hook   unit_test
  * @param string $type   system
  * @param mixed  $value  Array of tests
  * @param mixed  $params Params
@@ -1397,6 +1403,7 @@ function rest_handler() {
  */
 function api_unit_test($hook, $type, $value, $params) {
 	global $CONFIG;
+
 	$value[] = $CONFIG->path . 'engine/tests/services/api.php';
 	return $value;
 }
@@ -1418,15 +1425,18 @@ function api_init() {
 	elgg_echo("system.api.list"), "GET", false, false);
 
 	// The authentication token api
-	expose_function("auth.gettoken",
-					"auth_gettoken", array(
-											'username' => array ('type' => 'string'),
-											'password' => array ('type' => 'string'),
-											),
-					elgg_echo('auth.gettoken'),
-					'POST',
-					false,
-					false);
+	expose_function(
+		"auth.gettoken",
+		"auth_gettoken",
+		array(
+			'username' => array ('type' => 'string'),
+			'password' => array ('type' => 'string'),
+		),
+		elgg_echo('auth.gettoken'),
+		'POST',
+		false,
+		false
+	);
 }
 
 
diff --git a/engine/tests/api/metadata.php b/engine/tests/api/metadata.php
index 9933263d1..825290d80 100644
--- a/engine/tests/api/metadata.php
+++ b/engine/tests/api/metadata.php
@@ -102,14 +102,14 @@ class ElggCoreMetadataAPITest extends ElggCoreUnitTest {
 		$e = new ElggObject();
 		$e->save();
 
-		for ($i=0; $i<30; $i++) {
-			$name = "test_metadata" . rand(0, 10000);
+		for ($i = 0; $i < 30; $i++) {
+			$name = "test_metadata$i";
 			$e->$name = rand(0, 10000);
 		}
 
 		$options = array(
 			'guid' => $e->getGUID(),
-			'limit' => 0
+			'limit' => 0,
 		);
 
 		$md = elgg_get_metadata($options);