Browse Source

Move query execution and logging to the driver.

ADmad 4 years ago
parent
commit
1e0b03ae59

+ 7 - 32
src/Database/Connection.php

@@ -270,22 +270,6 @@ class Connection implements ConnectionInterface
     }
 
     /**
-     * Prepares a SQL statement to be executed.
-     *
-     * @param \Cake\Database\Query|string $query The SQL to convert into a prepared statement.
-     * @return \Cake\Database\StatementInterface
-     */
-    protected function prepare(Query|string $query): StatementInterface
-    {
-        $statement = $this->_driver->prepare($query);
-        if ($this->_logQueries) {
-            $statement->setLogger($this->getLogger());
-        }
-
-        return $statement;
-    }
-
-    /**
      * Executes a query using $params for interpolating values and $types as a hint for each
      * those params.
      *
@@ -296,15 +280,7 @@ class Connection implements ConnectionInterface
      */
     public function execute(string $sql, array $params = [], array $types = []): StatementInterface
     {
-        return $this->getDisconnectRetry()->run(function () use ($sql, $params, $types) {
-            $statement = $this->prepare($sql);
-            if (!empty($params)) {
-                $statement->bind($params, $types);
-            }
-            $statement->execute();
-
-            return $statement;
-        });
+        return $this->getDisconnectRetry()->run(fn () => $this->_driver->execute($sql, $params, $types));
     }
 
     /**
@@ -316,13 +292,7 @@ class Connection implements ConnectionInterface
      */
     public function run(Query $query): StatementInterface
     {
-        return $this->getDisconnectRetry()->run(function () use ($query) {
-            $statement = $this->prepare($query);
-            $query->getValueBinder()->attachTo($statement);
-            $statement->execute();
-
-            return $statement;
-        });
+        return $this->getDisconnectRetry()->run(fn () => $this->_driver->run($query));
     }
 
     /**
@@ -803,6 +773,7 @@ class Connection implements ConnectionInterface
     public function enableQueryLogging(bool $enable = true)
     {
         $this->_logQueries = $enable;
+        $this->_driver->setLogger($this->getLogger());
 
         return $this;
     }
@@ -815,6 +786,7 @@ class Connection implements ConnectionInterface
     public function disableQueryLogging()
     {
         $this->_logQueries = false;
+        $this->_driver->setLogger(null);
 
         return $this;
     }
@@ -838,6 +810,9 @@ class Connection implements ConnectionInterface
     public function setLogger(LoggerInterface $logger): void
     {
         $this->_logger = $logger;
+        if ($this->_logQueries) {
+            $this->_driver->setLogger($this->_logger);
+        }
     }
 
     /**

+ 84 - 0
src/Database/Driver.php

@@ -19,6 +19,7 @@ namespace Cake\Database;
 use Cake\Core\App;
 use Cake\Core\Retry\CommandRetry;
 use Cake\Database\Exception\MissingConnectionException;
+use Cake\Database\Log\LoggedQuery;
 use Cake\Database\Retry\ErrorCodeWaitStrategy;
 use Cake\Database\Schema\SchemaDialect;
 use Cake\Database\Schema\TableSchema;
@@ -28,6 +29,7 @@ use Closure;
 use InvalidArgumentException;
 use PDO;
 use PDOException;
+use Psr\Log\LoggerInterface;
 
 /**
  * Represents a database driver containing all specificities for
@@ -102,6 +104,13 @@ abstract class Driver implements DriverInterface
     protected SchemaDialect $_schemaDialect;
 
     /**
+     * Logger instance.
+     *
+     * @var \Psr\Log\LoggerInterface|null
+     */
+    protected ?LoggerInterface $logger = null;
+
+    /**
      * Constructor
      *
      * @param array<string, mixed> $config The configuration for the driver.
@@ -218,6 +227,73 @@ abstract class Driver implements DriverInterface
     /**
      * @inheritDoc
      */
