Merge branch '1.8' of git://github.com/Elgg/Elgg into lorea-preprod

1 files changed, 60 insertions, 58 deletions

diff --git a/engine/classes/ElggEntity.php b/engine/classes/ElggEntity.php
index dc38dafbe..77c2bbf4d 100644
--- a/engine/classes/ElggEntity.php
+++ b/engine/classes/ElggEntity.php
@@ -24,7 +24,6 @@
  *
  * @package    Elgg.Core
  * @subpackage DataModel.Entities
- * @link       http://docs.elgg.org/DataModel/ElggEntity
  * 
  * @property string $type           object, user, group, or site (read-only after save)
  * @property string $subtype        Further clarifies the nature of the entity (read-only after save)
@@ -201,8 +200,11 @@ abstract class ElggEntity extends ElggData implements
 	/**
 	 * Sets the value of a property.
 	 *
-	 * If $name is defined in $this->attributes that value is set, otherwise it will
-	 * set the appropriate item of metadata.
+	 * If $name is defined in $this->attributes that value is set, otherwise it is
+	 * saved as metadata.
+	 *
+	 * @warning Metadata set this way will inherit the entity's owner and access ID. If you want
+	 * to set metadata with a different owner, use create_metadata().
 	 *
 	 * @warning It is important that your class populates $this->attributes with keys
 	 * for all base attributes, anything not in their gets set as METADATA.
@@ -248,7 +250,12 @@ abstract class ElggEntity extends ElggData implements
 	public function getMetaData($name) {
 		if ((int) ($this->guid) == 0) {
 			if (isset($this->temp_metadata[$name])) {
-				return $this->temp_metadata[$name];
+				// md is returned as an array only if more than 1 entry
+				if (count($this->temp_metadata[$name]) == 1) {
+					return $this->temp_metadata[$name][0];
+				} else {
+					return $this->temp_metadata[$name];
+				}
 			} else {
 				return null;
 			}
@@ -291,80 +298,78 @@ abstract class ElggEntity extends ElggData implements
 	/**
 	 * Set a piece of metadata.
 	 *
-	 * @tip Plugin authors should use the magic methods.
+	 * Plugin authors should use the magic methods or create_metadata().
+	 *
+	 * @warning The metadata will inherit the parent entity's owner and access ID.
+	 * If you want to write metadata with a different owner, use create_metadata().
 	 *
 	 * @access private
 	 *
 	 * @param string $name       Name of the metadata
-	 * @param mixed  $value      Value of the metadata
+	 * @param mixed  $value      Value of the metadata (doesn't support assoc arrays)
 	 * @param string $value_type Types supported: integer and string. Will auto-identify if not set
 	 * @param bool   $multiple   Allow multiple values for a single name (doesn't support assoc arrays)
 	 *
 	 * @return bool
 	 */
-	public function setMetaData($name, $value, $value_type = "", $multiple = false) {
-		$delete_first = false;
-		// if multiple is set that always means don't delete.
-		// if multiple isn't set it means override. set it to true on arrays for the foreach.
-		if (!$multiple) {
-			$delete_first = true;
-			$multiple = is_array($value);
-		}
-
-		if (!$this->guid) {
-			// real metadata only returns as an array if there are multiple elements
-			if (is_array($value) && count($value) == 1) {
-				$value = $value[0];
-			}
+	public function setMetaData($name, $value, $value_type = null, $multiple = false) {
 
-			$value_is_array = is_array($value);
-
-			if (!isset($this->temp_metadata[$name]) || $delete_first) {
-				// need to remove the indexes because real metadata doesn't have them.
-				if ($value_is_array) {
-					$this->temp_metadata[$name] = array_values($value);
-				} else {
-					$this->temp_metadata[$name] = $value;
-				}
-			} else {
-				// multiple is always true at this point.
-				// if we're setting multiple and temp isn't array, it needs to be.
-				if (!is_array($this->temp_metadata[$name])) {
-					$this->temp_metadata[$name] = array($this->temp_metadata[$name]);
-				}
-
-				if ($value_is_array) {
-					$this->temp_metadata[$name] = array_merge($this->temp_metadata[$name], array_values($value));
-				} else {
-					$this->temp_metadata[$name][] = $value;
-				}
-			}
+		// normalize value to an array that we will loop over
+		// remove indexes if value already an array.
+		if (is_array($value)) {
+			$value = array_values($value);
 		} else {
-			if ($delete_first) {
+			$value = array($value);
+		}
+
+		// saved entity. persist md to db.
+		if ($this->guid) {
+			// if overwriting, delete first.
+			if (!$multiple) {
 				$options = array(
 					'guid' => $this->getGUID(),
 					'metadata_name' => $name,
 					'limit' => 0
 				);
-				// @todo this doesn't check if it exists so we can't handle failed deletes
-				// is it worth the overhead of more SQL calls to check?
-				elgg_delete_metadata($options);
-			}
-			// save into real metadata
-			if (!is_array($value)) {
-				$value = array($value);
+				// @todo in 1.9 make this return false if can't add metadata
+				// http://trac.elgg.org/ticket/4520
+				// 
+				// need to remove access restrictions right now to delete
+				// because this is the expected behavior
+				$ia = elgg_set_ignore_access(true);
+				if (false === elgg_delete_metadata($options)) {
+					return false;
+				}
+				elgg_set_ignore_access($ia);
 			}
-			foreach ($value as $v) {
-				$result = create_metadata($this->getGUID(), $name, $v, $value_type,
-					$this->getOwnerGUID(), $this->getAccessId(), $multiple);
 
-				if (!$result) {
+			// add new md
+			$result = true;
+			foreach ($value as $value_tmp) {
+				// at this point $value should be appended because it was cleared above if needed.
+				$md_id = create_metadata($this->getGUID(), $name, $value_tmp, $value_type,
+						$this->getOwnerGUID(), $this->getAccessId(), true);
+				if (!$md_id) {
 					return false;
 				}
 			}
+
+			return $result;
 		}
 
-		return true;
+		// unsaved entity. store in temp array
+		// returning single entries instead of an array of 1 element is decided in
+		// getMetaData(), just like pulling from the db.
+		else {
+			// if overwrite, delete first
+			if (!$multiple || !isset($this->temp_metadata[$name])) {
+				$this->temp_metadata[$name] = array();
+			}
+
+			// add new md
+			$this->temp_metadata[$name] = array_merge($this->temp_metadata[$name], $value);
+			return true;
+		}
 	}
 
 	/**
@@ -575,7 +580,6 @@ abstract class ElggEntity extends ElggData implements
 	 * @param mixed  $value Value of private setting
 	 *
 	 * @return bool
-	 * @link http://docs.elgg.org/DataModel/Entities/PrivateSettings
 	 */
 	function setPrivateSetting($name, $value) {
 		if ((int) $this->guid > 0) {
@@ -734,8 +738,6 @@ abstract class ElggEntity extends ElggData implements
 	 * @param string $vartype   The type of annotation value
 	 *
 	 * @return bool
-	 *
-	 * @link http://docs.elgg.org/DataModel/Annotations
 	 */
 	function annotate($name, $value, $access_id = ACCESS_PRIVATE, $owner_id = 0, $vartype = "") {
 		if ((int) $this->guid > 0) {