Assuming that deleteAll statement inside _unlink is always OK if no e…

…xceptions occurred. Removing unnecessary test
cakephp · Nov 4, 2015 · c99a54f · c99a54f
1 parent 9a853a1
commit c99a54f
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 107 deletions.
diff --git 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
@@ -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