+    public function execute(string $sql, array $params = [], array $types = []): StatementInterface
+    {
+        $statement = $this->prepare($sql);
+        if (!empty($params)) {
+            $statement->bind($params, $types);
+        }
+        $this->executeStatement($statement);
+
+        return $statement;
+    }
+
+    /**
+     * @inheritDoc
+     */
+    public function run(Query $query): StatementInterface
+    {
+        $statement = $this->prepare($query);
+        $query->getValueBinder()->attachTo($statement);
+        $this->executeStatement($statement);
+
+        return $statement;
+    }
+
+    /**
+     * Execute the statement and log the query string.
+     *
+     * @param \Cake\Database\StatementInterface $statement Statement to execute.
+     * @return void
+     */
+    protected function executeStatement(StatementInterface $statement): void
+    {
+        if ($this->logger === null) {
+            $statement->execute();
+
+            return;
+        }
+
+        $exception = null;
+        $took = 0.0;
+
+        try {
+            $start = microtime(true);
+            $statement->execute();
+            $took = (float)number_format((microtime(true) - $start) * 1000, 1);
+        } catch (PDOException $e) {
+            $exception = $e;
+        }
+
+        $loggedQuery = new LoggedQuery();
+        $loggedQuery->driver = $this;
+        $loggedQuery->query = $statement->queryString();
+        $loggedQuery->params = $statement->getBoundParams();
+        $loggedQuery->error = $exception;
+        if (!$exception) {
+            $loggedQuery->numRows = $statement->rowCount();
+            $loggedQuery->took = $took;
+        }
+        $this->logger->debug((string)$loggedQuery, ['query' => $loggedQuery]);
+
+        if ($exception) {
+            throw $exception;
+        }
+    }
+
+    /**
+     * @inheritDoc
+     */
     public function prepare(Query|string $query): StatementInterface
     {
         $statement = $this->getPdo()->prepare($query instanceof Query ? $query->sql() : $query);
@@ -461,6 +537,14 @@ abstract class Driver implements DriverInterface
     }
 
     /**
+     * @inheritDoc
+     */
+    public function setLogger(?LoggerInterface $logger): void
+    {
+        $this->logger = $logger;
+    }
+
+    /**
      * Destructor
      */
     public function __destruct()

+ 29 - 0
src/Database/DriverInterface.php

@@ -19,6 +19,7 @@ namespace Cake\Database;
 use Cake\Database\Schema\SchemaDialect;
 use Cake\Database\Schema\TableSchemaInterface;
 use Closure;
+use Psr\Log\LoggerInterface;
 
 /**
  * Interface for database driver.
@@ -105,6 +106,26 @@ interface DriverInterface
     public function prepare(Query|string $query): StatementInterface;
 
     /**
+     * Executes a query using $params for interpolating values and $types as a hint for each
+     * those params.
+     *
+     * @param string $sql SQL to be executed and interpolated with $params
+     * @param array $params List or associative array of params to be interpolated in $sql as values.
+     * @param array $types List or associative array of types to be used for casting values in query.
+     * @return \Cake\Database\StatementInterface Executed statement
+     */
+    public function execute(string $sql, array $params = [], array $types = []): StatementInterface;
+
+    /**
+     * Executes the provided query after compiling it for the specific driver
+     * dialect and returns the executed Statement object.
+     *
+     * @param \Cake\Database\Query $query The query to be executed.
+     * @return \Cake\Database\StatementInterface Executed statement
+     */
+    public function run(Query $query): StatementInterface;
+
+    /**
      * Starts a transaction.
      *
      * @return bool True on success, false otherwise.
@@ -302,4 +323,12 @@ interface DriverInterface
      * @return int|null
      */
     public function getMaxAliasLength(): ?int;
+
+    /**
+     * Set logger instance.
+     *
+     * @param \Psr\Log\LoggerInterface|null $logger Logger instance.
+     * @return void
+     */
+    public function setLogger(?LoggerInterface $logger): void;
 }

+ 9 - 53
src/Database/Statement/Statement.php

@@ -18,15 +18,12 @@ namespace Cake\Database\Statement;
 
 use Cake\Database\DriverInterface;
 use Cake\Database\FieldTypeConverter;
-use Cake\Database\Log\LoggedQuery;
 use Cake\Database\StatementInterface;
 use Cake\Database\TypeConverterTrait;
 use Cake\Database\TypeMap;
 use InvalidArgumentException;
 use PDO;
-use PDOException;
 use PDOStatement;
