Browse Source

Add DeleteQuery class.

Split code specific to DELETE query generation into a separate class.
ADmad 3 years ago
parent
commit
b0656fc466

+ 12 - 1
src/Database/Connection.php

@@ -24,6 +24,7 @@ use Cake\Database\Exception\MissingConnectionException;
 use Cake\Database\Exception\MissingDriverException;
 use Cake\Database\Exception\MissingExtensionException;
 use Cake\Database\Exception\NestedTransactionRollbackException;
+use Cake\Database\Query\DeleteQuery;
 use Cake\Database\Query\InsertQuery;
 use Cake\Database\Query\SelectQuery;
 use Cake\Database\Query\UpdateQuery;
@@ -318,6 +319,16 @@ class Connection implements ConnectionInterface
     }
 
     /**
+     * Create a new DeleteQuery instance for this connection.
+     *
+     * @return \Cake\Database\Query\DeleteQuery
+     */
+    public function newDeleteQuery(): DeleteQuery
+    {
+        return new DeleteQuery($this);
+    }
+
+    /**
      * Sets a Schema\Collection object for this connection.
      *
      * @param \Cake\Database\Schema\CollectionInterface $collection The schema collection object
@@ -397,7 +408,7 @@ class Connection implements ConnectionInterface
      */
     public function delete(string $table, array $conditions = [], array $types = []): StatementInterface
     {
-        return $this->newQuery()->delete($table)
+        return $this->newDeleteQuery()->delete($table)
             ->where($conditions, $types)
             ->execute();
     }

+ 6 - 1
src/Database/IdentifierQuoter.php

@@ -19,6 +19,7 @@ namespace Cake\Database;
 use Cake\Database\Expression\FieldInterface;
 use Cake\Database\Expression\IdentifierExpression;
 use Cake\Database\Expression\OrderByExpression;
+use InvalidArgumentException;
 
 /**
  * Contains all the logic related to quoting identifiers in a Query object
@@ -106,7 +107,11 @@ class IdentifierQuoter
     protected function _quoteParts(Query $query): void
     {
         foreach (['distinct', 'select', 'from', 'group'] as $part) {
-            $contents = $query->clause($part);
+            try {
+                $contents = $query->clause($part);
+            } catch (InvalidArgumentException) {
+                continue;
+            }
 
             if (!is_array($contents)) {
                 continue;

+ 0 - 20
src/Database/Query.php

@@ -1311,26 +1311,6 @@ class Query implements ExpressionInterface, Stringable
     }
 
     /**
-     * Create a delete query.
-     *
-     * Can be combined with from(), where() and other methods to
-     * create delete queries with specific conditions.
-     *
-     * @param string|null $table The table to use when deleting.
-     * @return $this
-     */
-    public function delete(?string $table = null)
-    {
-        $this->_dirty();
-        $this->_type = self::TYPE_DELETE;
-        if ($table !== null) {
-            $this->from($table);
-        }
-
-        return $this;
-    }
-
-    /**
      * A string or expression that will be appended to the generated query
      *
      * ### Examples:

+ 62 - 0
src/Database/Query/DeleteQuery.php

@@ -0,0 +1,62 @@
+<?php
+declare(strict_types=1);
+
+/**
+ * CakePHP(tm) : Rapid Development Framework (https://cakephp.org)
+ * Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
+ *
+ * Licensed under The MIT License
+ * For full copyright and license information, please see the LICENSE.txt
+ * Redistributions of files must retain the above copyright notice.
+ *
+ * @copyright     Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
+ * @link          https://cakephp.org CakePHP(tm) Project
+ * @since         5.0.0
+ * @license       https://opensource.org/licenses/mit-license.php MIT License
+ */
+namespace Cake\Database\Query;
+
+use Cake\Database\Query;
+
+/**
+ * This class is used to generate DELETE queries for the relational database.
+ */
+class DeleteQuery extends Query
+{
+    /**
+     * List of SQL parts that will be used to build this query.
+     *
+     * @var array<string, mixed>
+     */
+    protected array $_parts = [
+        'with' => [],
+        'delete' => true,
+        'modifier' => [],
+        'from' => [],
+        'join' => [],
+        'where' => null,
+        'order' => null,
+        'limit' => null,
+        'epilog' => null,
+    ];
+
+    /**
+     * Create a delete query.
+     *
+     * Can be combined with from(), where() and other methods to
+     * create delete queries with specific conditions.
+     *
+     * @param string|null $table The table to use when deleting.
+     * @return $this
+     */
+    public function delete(?string $table = null)
+    {
+        $this->_dirty();
+        $this->_type = self::TYPE_DELETE;
+        if ($table !== null) {
+            $this->from($table);
+        }
+
+        return $this;
+    }
+}

