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

diff --git a/engine/classes/ElggEntity.php b/engine/classes/ElggEntity.php
index 6828cab1fa3a3874688c1dade0488b89e0b19627..164ff3838f6ca663c17a7cda6982202a68cdddab 100644 (file)
--- 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 be8ac269ccf0a395a4013fc1154919014bcd7ba3..2461e975e1151e0f8b6dc4da6f6057aa201a99cc 100644 (file)
--- 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;