-use Psr\Log\LoggerInterface;
 
 class Statement implements StatementInterface
 {
@@ -60,11 +57,6 @@ class Statement implements StatementInterface
     protected ?FieldTypeConverter $typeConverter;
 
     /**
-     * @var \Psr\Log\LoggerInterface|null
-     */
-    protected ?LoggerInterface $logger = null;
-
-    /**
      * Cached bound parameters used for logging
      *
      * @var array<mixed>
@@ -72,11 +64,6 @@ class Statement implements StatementInterface
     protected array $params = [];
 
     /**
-     * @var float
-     */
-    protected float $took = 0.0;
-
-    /**
      * @param \PDOStatement $statement PDO statement
      * @param \Cake\Database\DriverInterface $driver Database driver
      * @param \Cake\Database\TypeMap|null $typeMap Results type map
@@ -94,16 +81,6 @@ class Statement implements StatementInterface
     /**
      * @inheritDoc
      */
-    public function setLogger(LoggerInterface $logger)
-    {
-        $this->logger = $logger;
-
-        return $this;
-    }
-
-    /**
-     * @inheritDoc
-     */
     public function bind(array $params, array $types): void
     {
         if (empty($params)) {
@@ -142,6 +119,14 @@ class Statement implements StatementInterface
     /**
      * @inheritDoc
      */
+    public function getBoundParams(): array
+    {
+        return $this->params;
+    }
+
+    /**
+     * @inheritDoc
+     */
     protected function performBind(string|int $column, mixed $value, int $type): void
     {
         $this->statement->bindValue($column, $value, $type);
@@ -152,36 +137,7 @@ class Statement implements StatementInterface
      */
     public function execute(?array $params = null): bool
     {
-        $success = false;
-        $exception = null;
-
-        try {
-            $start = microtime(true);
-            $success = $this->statement->execute($params);
-            $this->took = (microtime(true) - $start) * 1000;
-        } catch (PDOException $e) {
-            $exception = $e;
-            $this->took = 0.0;
-        }
-
-        if ($this->logger) {
-            $loggedQuery = new LoggedQuery();
-            $loggedQuery->driver = $this->_driver;
-            $loggedQuery->query = $this->queryString();
-            $loggedQuery->params = $params ?? $this->params;
-            $loggedQuery->error = $exception;
-            if (!$exception) {
-                $loggedQuery->numRows = $this->rowCount();
-                $loggedQuery->took = (int)round($this->took);
-            }
-            $this->logger->debug((string)$loggedQuery, ['query' => $loggedQuery]);
-        }
-
-        if ($exception) {
-            throw $exception;
-        }
-
-        return $success;
+        return $this->statement->execute($params);
     }
 
     /**

+ 10 - 5
src/Database/StatementInterface.php

@@ -17,7 +17,6 @@ declare(strict_types=1);
 namespace Cake\Database;
 
 use PDO;
-use Psr\Log\LoggerInterface;
 
 interface StatementInterface
 {
@@ -197,10 +196,16 @@ interface StatementInterface
     public function lastInsertId(?string $table = null, ?string $column = null): string|int;
 
     /**
-     * Sets query logger to use when calling execute().
+     * Returns prepared query string.
      *
-     * @param \Psr\Log\LoggerInterface $logger Query logger
-     * @return $this
+     * @return string
+     */
+    public function queryString(): string;
+
+    /**
+     * Get the bound params.
+     *
+     * @return array
      */
-    public function setLogger(LoggerInterface $logger);
+    public function getBoundParams(): array;
 }

+ 2 - 2
tests/TestCase/Database/ConnectionTest.php

@@ -1180,7 +1180,7 @@ class ConnectionTest extends TestCase
         $prop->setValue($conn, $newDriver);
 
         $newDriver->expects($this->exactly(2))
-            ->method('prepare')
+            ->method('execute')
             ->will($this->onConsecutiveCalls(
                 $this->throwException(new Exception('server gone away')),
                 $this->returnValue($statement)
@@ -1210,7 +1210,7 @@ class ConnectionTest extends TestCase
         $prop->setValue($conn, $newDriver);
 
         $newDriver->expects($this->once())
-            ->method('prepare')
+            ->method('execute')
             ->will($this->throwException(new Exception('server gone away')));
 
         try {

+ 108 - 1
tests/TestCase/Database/DriverTest.php

@@ -20,14 +20,19 @@ use Cake\Database\Driver;
 use Cake\Database\Driver\Sqlserver;
 use Cake\Database\DriverInterface;
 use Cake\Database\Exception\MissingConnectionException;
+use Cake\Database\Log\QueryLogger;
 use Cake\Database\Query;
 use Cake\Database\QueryCompiler;
 use Cake\Database\Schema\TableSchema;
+use Cake\Database\Statement\Statement;
 use Cake\Database\ValueBinder;
 use Cake\Datasource\ConnectionManager;
+use Cake\Log\Log;
 use Cake\TestSuite\TestCase;
+use DateTime;
 use Exception;
 use PDO;
+use PDOException;
 use PDOStatement;
 use TestApp\Database\Driver\RetryDriver;
 use TestApp\Database\Driver\StubDriver;
@@ -49,6 +54,11 @@ class DriverTest extends TestCase
     {
         parent::setUp();
 
+        Log::setConfig('queries', [
+            'className' => 'Array',
+            'scopes' => ['queriesLog'],
+        ]);
+
         $this->driver = $this->getMockForAbstractClass(
             StubDriver::class,
             [],
@@ -56,10 +66,16 @@ class DriverTest extends TestCase
             true,
             true,
             true,
-            ['createPdo']
+            ['createPdo', 'prepare']
         );
     }
 
+    public function tearDown(): void
+    {
+        parent::tearDown();
+        Log::drop('queries');
+    }
+
     /**
      * Test if building the object throws an exception if we're not passing
      * required config data.
@@ -310,4 +326,95 @@ class DriverTest extends TestCase
             ['42', '42'],
         ];
     }
+
+    /**
+     * Tests that queries are logged when executed without params
+     */
+    public function testExecuteNoParams(): void
+    {
+        $inner = $this->getMockBuilder(PDOStatement::class)->getMock();
+
+        $statement = $this->getMockBuilder(Statement::class)
+            ->setConstructorArgs([$inner, $this->driver])
+            ->onlyMethods(['queryString','rowCount','execute'])
+            ->getMock();
+        $statement->expects($this->any())->method('queryString')->will($this->returnValue('SELECT bar FROM foo'));
+        $statement->method('rowCount')->will($this->returnValue(3));
+        $statement->method('execute')->will($this->returnValue(true));
+
+        $this->driver->expects($this->any())
+            ->method('prepare')
+            ->willReturn($statement);
+        $this->driver->setLogger(new QueryLogger(['connection' => 'test']));
+
+        $this->driver->execute('SELECT bar FROM foo');
+
+        $messages = Log::engine('queries')->read();
+        $this->assertCount(1, $messages);
+        $this->assertMatchesRegularExpression('/^debug: connection=test duration=[\d\.]+ rows=3 SELECT bar FROM foo$/', $messages[0]);
+    }
+
+    /**
+     * Tests that queries are logged when executed with bound params
+     */
+    public function testExecuteWithBinding(): void
+    {
+        $inner = $this->getMockBuilder(PDOStatement::class)->getMock();
+
+        $statement = $this->getMockBuilder(Statement::class)
+            ->setConstructorArgs([$inner, $this->driver])
+            ->onlyMethods(['queryString','rowCount','execute'])
+            ->getMock();
+        $statement->method('rowCount')->will($this->returnValue(3));
+        $statement->method('execute')->will($this->returnValue(true));
+        $statement->expects($this->any())->method('queryString')->will($this->returnValue('SELECT bar FROM foo WHERE a=:a AND b=:b'));
+
+        $this->driver->setLogger(new QueryLogger(['connection' => 'test']));
+        $this->driver->expects($this->any())
+            ->method('prepare')
+            ->willReturn($statement);
+
+        $this->driver->execute(
+            'SELECT bar FROM foo WHERE a=:a AND b=:b',
+            [
+                'a' => 1,
+                'b' => new DateTime('2013-01-01'),
+            ],
+            ['b' => 'date']
+        );
+
+        $messages = Log::engine('queries')->read();
+        $this->assertCount(1, $messages);
+        $this->assertMatchesRegularExpression("/^debug: connection=test duration=\d+ rows=3 SELECT bar FROM foo WHERE a='1' AND b='2013-01-01'$/", $messages[0]);
+    }
+
+    /**
+     * Tests that queries are logged despite database errors
+     */
+    public function testExecuteWithError(): void
+    {
+        $inner = $this->getMockBuilder(PDOStatement::class)->getMock();
+
+        $statement = $this->getMockBuilder(Statement::class)
+            ->setConstructorArgs([$inner, $this->driver])
+            ->onlyMethods(['queryString','rowCount','execute'])
+            ->getMock();
+        $statement->expects($this->any())->method('queryString')->will($this->returnValue('SELECT bar FROM foo'));
+        $statement->method('rowCount')->will($this->returnValue(0));
+        $statement->method('execute')->will($this->throwException(new PDOException()));
+
+        $this->driver->setLogger(new QueryLogger(['connection' => 'test']));
+        $this->driver->expects($this->any())
+            ->method('prepare')
+            ->willReturn($statement);
+
+        try {
+            $this->driver->execute('SELECT foo FROM bar');
+        } catch (PDOException $e) {
+        }
+
+        $messages = Log::engine('queries')->read();
+        $this->assertCount(1, $messages);
+        $this->assertMatchesRegularExpression('/^debug: connection=test duration=\d+ rows=0 SELECT bar FROM foo$/', $messages[0]);
+    }
 }

+ 0 - 124
tests/TestCase/Database/Statement/StatementTest.php

@@ -1,124 +0,0 @@
-<?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\Statement;
-
-use Cake\Database\DriverInterface;
-use Cake\Database\Log\QueryLogger;
-use Cake\Database\Statement\Statement;
-use Cake\Log\Log;
-use Cake\TestSuite\TestCase;
-use DateTime;
-use PDOException;
-use PDOStatement;
-
-class StatementTest extends TestCase
-{
-    public function setUp(): void
-    {
-        parent::setUp();
-        Log::setConfig('queries', [
-            'className' => 'Array',
-            'scopes' => ['queriesLog'],
-        ]);
-    }
-
-    public function tearDown(): void
-    {
-        parent::tearDown();
-        Log::drop('queries');
-    }
-
-    /**
-     * Tests that queries are logged when executed without params
-     */
-    public function testExecuteNoParams(): void
-    {
-        $inner = $this->getMockBuilder(PDOStatement::class)->getMock();
-        $inner->method('rowCount')->will($this->returnValue(3));
-        $inner->method('execute')->will($this->returnValue(true));
-
-        $driver = $this->getMockBuilder(DriverInterface::class)->getMock();
-        $statement = $this->getMockBuilder(Statement::class)
-            ->setConstructorArgs([$inner, $driver])
-            ->onlyMethods(['queryString'])
-            ->getMock();
-        $statement->expects($this->any())->method('queryString')->will($this->returnValue('SELECT bar FROM foo'));
-        $statement->setLogger(new QueryLogger(['connection' => 'test']));
-        $statement->execute();
-
-        $messages = Log::engine('queries')->read();
-        $this->assertCount(1, $messages);
-        $this->assertMatchesRegularExpression('/^debug: connection=test duration=\d+ rows=3 SELECT bar FROM foo$/', $messages[0]);
-    }
-
-    /**
-     * Tests that queries are logged when executed with bound params
-     */
-    public function testExecuteWithBinding(): void
-    {
-        $inner = $this->getMockBuilder(PDOStatement::class)->getMock();
-        $inner->method('rowCount')->will($this->returnValue(3));
-        $inner->method('execute')->will($this->returnValue(true));
-
-        $driver = $this->getMockBuilder(DriverInterface::class)->getMock();
-        $statement = $this->getMockBuilder(Statement::class)
-            ->setConstructorArgs([$inner, $driver])
-            ->onlyMethods(['queryString'])
-            ->getMock();
-        $statement->expects($this->any())->method('queryString')->will($this->returnValue('SELECT bar FROM foo WHERE a=:a AND b=:b'));
-        $statement->setLogger(new QueryLogger(['connection' => 'test']));
-
-        $statement->bindValue('a', 1);
-        $statement->bindValue('b', new DateTime('2013-01-01'), 'date');
-        $statement->execute();
-
-        $statement->bindValue('b', new DateTime('2014-01-01'), 'date');
-        $statement->execute();
-
-        $messages = Log::engine('queries')->read();
-        $this->assertCount(2, $messages);
-        $this->assertMatchesRegularExpression("/^debug: connection=test duration=\d+ rows=3 SELECT bar FROM foo WHERE a='1' AND b='2013-01-01'$/", $messages[0]);
-        $this->assertMatchesRegularExpression("/^debug: connection=test duration=\d+ rows=3 SELECT bar FROM foo WHERE a='1' AND b='2014-01-01'$/", $messages[1]);
-    }
-
-    /**
-     * Tests that queries are logged despite database errors
-     */
-    public function testExecuteWithError(): void
-    {
-        $inner = $this->getMockBuilder(PDOStatement::class)->getMock();
-        $inner->method('rowCount')->will($this->returnValue(3));
-        $inner->method('execute')->will($this->throwException(new PDOException()));
-
-        $driver = $this->getMockBuilder(DriverInterface::class)->getMock();
-        $statement = $this->getMockBuilder(Statement::class)
-            ->setConstructorArgs([$inner, $driver])
-            ->onlyMethods(['queryString'])
-            ->getMock();
-        $statement->expects($this->any())->method('queryString')->will($this->returnValue('SELECT bar FROM foo'));
-        $statement->setLogger(new QueryLogger(['connection' => 'test']));
-
-        try {
-            $statement->execute();
-        } catch (PDOException $e) {
-        }
-
-        $messages = Log::engine('queries')->read();
-        $this->assertCount(1, $messages);
-        $this->assertMatchesRegularExpression('/^debug: connection=test duration=\d+ rows=0 SELECT bar FROM foo$/', $messages[0]);
-    }
-}