Fixing error with validateMany and validateAssociated not saving values altered in beforeValidate callbacks

lib/Cake/Model/Model.php

@@ -2107,14 +2107,15 @@ class Model extends Object implements CakeEventListener {
  *    Otherwise: array similar to the $data array passed, but values are set to true/false
  *    depending on whether each record validated successfully.
  */
-	public function validateMany($data, $options = array()) {
+	public function validateMany(&$data, $options = array()) {
 		$options = array_merge(array('atomic' => true, 'deep' => false), $options);
 		$this->validationErrors = $validationErrors = $return = array();
-		foreach ($data as $key => $record) {
+		foreach ($data as $key => &$record) {
 			if ($options['deep']) {
 				$validates = $this->validateAssociated($record, $options);
 			} else {
 				$validates = $this->create($record) && $this->validates($options);
+				$data[$key] = $this->data;
 			}
 			if ($validates === false || (is_array($validates) && in_array(false, $validates, true))) {
 				$validationErrors[$key] = $this->validationErrors;
@@ -2178,7 +2179,6 @@ class Model extends Object implements CakeEventListener {
 
 		if ($options['validate'] === 'first') {
 			$validates = $this->validateAssociated($data, $options);
-			$data = $this->data;
 			if ((!$validates && $options['atomic']) || (!$options['atomic'] && in_array(false, $validates, true))) {
 				return $validates;
 			}
@@ -2188,6 +2188,7 @@ class Model extends Object implements CakeEventListener {
 			$db = $this->getDataSource();
 			$transactionBegun = $db->begin();
 		}
+
 		$associations = $this->getAssociated();
 		$return = array();
 		$validates = true;
@@ -2306,7 +2307,7 @@ class Model extends Object implements CakeEventListener {
  *    Otherwise: array similar to the $data array passed, but values are set to true/false
  *    depending on whether each record validated successfully.
  */
-	public function validateAssociated($data, $options = array()) {
+	public function validateAssociated(&$data, $options = array()) {
 		$options = array_merge(array('atomic' => true, 'deep' => false), $options);
 		$this->validationErrors = $validationErrors = $return = array();
 		if (!($this->create($data) && $this->validates($options))) {
@@ -2315,8 +2316,9 @@ class Model extends Object implements CakeEventListener {
 		} else {
 			$return[$this->alias] = true;
 		}
+		$data[$this->alias] = $this->data[$this->alias];
 		$associations = $this->getAssociated();
-		foreach ($data as $association => $values) {
+		foreach ($data as $association => &$values) {
 			$validates = true;
 			if (isset($associations[$association])) {
 				if (in_array($associations[$association], array('belongsTo', 'hasOne'))) {

lib/Cake/Test/Case/Model/ModelValidationTest.php

@@ -991,4 +991,83 @@ class ModelValidationTest extends BaseModelTest {
 		$this->assertFalse($Article->validates());
 	}
 
-}
+/**
+ * Tests that altering data in a beforeValidate callback will lead to saving those
+ * values in database
+ *
+ * @return void
+ */
+	public function testValidateFirstWithBeforeValidate() {
+		$this->loadFixtures('Article', 'User');
+		$model = new CustomArticle();
+		$model->validate = array(
+			'title' => array(
+				'notempty' => array(
+					'rule' => 'notEmpty',
+					'required' => true,
+					'allowEmpty' => false
+				)
+			)
+		);
+		$data = array(
+			'CustomArticle' => array(
+				'body' => 'foo0'
+			)
+		);
+		$result = $model->saveAll($data, array('validate' => 'first'));
+		$this->assertTrue($result);
+
+		$title = $model->field('title', array('body' => 'foo0'));
+		$this->assertEquals('foo', $title);
+
+		$data = array(
+			array('body' => 'foo1'),
+			array('body' => 'foo2'),
+			array('body' => 'foo3')
+		);
+
+		$result = $model->saveAll($data, array('validate' => 'first'));
+		$this->assertTrue($result);
+
+		$this->assertEquals('foo', $model->field('title', array('body' => 'foo1')));
+		$this->assertEquals('foo', $model->field('title', array('body' => 'foo2')));
+		$this->assertEquals('foo', $model->field('title', array('body' => 'foo3')));
+	}
+
+/**
+ * Tests that altering data in a beforeValidate callback will lead to saving those
+ * values in database
+ *
+ * @return void
+ */
+	public function testValidateAssociatedWithBeforeValidate() {
+		$this->loadFixtures('Article', 'User');
+		$model = new CustomArticle();
+		$model->validate = array(
+			'title' => array(
+				'notempty' => array(
+					'rule' => 'notEmpty',
+					'required' => true
+				)
+			)
+		);
+		$articles = array(
+			array('body' => 'foo1'),
+			array('body' => 'foo2'),
+			array('body' => 'foo3')
+		);
+		$user = new User();
+		$user->hasMany['CustomArticle'] = array('foreignKey' => 'user_id');
+		$data = array(
+			'User' => array('user' => 'foo', 'password' => 'bar'),
+			'CustomArticle' => $articles
+		);
+		$result = $user->saveAll($data, array('validate' => 'first'));
+		$this->assertTrue($result);
+
+		$this->assertEquals('foo', $model->field('title', array('body' => 'foo1')));
+		$this->assertEquals('foo', $model->field('title', array('body' => 'foo2')));
+		$this->assertEquals('foo', $model->field('title', array('body' => 'foo3')));
+	}
+
+}

lib/Cake/Test/Case/Model/ModelWriteTest.php

@@ -4277,7 +4277,7 @@ class ModelWriteTest extends BaseModelTest {
 					'author_id' => '3',
 					'title' => 'Just update the title',
 					'body' => 'Second Post Body',
-					'published' => 'Y',
+					'published' => 'N',
 					'created' => '2007-03-18 10:41:23'
 			)),
 			array(
@@ -4366,7 +4366,7 @@ class ModelWriteTest extends BaseModelTest {
 					'author_id' => '3',
 					'title' => 'Just update the title',
 					'body' => 'Second Post Body',
-					'published' => 'Y',
+					'published' => 'N',
 					'created' => '2007-03-18 10:41:23'
 				)
 			),
@@ -5631,7 +5631,7 @@ class ModelWriteTest extends BaseModelTest {
 					'author_id' => '3',
 					'title' => 'Just update the title',
 					'body' => 'Second Post Body',
-					'published' => 'Y',
+					'published' => 'N',
 					'created' => '2007-03-18 10:41:23'
 				)
 			),
@@ -5727,7 +5727,7 @@ class ModelWriteTest extends BaseModelTest {
 					'author_id' => '3',
 					'title' => 'Just update the title',
 					'body' => 'Second Post Body',
-					'published' => 'Y',
+					'published' => 'N',
 			)),
 			array(
 				'Post' => array(
@@ -5779,24 +5779,24 @@ class ModelWriteTest extends BaseModelTest {
 	public function testValidateMany() {
 		$TestModel = new Article();
 		$TestModel->validate = array('title' => 'notEmpty');
-		$result = $TestModel->validateMany(
-			array(
+		$data = array(
 				0 => array('title' => ''),
 				1 => array('title' => 'title 1'),
 				2 => array('title' => 'title 2'),
-		));
+		);
+		$result = $TestModel->validateMany($data);
 		$this->assertFalse($result);
 		$expected = array(
 			0 => array('title' => array('This field cannot be left blank')),
 		);
 		$this->assertEquals($expected, $TestModel->validationErrors);
 
-		$result = $TestModel->validateMany(
-			array(
+		$data = array(
 				0 => array('title' => 'title 0'),
 				1 => array('title' => ''),
 				2 => array('title' => 'title 2'),
-		));
+		);
+		$result = $TestModel->validateMany($data);
 		$this->assertFalse($result);
 		$expected = array(
 			1 => array('title' => array('This field cannot be left blank')),
@@ -5966,47 +5966,46 @@ class ModelWriteTest extends BaseModelTest {
 		$TestModel->belongsTo = $TestModel->hasAndBelongsToMany = array();
 		$TestModel->Comment->validate = array('comment' => 'notEmpty');
 
-		$result = $TestModel->validateAssociated(
-			array(
-				'Article' => array('id' => 2),
-				'Comment' => array(
-					array(
-						'id' => 1,
-						'comment' => '',
-						'published' => 'Y',
-						'user_id' => 1),
-					array(
-						'id' => 2,
-						'comment' =>
-						'comment',
-						'published' => 'Y',
-						'user_id' => 1
-		))));
+		$data = array(
+			'Article' => array('id' => 2),
+			'Comment' => array(
+				array(
+					'id' => 1,
+					'comment' => '',
+					'published' => 'Y',
+					'user_id' => 1),
+				array(
+					'id' => 2,
+					'comment' =>
+					'comment',
+					'published' => 'Y',
+					'user_id' => 1
+		)));
+		$result = $TestModel->validateAssociated($data);
 		$this->assertFalse($result);
 
-		$result = $TestModel->validateAssociated(
-			array(
-				'Article' => array('id' => 2),
-				'Comment' => array(
-					array(
-						'id' => 1,
-						'comment' => '',
-						'published' => 'Y',
-						'user_id' => 1
-					),
-					array(
-						'id' => 2,
-						'comment' => 'comment',
-						'published' => 'Y',
-						'user_id' => 1
-					),
-					array(
-						'id' => 3,
-						'comment' => '',
-						'published' => 'Y',
-						'user_id' => 1
-			))),
-			array(
+		$data = array(
+			'Article' => array('id' => 2),
+			'Comment' => array(
+				array(
+					'id' => 1,
+					'comment' => '',
+					'published' => 'Y',
+					'user_id' => 1
+				),
+				array(
+					'id' => 2,
+					'comment' => 'comment',
+					'published' => 'Y',
+					'user_id' => 1
+				),
+				array(
+					'id' => 3,
+					'comment' => '',
+					'published' => 'Y',
+					'user_id' => 1
+		)));
+		$result = $TestModel->validateAssociated($data, array(
 				'atomic' => false
 		));
 		$expected = array(

lib/Cake/Test/Case/Model/models.php

@@ -4948,6 +4948,13 @@ class CustomArticle extends AppModel {
  */
 	public $findMethods = array('unPublished' => true);
 
+/**
+ * belongsTo property
+ *
+ * @var array
+ */
+	public $belongsTo = array('User');
+
 /**
  * _findUnPublished custom find
  *
@@ -4961,4 +4968,13 @@ class CustomArticle extends AppModel {
 		return $results;
 	}
 
+/**
+ * Alters title data
+ *
+ * @return void
+ **/
+	public function beforeValidate() {
+		$this->data[$this->alias]['title'] = 'foo';
+	}
+
 }