Fixes #4290: adds volatile metadata cache, unit tests, and pre-loading for fetched entities

2 files changed, 118 insertions, 56 deletions

diff --git a/engine/lib/entities.php b/engine/lib/entities.php
index 3896cd58f..e7f84b15b 100644
--- a/engine/lib/entities.php
+++ b/engine/lib/entities.php
@@ -39,6 +39,8 @@ function invalidate_cache_for_entity($guid) {
 	$guid = (int)$guid;
 
 	unset($ENTITY_CACHE[$guid]);
+
+	elgg_get_metadata_cache()->clear($guid);
 }
 
 /**
@@ -606,12 +608,14 @@ function get_entity_as_row($guid) {
  *
  * @param stdClass $row The row of the entry in the entities table.
  *
- * @return object|false
+ * @return ElggEntity|false
  * @link http://docs.elgg.org/DataModel/Entities
  * @see get_entity_as_row()
  * @see add_subtype()
  * @see get_entity()
  * @access private
+ *
+ * @throws ClassException|InstallationException
  */
 function entity_row_to_elggstar($row) {
 	if (!($row instanceof stdClass)) {
@@ -969,17 +973,25 @@ function elgg_get_entities(array $options = array()) {
 
 		$dt = get_data($query, $options['callback']);
 		if ($dt) {
-			foreach ($dt as $entity) {
-				// If a custom callback is provided, it could return something other than ElggEntity,
-				// so we have to do an explicit check here.
-				if ($entity instanceof ElggEntity) {
-					cache_entity($entity);
+			// populate entity and metadata caches
+			$guids = array();
+			foreach ($dt as $item) {
+				// A custom callback could result in items that aren't ElggEntity's, so check for them
+				if ($item instanceof ElggEntity) {
+					cache_entity($item);
+					// plugins usually have only settings
+					if (!$item instanceof ElggPlugin) {
+						$guids[] = $item->guid;
+					}
 				}
 			}
 			// @todo Without this, recursive delete fails. See #4568
 			reset($dt);
-		}
 
+			if ($guids) {
+				elgg_get_metadata_cache()->populateFromEntities($guids);
+			}
+		}
 		return $dt;
 	} else {
 		$total = get_data_row($query);
@@ -1153,7 +1165,7 @@ function elgg_get_entity_type_subtype_where_sql($table, $types, $subtypes, $pair
  *                           best to provide in table.column format.
  * @param NULL|array $guids  Array of GUIDs.
  *
- * @return false|str
+ * @return false|string
  * @since 1.8.0
  * @access private
  */
@@ -1202,7 +1214,7 @@ function elgg_get_guid_based_where_sql($column, $guids) {
  * @param NULL|int $time_updated_upper Time updated upper limit
  * @param NULL|int $time_updated_lower Time updated lower limit
  *
- * @return FALSE|str FALSE on fail, string on success.
+ * @return FALSE|string FALSE on fail, string on success.
  * @since 1.7.0
  * @access private
  */
@@ -1304,7 +1316,7 @@ function elgg_list_entities(array $options = array(), $getter = 'elgg_get_entiti
  * @param string $subtype        The subtype of entity
  * @param int    $container_guid The container GUID that the entinties belong to
  * @param int    $site_guid      The site GUID
- * @param str    $order_by       Order_by SQL order by clause
+ * @param string $order_by       Order_by SQL order by clause
  *
  * @return array|false Either an array months as YYYYMM, or false on failure
  */
@@ -1649,7 +1661,7 @@ function delete_entity($guid, $recursive = true) {
  * @param string $returnvalue Return value from previous hook
  * @param array  $params      The parameters, passed 'guid' and 'varname'
  *
- * @return void
+ * @return ElggMetadata|null
  * @elgg_plugin_hook_handler volatile metadata
  * @todo investigate more.
  * @access private
@@ -1694,6 +1706,8 @@ function volatile_data_export_plugin_hook($hook, $entity_type, $returnvalue, $pa
  * @elgg_event_handler export all
  * @return mixed
  * @access private
+ *
+ * @throws InvalidParameterException|InvalidClassException
  */
 function export_entity_plugin_hook($hook, $entity_type, $returnvalue, $params) {
 	// Sanity check values
@@ -1736,6 +1750,8 @@ function export_entity_plugin_hook($hook, $entity_type, $returnvalue, $params) {
  * @return ElggEntity the unsaved entity which should be populated by items.
  * @todo Remove this.
  * @access private
+ *
+ * @throws ClassException|InstallationException|ImportException
  */
 function oddentity_to_elggentity(ODDEntity $element) {
 	$class = $element->getAttribute('class');
@@ -1807,6 +1823,8 @@ function oddentity_to_elggentity(ODDEntity $element) {
  * @elgg_plugin_hook_handler import all
  * @todo document
  * @access private
+ *
+ * @throws ImportException
  */
 function import_entity_plugin_hook($hook, $entity_type, $returnvalue, $params) {
 	$element = $params['element'];
@@ -1853,8 +1871,6 @@ function import_entity_plugin_hook($hook, $entity_type, $returnvalue, $params) {
  * @link http://docs.elgg.org/Entities/AccessControl
  */
 function can_edit_entity($entity_guid, $user_guid = 0) {
-	global $CONFIG;
-
 	$user_guid = (int)$user_guid;
 	$user = get_entity($user_guid);
 	if (!$user) {
@@ -1978,7 +1994,7 @@ function get_entity_url($entity_guid) {
  * @param string $entity_subtype The entity subtype
  * @param string $function_name  The function to register
  *
- * @return true|false Depending on success
+ * @return bool Depending on success
  * @see get_entity_url()
  * @see ElggEntity::getURL()
  * @since 1.8.0
@@ -2014,7 +2030,7 @@ function elgg_register_entity_url_handler($entity_type, $entity_subtype, $functi
  * @param string $type    The type of entity (object, site, user, group)
  * @param string $subtype The subtype to register (may be blank)
  *
- * @return true|false Depending on success
+ * @return bool Depending on success
  * @see get_registered_entity_types()
  * @link http://docs.elgg.org/Search
  * @link http://docs.elgg.org/Tutorials/Search
@@ -2051,7 +2067,7 @@ function elgg_register_entity_type($type, $subtype = null) {
  * @param string $type    The type of entity (object, site, user, group)
  * @param string $subtype The subtype to register (may be blank)
  *
- * @return true|false Depending on success
+ * @return bool Depending on success
  * @see elgg_register_entity_type()
  */
 function unregister_entity_type($type, $subtype) {
@@ -2118,7 +2134,7 @@ function get_registered_entity_types($type = null) {
  * @param string $type    The type of entity (object, site, user, group)
  * @param string $subtype The subtype (may be blank)
  *
- * @return true|false Depending on whether or not the type has been registered
+ * @return bool Depending on whether or not the type has been registered
  */
 function is_registered_entity_type($type, $subtype = null) {
 	global $CONFIG;
@@ -2318,7 +2334,7 @@ function entities_gc() {
 /**
  * Runs unit tests for the entity objects.
  *
- * @param sting  $hook   unit_test
+ * @param string  $hook   unit_test
  * @param string $type   system
  * @param mixed  $value  Array of tests
  * @param mixed  $params Params
diff --git a/engine/lib/metadata.php b/engine/lib/metadata.php
index 77fa30e41..f76c20f24 100644
--- a/engine/lib/metadata.php
+++ b/engine/lib/metadata.php
@@ -12,7 +12,7 @@
  *
  * @param stdClass $row An object from the database
  *
- * @return stdClass or ElggMetadata
+ * @return stdClass|ElggMetadata
  * @access private
  */
 function row_to_elggmetadata($row) {
@@ -30,7 +30,7 @@ function row_to_elggmetadata($row) {
  *
  * @param int $id The id of the metadata object being retrieved.
  *
- * @return false|ElggMetadata
+ * @return ElggMetadata|false  FALSE if not found
  */
 function elgg_get_metadata_from_id($id) {
 	return elgg_get_metastring_based_object_from_id($id, 'metadata');
@@ -64,7 +64,7 @@ function elgg_delete_metadata_by_id($id) {
  * @param int    $access_id      Default is ACCESS_PRIVATE
  * @param bool   $allow_multiple Allow multiple values for one key. Default is FALSE
  *
- * @return int/bool id of metadata or FALSE if failure
+ * @return int|false id of metadata or FALSE if failure
  */
 function create_metadata($entity_guid, $name, $value, $value_type = '', $owner_guid = 0,
 	$access_id = ACCESS_PRIVATE, $allow_multiple = false) {
@@ -90,8 +90,6 @@ function create_metadata($entity_guid, $name, $value, $value_type = '', $owner_g
 
 	$access_id = (int)$access_id;
 
-	$id = false;
-
 	$query = "SELECT * from {$CONFIG->dbprefix}metadata"
 		. " WHERE entity_guid = $entity_guid and name_id=" . add_metastring($name) . " limit 1";
 
@@ -106,34 +104,33 @@ function create_metadata($entity_guid, $name, $value, $value_type = '', $owner_g
 	} else {
 		// Support boolean types
 		if (is_bool($value)) {
-			if ($value) {
-				$value = 1;
-			} else {
-				$value = 0;
-			}
+			$value = (int) $value;
 		}
 
 		// Add the metastrings
-		$value = add_metastring($value);
-		if (!$value) {
+		$value_id = add_metastring($value);
+		if (!$value_id) {
 			return false;
 		}
 
-		$name = add_metastring($name);
-		if (!$name) {
+		$name_id = add_metastring($name);
+		if (!$name_id) {
 			return false;
 		}
 
 		// If ok then add it
 		$query = "INSERT into {$CONFIG->dbprefix}metadata"
 			. " (entity_guid, name_id, value_id, value_type, owner_guid, time_created, access_id)"
-			. " VALUES ($entity_guid, '$name','$value','$value_type', $owner_guid, $time, $access_id)";
+			. " VALUES ($entity_guid, '$name_id','$value_id','$value_type', $owner_guid, $time, $access_id)";
 
 		$id = insert_data($query);
 
 		if ($id !== false) {
 			$obj = elgg_get_metadata_from_id($id);
 			if (elgg_trigger_event('create', 'metadata', $obj)) {
+
+				elgg_get_metadata_cache()->save($entity_guid, $name, $value, $allow_multiple);
+
 				return $id;
 			} else {
 				elgg_delete_metadata_by_id($id);
@@ -175,6 +172,7 @@ function update_metadata($id, $name, $value, $value_type, $owner_guid, $access_i
 	}
 
 	if ($metabyname_memcache) {
+		// @todo fix memcache (name_id is not a property of ElggMetadata)
 		$metabyname_memcache->delete("{$md->entity_guid}:{$md->name_id}");
 	}
 
@@ -187,15 +185,9 @@ function update_metadata($id, $name, $value, $value_type, $owner_guid, $access_i
 
 	$access_id = (int)$access_id;
 
-	$access = get_access_sql_suffix();
-
 	// Support boolean types (as integers)
 	if (is_bool($value)) {
-		if ($value) {
-			$value = 1;
-		} else {
-			$value = 0;
-		}
+		$value = (int) $value;
 	}
 
 	// Add the metastring
@@ -216,6 +208,9 @@ function update_metadata($id, $name, $value, $value_type, $owner_guid, $access_i
 
 	$result = update_data($query);
 	if ($result !== false) {
+
+		elgg_get_metadata_cache()->save($md->entity_guid, $name, $value);
+
 		// @todo this event tells you the metadata has been updated, but does not
 		// let you do anything about it. What is needed is a plugin hook before
 		// the update that passes old and new values.
@@ -234,7 +229,7 @@ function update_metadata($id, $name, $value, $value_type, $owner_guid, $access_i
  * associative arrays and there is no guarantee on the ordering in the array.
  *
  * @param int    $entity_guid     The entity to attach the metadata to
- * @param string $name_and_values Associative array - a value can be a string, number, bool
+ * @param array  $name_and_values Associative array - a value can be a string, number, bool
  * @param string $value_type      'text', 'integer', or '' for automatic detection
  * @param int    $owner_guid      GUID of entity that owns the metadata
  * @param int    $access_id       Default is ACCESS_PRIVATE
@@ -308,6 +303,8 @@ function elgg_delete_metadata(array $options) {
 		return false;
 	}
 
+	elgg_get_metadata_cache()->invalidateByOptions('delete', $options);
+
 	$options['metastring_type'] = 'metadata';
 	return elgg_batch_metastring_based_objects($options, 'elgg_batch_delete_callback', false);
 }
@@ -328,6 +325,8 @@ function elgg_disable_metadata(array $options) {
 		return false;
 	}
 
+	elgg_get_metadata_cache()->invalidateByOptions('disable', $options);
+
 	$options['metastring_type'] = 'metadata';
 	return elgg_batch_metastring_based_objects($options, 'elgg_batch_disable_callback', false);
 }
@@ -348,6 +347,8 @@ function elgg_enable_metadata(array $options) {
 		return false;
 	}
 
+	elgg_get_metadata_cache()->invalidateByOptions('enable', $options);
+
 	$options['metastring_type'] = 'metadata';
 	return elgg_batch_metastring_based_objects($options, 'elgg_batch_enable_callback');
 }
@@ -449,16 +450,16 @@ function elgg_get_entities_from_metadata(array $options = array()) {
  * This function is reused for annotations because the tables are
  * exactly the same.
  *
- * @param string   $e_table           Entities table name
- * @param string   $n_table           Normalized metastrings table name (Where entities,
+ * @param string     $e_table           Entities table name
+ * @param string     $n_table           Normalized metastrings table name (Where entities,
  *                                    values, and names are joined. annotations / metadata)
- * @param arr|null $names             Array of names
- * @param arr|null $values            Array of values
- * @param arr|null $pairs             Array of names / values / operands
- * @param and|or   $pair_operator     Operator to use to join the where clauses for pairs
- * @param bool     $case_sensitive    Case sensitive metadata names?
- * @param arr|null $order_by_metadata Array of names / direction
- * @param arr|null $owner_guids       Array of owner GUIDs
+ * @param array|null $names             Array of names
+ * @param array|null $values            Array of values
+ * @param array|null $pairs             Array of names / values / operands
+ * @param string     $pair_operator     ("AND" or "OR") Operator to use to join the where clauses for pairs
+ * @param bool       $case_sensitive    Case sensitive metadata names?
+ * @param array|null $order_by_metadata Array of names / direction
+ * @param array|null $owner_guids       Array of owner GUIDs
  *
  * @return FALSE|array False on fail, array('joins', 'wheres')
  * @since 1.7.0
@@ -732,6 +733,8 @@ function elgg_list_entities_from_metadata($options) {
  *
  * @return array
  * @access private
+ *
+ * @throws InvalidParameterException
  */
 function export_metadata_plugin_hook($hook, $entity_type, $returnvalue, $params) {
 	// Sanity check values
@@ -743,15 +746,13 @@ function export_metadata_plugin_hook($hook, $entity_type, $returnvalue, $params)
 		throw new InvalidParameterException(elgg_echo('InvalidParameterException:NonArrayReturnValue'));
 	}
 
-	$guid = (int)$params['guid'];
-	$name = $params['name'];
-
 	$result = elgg_get_metadata(array(
-		'guid' => $guid,
-		'limit' => 0
+		'guid' => (int)$params['guid'],
+		'limit' => 0,
 	));
 
 	if ($result) {
+		/* @var ElggMetadata[] $result */
 		foreach ($result as $r) {
 			$returnvalue[] = $r->export();
 		}
@@ -889,6 +890,50 @@ function elgg_register_metadata_url_handler($extender_name, $function) {
 	return elgg_register_extender_url_handler('metadata', $extender_name, $function);
 }
 
+/**
+ * Get the global metadata cache instance
+ *
+ * @return ElggVolatileMetadataCache
+ *
+ * @access private
+ */
+function elgg_get_metadata_cache() {
+	global $CONFIG;
+	if (empty($CONFIG->local_metadata_cache)) {
+		$CONFIG->local_metadata_cache = new ElggVolatileMetadataCache();
+	}
+	return $CONFIG->local_metadata_cache;
+}
+
+/**
+ * Invalidate the metadata cache based on options passed to various *_metadata functions
+ *
+ * @param string $action  Action performed on metadata. "delete", "disable", or "enable"
+ *
+ * @param array $options  Options passed to elgg_(delete|disable|enable)_metadata
+ */
+function elgg_invalidate_metadata_cache($action, array $options) {
+	// remove as little as possible, optimizing for common cases
+	$cache = elgg_get_metadata_cache();
+	if (empty($options['guid'])) {
+		// safest to clear everything unless we want to make this even more complex :(
+		$cache->flush();
+	} else {
+		if (empty($options['metadata_name'])) {
+			// safest to clear the whole entity
+			$cache->clear($options['guid']);
+		} else {
+			switch ($action) {
+				case 'delete':
+					$cache->markEmpty($options['guid'], $options['metadata_name']);
+					break;
+				default:
+					$cache->markUnknown($options['guid'], $options['metadata_name']);
+			}
+		}
+	}
+}
+
 /** Register the hook */
 elgg_register_plugin_hook_handler("export", "all", "export_metadata_plugin_hook", 2);
 
@@ -912,5 +957,6 @@ elgg_register_plugin_hook_handler('unit_test', 'system', 'metadata_test');
 function metadata_test($hook, $type, $value, $params) {
 	global $CONFIG;
 	$value[] = $CONFIG->path . 'engine/tests/api/metadata.php';
+	$value[] = $CONFIG->path . 'engine/tests/api/metadata_cache.php';
 	return $value;
-}
\ No newline at end of file
+}