Skip to content
Permalink
Browse files

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
  • Loading branch information...
markstory committed Jun 21, 2015
1 parent c77cdad commit cad11892d59776e400cfdbcc3b20b21ce062e320
Showing with 53 additions and 7 deletions.
  1. +5 −5 src/ORM/Association/BelongsToMany.php
  2. +48 −2 tests/TestCase/ORM/Association/BelongsToManyTest.php
@@ -430,13 +430,13 @@ public function saveAssociated(EntityInterface $entity, array $options = [])
$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);
@@ -828,12 +828,28 @@ public function testReplaceLinkSuccess()
$this->assertFalse($entity->dirty('tags'));
}
/**
* 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 @@ public function testSaveAssociatedEmptySetSuccess()
);
$entity = new Entity([
'id' => 1,
'tags' => []
'tags' => $value,
], ['markNew' => true]);
$assoc->saveStrategy(BelongsToMany::SAVE_REPLACE);
@@ -853,6 +869,36 @@ public function testSaveAssociatedEmptySetSuccess()
$this->assertSame($entity, $assoc->saveAssociated($entity));
}
/**
* 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
*

0 comments on commit cad1189

Please sign in to comment.
You can’t perform that action at this time.