Browse Source

Throw an exception instead of triggering a notice.

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.
ndm2 9 years ago
parent
commit
2cb85337a6
2 changed files with 9 additions and 33 deletions
  1. 4 3
      src/Database/SqlDialectTrait.php
  2. 5 30
      tests/TestCase/Database/QueryTest.php

+ 4 - 3
src/Database/SqlDialectTrait.php

@@ -203,14 +203,15 @@ trait SqlDialectTrait
      *
      * @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.'
             );
         }
 

+ 5 - 30
tests/TestCase/Database/QueryTest.php

@@ -2655,19 +2655,13 @@ class QueryTest extends TestCase
      * 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
@@ -2677,12 +2671,6 @@ class QueryTest extends TestCase
             ->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);
     }
 
     /**
@@ -2913,19 +2901,12 @@ class QueryTest extends TestCase
      * 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
@@ -2935,12 +2916,6 @@ class QueryTest extends TestCase
             ->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);
     }
 
     /**