Browse Source

Merge pull request #10844 from cakephp/issue-10658

Make one()/many() more consistent with empty associations
José Lorenzo Rodríguez 8 years ago
parent
commit
b79aeaebe2
3 changed files with 57 additions and 7 deletions
  1. 1 2
      src/Datasource/EntityTrait.php
  2. 14 5
      src/ORM/Marshaller.php
  3. 42 0
      tests/TestCase/ORM/MarshallerTest.php

+ 1 - 2
src/Datasource/EntityTrait.php

@@ -227,8 +227,7 @@ trait EntityTrait
      */
     public function set($property, $value = null, array $options = [])
     {
-        $isString = is_string($property);
-        if ($isString && $property !== '') {
+        if (is_string($property) && $property !== '') {
             $guard = false;
             $property = [$property => $value];
         } else {

+ 14 - 5
src/ORM/Marshaller.php

@@ -212,6 +212,14 @@ class Marshaller
             $entity->set($properties);
         }
 
+        // Don't flag clean association entities as
+        // dirty so we don't persist empty records.
+        foreach ($properties as $field => $value) {
+            if ($value instanceof EntityInterface) {
+                $entity->dirty($field, $value->dirty());
+            }
+        }
+
         $entity->setErrors($errors);
 
         return $entity;
@@ -587,11 +595,12 @@ class Marshaller
         }
 
         foreach ((array)$options['fields'] as $field) {
-            if (array_key_exists($field, $properties)) {
-                $entity->set($field, $properties[$field]);
-                if ($properties[$field] instanceof EntityInterface) {
-                    $entity->setDirty($field, $properties[$field]->isDirty());
-                }
+            if (!array_key_exists($field, $properties)) {
+                continue;
+            }
+            $entity->set($field, $properties[$field]);
+            if ($properties[$field] instanceof EntityInterface) {
+                $entity->setDirty($field, $properties[$field]->isDirty());
             }
         }
 

+ 42 - 0
tests/TestCase/ORM/MarshallerTest.php

@@ -367,6 +367,48 @@ class MarshallerTest extends TestCase
     }
 
     /**
+     * Test that one() correctly handles an association beforeMarshal
+     * making the association empty.
+     *
+     * @return void
+     */
+    public function testOneAssociationBeforeMarshalMutation()
+    {
+        $users = TableRegistry::get('Users');
+        $articles = TableRegistry::get('Articles');
+
+        $users->hasOne('Articles', [
+            'foreignKey' => 'author_id'
+        ]);
+        $articles->eventManager()->on('Model.beforeMarshal', function ($event, $data, $options) {
+            // Blank the association, so it doesn't become dirty.
+            unset($data['not_a_real_field']);
+        });
+
+        $data = [
+            'username' => 'Jen',
+            'article' => [
+                'not_a_real_field' => 'whatever'
+            ]
+        ];
+        $marshall = new Marshaller($users);
+        $entity = $marshall->one($data, ['associated' => ['Articles']]);
+        $this->assertTrue($entity->isDirty('username'));
+        $this->assertFalse($entity->isDirty('article'));
+
+        // Ensure consistency with merge()
+        $entity = new Entity([
+            'username' => 'Jenny',
+        ]);
+        // Make the entity think it is new.
+        $entity->accessible('*', true);
+        $entity->clean();
+        $entity = $marshall->merge($entity, $data, ['associated' => ['Articles']]);
+        $this->assertTrue($entity->isDirty('username'));
+        $this->assertFalse($entity->isDirty('article'));
+    }
+
+    /**
      * Test one() supports accessibleFields option for associations
      *
      * @return void