diff --git a/src/ORM/Association/HasMany.php b/src/ORM/Association/HasMany.php index 79bbf623be7..185527fd367 100644 --- a/src/ORM/Association/HasMany.php +++ b/src/ORM/Association/HasMany.php @@ -148,11 +148,12 @@ public function saveAssociated(EntityInterface $entity, array $options = []) $original = $targetEntities; $options['_sourceTable'] = $this->source(); + $unlinkSuccessful = null; if ($this->_saveStrategy === self::SAVE_REPLACE) { $unlinkSuccessful = $this->_unlinkAssociated($properties, $entity, $target, $targetEntities); } - if (isset($unlinkSuccessful) && !$unlinkSuccessful) { + if ($unlinkSuccessful === false) { return false; } @@ -414,27 +415,25 @@ function ($v) { */ protected function _unlink(array $foreignKey, Table $target, array $conditions = []) { - $ok = true; $mustBeDependent = (!$this->_foreignKeyAcceptsNull($target, $foreignKey) || $this->dependent()); - $persistedEntitiesExist = $this->exists($conditions); - - if ($persistedEntitiesExist) { - if ($mustBeDependent) { - if ($this->_cascadeCallbacks) { - $query = $this->find('all')->where($conditions); - foreach ($query as $assoc) { - $ok = $ok && $target->delete($assoc); - } - } else { - $ok = ($target->deleteAll($conditions) > 0); + + if ($mustBeDependent) { + if ($this->_cascadeCallbacks) { + $query = $this->find('all')->where($conditions); + $ok = true; + foreach ($query as $assoc) { + $ok = $ok && $target->delete($assoc); } + return $ok; } else { - $updateFields = array_fill_keys($foreignKey, null); - $ok = $target->updateAll($updateFields, $conditions); - + $target->deleteAll($conditions); + return true; } + } else { + $updateFields = array_fill_keys($foreignKey, null); + return $target->updateAll($updateFields, $conditions); + } - return $ok; } /** diff --git a/tests/TestCase/ORM/TableTest.php b/tests/TestCase/ORM/TableTest.php index 242465d8fd6..042e9e512b6 100644 --- a/tests/TestCase/ORM/TableTest.php +++ b/tests/TestCase/ORM/TableTest.php @@ -4215,96 +4215,6 @@ public function testReplaceHasManyOnError() } - /** - * Integration test for replacing entities which depend on their source entity with HasMany and failing transaction. False should be returned when - * unlinking fails while replacing - * - * @return void - */ - public function testReplaceHasManyOnErrorDependent() - { - $articles = $this->getMock( - 'Cake\ORM\Table', - ['deleteAll'], - [[ - 'connection' => $this->connection, - 'alias' => 'Articles', - 'table' => 'articles', - ]] - ); - - $articles->method('deleteAll')->willReturn(false); - - $associations = new AssociationCollection(); - - $hasManyArticles = $this->getMock( - 'Cake\ORM\Association\HasMany', - ['target'], - [ - 'articles', - [ - 'target' => $articles, - 'foreignKey' => 'author_id', - 'dependent' => true - ] - ] - ); - $hasManyArticles->method('target')->willReturn($articles); - - $associations->add('articles', $hasManyArticles); - - $authors = new Table([ - 'connection' => $this->connection, - 'alias' => 'Authors', - 'table' => 'authors', - 'associations' => $associations - ]); - $authors->Articles->source($authors); - - $author = $authors->newEntity(['name' => 'mylux']); - $author = $authors->save($author); - - $newArticles = $articles->newEntities( - [ - [ - 'title' => 'New bakery next corner', - 'body' => 'They sell tastefull cakes' - ], - [ - 'title' => 'Spicy cake recipe', - 'body' => 'chocolate and peppers' - ] - ] - ); - - $sizeArticles = count($newArticles); - - $this->assertTrue($authors->Articles->link($author, $newArticles)); - $this->assertEquals($authors->Articles->findAllByAuthorId($author->id)->count(), $sizeArticles); - $this->assertEquals(count($author->articles), $sizeArticles); - - $newArticles = array_merge( - $author->articles, - $articles->newEntities( - [ - [ - 'title' => 'Cheese cake recipe', - 'body' => 'The secrets of mixing salt and sugar' - ], - [ - 'title' => 'Not another piece of cake', - 'body' => 'This is the best' - ] - ] - ) - ); - unset($newArticles[0]); - - $this->assertFalse($authors->Articles->replace($author, $newArticles)); - $this->assertCount($sizeArticles, $authors->Articles->findAllByAuthorId($author->id)); - } - - /** * Integration test for replacing entities which depend on their source entity with HasMany and failing transaction. False should be returned when * unlinking fails while replacing even when cascadeCallbacks is enabled