Fix errors when updating belongsToMany associations.

* Fix removing all linked records. * Fix removing some linked records, but not adding new ones. Refs #3485
cakephp · May 19, 2014 · e16a34a · e16a34a
1 parent bda0f73
commit e16a34a
Show file tree

Hide file tree

Showing 3 changed files with 172 additions and 10 deletions.
diff --git a/src/ORM/Association/BelongsToMany.php b/src/ORM/Association/BelongsToMany.php
@@ -385,10 +385,14 @@ public function save(Entity $entity, array $options = []) {
 		$targetEntity = $entity->get($this->property());
 		$strategy = $this->saveStrategy();
 
-		if (!$targetEntity) {
+		if ($targetEntity === null) {
 			return false;
 		}
 
+		if ($targetEntity === [] && $entity->isNew()) {
+			return $entity;
+		}
+
 		if ($strategy === self::SAVE_APPEND) {
 			return $this->_saveTarget($entity, $targetEntity, $options);
 		}
@@ -516,7 +520,7 @@ protected function _saveLinks(Entity $sourceEntity, $targetEntities, $options) {
  *
  * This method does not check link uniqueness.
  *
- * ###Example:
+ * ### Example:
  *
  * {{{
  * $newTags = $tags->find('relevant')->execute();
@@ -682,12 +686,17 @@ function() use ($sourceEntity, $targetEntities, $primaryValue, $options) {
 					return false;
 				}
 
+				$inserted = [];
 				$property = $this->property();
-				$inserted = array_combine(
-					array_keys($inserts),
-					(array)$sourceEntity->get($property)
-				);
-				$targetEntities = $inserted + $targetEntities;
+
+				if (count($inserts)) {
+					$inserted = array_combine(
+						array_keys($inserts),
+						(array)$sourceEntity->get($property)
+					);
+					$targetEntities = $inserted + $targetEntities;
+				}
+
 				ksort($targetEntities);
 				$sourceEntity->set($property, array_values($targetEntities));
 				$sourceEntity->dirty($property, false);

diff --git a/tests/TestCase/ORM/Association/BelongsToManyTest.php b/tests/TestCase/ORM/Association/BelongsToManyTest.php
@@ -1240,7 +1240,83 @@ public function testReplaceWithMissingPrimaryKey() {
 	}
 
 /**
- * Tests that replaceLinks will delete entities not present in the passed
+ * Test that replaceLinks() can save an empty set, removing all rows.
+ *
+ * @return void
+ */
+	public function testReplaceLinksUpdateToEmptySet() {
+		$connection = ConnectionManager::get('test');
+		$joint = $this->getMock(
+			'\Cake\ORM\Table',
+			['delete', 'find'],
+			[['alias' => 'ArticlesTags', 'connection' => $connection]]
+		);
+		$config = [
+			'sourceTable' => $this->article,
+			'targetTable' => $this->tag,
+			'through' => $joint,
+			'joinTable' => 'tags_articles'
+		];
+		$assoc = $this->getMock(
+			'\Cake\ORM\Association\BelongsToMany',
+			['_collectJointEntities', '_saveTarget'],
+			['tags', $config]
+		);
+		$assoc->junction();
+
+		$this->article
+			->association('ArticlesTags')
+			->conditions(['foo' => 1]);
+
+		$query1 = $this->getMock(
+			'\Cake\ORM\Query',
+			['where', 'andWhere', 'addDefaultTypes'],
+			[$connection, $joint]
+		);
+
+		$joint->expects($this->at(0))->method('find')
+			->with('all')
+			->will($this->returnValue($query1));
+
+		$query1->expects($this->at(0))
+			->method('where')
+			->with(['foo' => 1])
+			->will($this->returnSelf());
+		$query1->expects($this->at(1))
+			->method('where')
+			->with(['article_id' => 1])
+			->will($this->returnSelf());
+
+		$existing = [
+			new Entity(['article_id' => 1, 'tag_id' => 2]),
+			new Entity(['article_id' => 1, 'tag_id' => 4]),
+		];
+		$query1->setResult(new \ArrayIterator($existing));
+
+		$tags = [];
+		$entity = new Entity(['id' => 1, 'test' => $tags]);
+
+		$assoc->expects($this->once())->method('_collectJointEntities')
+			->with($entity, $tags)
+			->will($this->returnValue([]));
+
+		$joint->expects($this->at(1))
+			->method('delete')
+			->with($existing[0]);
+		$joint->expects($this->at(2))
+			->method('delete')
+			->with($existing[1]);
+
+		$assoc->expects($this->never())
+			->method('_saveTarget');
+
+		$assoc->replaceLinks($entity, $tags);
+		$this->assertSame([], $entity->tags);
+		$this->assertFalse($entity->dirty('tags'));
+	}
+
+/**
+ * Tests that replaceLinks will delete entities not present in the passed,
  * array, maintain those are already persisted and were passed and also
  * insert the rest.
  *
@@ -1334,6 +1410,30 @@ public function testReplaceLinkSuccess() {
 		$this->assertFalse($entity->dirty('tags'));
 	}
 
+/**
+ * Test that saving an empty set on create works.
+ *
+ * @return void
+ */
+	public function testSaveEmptySetSuccess() {
+		$assoc = $this->getMock(
+			'\Cake\ORM\Association\BelongsToMany',
+			['_saveTarget', 'replaceLinks'],
+			['tags']
+		);
+		$entity = new Entity([
+			'id' => 1,
+			'tags' => []
+		], ['markNew' => true]);
+
+		$assoc->saveStrategy(BelongsToMany::SAVE_REPLACE);
+		$assoc->expects($this->never())
+			->method('replaceLinks');
+		$assoc->expects($this->never())
+			->method('_saveTarget');
+		$this->assertSame($entity, $assoc->save($entity));
+	}
+
 /**
  * Tests saving with replace strategy returning true
  *

diff --git a/tests/TestCase/ORM/TableTest.php b/tests/TestCase/ORM/TableTest.php
@@ -1503,7 +1503,7 @@ public function testSaveOnlyDirtyProperties() {
  * @group save
  * @return void
  */
-	public function testsASavedEntityIsClean() {
+	public function testASavedEntityIsClean() {
 		$entity = new \Cake\ORM\Entity([
 			'username' => 'superuser',
 			'password' => 'root',
@@ -1524,7 +1524,7 @@ public function testsASavedEntityIsClean() {
  * @group save
  * @return void
  */
-	public function testsASavedEntityIsNotNew() {
+	public function testASavedEntityIsNotNew() {
 		$entity = new \Cake\ORM\Entity([
 			'username' => 'superuser',
 			'password' => 'root',
@@ -2593,6 +2593,59 @@ public function testSaveBelongsToMany() {
 		$this->assertEquals(5, $entity->tags[1]->_joinData->tag_id);
 	}
 
+/**
+ * Tests saving belongsToMany records can delete all links.
+ *
+ * @group save
+ * @return void
+ */
+	public function testSaveBelongsToManyDeleteAllLinks() {
+		$table = TableRegistry::get('articles');
+		$table->belongsToMany('tags', [
+			'saveStrategy' => 'replace',
+		]);
+
+		$entity = $table->get(1, ['contain' => 'Tags']);
+		$this->assertCount(2, $entity->tags, 'Fixture data did not change.');
+
+		$entity->tags = [];
+		$result = $table->save($entity);
+		$this->assertSame($result, $entity);
+		$this->assertSame([], $entity->tags, 'No tags on the entity.');
+
+		$entity = $table->get(1, ['contain' => 'Tags']);
+		$this->assertSame([], $entity->tags, 'No tags in the db either.');
+	}
+
+/**
+ * Tests saving belongsToMany records can delete some links.
+ *
+ * @group save
+ * @return void
+ */
+	public function testSaveBelongsToManyDeleteSomeLinks() {
+		$table = TableRegistry::get('articles');
+		$table->belongsToMany('tags', [
+			'saveStrategy' => 'replace',
+		]);
+
+		$entity = $table->get(1, ['contain' => 'Tags']);
+		$this->assertCount(2, $entity->tags, 'Fixture data did not change.');
+
+		$tag = new \Cake\ORM\Entity([
+			'id' => 2,
+		]);
+		$entity->tags = [$tag];
+		$result = $table->save($entity);
+		$this->assertSame($result, $entity);
+		$this->assertCount(1, $entity->tags, 'Only one tag left.');
+		$this->assertEquals($tag, $entity->tags[0]);
+
+		$entity = $table->get(1, ['contain' => 'Tags']);
+		$this->assertCount(1, $entity->tags, 'Only one tag in the db.');
+		$this->assertEquals($tag->id, $entity->tags[0]->id);
+	}
+
 /**
  * Tests saving belongsToMany records with a validation error and atomic set
  * to true