Browse Source

Fixed CTE name identifier quoting.

Corey Taylor 5 years ago
parent
commit
d7411cbddb

+ 6 - 5
src/Database/Expression/CommonTableExpression.php

@@ -29,7 +29,7 @@ class CommonTableExpression implements ExpressionInterface
     /**
      * The CTE name.
      *
-     * @var string|null
+     * @var \Cake\Database\Expression\IdentifierExpression
      */
     protected $name;
 
@@ -67,9 +67,9 @@ class CommonTableExpression implements ExpressionInterface
      * @param string $name The CTE name.
      * @param \Closure|\Cake\Database\ExpressionInterface $query CTE query
      */
-    public function __construct(?string $name = null, $query = null)
+    public function __construct(string $name = '', $query = null)
     {
-        $this->name = $name;
+        $this->name = new IdentifierExpression($name);
         if ($query) {
             $this->query($query);
         }
@@ -86,7 +86,7 @@ class CommonTableExpression implements ExpressionInterface
      */
     public function name(string $name)
     {
-        $this->name = $name;
+        $this->name = new IdentifierExpression($name);
 
         return $this;
     }
@@ -194,7 +194,7 @@ class CommonTableExpression implements ExpressionInterface
 
         return sprintf(
             '%s%s AS %s(%s)',
-            (string)$this->name,
+            $this->name->sql($generator),
             $fields,
             $suffix,
             $this->query ? $this->query->sql($generator) : ''
@@ -206,6 +206,7 @@ class CommonTableExpression implements ExpressionInterface
      */
     public function traverse(Closure $visitor)
     {
+        $visitor($this->name);
         foreach ($this->fields as $field) {
             $visitor($field);
             $field->traverse($visitor);

+ 29 - 10
src/TestSuite/TestCase.php

@@ -477,33 +477,52 @@ abstract class TestCase extends BaseTestCase
     /**
      * Assert that a string matches SQL with db-specific characters like quotes removed.
      *
-     * @param string $needle The string to compare
-     * @param string $haystack The SQL to filter
+     * @param string $expected The expected sql
+     * @param string $actual The sql to compare
      * @param string $message The message to display on failure
      * @return void
      */
     public function assertEqualsSql(
-        string $needle,
-        string $haystack,
+        string $expected,
+        string $actual,
         string $message = ''
     ): void {
-        $this->assertEquals($needle, preg_replace('/[`"\[\]]/', '', $haystack), $message);
+        $this->assertEquals($expected, preg_replace('/[`"\[\]]/', '', $actual), $message);
     }
 
     /**
      * Assert that a string starts with SQL with db-specific characters like quotes removed.
      *
-     * @param string $needle The string to compare
-     * @param string $haystack The SQL to filter
+     * @param string $expected The expected sql
+     * @param string $actual The sql to compare
      * @param string $message The message to display on failure
      * @return void
      */
     public function assertStartsWithSql(
-        string $needle,
-        string $haystack,
+        string $expected,
+        string $actual,
         string $message = ''
     ): void {
-        $this->assertStringStartsWith($needle, preg_replace('/[`"\[\]]/', '', $haystack), $message);
+        $this->assertStringStartsWith($expected, preg_replace('/[`"\[\]]/', '', $actual), $message);
+    }
+
+    /**
+     * Assertion for comparing a regex pattern against a query having its identifiers
+     * quoted. It accepts queries quoted with the characters `<` and `>`. If the third
+     * parameter is set to true, it will alter the pattern to both accept quoted and
+     * unquoted queries
+     *
+     * @param string $pattern The expected sql pattern
+     * @param string $actual The sql to compare
+     * @param bool $optional Whether quote characters (marked with <>) are optional
+     * @return void
+     */
+    public function assertRegExpSql(string $pattern, string $actual, bool $optional = false): void
+    {
+        $optional = $optional ? '?' : '';
+        $pattern = str_replace('<', '[`"\[]' . $optional, $pattern);
+        $pattern = str_replace('>', '[`"\]]' . $optional, $pattern);
+        $this->assertRegExp('#' . $pattern . '#', $actual);
     }
 
     /**

+ 4 - 4
tests/TestCase/Database/Expression/CommonTableExpressionTest.php

@@ -123,16 +123,16 @@ class CommonTableExpressionTest extends TestCase
     public function testTraverse()
     {
         $query = $this->connection->newQuery()->select('1');
-        $field = new IdentifierExpression('field');
-        $cte = (new CommonTableExpression('test', $query))->field($field);
+        $cte = (new CommonTableExpression('test', $query))->field('field');
 
         $expressions = [];
         $cte->traverse(function ($expression) use (&$expressions) {
             $expressions[] = $expression;
         });
 
-        $this->assertEquals($field, $expressions[0]);
-        $this->assertEquals($query, $expressions[1]);
+        $this->assertEquals(new IdentifierExpression('test'), $expressions[0]);
+        $this->assertEquals(new IdentifierExpression('field'), $expressions[1]);
+        $this->assertEquals($query, $expressions[2]);
     }
 
     /**

+ 5 - 3
tests/TestCase/Database/QueryTests/CommonTableExpressionQueryTests.php

@@ -54,6 +54,7 @@ class CommonTableExpressionQueryTests extends TestCase
     {
         parent::setUp();
         $this->connection = ConnectionManager::get('test');
+        $this->autoQuote = $this->connection->getDriver()->isAutoQuotingEnabled();
 
         $this->skipIf(
             !$this->connection->getDriver()->supportsCTEs(),
@@ -81,9 +82,10 @@ class CommonTableExpressionQueryTests extends TestCase
             ->select('col')
             ->from('cte');
 
-        $this->assertEqualsSql(
-            'WITH cte AS (SELECT 1 AS col) SELECT col FROM cte',
-            $query->sql(new ValueBinder())
+        $this->assertRegExpSql(
+            'WITH <cte> AS \(SELECT 1 AS <col>\) SELECT <col> FROM <cte>',
+            $query->sql(new ValueBinder()),
+            !$this->autoQuote
         );
 
         $expected = [