Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix HasMany::unlink() with empty arrays.
Return early as there is no work to be done when the target entities is
an empty list.

I've removed some of the previously added tests as they violated the
typehints for the unlink() method. I've also fixed up some of the logic
issues in the previous test.

Refs #9323
  • Loading branch information
markstory committed Aug 23, 2016
1 parent a7573be commit d0302ed
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 31 deletions.
3 changes: 3 additions & 0 deletions src/ORM/Association/HasMany.php
Expand Up @@ -293,6 +293,9 @@ public function unlink(EntityInterface $sourceEntity, array $targetEntities, $op
} else {
$options += ['cleanProperty' => true];
}
if (count($targetEntities) === 0) {
return;
}

$foreignKey = (array)$this->foreignKey();
$target = $this->target();
Expand Down
42 changes: 11 additions & 31 deletions tests/TestCase/ORM/Association/HasManyTest.php
Expand Up @@ -50,7 +50,7 @@ public function setUp()
$this->author = TableRegistry::get('Authors', [
'schema' => [
'id' => ['type' => 'integer'],
'username' => ['type' => 'string'],
'name' => ['type' => 'string'],
'_constraints' => [
'primary' => ['type' => 'primary', 'columns' => ['id']]
]
Expand Down Expand Up @@ -651,44 +651,23 @@ protected function assertSelectClause($expected, $query)
*/
public function testUnlinkSuccess()
{
$articles = TableRegistry::get('Articles');
$assoc = $this->author->hasMany('Articles', [
'sourceTable' => $this->author,
'targetTable' => $this->article
'targetTable' => $articles
]);

$entity = $this->author->get(1, ['contain' => 'Articles']);
$initial = $entity->articles;
$this->assertCount(2, $initial);

$this->author->unlink($entity, $entity->tags);
$assoc->unlink($entity, $entity->articles);
$this->assertEmpty($entity->get('articles'), 'Property should be empty');

$new = $this->article->get(2, ['contain' => 'Articles']);
$new = $this->author->get(2, ['contain' => 'Articles']);
$this->assertCount(0, $new->articles, 'DB should be clean');
$this->assertSame(4, $this->author->find()->count(), 'Authors should still exist');
$this->assertSame(3, $this->article->find()->count(), 'Articles should still exist');
}

/**
* Tests that unlink will work without contain() being called
*/
public function testUnlinkWithoutContain()
{
$assoc = $this->author->hasMany('Articles', [
'sourceTable' => $this->author,
'targetTable' => $this->article
]);

$entity = $this->author->get(1);
$entities = $this->article->find()->where(['Articles.author_id' => $entity->id]); // Without ->all()
$this->assertCount(2, $entities);

$assoc->unlink($entity, $entities);

$new = $this->article->get(2, ['contain' => 'Articles']);
$this->assertCount(0, $new->articles, 'DB should be clean');
$this->assertSame(4, $this->author->find()->count(), 'Authors should still exist');
$this->assertSame(3, $this->article->find()->count(), 'Articles should still exist');
$this->assertSame(3, $articles->find()->count(), 'Articles should still exist');
}

/**
Expand All @@ -698,20 +677,21 @@ public function testUnlinkWithoutContain()
*/
public function testUnlinkWithEmptyArray()
{
$articles = TableRegistry::get('Articles');
$assoc = $this->author->hasMany('Articles', [
'sourceTable' => $this->author,
'targetTable' => $this->article
'targetTable' => $articles
]);

$entity = $this->author->get(1, ['contain' => 'Articles']);
$initial = $entity->articles;
$this->assertCount(2, $initial);

$assoc->unlink($entity, []); // Unlink with empty array
$assoc->unlink($entity, []);

$new = $this->article->get(2, ['contain' => 'Articles']);
$new = $this->author->get(1, ['contain' => 'Articles']);
$this->assertCount(2, $new->articles, 'Articles should remain linked');
$this->assertSame(4, $this->author->find()->count(), 'Authors should still exist');
$this->assertSame(3, $this->article->find()->count(), 'Articles should still exist');
$this->assertSame(3, $articles->find()->count(), 'Articles should still exist');
}
}

0 comments on commit d0302ed

Please sign in to comment.