Merge pull request #377 from mrclay/4290-1.8

Fixes #4290: adds preloading volatile metadata cache and unit tests

1 files changed, 84 insertions, 38 deletions

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
+}