Fixed issue where Model::saveAll() would incorrectly commit a transac…

…tion which was not started in that function call itself.

cake/libs/model/model.php

@@ -559,9 +559,9 @@ function bindModel($params, $reset = true) {
  *
  * Example: Turn off the associated Model Support request,
  * to temporarily lighten the User model:
- * 
+ *
  * `$this->User->unbindModel( array('hasMany' => array('Supportrequest')) );`
- * 
+ *
  * unbound models that are not made permanent will reset with the next call to Model::find()
  *
  * @param array $params Set of bindings to unbind (indexed by binding type)
@@ -1589,7 +1589,7 @@ function saveAll($data = null, $options = array()) {
 		}
 
 		if ($options['atomic'] && $options['validate'] !== 'only') {
-			$db->begin($this);
+			$transactionBegun = $db->begin($this);
 		}
 
 		if (Set::numeric(array_keys($data))) {
@@ -1629,8 +1629,12 @@ function saveAll($data = null, $options = array()) {
 					break;
 					default:
 						if ($options['atomic']) {
-							if ($validates && ($db->commit($this) !== false)) {
-								return true;
+							if ($validates) {
+								if ($transactionBegun) {
+									return $db->commit($this) !== false;
+								} else {
+									return true;
+								}
 							}
 							$db->rollback($this);
 							return false;
@@ -1740,7 +1744,11 @@ function saveAll($data = null, $options = array()) {
 				default:
 					if ($options['atomic']) {
 						if ($validates) {
-							return ($db->commit($this) !== false);
+							if ($transactionBegun) {
+								return $db->commit($this) !== false;
+							} else {
+								return true;
+							}
 						} else {
 							$db->rollback($this);
 						}

cake/tests/cases/libs/model/model_write.test.php

@@ -3121,6 +3121,22 @@ function testSaveAllAssociatedTransactionNoRollback() {
 		$Post->saveAll($data);
 	}
 
+/**
+ * test saveAll with nested saveAll call.
+ *
+ * @return void
+ */
+	function testSaveAllNestedSaveAll() {
+		$this->loadFixtures('Sample');
+		$TransactionTestModel =& new TransactionTestModel();
+
+		$data = array(
+			array('apple_id' => 1, 'name' => 'sample5'),
+		);
+
+		$this->assertTrue($TransactionTestModel->saveAll($data, array('atomic' => true)));
+	}
+
 /**
  * testSaveAllTransaction method
  *

cake/tests/cases/libs/model/models.php

@@ -290,7 +290,7 @@ function titleDuplicate ($title) {
  */
 class BeforeDeleteComment extends CakeTestModel {
 	var $name = 'BeforeDeleteComment';
-	
+
 	var $useTable = 'comments';
 
 	function beforeDelete($cascade = true) {
@@ -3557,6 +3557,7 @@ class FruitNoWith extends CakeTestModel {
 		)
 	);
 }
+
 class UuidTagNoWith extends CakeTestModel {
 	var $name = 'UuidTag';
 	var $useTable = 'uuid_tags';
@@ -3573,11 +3574,21 @@ class UuidTagNoWith extends CakeTestModel {
 class ProductUpdateAll extends CakeTestModel {
 	var $name = 'ProductUpdateAll';
 	var $useTable = 'product_update_all';
-
 }
 
 class GroupUpdateAll extends CakeTestModel {
 	var $name = 'GroupUpdateAll';
 	var $useTable = 'group_update_all';
-
 }
+
+class TransactionTestModel extends CakeTestModel {
+	var $name = 'TransactionTestModel';
+	var $useTable = 'samples';
+
+	function afterSave($created) {
+		$data = array(
+			array('apple_id' => 1, 'name' => 'sample6'),
+		);
+		$this->saveAll($data, array('atomic' => true, 'callbacks' => false));
+	}
+}