Browse Source

Treat [] and '' the same when saving belongsToMany associations.

Both [] and '' are semantically the same in this association type (empty
data). Because '', false, and null can all be interpreted as no data,
we'll use all 3 to represent an empty set. This matches the behavior we
use when handling hasMany associations well. Going forward when updating
a belongsToMany association these empty values will clear out links.

Previously '' would cause saving to silently fail and possibly emit
errors when doing updates which was bad.

Refs #6817
Mark Story 10 years ago
parent
commit
cad11892d5

+ 5 - 5
src/ORM/Association/BelongsToMany.php

@@ -430,13 +430,13 @@ class BelongsToMany extends Association
         $targetEntity = $entity->get($this->property());
         $strategy = $this->saveStrategy();
 
-        if ($targetEntity === null) {
-            return false;
-        }
-
-        if ($targetEntity === [] && $entity->isNew()) {
+        $isEmpty = in_array($targetEntity, [null, [], '', false], true);
+        if ($isEmpty && $entity->isNew()) {
             return $entity;
         }
+        if ($isEmpty) {
+            $targetEntity = [];
+        }
 
         if ($strategy === self::SAVE_APPEND) {
             return $this->_saveTarget($entity, $targetEntity, $options);

+ 48 - 2
tests/TestCase/ORM/Association/BelongsToManyTest.php

@@ -829,11 +829,27 @@ class BelongsToManyTest extends TestCase
     }
 
     /**
+     * Provider for empty values
+     *
+     * @return array
+     */
+    public function emptyProvider()
+    {
+        return [
+            [''],
+            [false],
+            [null],
+            [[]]
+        ];
+    }
+
+    /**
      * Test that saving an empty set on create works.
      *
+     * @dataProvider emptyProvider
      * @return void
      */
-    public function testSaveAssociatedEmptySetSuccess()
+    public function testSaveAssociatedEmptySetSuccess($value)
     {
         $assoc = $this->getMock(
             '\Cake\ORM\Association\BelongsToMany',
@@ -842,7 +858,7 @@ class BelongsToManyTest extends TestCase
         );
         $entity = new Entity([
             'id' => 1,
-            'tags' => []
+            'tags' => $value,
         ], ['markNew' => true]);
 
         $assoc->saveStrategy(BelongsToMany::SAVE_REPLACE);
@@ -854,6 +870,36 @@ class BelongsToManyTest extends TestCase
     }
 
     /**
+     * Test that saving an empty set on update works.
+     *
+     * @dataProvider emptyProvider
+     * @return void
+     */
+    public function testSaveAssociatedEmptySetUpdateSuccess($value)
+    {
+        $assoc = $this->getMock(
+            '\Cake\ORM\Association\BelongsToMany',
+            ['_saveTarget', 'replaceLinks'],
+            ['tags']
+        );
+        $entity = new Entity([
+            'id' => 1,
+            'tags' => $value,
+        ], ['markNew' => false]);
+
+        $assoc->saveStrategy(BelongsToMany::SAVE_REPLACE);
+        $assoc->expects($this->once())
+            ->method('replaceLinks')
+            ->with($entity, [])
+            ->will($this->returnValue(true));
+
+        $assoc->expects($this->never())
+            ->method('_saveTarget');
+
+        $this->assertSame($entity, $assoc->saveAssociated($entity));
+    }
+
+    /**
      * Tests saving with replace strategy returning true
      *
      * @return void