Browse Source

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.
mark_story 11 years ago
parent
commit
fe2846f5a0

+ 4 - 0
src/ORM/Marshaller.php

@@ -106,12 +106,16 @@ class Marshaller {
 			$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);

+ 0 - 3
src/ORM/Table.php

@@ -1152,9 +1152,6 @@ class Table implements RepositoryInterface, EventListener {
 			$alias = $this->alias();
 			$conditions = [];
 			foreach ($primary as $k => $v) {
-				if ($v === '') {
-					$entity->unsetProperty($k);
-				}
 				$conditions["$alias.$k"] = $v;
 			}
 			$entity->isNew(!$this->exists($conditions));

+ 21 - 0
tests/TestCase/ORM/MarshallerTest.php

@@ -115,6 +115,27 @@ class MarshallerTest extends TestCase {
 	}
 
 /**
+ * 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.
  *
  * @return void

+ 0 - 20
tests/TestCase/ORM/TableTest.php

@@ -1213,26 +1213,6 @@ class TableTest extends \Cake\TestSuite\TestCase {
 	}
 
 /**
- * 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
  *