Skip to content

Commit

Permalink
Throw an exception instead of triggering a notice.
Browse files Browse the repository at this point in the history
This situation is unlikely to happen currently, given that joins
aren't supported for delete/update queries, therefore introducing
an exception at this point should be safe.
  • Loading branch information
ndm2 committed Sep 8, 2016
1 parent 969c8c6 commit 2cb8533
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 33 deletions.
7 changes: 4 additions & 3 deletions src/Database/SqlDialectTrait.php
Expand Up @@ -203,14 +203,15 @@ protected function _updateQueryTranslator($query)
*
* @param \Cake\Database\Query $query The query to process.
* @return \Cake\Database\Query The modified query.
* @throws \RuntimeException In case the processed query contains any joins, as removing
* aliases from the conditions can break references to the joined tables.
*/
protected function _removeAliasesFromConditions($query)
{
if (!empty($query->clause('join'))) {
trigger_error(
throw new \RuntimeException(
'Aliases are being removed from conditions for UPDATE/DELETE queries, ' .
'this can break references to joined tables.',
E_USER_NOTICE
'this can break references to joined tables.'
);
}

Expand Down
35 changes: 5 additions & 30 deletions tests/TestCase/Database/QueryTest.php
Expand Up @@ -2655,19 +2655,13 @@ public function testDeleteNoFrom()
* warning about possible incompatibilities with aliases being removed
* from the conditions.
*
*
* @expectedException \RuntimeException
* @expectedExceptionMessage Aliases are being removed from conditions for UPDATE/DELETE queries, this can break references to joined tables.
* @return void
*/
public function testDeleteRemovingAliasesCanBreakJoins()
{
$message = null;
$oldHandler = set_error_handler(function ($errno, $errstr) use (&$oldHandler, &$message) {
if ($errno === E_USER_NOTICE) {
$message = $errstr;
} else {
call_user_func_array($oldHandler, func_get_args());
}
});

$query = new Query($this->connection);

$query
Expand All @@ -2677,12 +2671,6 @@ public function testDeleteRemovingAliasesCanBreakJoins()
->where(['a.id' => 1]);

$query->sql();

restore_error_handler();

$expected = 'Aliases are being removed from conditions for UPDATE/DELETE queries, ' .
'this can break references to joined tables.';
$this->assertEquals($expected, $message);
}

/**
Expand Down Expand Up @@ -2913,19 +2901,12 @@ public function testUpdateStripAliasesFromConditions()
* warning about possible incompatibilities with aliases being removed
* from the conditions.
*
* @expectedException \RuntimeException
* @expectedExceptionMessage Aliases are being removed from conditions for UPDATE/DELETE queries, this can break references to joined tables.
* @return void
*/
public function testUpdateRemovingAliasesCanBreakJoins()
{
$message = null;
$oldHandler = set_error_handler(function ($errno, $errstr) use (&$oldHandler, &$message) {
if ($errno === E_USER_NOTICE) {
$message = $errstr;
} else {
call_user_func_array($oldHandler, func_get_args());
}
});

$query = new Query($this->connection);

$query
Expand All @@ -2935,12 +2916,6 @@ public function testUpdateRemovingAliasesCanBreakJoins()
->where(['a.id' => 1]);

$query->sql();

restore_error_handler();

$expected = 'Aliases are being removed from conditions for UPDATE/DELETE queries, ' .
'this can break references to joined tables.';
$this->assertEquals($expected, $message);
}

/**
Expand Down

0 comments on commit 2cb8533

Please sign in to comment.