From fe2846f5a062a0376aa5db0a8ef84faab63b6bf2 Mon Sep 17 00:00:00 2001
From: mark_story <mark@mark-story.com>
Date: Tue, 12 Aug 2014 10:10:43 -0400
Subject: [PATCH] Move blank primary key filtering to the marshaller.

I agree with jose_zap that having it here makes more sense, as the
marshaller is responsible for converting request data into sensical
entities.
---
 src/ORM/Marshaller.php                |  4 ++++
 src/ORM/Table.php                     |  3 ---
 tests/TestCase/ORM/MarshallerTest.php | 21 +++++++++++++++++++++
 tests/TestCase/ORM/TableTest.php      | 20 --------------------
 4 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/src/ORM/Marshaller.php b/src/ORM/Marshaller.php
index 639b6d55005..57222ba93af 100644
--- a/src/ORM/Marshaller.php
+++ b/src/ORM/Marshaller.php
@@ -106,12 +106,16 @@ public function one(array $data, array $options = []) {
 			$data = $data[$tableName];
 		}
 
+		$primaryKey = $schema->primaryKey();
 		$properties = [];
 		foreach ($data as $key => $value) {
 			$columnType = $schema->columnType($key);
 			if (isset($propertyMap[$key])) {
 				$assoc = $propertyMap[$key]['association'];
 				$value = $this->_marshalAssociation($assoc, $value, $propertyMap[$key]);
+			} elseif ($value === '' && in_array($key, $primaryKey, true)) {
+				// Skip marshalling '' for pk fields.
+				continue;
 			} elseif ($columnType) {
 				$converter = Type::build($columnType);
 				$value = $converter->marshal($value);
diff --git a/src/ORM/Table.php b/src/ORM/Table.php
index d89c1cc044c..5e27fc9bb7b 100644
--- a/src/ORM/Table.php
+++ b/src/ORM/Table.php
@@ -1152,9 +1152,6 @@ protected function _processSave($entity, $options) {
 			$alias = $this->alias();
 			$conditions = [];
 			foreach ($primary as $k => $v) {
-				if ($v === '') {
-					$entity->unsetProperty($k);
-				}
 				$conditions["$alias.$k"] = $v;
 			}
 			$entity->isNew(!$this->exists($conditions));
diff --git a/tests/TestCase/ORM/MarshallerTest.php b/tests/TestCase/ORM/MarshallerTest.php
index cdac16d5e7e..264b7d5e85b 100644
--- a/tests/TestCase/ORM/MarshallerTest.php
+++ b/tests/TestCase/ORM/MarshallerTest.php
@@ -114,6 +114,27 @@ public function testOneSimple() {
 		$this->assertEquals('Articles', $result->source());
 	}
 
+/**
+ * Test that marshalling an entity with '' for pk values results
+ * in no pk value being set.
+ *
+ * @return void
+ */
+	public function testOneEmptyStringPrimaryKey() {
+		$data = [
+			'id' => '',
+			'username' => 'superuser',
+			'password' => 'root',
+			'created' => new Time('2013-10-10 00:00'),
+			'updated' => new Time('2013-10-10 00:00')
+		];
+		$marshall = new Marshaller($this->articles);
+		$result = $marshall->one($data, []);
+
+		$this->assertFalse($result->dirty('id'));
+		$this->assertNull($result->id);
+	}
+
 /**
  * Test marshalling datetime/date field.
  *
diff --git a/tests/TestCase/ORM/TableTest.php b/tests/TestCase/ORM/TableTest.php
index 63773b42384..30c6e4c0343 100644
--- a/tests/TestCase/ORM/TableTest.php
+++ b/tests/TestCase/ORM/TableTest.php
@@ -1212,26 +1212,6 @@ public function testSaveNewEmptyEntity() {
 		$this->assertFalse($table->save($entity));
 	}
 
-/**
- * Test that saving a new entity with an empty id populates
- * the entity's id field.
- *
- * @group save
- * @return void
- */
-	public function testSaveNewEntityEmptyIdField() {
-		$entity = new \Cake\ORM\Entity([
-			'id' => '',
-			'username' => 'superuser',
-			'password' => 'root',
-			'created' => new Time('2013-10-10 00:00'),
-			'updated' => new Time('2013-10-10 00:00')
-		]);
-		$table = TableRegistry::get('users');
-		$this->assertSame($entity, $table->save($entity));
-		$this->assertEquals($entity->id, self::$nextUserId);
-	}
-
 /**
  * Tests that saving an entity will filter out properties that
  * are not present in the table schema when saving