Skip to content

Commit

Permalink
Models now require a primary key. This is partially to keep the code …
Browse files Browse the repository at this point in the history
…from becoming a tangled mess, and partially to ensure all Model convenience features work both now and in the future. Closes #1597
  • Loading branch information
lonnieezell committed Mar 14, 2019
1 parent fed73d7 commit 9d7ea5d
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 12 deletions.
13 changes: 13 additions & 0 deletions system/Exceptions/ModelException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?php namespace CodeIgniter\Exceptions;

/**
* Model Exceptions.
*/

class ModelException extends FrameworkException
{
public static function forNoPrimaryKey(string $modelName)
{
return new static(lang('Database.noPrimaryKey', [$modelName]));
}
}
1 change: 1 addition & 0 deletions system/Language/en/Database.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,5 @@
'parseStringFail' => 'Parsing key string failed.',
'featureUnavailable' => 'This feature is not available for the database you are using.',
'tableNotFound' => 'Table `{0}` was not found in the current database.',
'noPrimaryKey' => '`{0}` model class does not specify a Primary Key.',
];
9 changes: 9 additions & 0 deletions system/Model.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
* @filesource
*/

use CodeIgniter\Exceptions\ModelException;
use Config\Database;
use CodeIgniter\I18n\Time;
use CodeIgniter\Pager\Pager;
Expand Down Expand Up @@ -1093,6 +1094,14 @@ protected function builder(string $table = null)
return $this->builder;
}

// We're going to force a primary key to exist
// so we don't have overly convoluted code,
// and future features are likely to require them.
if (empty($this->primaryKey))
{
throw ModelException::forNoPrimaryKey(get_class($this));
}

$table = empty($table) ? $this->table : $table;

// Ensure we have a good db connection
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,11 @@ public function up()

//No Primary Key
$this->forge->addField([
'id' => [
'type' => 'INTEGER',
'constraint' => 3,
$unique_or_auto => true,
],
'key' => [
'type' => 'VARCHAR',
'constraint' => 40,
Expand Down
2 changes: 1 addition & 1 deletion tests/_support/Models/SecondaryModel.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ class SecondaryModel extends Model
{
protected $table = 'secondary';

protected $primaryKey = null;
protected $primaryKey = 'id';

protected $returnType = 'object';

Expand Down
29 changes: 21 additions & 8 deletions tests/system/Database/Live/ModelTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -247,11 +247,13 @@ public function testFirstWithNoPrimaryKey()

$this->db->table('secondary')
->insert([
'id' => 1,
'key' => 'foo',
'value' => 'bar',
]);
$this->db->table('secondary')
->insert([
'id' => 2,
'key' => 'bar',
'value' => 'baz',
]);
Expand Down Expand Up @@ -519,14 +521,12 @@ public function testValidationWithSetValidationMessages()
'description' => 'some great marketing stuff',
];

$model->setValidationMessages(
[
'name' => [
'required' => 'Your baby name is missing.',
'min_length' => 'Too short, man!',
],
]
);
$model->setValidationMessages([
'name' => [
'required' => 'Your baby name is missing.',
'min_length' => 'Too short, man!',
],
]);

$this->assertFalse($model->insert($data));

Expand Down Expand Up @@ -979,6 +979,7 @@ public function testUpdateNoPrimaryKey()

$this->db->table('secondary')
->insert([
'id' => 1,
'key' => 'foo',
'value' => 'bar',
]);
Expand Down Expand Up @@ -1103,4 +1104,16 @@ public function testValidationIncludingErrors()

$this->assertEquals('Minimum Length Error', $model->errors()['name']);
}

/**
* @expectedException CodeIgniter\Exceptions\ModelException
* @expectedExceptionMessage `Tests\Support\Models\UserModel` model class does not specify a Primary Key.
*/
public function testThrowsWithNoPrimaryKey()
{
$model = new UserModel();
$this->setPrivateProperty($model, 'primaryKey', '');

$model->find(1);
}
}
9 changes: 6 additions & 3 deletions user_guide_src/source/models/model.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ instance of the database connection and you're good to go.

::

<?php namespace App\Models;
<?php namespace App\Models;

use CodeIgniter\Database\ConnectionInterface;

Expand Down Expand Up @@ -70,9 +70,9 @@ This ensures that within the model any references to ``$this->db`` are made thro
connection.
::

<?php namespace App\Models;
<?php namespace App\Models;

use CodeIgniter\Model;
use CodeIgniter\Model;

class UserModel extends Model
{
Expand Down Expand Up @@ -122,6 +122,9 @@ This is the name of the column that uniquely identifies the records in this tabl
does not necessarily have to match the primary key that is specified in the database, but
is used with methods like ``find()`` to know what column to match the specified value to.

.. note:: All Models must have a primaryKey specified to allow all of the features to work
as expected.

**$returnType**

The Model's CRUD methods will take a step of work away from you and automatically return
Expand Down

0 comments on commit 9d7ea5d

Please sign in to comment.