Skip to content

Commit

Permalink
Validate the primary key when registering or saving a model (see cont…
Browse files Browse the repository at this point in the history
…ao#230)

Description
-----------

Consider the following:
```
$model = new Contao\PageModel();
$model->title = 'Foo';
$model->attach();
```
This works once per table (in this example, `tl_page`), because `Contao\Model\Registry::arrRegistry` then has one key for that table that is `""` (empty string):
```
array(13) {
  // …
  ["tl_page"]=>
  array(58) {
    [1]=>
    object(Contao\PageModel)contao#641 (6) { /* … */ }
    [4]=>
    object(Contao\PageModel)contao#768 (6) { /* … */ }
    // …
    [""]=>
    object(Contao\PageModel)contao#2094 (6) { /* Our Model */ }
  }
  // …
}
```
Once we do the same thing again:
```
$model2 = new Contao\PageModel();
$model2->title = 'Bar';
$model2->attach();
```
we suddenly get an exception:
> The registry already contains an instance for tl_page::id()

Obviously, this is not the expected behavior. The Registry should either categorically allow registering a model without a primary key or categorically not allow it. This fix implements the latter. It does not use `empty()` because that would overshoot and prevent the primary key from being `0` or `"0"`, which may not be recommendable, but does not have to be forbidden in this context.

Commits
-------

3f33fc2 Do not allow registering a Model with an empty primary key
5011f8e Do not allow saving a Model with an invalid primary key
3aaaee2 Revert some changes and allow the primary key to be an empty string
  • Loading branch information
gmpf authored and leofeyer committed Jan 22, 2019
1 parent 58e17e7 commit 935fa81
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 0 deletions.
5 changes: 5 additions & 0 deletions core-bundle/src/Resources/contao/library/Contao/Model.php
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,11 @@ public function save()
$intPk = $this->arrModified[static::$strPk];
}

if ($intPk === null)
{
throw new \RuntimeException('The primary key has not been set');
}

// Update the row
$objDatabase->prepare("UPDATE " . static::$strTable . " %s WHERE " . \Database::quoteIdentifier(static::$strPk) . "=?")
->set($arrSet)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,11 @@ public function register(Model $objModel)
$strPk = $objModel->getPk();
$varPk = $objModel->$strPk;

if ($varPk === null)
{
throw new \RuntimeException('The primary key has not been set');
}

// Another model object is pointing to the DB record already
if (isset($this->arrRegistry[$strTable][$varPk]))
{
Expand Down

0 comments on commit 935fa81

Please sign in to comment.