Fixes #2776. Overriding permissions to delete metadata when overwriting multiple values.

2 files changed, 87 insertions, 5 deletions

diff --git a/engine/classes/ElggEntity.php b/engine/classes/ElggEntity.php
index 6828cab1f..164ff3838 100644
--- a/engine/classes/ElggEntity.php
+++ b/engine/classes/ElggEntity.php
@@ -325,10 +325,6 @@ abstract class ElggEntity extends ElggData implements
 
 		// saved entity. persist md to db.
 		if ($this->guid) {
-			if (!$this->canEditMetadata()) {
-				return false;
-			}
-
 			// if overwriting, delete first.
 			if (!$multiple) {
 				$options = array(
@@ -336,23 +332,35 @@ abstract class ElggEntity extends ElggData implements
 					'metadata_name' => $name,
 					'limit' => 0
 				);
+				// @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);
 			}
 
 			// add new md
 			$result = true;
 			foreach ($value as $value_tmp) {
 				// at this point $value should be appended because it was cleared above if needed.
-				$result &= create_metadata($this->getGUID(), $name, $value_tmp, $value_type,
+				$md_id = create_metadata($this->getGUID(), $name, $value_tmp, $value_type,
 						$this->getOwnerGUID(), $this->getAccessId(), true);
+				if (!$md_id) {
+					return false;
+				}
 			}
 
 			return $result;
 		}
 
 		// 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])) {
diff --git a/engine/tests/api/metadata.php b/engine/tests/api/metadata.php
index be8ac269c..2461e975e 100644
--- a/engine/tests/api/metadata.php
+++ b/engine/tests/api/metadata.php
@@ -124,6 +124,80 @@ class ElggCoreMetadataAPITest extends ElggCoreUnitTest {
 		$e->delete();
 	}
 
+	// Make sure metadata with multiple values is correctly deleted when re-written
+	// by another user
+	// http://trac.elgg.org/ticket/2776
+	public function test_elgg_metadata_multiple_values() {
+		$u1 = new ElggUser();
+		$u1->username = rand();
+		$u1->save();
+
+		$u2 = new ElggUser();
+		$u2->username = rand();
+		$u2->save();
+
+		$obj = new ElggObject();
+		$obj->owner_guid = $u1->guid;
+		$obj->container_guid = $u1->guid;
+		$obj->access_id = ACCESS_PUBLIC;
+		$obj->save();
+
+		$md_values = array(
+			'one',
+			'two',
+			'three'
+		);
+
+		// need to fake different logins.
+		// good times without mocking.
+		$original_user = elgg_get_logged_in_user_entity();
+		$_SESSION['user'] = $u1;
+		
+		elgg_set_ignore_access(false);
+
+		// add metadata as one user
+		$obj->test = $md_values;
+
+		// check only these md exists
+		$db_prefix = elgg_get_config('dbprefix');
+		$q = "SELECT * FROM {$db_prefix}metadata WHERE entity_guid = $obj->guid";
+		$data = get_data($q);
+
+		$this->assertEqual(count($md_values), count($data));
+		foreach ($data as $md_row) {
+			$md = elgg_get_metadata_from_id($md_row->id);
+			$this->assertTrue(in_array($md->value, $md_values));
+			$this->assertEqual('test', $md->name);
+		}
+
+		// add md w/ same name as a different user
+		$_SESSION['user'] = $u2;
+		$md_values2 = array(
+			'four',
+			'five',
+			'six',
+			'seven'
+		);
+
+		$obj->test = $md_values2;
+
+		$q = "SELECT * FROM {$db_prefix}metadata WHERE entity_guid = $obj->guid";
+		$data = get_data($q);
+
+		$this->assertEqual(count($md_values2), count($data));
+		foreach ($data as $md_row) {
+			$md = elgg_get_metadata_from_id($md_row->id);
+			$this->assertTrue(in_array($md->value, $md_values2));
+			$this->assertEqual('test', $md->name);
+		}
+
+		$_SESSION['user'] = $original_user;
+
+		$obj->delete();
+		$u1->delete();
+		$u2->delete();
+	}
+
 
 	protected function create_metastring($string) {
 		global $CONFIG, $METASTRINGS_CACHE, $METASTRINGS_DEADNAME_CACHE;