Merge pull request #205 from cainmi/develop

Restrict null primary keys on update/delete, resolves #203
j4mie · Jun 21, 2014 · 6dbd879 · 6dbd879
2 parents b616165 + 90469df
commit 6dbd879
Show file tree

Hide file tree

Showing 2 changed files with 95 additions and 4 deletions.
diff --git a/idiorm.php b/idiorm.php
@@ -1879,8 +1879,22 @@ protected function _get_id_column_name() {
         /**
          * Get the primary key ID of this object.
          */
-        public function id() {
-            return $this->get($this->_get_id_column_name());
+        public function id($disallow_null = false) {
+            $id = $this->get($this->_get_id_column_name());
+
+            if ($disallow_null) {
+                if (is_array($id)) {
+                    foreach ($id as $id_part) {
+                        if ($id_part === null) {
+                            throw new Exception('Primary key ID contains null value(s)');
+                        }
+                    }
+                } else if ($id === null) {
+                    throw new Exception('Primary key ID missing from row or is null');
+                }
+            }
+
+            return $id;
         }
 
         /**
@@ -1961,7 +1975,7 @@ public function save() {
                     return true;
                 }
                 $query = $this->_build_update();
-                $id = $this->id();
+                $id = $this->id(true);
                 if (is_array($id)) {
                     $values = array_merge($values, array_values($id));
                 } else {
@@ -2068,7 +2082,7 @@ public function delete() {
                 $this->_quote_identifier($this->_table_name)
             );
             $this->_add_id_column_conditions($query);
-            return self::_execute(join(" ", $query), is_array($this->id()) ? array_values($this->id()) : array($this->id()), $this->_connection_name);
+            return self::_execute(join(" ", $query), is_array($this->id(true)) ? array_values($this->id(true)) : array($this->id(true)), $this->_connection_name);
         }
 
         /**

diff --git a/test/ORMTest.php b/test/ORMTest.php
@@ -99,4 +99,81 @@ public function testInvalidResultsSetFunctionCallShouldCreateException() {
         $resultSet = ORM::for_table('test')->find_result_set();
         $resultSet->invalidFunctionCall();
     }
+
+    /**
+     * These next two tests are needed because if you have select()ed some fields,
+     * but not the primary key, then the primary key is not available for the
+     * update/delete query - see issue #203.
+     * We need to change the primary key here to something other than `id`
+     * becuase MockPDOStatement->fetch() always returns an id.
+     */
+    public function testUpdateNullPrimaryKey() {
+        try {
+            $widget = ORM::for_table('widget')
+                ->use_id_column('primary')
+                ->select('foo')
+                ->where('primary', 1)
+                ->find_one()
+            ;
+
+            $widget->foo = 'bar';
+            $widget->save();
+
+            throw new Exception('Test did not throw expected exception');
+        } catch (Exception $e) {
+            $this->assertEquals($e->getMessage(), 'Primary key ID missing from row or is null');
+        }
+    }
+
+    public function testDeleteNullPrimaryKey() {
+        try {
+            $widget = ORM::for_table('widget')
+                ->use_id_column('primary')
+                ->select('foo')
+                ->where('primary', 1)
+                ->find_one()
+            ;
+
+            $widget->delete();
+
+            throw new Exception('Test did not throw expected exception');
+        } catch (Exception $e) {
+            $this->assertEquals($e->getMessage(), 'Primary key ID missing from row or is null');
+        }
+    }
+
+    public function testNullPrimaryKey() {
+        try {
+            $widget = ORM::for_table('widget')
+                ->use_id_column('primary')
+                ->select('foo')
+                ->where('primary', 1)
+                ->find_one()
+            ;
+
+            $widget->id(true);
+
+            throw new Exception('Test did not throw expected exception');
+        } catch (Exception $e) {
+            $this->assertEquals($e->getMessage(), 'Primary key ID missing from row or is null');
+        }
+    }
+
+    public function testNullPrimaryKeyPart() {
+        try {
+            $widget = ORM::for_table('widget')
+                ->use_id_column(array('id', 'primary'))
+                ->select('foo')
+                ->where('id', 1)
+                ->where('primary', 1)
+                ->find_one()
+            ;
+
+            $widget->id(true);
+
+            throw new Exception('Test did not throw expected exception');
+        } catch (Exception $e) {
+            $this->assertEquals($e->getMessage(), 'Primary key ID contains null value(s)');
+        }
+    }
 }