Skip to content

Commit

Permalink
Bump minimum required DBAL version to 2.13 and rework compatibility…
Browse files Browse the repository at this point in the history
… layer (#527)
  • Loading branch information
Jean85 committed Jul 29, 2021
1 parent 070486e commit 1bffee8
Show file tree
Hide file tree
Showing 14 changed files with 108 additions and 67 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
- Make the list of commands for which distributed tracing is active configurable (#515)
- Introduce `TracingDriverConnection::getWrappedConnection()` (#536)
- Add the `logger` config option to ease setting a PSR-3 logger to debug the SDK (#538)
- Bump requirement for DBAL tracing to `^2.13|^3`; simplify the DBAL tracing feature (#527)

## 4.1.4 (2021-06-18)

Expand Down
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
"symfony/security-core": "^3.4.44||^4.4.20||^5.0.11"
},
"require-dev": {
"doctrine/dbal": "^2.10||^3.0",
"doctrine/dbal": "^2.13||^3.0",
"doctrine/doctrine-bundle": "^1.12||^2.0",
"friendsofphp/php-cs-fixer": "^2.18",
"jangregor/phpstan-prophecy": "^0.8",
Expand Down
24 changes: 22 additions & 2 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,12 @@ parameters:
path: src/Tracing/Doctrine/DBAL/TracingDriver.php

-
message: "#^Parameter \\#2 \\$query of class Doctrine\\\\DBAL\\\\Exception\\\\DriverException constructor expects Doctrine\\\\DBAL\\\\Query\\|null, Doctrine\\\\DBAL\\\\Driver\\\\Exception given\\.$#"
message: "#^Parameter \\#2 \\$query of class Doctrine\\\\DBAL\\\\Exception\\\\DriverException constructor expects Doctrine\\\\DBAL\\\\Query\\|null, Doctrine\\\\DBAL\\\\Driver\\\\DriverException given\\.$#"
count: 1
path: src/Tracing/Doctrine/DBAL/TracingDriver.php

-
message: "#^Parameter \\$exception of method Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\TracingDriver\\:\\:convertException\\(\\) has invalid typehint type Doctrine\\\\DBAL\\\\Driver\\\\DriverException\\.$#"
count: 1
path: src/Tracing/Doctrine/DBAL/TracingDriver.php

Expand Down Expand Up @@ -115,6 +120,11 @@ parameters:
count: 1
path: src/aliases.php

-
message: "#^Class Symfony\\\\Bundle\\\\FrameworkBundle\\\\Client not found\\.$#"
count: 1
path: tests/End2End/TracingEnd2EndTest.php

-
message: "#^Call to method getException\\(\\) on an unknown class Symfony\\\\Component\\\\HttpKernel\\\\Event\\\\GetResponseForExceptionEvent\\.$#"
count: 1
Expand Down Expand Up @@ -235,13 +245,23 @@ parameters:
count: 1
path: tests/Tracing/Cache/AbstractTraceableCacheAdapterTest.php

-
message: "#^Class Doctrine\\\\DBAL\\\\Driver\\\\DriverException not found\\.$#"
count: 3
path: tests/Tracing/Doctrine/DBAL/TracingDriverTest.php

-
message: "#^Parameter \\#1 \\$driverException of class Doctrine\\\\DBAL\\\\Exception\\\\DriverException constructor expects Doctrine\\\\DBAL\\\\Driver\\\\Exception, string given\\.$#"
count: 1
path: tests/Tracing/Doctrine/DBAL/TracingDriverTest.php

-
message: "#^Parameter \\#2 \\$query of class Doctrine\\\\DBAL\\\\Exception\\\\DriverException constructor expects Doctrine\\\\DBAL\\\\Query\\|null, Doctrine\\\\DBAL\\\\Driver\\\\Exception&PHPUnit\\\\Framework\\\\MockObject\\\\MockObject given\\.$#"
message: "#^Parameter \\#1 \\$originalClassName of method PHPUnit\\\\Framework\\\\TestCase\\:\\:createMock\\(\\) expects class\\-string\\<Doctrine\\\\DBAL\\\\Driver\\\\DriverException\\>, string given\\.$#"
count: 2
path: tests/Tracing/Doctrine/DBAL/TracingDriverTest.php

-
message: "#^Parameter \\#2 \\$query of class Doctrine\\\\DBAL\\\\Exception\\\\DriverException constructor expects Doctrine\\\\DBAL\\\\Query\\|null, Doctrine\\\\DBAL\\\\Driver\\\\DriverException&PHPUnit\\\\Framework\\\\MockObject\\\\MockObject given\\.$#"
count: 1
path: tests/Tracing/Doctrine/DBAL/TracingDriverTest.php

Expand Down
7 changes: 1 addition & 6 deletions psalm-baseline.xml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="UTF-8"?>
<files psalm-version="4.7.0@d4377c0baf3ffbf0b1ec6998e8d1be2a40971005">
<files psalm-version="4.8.1@f73f2299dbc59a3e6c4d66cff4605176e728ee69">
<file src="src/EventListener/ConsoleCommandListener.php">
<InvalidExtendClass occurrences="1">
<code>ConsoleListener</code>
Expand All @@ -8,9 +8,4 @@
<code>public function __construct(HubInterface $hub, bool $captureErrors = true)</code>
</MethodSignatureMismatch>
</file>
<file src="src/Tracing/Cache/TraceableCacheAdapterTrait.php">
<InvalidReturnType occurrences="1">
<code>getItems</code>
</InvalidReturnType>
</file>
</files>
18 changes: 18 additions & 0 deletions src/DependencyInjection/Compiler/DbalTracingPass.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace Sentry\SentryBundle\DependencyInjection\Compiler;

use Doctrine\DBAL\Driver\ResultStatement;
use Doctrine\DBAL\Result;
use Sentry\SentryBundle\Tracing\Doctrine\DBAL\ConnectionConfigurator;
use Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverMiddleware;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
Expand Down Expand Up @@ -39,6 +40,8 @@ public function process(ContainerBuilder $container): void
return;
}

$this->assertRequiredDbalVersion();

/** @var string[] $connectionsToTrace */
$connectionsToTrace = $container->getParameter('sentry.tracing.dbal.connections');

Expand Down Expand Up @@ -92,4 +95,19 @@ private function getSetMiddlewaresMethodCallArguments(Definition $definition): a

return [];
}

private function assertRequiredDbalVersion(): void
{
if (interface_exists(Result::class)) {
// DBAL ^2.13
return;
}

if (class_exists(Result::class)) {
// DBAL ^3
return;
}

throw new \LogicException('Tracing support cannot be enabled as the Doctrine DBAL 2.13+ package is not installed. Try running "composer require doctrine/dbal:^2.13".');
}
}
12 changes: 0 additions & 12 deletions src/Tracing/Doctrine/DBAL/Compatibility/DriverInterface.php

This file was deleted.

4 changes: 1 addition & 3 deletions src/Tracing/Doctrine/DBAL/TracingDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
* This is a simple implementation of the {@see DriverInterface} interface that
* decorates an existing driver to support distributed tracing capabilities.
* This implementation IS and MUST be compatible with all versions of Doctrine
* DBAL >= 2.10.
* DBAL >= 2.13.
*
* @internal
*/
Expand All @@ -36,8 +36,6 @@ final class TracingDriver implements DriverInterface, VersionAwarePlatformDriver
private $decoratedDriver;

/**
* Constructor.
*
* @param HubInterface $hub The current hub
* @param DriverInterface $decoratedDriver The instance of the driver to decorate
*/
Expand Down
10 changes: 5 additions & 5 deletions src/Tracing/Doctrine/DBAL/TracingDriverMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@

namespace Sentry\SentryBundle\Tracing\Doctrine\DBAL;

use Doctrine\DBAL\Driver as DriverInterface;
use Doctrine\DBAL\Driver\Middleware as MiddlewareInterface;
use Doctrine\DBAL\Driver;
use Doctrine\DBAL\Driver\Middleware;
use Sentry\State\HubInterface;

/**
* This middleware wraps a {@see DriverInterface} instance into one that
* This middleware wraps a {@see Driver} instance into one that
* supports the distributed tracing feature of Sentry.
*/
final class TracingDriverMiddleware implements MiddlewareInterface
final class TracingDriverMiddleware implements Middleware
{
/**
* @var HubInterface The current hub
Expand All @@ -32,7 +32,7 @@ public function __construct(HubInterface $hub)
/**
* {@inheritdoc}
*/
public function wrap(DriverInterface $driver): DriverInterface
public function wrap(Driver $driver): Driver
{
return new TracingDriver($this->hub, $driver);
}
Expand Down
21 changes: 0 additions & 21 deletions src/aliases.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,14 @@

namespace Sentry\SentryBundle;

use Doctrine\DBAL\Driver as DoctrineDriverInterface;
use Doctrine\DBAL\Driver\DriverException as LegacyDriverExceptionInterface;
use Doctrine\DBAL\Driver\Exception as DriverExceptionInterface;
use Doctrine\DBAL\Driver\ExceptionConverterDriver as LegacyExceptionConverterDriverInterface;
use Doctrine\DBAL\Driver\Middleware as DoctrineMiddlewareInterface;
use Doctrine\DBAL\Driver\Result;
use Doctrine\DBAL\Driver\Statement;
use Sentry\SentryBundle\EventListener\ErrorListenerExceptionEvent;
use Sentry\SentryBundle\EventListener\RequestListenerControllerEvent;
use Sentry\SentryBundle\EventListener\RequestListenerRequestEvent;
use Sentry\SentryBundle\EventListener\RequestListenerResponseEvent;
use Sentry\SentryBundle\EventListener\RequestListenerTerminateEvent;
use Sentry\SentryBundle\EventListener\SubRequestListenerRequestEvent;
use Sentry\SentryBundle\Tracing\Doctrine\DBAL\Compatibility\DriverInterface;
use Sentry\SentryBundle\Tracing\Doctrine\DBAL\Compatibility\ExceptionConverterDriverInterface;
use Sentry\SentryBundle\Tracing\Doctrine\DBAL\Compatibility\MiddlewareInterface;
use Symfony\Component\HttpKernel\Event\ControllerEvent;
Expand Down Expand Up @@ -94,26 +88,11 @@ class_alias(GetResponseEvent::class, SubRequestListenerRequestEvent::class);
}
}

if (interface_exists(Statement::class) && !interface_exists(Result::class)) {
/** @psalm-suppress UndefinedClass */
class_alias(Statement::class, Result::class);
}

if (interface_exists(DriverExceptionInterface::class) && !interface_exists(LegacyDriverExceptionInterface::class)) {
/** @psalm-suppress UndefinedClass */
class_alias(DriverExceptionInterface::class, LegacyDriverExceptionInterface::class);
}

if (!interface_exists(DoctrineMiddlewareInterface::class)) {
/** @psalm-suppress UndefinedClass */
class_alias(MiddlewareInterface::class, DoctrineMiddlewareInterface::class);
}

if (!interface_exists(DoctrineDriverInterface::class)) {
/** @psalm-suppress UndefinedClass */
class_alias(DriverInterface::class, DoctrineDriverInterface::class);
}

if (!interface_exists(LegacyExceptionConverterDriverInterface::class)) {
/** @psalm-suppress UndefinedClass */
class_alias(ExceptionConverterDriverInterface::class, LegacyExceptionConverterDriverInterface::class);
Expand Down
27 changes: 25 additions & 2 deletions tests/DependencyInjection/Compiler/DbalTracingPassTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ public function testProcessWithDoctrineDBALVersionAtLeast30(): void

public function testProcessWithDoctrineDBALVersionLowerThan30(): void
{
if (self::isDoctrineDBALVersion3Installed()) {
$this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be < 3.0.');
if (!self::isDoctrineDBALVersion2Installed()) {
$this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be ^2.13.');
}

$connection1 = (new Definition(Connection::class))->setPublic(true);
Expand All @@ -103,11 +103,30 @@ public function testProcessWithDoctrineDBALVersionLowerThan30(): void
$this->assertNull($connection2->getConfigurator());
}

public function testProcessWithDoctrineDBALMissing(): void
{
if (self::isDoctrineDBALInstalled()) {
$this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be missing.');
}

$container = $this->createContainerBuilder();
$container->setParameter('sentry.tracing.dbal.connections', ['foo', 'baz']);

$this->expectException(\LogicException::class);
$this->expectExceptionMessage('Tracing support cannot be enabled as the Doctrine DBAL 2.13+ package is not installed. Try running "composer require doctrine/dbal:^2.13".');

$container->compile();
}

/**
* @dataProvider processDoesNothingIfConditionsForEnablingTracingAreMissingDataProvider
*/
public function testProcessDoesNothingIfConditionsForEnablingTracingAreMissing(ContainerBuilder $container): void
{
if (!self::isDoctrineDBALInstalled()) {
$this->markTestSkipped('This test requires the "doctrine/dbal" Composer package.');
}

$connectionConfigDefinition = new Definition();
$connectionConfigDefinition->setClass(Configuration::class);
$connectionConfigDefinition->setPublic(true);
Expand Down Expand Up @@ -141,6 +160,10 @@ public function processDoesNothingIfConditionsForEnablingTracingAreMissingDataPr

public function testContainerCompilationFailsIfConnectionDoesntExist(): void
{
if (!self::isDoctrineDBALInstalled()) {
$this->markTestSkipped('This test requires the "doctrine/dbal" Composer package.');
}

$container = $this->createContainerBuilder();
$container->setParameter('sentry.tracing.dbal.connections', ['missing']);

Expand Down
15 changes: 14 additions & 1 deletion tests/DoctrineTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,27 @@
namespace Sentry\SentryBundle\Tests;

use Doctrine\Bundle\DoctrineBundle\DoctrineBundle;
use Doctrine\DBAL\Driver;
use Doctrine\DBAL\Driver\ResultStatement;
use PHPUnit\Framework\TestCase;

abstract class DoctrineTestCase extends TestCase
{
protected static function isDoctrineDBALInstalled(): bool
{
return interface_exists(Driver::class);
}

protected static function isDoctrineDBALVersion2Installed(): bool
{
return self::isDoctrineDBALInstalled()
&& interface_exists(ResultStatement::class);
}

protected static function isDoctrineDBALVersion3Installed(): bool
{
return !interface_exists(ResultStatement::class);
return self::isDoctrineDBALInstalled()
&& !self::isDoctrineDBALVersion2Installed();
}

protected static function isDoctrineBundlePackageInstalled(): bool
Expand Down
1 change: 0 additions & 1 deletion tests/End2End/TracingEnd2EndTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
use Symfony\Component\HttpFoundation\Response;

if (!class_exists(KernelBrowser::class)) {
/** @phpstan-ignore-next-line */
class_alias(Client::class, KernelBrowser::class);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ final class TracingDriverMiddlewareTest extends DoctrineTestCase

public static function setUpBeforeClass(): void
{
if (!self::isDoctrineBundlePackageInstalled()) {
if (!self::isDoctrineDBALVersion3Installed()) {
self::markTestSkipped();
}
}
Expand Down
31 changes: 19 additions & 12 deletions tests/Tracing/Doctrine/DBAL/TracingDriverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use Doctrine\DBAL\Driver\Connection as DriverConnectionInterface;
use Doctrine\DBAL\Driver\DriverException as DriverExceptionInterface;
use Doctrine\DBAL\Driver\ExceptionConverterDriver as ExceptionConverterDriverInterface;
use Doctrine\DBAL\Exception;
use Doctrine\DBAL\Exception\DriverException as DBALDriverException;
use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Schema\AbstractSchemaManager;
Expand Down Expand Up @@ -217,8 +218,8 @@ public function testCreateDatabasePlatformForVersionWhenDriverDoesNotImplementIn

public function testConvertException(): void
{
if (self::isDoctrineDBALVersion3Installed()) {
$this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be <= 3.0.');
if (!self::isDoctrineDBALVersion2Installed()) {
$this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be ^2.13.');
}

$exception = $this->createMock(DriverExceptionInterface::class);
Expand Down Expand Up @@ -249,18 +250,24 @@ public function testConvertExceptionThrowsIfDoctrineDBALVersionIsAtLeast30(): vo
}
}

if (interface_exists(VersionAwarePlatformDriverInterface::class)) {
interface StubVersionAwarePlatformDriverInterface extends DriverInterface, VersionAwarePlatformDriverInterface
{
if (interface_exists(DriverInterface::class)) {
if (interface_exists(VersionAwarePlatformDriverInterface::class)) {
interface StubVersionAwarePlatformDriverInterface extends DriverInterface, VersionAwarePlatformDriverInterface
{
}
}
}

if (interface_exists(ExceptionConverterDriverInterface::class)) {
interface StubExceptionConverterDriverInterface extends ExceptionConverterDriverInterface, DriverInterface
{
if (interface_exists(ExceptionConverterDriverInterface::class)) {
interface StubExceptionConverterDriverInterface extends ExceptionConverterDriverInterface, DriverInterface
{
}
} else {
interface StubExceptionConverterDriverInterface extends DriverInterface
{
}
}
} else {
interface StubExceptionConverterDriverInterface extends DriverInterface
{

if (!interface_exists(DriverExceptionInterface::class)) {
class_alias(Exception::class, DriverExceptionInterface::class);
}
}

0 comments on commit 1bffee8

Please sign in to comment.