+ 233 - 0
tests/TestCase/Database/Query/DeleteQueryTest.php

@@ -0,0 +1,233 @@
+<?php
+declare(strict_types=1);
+
+/**
+ * CakePHP(tm) : Rapid Development Framework (https://cakephp.org)
+ * Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
+ *
+ * Licensed under The MIT License
+ * For full copyright and license information, please see the LICENSE.txt
+ * Redistributions of files must retain the above copyright notice.
+ *
+ * @copyright     Copyright (c) Cake Software Foundation, Inc. (https://cakefoundation.org)
+ * @link          https://cakephp.org CakePHP(tm) Project
+ * @since         5.0.0
+ * @license       https://opensource.org/licenses/mit-license.php MIT License
+ */
+namespace Cake\Test\TestCase\Database\Query;
+
+use Cake\Database\Exception\DatabaseException;
+use Cake\Database\Expression\IdentifierExpression;
+use Cake\Database\Query\DeleteQuery;
+use Cake\Database\Query\SelectQuery;
+use Cake\Database\StatementInterface;
+use Cake\Datasource\ConnectionManager;
+use Cake\Test\TestCase\Database\QueryAssertsTrait;
+use Cake\TestSuite\TestCase;
+
+/**
+ * Tests DeleteQuery class
+ */
+class DeleteQueryTest extends TestCase
+{
+    use QueryAssertsTrait;
+
+    /**
+     * @var int
+     */
+    public const AUTHOR_COUNT = 4;
+
+    protected array $fixtures = [
+        'core.Articles',
+        'core.Authors',
+    ];
+
+    /**
+     * @var \Cake\Database\Connection
+     */
+    protected $connection;
+
+    /**
+     * @var bool
+     */
+    protected $autoQuote;
+
+    public function setUp(): void
+    {
+        parent::setUp();
+        $this->connection = ConnectionManager::get('test');
+        $this->autoQuote = $this->connection->getDriver()->isAutoQuotingEnabled();
+    }
+
+    public function tearDown(): void
+    {
+        parent::tearDown();
+        $this->connection->getDriver()->enableAutoQuoting($this->autoQuote);
+        unset($this->connection);
+    }
+
+    /**
+     * Test a basic delete using from()
+     */
+    public function testDeleteWithFrom(): void
+    {
+        $query = new DeleteQuery($this->connection);
+
+        $query->delete()
+            ->from('authors')
+            ->where('1 = 1');
+
+        $result = $query->sql();
+        $this->assertQuotedQuery('DELETE FROM <authors>', $result, !$this->autoQuote);
+
+        $result = $query->execute();
+        $this->assertInstanceOf(StatementInterface::class, $result);
+        $this->assertSame(self::AUTHOR_COUNT, $result->rowCount());
+        $result->closeCursor();
+    }
+
+    /**
+     * Test delete with from and alias.
+     */
+    public function testDeleteWithAliasedFrom(): void
+    {
+        $query = new DeleteQuery($this->connection);
+
+        $query->delete()
+            ->from(['a ' => 'authors'])
+            ->where(['a.id !=' => 99]);
+
+        $result = $query->sql();
+        $this->assertQuotedQuery('DELETE FROM <authors> WHERE <id> != :c0', $result, !$this->autoQuote);
+
+        $result = $query->execute();
+        $this->assertInstanceOf(StatementInterface::class, $result);
+        $this->assertSame(self::AUTHOR_COUNT, $result->rowCount());
+        $result->closeCursor();
+    }
+
+    /**
+     * Test a basic delete with no from() call.
+     */
+    public function testDeleteNoFrom(): void
+    {
+        $query = new DeleteQuery($this->connection);
+
+        $query->delete('authors')
+            ->where('1 = 1');
+
+        $result = $query->sql();
+        $this->assertQuotedQuery('DELETE FROM <authors>', $result, !$this->autoQuote);
+
+        $result = $query->execute();
+        $this->assertInstanceOf(StatementInterface::class, $result);
+        $this->assertSame(self::AUTHOR_COUNT, $result->rowCount());
+        $result->closeCursor();
+    }
+
+    /**
+     * Tests that delete queries that contain joins do trigger a notice,
+     * warning about possible incompatibilities with aliases being removed
+     * from the conditions.
+     */
+    public function testDeleteRemovingAliasesCanBreakJoins(): void
+    {
+        $this->expectException(DatabaseException::class);
+        $this->expectExceptionMessage('Aliases are being removed from conditions for UPDATE/DELETE queries, this can break references to joined tables.');
+        $query = new DeleteQuery($this->connection);
+
+        $query
+            ->delete('authors')
+            ->from(['a ' => 'authors'])
+            ->innerJoin('articles')
+            ->where(['a.id' => 1]);
+
+        $query->sql();
+    }
+
+    /**
+     * Tests that aliases are stripped from delete query conditions
+     * where possible.
+     */
+    public function testDeleteStripAliasesFromConditions(): void
+    {
+        $query = new DeleteQuery($this->connection);
+
+        $query
+            ->delete()
+            ->from(['a' => 'authors'])
+            ->where([
+                'OR' => [
+                    'a.id' => 1,
+                    'a.name IS' => null,
+                    'a.email IS NOT' => null,
+                    'AND' => [
+                        'b.name NOT IN' => ['foo', 'bar'],
+                        'OR' => [
+                            $query->newExpr()->eq(new IdentifierExpression('c.name'), 'zap'),
+                            'd.name' => 'baz',
+                            (new SelectQuery($this->connection))->select(['e.name'])->where(['e.name' => 'oof']),
+                        ],
+                    ],
+                ],
+            ]);
+
+        $this->assertQuotedQuery(
+            'DELETE FROM <authors> WHERE \(' .
+                '<id> = :c0 OR \(<name>\) IS NULL OR \(<email>\) IS NOT NULL OR \(' .
+                    '<name> NOT IN \(:c1,:c2\) AND \(' .
+                        '<name> = :c3 OR <name> = :c4 OR \(SELECT <e>\.<name> WHERE <e>\.<name> = :c5\)' .
+                    '\)' .
+                '\)' .
+            '\)',
+            $query->sql(),
+            !$this->autoQuote
+        );
+    }
+
+    /**
+     * Test that epilog() will actually append a string to a delete query
+     */
+    public function testAppendDelete(): void
+    {
+        $query = new DeleteQuery($this->connection);
+        $sql = $query
+            ->delete('articles')
+            ->where(['id' => 1])
+            ->epilog('RETURNING id')
+            ->sql();
+        $this->assertStringContainsString('DELETE FROM', $sql);
+        $this->assertStringContainsString('WHERE', $sql);
+        $this->assertSame(' RETURNING id', substr($sql, -13));
+    }
+
+    /**
+     * Test use of modifiers in a DELETE query
+     *
+     * Testing the generated SQL since the modifiers are usually different per driver
+     */
+    public function testDeleteModifiers(): void
+    {
+        $query = new DeleteQuery($this->connection);
+        $result = $query->delete()
+            ->from('authors')
+            ->where('1 = 1')
+            ->modifier('IGNORE');
+        $this->assertQuotedQuery(
+            'DELETE IGNORE FROM <authors> WHERE 1 = 1',
+            $result->sql(),
+            !$this->autoQuote
+        );
+
+        $query = new DeleteQuery($this->connection);
+        $result = $query->delete()
+            ->from('authors')
+            ->where('1 = 1')
+            ->modifier(['IGNORE', 'QUICK']);
+        $this->assertQuotedQuery(
+            'DELETE IGNORE QUICK FROM <authors> WHERE 1 = 1',
+            $result->sql(),
+            !$this->autoQuote
+        );
+    }
+}

