HasMany::replace taking into consideration success of delete statements while unlinking

src/ORM/Association/HasMany.php

@@ -149,7 +149,11 @@ class HasMany extends Association
         $options['_sourceTable'] = $this->source();
 
         if ($this->_saveStrategy === self::SAVE_REPLACE) {
-            $this->_unlinkAssociated($properties, $entity, $target, $targetEntities);
+            $unlinkSuccessful = $this->_unlinkAssociated($properties, $entity, $target, $targetEntities);
+        }
+
+        if (isset($unlinkSuccessful) && !$unlinkSuccessful) {
+            return false;
         }
 
         foreach ($targetEntities as $k => $targetEntity) {
@@ -367,7 +371,7 @@ class HasMany extends Association
      * @param EntityInterface $entity the entity which should have its associated entities unassigned
      * @param Table $target The associated table
      * @param array $remainingEntities Entities that should not be deleted
-     * @return void
+     * @return bool success
      */
     protected function _unlinkAssociated(array $properties, EntityInterface $entity, Table $target, array $remainingEntities = [])
     {
@@ -396,7 +400,7 @@ class HasMany extends Association
             ];
         }
 
-        $this->_unlink(array_keys($properties), $target, $conditions);
+        return $this->_unlink(array_keys($properties), $target, $conditions);
     }
 
     /**
@@ -406,25 +410,31 @@ class HasMany extends Association
      * @param array $foreignKey array of foreign key properties
      * @param Table $target The associated table
      * @param array $conditions The conditions that specifies what are the objects to be unlinked
-     * @return void
+     * @return bool success
      */
     protected function _unlink(array $foreignKey, Table $target, array $conditions = [])
     {
+        $ok = true;
         $mustBeDependent = (!$this->_foreignKeyAcceptsNull($target, $foreignKey) || $this->dependent());
-        if ($mustBeDependent) {
-            if ($this->_cascadeCallbacks) {
-                $query = $this->find('all')->where($conditions);
-                foreach ($query as $assoc) {
-                    $target->delete($assoc);
+        $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);
                 }
             } else {
-                $target->deleteAll($conditions);
-            }
-        } else {
-            $updateFields = array_fill_keys($foreignKey, null);
-            $target->updateAll($updateFields, $conditions);
+                $updateFields = array_fill_keys($foreignKey, null);
+                $ok = $target->updateAll($updateFields, $conditions);
 
+            }
         }
+        return $ok;
     }
 
     /**

tests/TestCase/ORM/TableTest.php

@@ -25,6 +25,8 @@ use Cake\Datasource\ConnectionManager;
 use Cake\Event\Event;
 use Cake\Event\EventManager;
 use Cake\I18n\Time;
+use Cake\ORM\AssociationCollection;
+use Cake\ORM\Association\HasMany;
 use Cake\ORM\Entity;
 use Cake\ORM\Query;
 use Cake\ORM\RulesChecker;
@@ -4125,6 +4127,318 @@ class TableTest extends TestCase
     }
 
     /**
+     * Integration test for replacing entities with HasMany and failing transaction. False should be returned when
+     * unlinking fails while replacing
+     *
+     * @return void
+     */
+    public function testReplaceHasManyOnError()
+    {
+        $articles = $this->getMock(
+            'Cake\ORM\Table',
+            ['updateAll'],
+            [[
+                'connection' => $this->connection,
+                'alias' => 'Articles',
+                'table' => 'articles',
+            ]]
+        );
+
+        $articles->method('updateAll')->willReturn(false);
+
+        $associations = new AssociationCollection();
+
+        $hasManyArticles = $this->getMock(
+            'Cake\ORM\Association\HasMany',
+            ['target'],
+            [
+                'articles',
+                [
+                    'target' => $articles,
+                    'foreignKey' => 'author_id',
+                ]
+            ]
+        );
+        $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
+     *
+     * @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
+     *
+     * @return void
+     */
+    public function testReplaceHasManyOnErrorDependentCascadeCallbacks()
+    {
+        $articles = $this->getMock(
+            'Cake\ORM\Table',
+            ['delete'],
+            [[
+                'connection' => $this->connection,
+                'alias' => 'Articles',
+                'table' => 'articles',
+            ]]
+        );
+
+        $articles->method('delete')->willReturn(false);
+
+        $associations = new AssociationCollection();
+
+        $hasManyArticles = $this->getMock(
+            'Cake\ORM\Association\HasMany',
+            ['target'],
+            [
+                'articles',
+                [
+                    'target' => $articles,
+                    'foreignKey' => 'author_id',
+                    'dependent' => true,
+                    'cascadeCallbacks' => 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 with HasMany and an empty target list. The transaction must be successfull
+     *
+     * @return void
+     */
+    public function testReplaceHasManyEmptyList()
+    {
+        $authors = new Table([
+            'connection' => $this->connection,
+            'alias' => 'Authors',
+            'table' => 'authors',
+        ]);
+        $authors->hasMany('Articles');
+
+        $author = $authors->newEntity(['name' => 'mylux']);
+        $author = $authors->save($author);
+
+        $newArticles = $authors->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 = [];
+
+        $this->assertTrue($authors->Articles->replace($author, $newArticles));
+        $this->assertCount(0, $authors->Articles->findAllByAuthorId($author->id));
+    }
+
+    /**
      * Integration test for replacing entities with HasMany.
      *
      * @return void