Adding performance enhancement to Set::numeric(), adding $filterKey param to Model::create() to overwrite primary keys, closes #4046

git-svn-id: https://svn.cakephp.org/repo/branches/1.2.x.x@6438 3807eeeb-6ff5-0310-8944-8be069107fe0

cake/libs/model/model.php

@@ -1039,10 +1039,11 @@ class Model extends Overloadable {
  *
  * @param mixed $data Optional data array to assign to the model after it is created.  If null or false,
  *                    schema data defaults are not merged.
+ * @param boolean $filterKey If true, overwrites any primary key input with an empty value
  * @return mixed The current data of the model, or true if $data is empty
  * @access public
  */
-	function create($data = array()) {
+	function create($data = array(), $filterKey = false) {
 		$defaults = array();
 		$this->id = false;
 		$this->data = array();
@@ -1057,6 +1058,9 @@ class Model extends Overloadable {
 			$this->set(Set::filter($defaults));
 			$this->set($data);
 		}
+		if ($filterKey) {
+			$this->set($this->primaryKey, false);
+		}
 		if (empty($this->data)) {
 			return true;
 		}
@@ -1406,7 +1410,7 @@ class Model extends Overloadable {
  */
 	function saveAll($data = null, $options = array()) {
 		if (empty($data)) {
-			return false;
+			$data = $this->data;
 		}
 		$db =& ConnectionManager::getDataSource($this->useDbConfig);
 
@@ -1414,17 +1418,30 @@ class Model extends Overloadable {
 			array('validate' => true, 'fieldList' => array(), 'atomic' => true),
 			$options
 		);
+		$return = array();
 
 		if ($options['atomic']) {
 			$db->begin($this);
 		}
 
 		if (Set::numeric(array_keys($data))) {
+			if ($options['validate'] === 'first') {
+				foreach ($data as $record) {
+					if (!($this->create($record) && $result = $this->validates())) {
+						return false;
+					}
+					$return[] = $result[$this->alias];
+				}
+			}
+
 			foreach ($data as $record) {
-				if (!($this->create($record) && $this->save(null, $options['validate'], $options['fieldList'])) && $options['atomic']) {
+				if (!($this->create($record) && $result = $this->save(null, $options['validate'] !== false, $options['fieldList'])) && $options['atomic']) {
-					$db->rollback($this);
+					if ($options['atomic']) {
+						$db->rollback($this);
+					}
 					return false;
 				}
+				$return[] = $result[$this->alias];
 			}
 		} else {
 			$associations = $this->getAssociated();
@@ -1434,17 +1451,22 @@ class Model extends Overloadable {
 					$alias = $this->{$association}->alias;
 					$foreignKey = $this->{$type}[$association]['foreignKey'];
 
-					if (!$result = $this->{$association}->save($values, $options['validate'], $options['fieldList'])) {
+					if (!$result = $this->{$association}->save($values, ($options['validate'] !== false), $options['fieldList'])) {
-						$db->rollback($this);
+						if ($options['atomic']) {
+							$db->rollback($this);
+						}
 						return false;
 					} elseif (!isset($data[$foreignKey]) || empty($data[$foreignKey])) {
 						$data[$this->alias][$foreignKey] = $this->{$association}->id;
 					}
+					$return[$association] = $result[$association];
 				}
 			}
 
-			if (!$this->save($data[$this->alias], $options['validate'], $options['fieldList'])) {
+			if (!$result = $this->save($data[$this->alias], ($options['validate'] !== false), $options['fieldList'])) {
-				$db->rollback($this);
+				if ($options['atomic']) {
+					$db->rollback($this);
+				}
 				return false;
 			}
 
@@ -1454,16 +1476,21 @@ class Model extends Overloadable {
 						case 'hasOne':
 							$type = $associations[$association];
 							$this->{$association}->set($this->{$type}[$association]['foreignKey'], $this->id);
-							if (!$result = $this->{$association}->save($values, $options['validate'], $options['fieldList'])) {
+							if (!$result = $this->{$association}->save($values, ($options['validate'] !== false), $options['fieldList'])) {
-								$db->rollback($this);
+								if ($options['atomic']) {
+									$db->rollback($this);
+								}
 								return false;
 							}
 						break;
 						case 'hasMany':
-							if (!$this->{$association}->saveAll($values, $options)) {
+							if (!$result = $this->{$association}->saveAll($values, $options)) {
-								$db->rollback($this);
+								if ($options['atomic']) {
+									$db->rollback($this);
+								}
 								return false;
 							}
+							$return[$association] = $result[$association];
 						break;
 					}
 				}
@@ -1471,7 +1498,7 @@ class Model extends Overloadable {
 		}
 
 		if ($options['atomic']) {
-			$db->commit($this);
+			return ($db->commit($this) !== false);
 		}
 		return true;
 	}

cake/libs/router.php

@@ -743,7 +743,7 @@ class Router extends Object {
 			}
 
 			if ($match === false) {
-				list($args, $named)  = array(Set::filter($args, true), Set::filter($named, true));
+				list($args, $named)  = array(Set::filter($args, true), Set::filter($named));
 
 				if (empty($named) && empty($args) && (!isset($url['action']) || $url['action'] == 'index')) {
 					$url['action'] = null;

cake/libs/set.php

@@ -257,9 +257,14 @@ class Set extends Object {
 			$array = $this->get();
 		}
 
+		if ($array == range(0, count($array) - 1)) {
+			return true;
+		}
+
 		$numeric = true;
 		$keys = array_keys($array);
 		$count = count($keys);
+
 		for ($i = 0; $i < $count; $i++) {
 			if (!is_numeric($array[$keys[$i]])) {
 				$numeric = false;

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

@@ -789,6 +789,26 @@ class ModelTest extends CakeTestCase {
 		$this->assertEqual($this->model->find('count'), 1);
 	}
 
+	function testCreateWithPKFiltering() {
+		$this->model =& new Article();
+		$data = array('id' => 5, 'user_id' => 2, 'title' => 'My article', 'body' => 'Some text');
+
+		$result = $this->model->create($data);
+		$expected = array('Article' => array('published' => 'N', 'id' => 5, 'user_id' => 2, 'title' => 'My article', 'body' => 'Some text'));
+		$this->assertEqual($result, $expected);
+		$this->assertEqual($this->model->id, 5);
+
+		$result = $this->model->create($data, true);
+		$expected = array('Article' => array('published' => 'N', 'id' => false, 'user_id' => 2, 'title' => 'My article', 'body' => 'Some text'));
+		$this->assertEqual($result, $expected);
+		$this->assertFalse($this->model->id);
+
+		$result = $this->model->create(array('Article' => $data), true);
+		$expected = array('Article' => array('published' => 'N', 'id' => false, 'user_id' => 2, 'title' => 'My article', 'body' => 'Some text'));
+		$this->assertEqual($result, $expected);
+		$this->assertFalse($this->model->id);
+	}
+
 	function testCreationWithMultipleData() {
 		$this->loadFixtures('Article', 'Comment');
 		$this->Article =& new Article();

cake/tests/cases/libs/set.test.php

@@ -44,6 +44,26 @@ class SetTest extends UnitTestCase {
 		$this->assertIdentical(Set::diff($data, Set::extract($data, '{n}')), array('plugin' => null, 'controller' => '', 'action' => ''));
 	}
 
+	function testFilter() {
+		$result = Set::filter(array('0', false, true, 0, array('one thing', 'I can tell you', 'is you got to be', false)));
+		$expected = array('0', 2 => true, 3 => 0, 4 => array('one thing', 'I can tell you', 'is you got to be', false));
+		$this->assertIdentical($result, $expected);
+	}
+
+	function testNumericArrayCheck() {
+		$data = array('one', 'two', 'three', 'four', 'five');
+		$this->assertTrue(Set::numeric(array_keys($data)));
+
+		$data = array(1 => 'one', 2 => 'two', 3 => 'three', 4 => 'four', 5 => 'five');
+		$this->assertTrue(Set::numeric(array_keys($data)));
+
+		$data = array('1' => 'one', 2 => 'two', 3 => 'three', 4 => 'four', 5 => 'five');
+		$this->assertTrue(Set::numeric(array_keys($data)));
+
+		$data = array('one', 2 => 'two', 3 => 'three', 4 => 'four', 'a' => 'five');
+		$this->assertFalse(Set::numeric(array_keys($data)));
+	}
+
 	function testMerge() {
 		$r = Set::merge(array('foo'));
 		$this->assertIdentical($r, array('foo'));
@@ -91,7 +111,6 @@ class SetTest extends UnitTestCase {
 		unset($Set);
 
 		$Set =& new Set();
-
 		$SetA =& new Set($a);
 		$SetB =& new Set($b);
 		$SetC =& new Set($c);