From 371d80c4184a1358e04355d12199fc93a3de1a03 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Sun, 14 Nov 2021 13:03:33 -0800 Subject: [PATCH 1/4] Remove redundant test This case is already covered by `ConnectionTest::testExceptionOnPrepareAndExecute()` --- .../Driver/IBMDB2/ConnectionTest.php | 43 ------------------- 1 file changed, 43 deletions(-) delete mode 100644 tests/Functional/Driver/IBMDB2/ConnectionTest.php diff --git a/tests/Functional/Driver/IBMDB2/ConnectionTest.php b/tests/Functional/Driver/IBMDB2/ConnectionTest.php deleted file mode 100644 index a9e1479bfbe..00000000000 --- a/tests/Functional/Driver/IBMDB2/ConnectionTest.php +++ /dev/null @@ -1,43 +0,0 @@ -markConnectionNotReusable(); - } - - public function testPrepareFailure(): void - { - $driverConnection = $this->connection->getWrappedConnection(); - - $re = new ReflectionProperty($driverConnection, 'connection'); - $re->setAccessible(true); - $conn = $re->getValue($driverConnection); - db2_close($conn); - - $this->expectException(PrepareFailed::class); - $driverConnection->prepare('SELECT 1'); - } -} From e6a04f9c2072143f7575404dfbfa80fff5477fca Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Mon, 15 Nov 2021 16:00:45 -0800 Subject: [PATCH 2/4] Remove assertion in testDeterminesDatabasePlatformWhenConnectingToNonExistentDatabase() If a middleware driver is a `VersionAwarePlatformDriver` but the actual driver isn't, this assertion will fail. --- tests/Functional/ConnectionTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Functional/ConnectionTest.php b/tests/Functional/ConnectionTest.php index da129919c2e..68e13d50966 100644 --- a/tests/Functional/ConnectionTest.php +++ b/tests/Functional/ConnectionTest.php @@ -360,7 +360,6 @@ public function testDeterminesDatabasePlatformWhenConnectingToNonExistentDatabas ); self::assertInstanceOf(AbstractPlatform::class, $connection->getDatabasePlatform()); - self::assertFalse($connection->isConnected()); self::assertSame($params, $connection->getParams()); $connection->close(); From 843483d1c7655c6ed12d12ea8063328461d94f45 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Wed, 3 Nov 2021 18:01:48 -0700 Subject: [PATCH 3/4] Introduce logging middleware --- .github/workflows/continuous-integration.yml | 32 ++++ composer.json | 3 +- src/Logging/Connection.php | 126 ++++++++++++++++ src/Logging/Driver.php | 90 ++++++++++++ src/Logging/Middleware.php | 25 ++++ src/Logging/Statement.php | 77 ++++++++++ tests/Functional/PortabilityTest.php | 13 +- tests/Logging/MiddlewareTest.php | 146 +++++++++++++++++++ 8 files changed, 507 insertions(+), 5 deletions(-) create mode 100644 src/Logging/Connection.php create mode 100644 src/Logging/Driver.php create mode 100644 src/Logging/Middleware.php create mode 100644 src/Logging/Statement.php create mode 100644 tests/Logging/MiddlewareTest.php diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml index 2ac9c8639cd..0b0d3f086ed 100644 --- a/.github/workflows/continuous-integration.yml +++ b/.github/workflows/continuous-integration.yml @@ -38,6 +38,10 @@ jobs: with: fetch-depth: 2 + - name: "Temporarily remove support for PSR Log 3 on PHP 8.1" + run: 'sed -i "s/\"psr\/log\": \"^1|^2|^3\"/\"psr\/log\": \"^1|^2\"/" composer.json' + if: "${{ matrix.php-version=='8.1' }}" + - name: "Install PHP" uses: "shivammathur/setup-php@v2" with: @@ -82,6 +86,10 @@ jobs: with: fetch-depth: 2 + - name: "Temporarily remove support for PSR Log 3 on PHP 8.1" + run: 'sed -i "s/\"psr\/log\": \"^1|^2|^3\"/\"psr\/log\": \"^1|^2\"/" composer.json' + if: "${{ matrix.php-version=='8.1' }}" + - name: "Install PHP" uses: "shivammathur/setup-php@v2" with: @@ -125,6 +133,10 @@ jobs: with: fetch-depth: 2 + - name: "Temporarily remove support for PSR Log 3 on PHP 8.1" + run: 'sed -i "s/\"psr\/log\": \"^1|^2|^3\"/\"psr\/log\": \"^1|^2\"/" composer.json' + if: "${{ matrix.php-version=='8.1' }}" + - name: "Install PHP" uses: "shivammathur/setup-php@v2" with: @@ -180,6 +192,10 @@ jobs: with: fetch-depth: 2 + - name: "Temporarily remove support for PSR Log 3 on PHP 8.1" + run: 'sed -i "s/\"psr\/log\": \"^1|^2|^3\"/\"psr\/log\": \"^1|^2\"/" composer.json' + if: "${{ matrix.php-version=='8.1' }}" + - name: "Install PHP" uses: "shivammathur/setup-php@v2" with: @@ -242,6 +258,10 @@ jobs: with: fetch-depth: 2 + - name: "Temporarily remove support for PSR Log 3 on PHP 8.1" + run: 'sed -i "s/\"psr\/log\": \"^1|^2|^3\"/\"psr\/log\": \"^1|^2\"/" composer.json' + if: "${{ matrix.php-version=='8.1' }}" + - name: "Install PHP" uses: "shivammathur/setup-php@v2" with: @@ -330,6 +350,10 @@ jobs: with: fetch-depth: 2 + - name: "Temporarily remove support for PSR Log 3 on PHP 8.1" + run: 'sed -i "s/\"psr\/log\": \"^1|^2|^3\"/\"psr\/log\": \"^1|^2\"/" composer.json' + if: "${{ matrix.php-version=='8.1' }}" + - name: "Install PHP" uses: "shivammathur/setup-php@v2" with: @@ -398,6 +422,10 @@ jobs: with: fetch-depth: 2 + - name: "Temporarily remove support for PSR Log 3 on PHP 8.1" + run: 'sed -i "s/\"psr\/log\": \"^1|^2|^3\"/\"psr\/log\": \"^1|^2\"/" composer.json' + if: "${{ matrix.php-version=='8.1' }}" + - name: "Install PHP" uses: "shivammathur/setup-php@v2" with: @@ -455,6 +483,10 @@ jobs: with: fetch-depth: 2 + - name: "Temporarily remove support for PSR Log 3 on PHP 8.1" + run: 'sed -i "s/\"psr\/log\": \"^1|^2|^3\"/\"psr\/log\": \"^1|^2\"/" composer.json' + if: "${{ matrix.php-version=='8.1' }}" + - name: "Install PHP" uses: "shivammathur/setup-php@v2" with: diff --git a/composer.json b/composer.json index 9dec3533283..6d5f58437ce 100644 --- a/composer.json +++ b/composer.json @@ -36,7 +36,8 @@ "doctrine/cache": "^1.11|^2.0", "doctrine/deprecations": "^0.5.3", "doctrine/event-manager": "^1.0", - "psr/cache": "^1|^2|^3" + "psr/cache": "^1|^2|^3", + "psr/log": "^1|^2|^3" }, "require-dev": { "doctrine/coding-standard": "9.0.0", diff --git a/src/Logging/Connection.php b/src/Logging/Connection.php new file mode 100644 index 00000000000..9bb11ac0593 --- /dev/null +++ b/src/Logging/Connection.php @@ -0,0 +1,126 @@ +connection = $connection; + $this->logger = $logger; + } + + public function __destruct() + { + $this->logger->info('Disconnecting'); + } + + public function prepare(string $sql): DriverStatement + { + return new Statement( + $this->connection->prepare($sql), + $this->logger, + $sql + ); + } + + public function query(string $sql): Result + { + $this->logger->debug('Executing query: {sql}', ['sql' => $sql]); + + return $this->connection->query($sql); + } + + /** + * {@inheritDoc} + */ + public function quote($value, $type = ParameterType::STRING) + { + return $this->connection->quote($value, $type); + } + + public function exec(string $sql): int + { + $this->logger->debug('Executing statement: {sql}', ['sql' => $sql]); + + return $this->connection->exec($sql); + } + + /** + * {@inheritDoc} + */ + public function lastInsertId($name = null) + { + if ($name !== null) { + Deprecation::triggerIfCalledFromOutside( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/issues/4687', + 'The usage of Connection::lastInsertId() with a sequence name is deprecated.' + ); + } + + return $this->connection->lastInsertId($name); + } + + /** + * {@inheritDoc} + */ + public function beginTransaction() + { + $this->logger->debug('Beginning transaction'); + + return $this->connection->beginTransaction(); + } + + /** + * {@inheritDoc} + */ + public function commit() + { + $this->logger->debug('Committing transaction'); + + return $this->connection->commit(); + } + + /** + * {@inheritDoc} + */ + public function rollBack() + { + $this->logger->debug('Rolling back transaction'); + + return $this->connection->rollBack(); + } + + /** + * {@inheritDoc} + */ + public function getServerVersion() + { + if (! $this->connection instanceof ServerInfoAwareConnection) { + throw new LogicException('The underlying connection is not a ServerInfoAwareConnection'); + } + + return $this->connection->getServerVersion(); + } +} diff --git a/src/Logging/Driver.php b/src/Logging/Driver.php new file mode 100644 index 00000000000..5ee12b4312a --- /dev/null +++ b/src/Logging/Driver.php @@ -0,0 +1,90 @@ +driver = $driver; + $this->logger = $logger; + } + + /** + * {@inheritDoc} + */ + public function connect(array $params) + { + $this->logger->info('Connecting with parameters {params}', ['params' => $this->maskPassword($params)]); + + return new Connection( + $this->driver->connect($params), + $this->logger + ); + } + + /** + * {@inheritDoc} + */ + public function getDatabasePlatform() + { + return $this->driver->getDatabasePlatform(); + } + + /** + * {@inheritDoc} + */ + public function getSchemaManager(DBALConnection $conn, AbstractPlatform $platform) + { + return $this->driver->getSchemaManager($conn, $platform); + } + + public function getExceptionConverter(): ExceptionConverter + { + return $this->driver->getExceptionConverter(); + } + + /** + * {@inheritDoc} + */ + public function createDatabasePlatformForVersion($version) + { + if ($this->driver instanceof VersionAwarePlatformDriver) { + return $this->driver->createDatabasePlatformForVersion($version); + } + + return $this->driver->getDatabasePlatform(); + } + + /** + * @param array $params Connection parameters + * + * @return array + */ + private function maskPassword(array $params): array + { + if (isset($params['password'])) { + $params['password'] = ''; + } + + return $params; + } +} diff --git a/src/Logging/Middleware.php b/src/Logging/Middleware.php new file mode 100644 index 00000000000..4d5c6b0611b --- /dev/null +++ b/src/Logging/Middleware.php @@ -0,0 +1,25 @@ +logger = $logger; + } + + public function wrap(DriverInterface $driver): DriverInterface + { + return new Driver($driver, $this->logger); + } +} diff --git a/src/Logging/Statement.php b/src/Logging/Statement.php new file mode 100644 index 00000000000..37ebf343da9 --- /dev/null +++ b/src/Logging/Statement.php @@ -0,0 +1,77 @@ +|array */ + private $params = []; + + /** @var array|array */ + private $types = []; + + /** + * @internal This statement can be only instantiated by its connection. + */ + public function __construct(StatementInterface $statement, LoggerInterface $logger, string $sql) + { + $this->statement = $statement; + $this->logger = $logger; + $this->sql = $sql; + } + + /** + * {@inheritdoc} + */ + public function bindParam($param, &$variable, $type = ParameterType::STRING, $length = null) + { + $this->params[$param] = &$variable; + $this->types[$param] = $type; + + return $this->statement->bindParam($param, $variable, $type, ...array_slice(func_get_args(), 3)); + } + + /** + * {@inheritdoc} + */ + public function bindValue($param, $value, $type = ParameterType::STRING) + { + $this->params[$param] = $value; + $this->types[$param] = $type; + + return $this->statement->bindValue($param, $value, $type); + } + + /** + * {@inheritdoc} + */ + public function execute($params = null): ResultInterface + { + $this->logger->debug('Executing statement: {sql} (parameters: {params}, types: {types})', [ + 'sql' => $this->sql, + 'params' => $params ?? $this->params, + 'types' => $this->types, + ]); + + return $this->statement->execute($params); + } +} diff --git a/tests/Functional/PortabilityTest.php b/tests/Functional/PortabilityTest.php index 3fef5445a11..ddac7010d03 100644 --- a/tests/Functional/PortabilityTest.php +++ b/tests/Functional/PortabilityTest.php @@ -10,18 +10,23 @@ use Doctrine\DBAL\Tests\FunctionalTestCase; use Throwable; +use function array_merge; use function strlen; class PortabilityTest extends FunctionalTestCase { protected function setUp(): void { - $this->connection = DriverManager::getConnection( - $this->connection->getParams(), - $this->connection->getConfiguration() - ->setMiddlewares([new Middleware(Connection::PORTABILITY_ALL, ColumnCase::LOWER)]) + $configuration = $this->connection->getConfiguration(); + $configuration->setMiddlewares( + array_merge( + $configuration->getMiddlewares(), + [new Middleware(Connection::PORTABILITY_ALL, ColumnCase::LOWER)] + ) ); + $this->connection = DriverManager::getConnection($this->connection->getParams(), $configuration); + try { $table = new Table('portability_table'); $table->addColumn('Test_Int', 'integer'); diff --git a/tests/Logging/MiddlewareTest.php b/tests/Logging/MiddlewareTest.php new file mode 100644 index 00000000000..4cd565e52d1 --- /dev/null +++ b/tests/Logging/MiddlewareTest.php @@ -0,0 +1,146 @@ +createMock(Connection::class); + + $driver = $this->createMock(Driver::class); + $driver->method('connect') + ->willReturn($connection); + + $this->logger = $this->createMock(LoggerInterface::class); + + $middleware = new Middleware($this->logger); + $this->driver = $middleware->wrap($driver); + } + + public function testConnectAndDisconnect(): void + { + $this->logger->expects(self::exactly(2)) + ->method('info') + ->withConsecutive( + [ + 'Connecting with parameters {params}', + [ + 'params' => [ + 'username' => 'admin', + 'password' => '', + ], + ], + ], + ['Disconnecting', []], + ); + + $this->driver->connect([ + 'username' => 'admin', + 'password' => 'Passw0rd!', + ]); + } + + public function testQuery(): void + { + $this->logger->expects(self::once()) + ->method('debug') + ->with('Executing query: {sql}', ['sql' => 'SELECT 1']); + + $connection = $this->driver->connect([]); + $connection->query('SELECT 1'); + } + + public function testExec(): void + { + $this->logger->expects(self::once()) + ->method('debug') + ->with('Executing statement: {sql}', ['sql' => 'DROP DATABASE doctrine']); + + $connection = $this->driver->connect([]); + $connection->exec('DROP DATABASE doctrine'); + } + + public function testBeginCommitRollback(): void + { + $this->logger->expects(self::exactly(3)) + ->method('debug') + ->withConsecutive( + ['Beginning transaction'], + ['Committing transaction'], + ['Rolling back transaction'], + ); + + $connection = $this->driver->connect([]); + $connection->beginTransaction(); + $connection->commit(); + $connection->rollBack(); + } + + public function testExecuteStatementWithUntypedParameters(): void + { + $this->logger->expects(self::once()) + ->method('debug') + ->with('Executing statement: {sql} (parameters: {params}, types: {types})', [ + 'sql' => 'SELECT ?', + 'params' => [42], + 'types' => [], + ]); + + $connection = $this->driver->connect([]); + $statement = $connection->prepare('SELECT ?'); + $statement->execute([42]); + } + + public function testExecuteStatementWithTypedParameters(): void + { + $this->logger->expects(self::once()) + ->method('debug') + ->with('Executing statement: {sql} (parameters: {params}, types: {types})', [ + 'sql' => 'SELECT ?, ?', + 'params' => [1 => 42, 2 => 'Test'], + 'types' => [1 => ParameterType::INTEGER, 2 => ParameterType::STRING], + ]); + + $connection = $this->driver->connect([]); + $statement = $connection->prepare('SELECT ?, ?'); + $statement->bindValue(1, 42, ParameterType::INTEGER); + $statement->bindParam(2, $byRef, ParameterType::STRING); + + $byRef = 'Test'; + $statement->execute(); + } + + public function testExecuteStatementWithNamedParameters(): void + { + $this->logger->expects(self::once()) + ->method('debug') + ->with('Executing statement: {sql} (parameters: {params}, types: {types})', [ + 'sql' => 'SELECT :value', + 'params' => ['value' => 'Test'], + 'types' => ['value' => ParameterType::STRING], + ]); + + $connection = $this->driver->connect([]); + $statement = $connection->prepare('SELECT :value'); + $statement->bindValue('value', 'Test'); + + $statement->execute(); + } +} From 8fcd4541c914ab837f4ac8d354910f1948d991d5 Mon Sep 17 00:00:00 2001 From: Sergei Morozov Date: Mon, 15 Nov 2021 16:12:53 -0800 Subject: [PATCH 4/4] Deprecate SQLLogger --- UPGRADE.md | 5 +++++ psalm.xml.dist | 12 ++++++++++++ src/Logging/DebugStack.php | 13 +++++++++++++ src/Logging/LoggerChain.php | 10 ++++++++++ src/Logging/SQLLogger.php | 3 +++ 5 files changed, 43 insertions(+) diff --git a/UPGRADE.md b/UPGRADE.md index a0e21d83d8d..e51562bd0ab 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -8,6 +8,11 @@ awareness about deprecated code. # Upgrade to 3.2 +## Deprecated `SQLLogger` and its implementations. + +The `SQLLogger` and its implementations `DebugStack` and `LoggerChain` have been deprecated. +For logging purposes, use `Doctrine\DBAL\Logging\Middleware` instead. No replacement for `DebugStack` is provided. + ## Deprecated `SqliteSchemaManager::createDatabase()` and `dropDatabase()` methods. The `SqliteSchemaManager::createDatabase()` and `dropDatabase()` methods have been deprecated. The SQLite engine diff --git a/psalm.xml.dist b/psalm.xml.dist index eb1afaf6ba6..4b194dda9f3 100644 --- a/psalm.xml.dist +++ b/psalm.xml.dist @@ -67,6 +67,13 @@ TODO: remove in 4.0.0 --> + + + + @@ -80,6 +87,11 @@ TODO: remove in 4.0.0 --> + + diff --git a/src/Logging/DebugStack.php b/src/Logging/DebugStack.php index 6a9fab5a9e4..24a2d68cfbe 100644 --- a/src/Logging/DebugStack.php +++ b/src/Logging/DebugStack.php @@ -2,10 +2,14 @@ namespace Doctrine\DBAL\Logging; +use Doctrine\Deprecations\Deprecation; + use function microtime; /** * Includes executed SQLs in a Debug Stack. + * + * @deprecated */ class DebugStack implements SQLLogger { @@ -29,6 +33,15 @@ class DebugStack implements SQLLogger /** @var int */ public $currentQuery = 0; + public function __construct() + { + Deprecation::trigger( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/pull/4967', + 'DebugStack is deprecated.' + ); + } + /** * {@inheritdoc} */ diff --git a/src/Logging/LoggerChain.php b/src/Logging/LoggerChain.php index 9b44dc0e3e8..c256dd72cd6 100644 --- a/src/Logging/LoggerChain.php +++ b/src/Logging/LoggerChain.php @@ -2,8 +2,12 @@ namespace Doctrine\DBAL\Logging; +use Doctrine\Deprecations\Deprecation; + /** * Chains multiple SQLLogger. + * + * @deprecated */ class LoggerChain implements SQLLogger { @@ -15,6 +19,12 @@ class LoggerChain implements SQLLogger */ public function __construct(iterable $loggers = []) { + Deprecation::trigger( + 'doctrine/dbal', + 'https://github.com/doctrine/dbal/pull/4967', + 'LoggerChain is deprecated' + ); + $this->loggers = $loggers; } diff --git a/src/Logging/SQLLogger.php b/src/Logging/SQLLogger.php index a0bdf1bf6a8..40eb707addd 100644 --- a/src/Logging/SQLLogger.php +++ b/src/Logging/SQLLogger.php @@ -6,6 +6,9 @@ /** * Interface for SQL loggers. + * + * @deprecated Use {@link \Doctrine\DBAL\Logging\Middleware} or implement + * {@link \Doctrine\DBAL\Driver\Middleware} instead. */ interface SQLLogger {