+ 0 - 181
tests/TestCase/Database/QueryTest.php

@@ -16,13 +16,10 @@ declare(strict_types=1);
  */
 namespace Cake\Test\TestCase\Database;
 
-use Cake\Database\Exception\DatabaseException;
 use Cake\Database\Expression\CommonTableExpression;
 use Cake\Database\Expression\IdentifierExpression;
 use Cake\Database\ExpressionInterface;
 use Cake\Database\Query;
-use Cake\Database\Query\SelectQuery;
-use Cake\Database\StatementInterface;
 use Cake\Datasource\ConnectionManager;
 use Cake\TestSuite\TestCase;
 use InvalidArgumentException;
@@ -43,19 +40,6 @@ class QueryTest extends TestCase
     ];
 
     /**
-     * @var int
-     */
-    public const ARTICLE_COUNT = 3;
-    /**
-     * @var int
-     */
-    public const AUTHOR_COUNT = 4;
-    /**
-     * @var int
-     */
-    public const COMMENT_COUNT = 6;
-
-    /**
      * @var \Cake\Database\Connection
      */
     protected $connection;
@@ -95,125 +79,6 @@ class QueryTest extends TestCase
     }
 
     /**
-     * Test a basic delete using from()
-     */
-    public function testDeleteWithFrom(): void
-    {
-        $query = new Query($this->connection);
-
-        $query->delete()
-            ->from('authors')
-            ->where('1 = 1');
-
-        $result = $query->sql();
-        $this->assertQuotedQuery('DELETE FROM <authors>', $result, !$this->autoQuote);
-
-        $result = $query->execute();
-        $this->assertInstanceOf(StatementInterface::class, $result);
-        $this->assertSame(self::AUTHOR_COUNT, $result->rowCount());
-        $result->closeCursor();
-    }
-
-    /**
-     * Test delete with from and alias.
-     */
-    public function testDeleteWithAliasedFrom(): void
-    {
-        $query = new Query($this->connection);
-
-        $query->delete()
-            ->from(['a ' => 'authors'])
-            ->where(['a.id !=' => 99]);
-
-        $result = $query->sql();
-        $this->assertQuotedQuery('DELETE FROM <authors> WHERE <id> != :c0', $result, !$this->autoQuote);
-
-        $result = $query->execute();
-        $this->assertInstanceOf(StatementInterface::class, $result);
-        $this->assertSame(self::AUTHOR_COUNT, $result->rowCount());
-        $result->closeCursor();
-    }
-
-    /**
-     * Test a basic delete with no from() call.
-     */
-    public function testDeleteNoFrom(): void
-    {
-        $query = new Query($this->connection);
-
-        $query->delete('authors')
-            ->where('1 = 1');
-
-        $result = $query->sql();
-        $this->assertQuotedQuery('DELETE FROM <authors>', $result, !$this->autoQuote);
-
-        $result = $query->execute();
-        $this->assertInstanceOf(StatementInterface::class, $result);
-        $this->assertSame(self::AUTHOR_COUNT, $result->rowCount());
-        $result->closeCursor();
-    }
-
-    /**
-     * Tests that delete queries that contain joins do trigger a notice,
-     * warning about possible incompatibilities with aliases being removed
-     * from the conditions.
-     */
-    public function testDeleteRemovingAliasesCanBreakJoins(): void
-    {
-        $this->expectException(DatabaseException::class);
-        $this->expectExceptionMessage('Aliases are being removed from conditions for UPDATE/DELETE queries, this can break references to joined tables.');
-        $query = new Query($this->connection);
-
-        $query
-            ->delete('authors')
-            ->from(['a ' => 'authors'])
-            ->innerJoin('articles')
-            ->where(['a.id' => 1]);
-
-        $query->sql();
-    }
-
-    /**
-     * Tests that aliases are stripped from delete query conditions
-     * where possible.
-     */
-    public function testDeleteStripAliasesFromConditions(): void
-    {
-        $query = new Query($this->connection);
-
-        $query
-            ->delete()
-            ->from(['a' => 'authors'])
-            ->where([
-                'OR' => [
-                    'a.id' => 1,
-                    'a.name IS' => null,
-                    'a.email IS NOT' => null,
-                    'AND' => [
-                        'b.name NOT IN' => ['foo', 'bar'],
-                        'OR' => [
-                            $query->newExpr()->eq(new IdentifierExpression('c.name'), 'zap'),
-                            'd.name' => 'baz',
-                            (new SelectQuery($this->connection))->select(['e.name'])->where(['e.name' => 'oof']),
-                        ],
-                    ],
-                ],
-            ]);
-
-        $this->assertQuotedQuery(
-            'DELETE FROM <authors> WHERE \(' .
-                '<id> = :c0 OR \(<name>\) IS NULL OR \(<email>\) IS NOT NULL OR \(' .
-                    '<name> NOT IN \(:c1,:c2\) AND \(' .
-                        '<name> = :c3 OR <name> = :c4 OR \(SELECT <e>\.<name> WHERE <e>\.<name> = :c5\)' .
-                    '\)' .
-                '\)' .
-            '\)',
-            $query->sql(),
-            !$this->autoQuote
-        );
-    }
-
-    /**
      * Tests that the identifier method creates an expression object.
      */
     public function testIdentifierExpression(): void
@@ -242,22 +107,6 @@ class QueryTest extends TestCase
     }
 
     /**
-     * Test that epilog() will actually append a string to a delete query
-     */
-    public function testAppendDelete(): void
-    {
-        $query = new Query($this->connection);
-        $sql = $query
-            ->delete('articles')
-            ->where(['id' => 1])
-            ->epilog('RETURNING id')
-            ->sql();
-        $this->assertStringContainsString('DELETE FROM', $sql);
-        $this->assertStringContainsString('WHERE', $sql);
-        $this->assertSame(' RETURNING id', substr($sql, -13));
-    }
-
-    /**
      * Tests __debugInfo on incomplete query
      */
     public function testDebugInfoIncompleteQuery(): void
@@ -435,36 +284,6 @@ class QueryTest extends TestCase
     }
 
     /**
-     * Test use of modifiers in a DELETE query
-     *
-     * Testing the generated SQL since the modifiers are usually different per driver
-     */
-    public function testDeleteModifiers(): void
-    {
-        $query = new Query($this->connection);
-        $result = $query->delete()
-            ->from('authors')
-            ->where('1 = 1')
-            ->modifier('IGNORE');
-        $this->assertQuotedQuery(
-            'DELETE IGNORE FROM <authors> WHERE 1 = 1',
-            $result->sql(),
-            !$this->autoQuote
-        );
-
-        $query = new Query($this->connection);
-        $result = $query->delete()
-            ->from('authors')
-            ->where('1 = 1')
-            ->modifier(['IGNORE', 'QUICK']);
-        $this->assertQuotedQuery(
-            'DELETE IGNORE QUICK FROM <authors> WHERE 1 = 1',
-            $result->sql(),
-            !$this->autoQuote
-        );
-    }
-
-    /**
      * Test getValueBinder()
      */
     public function testGetValueBinder(): void

+ 1 - 1
tests/TestCase/Database/QueryTests/CommonTableExpressionQueryTest.php

@@ -384,7 +384,7 @@ class CommonTableExpressionQueryTest extends TestCase
         $this->assertEquals(['count' => '3'], $result->fetch('assoc'));
         $result->closeCursor();
 
-        $query = $this->connection->newQuery()
+        $query = $this->connection->newDeleteQuery()
             ->with(function (CommonTableExpression $cte, SelectQuery $query) {
                 $query->select('articles.id')
                     ->from('articles')