Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump DBAL support to 2.13+ #527

Merged
merged 27 commits into from
Jul 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
cbe825d
Require at least DBAL 2.13 to leverage the FC layer
Jean85 Jun 23, 2021
4643c20
Remove Driver and Middleware aliases
Jean85 Jun 23, 2021
72c0625
Add DBAL version check to compiler pass
Jean85 Jun 23, 2021
a524d17
Restore deleted use statement
Jean85 Jun 23, 2021
3b2918b
Fix edge case test with class_alias
Jean85 Jun 23, 2021
8fc8f57
Wrap stub definition in check for DBAL presence
Jean85 Jun 23, 2021
3675b2f
Fix test skipping in CI
Jean85 Jun 23, 2021
9e55351
Fix CS
Jean85 Jun 23, 2021
4c28ca9
Fix test
Jean85 Jun 23, 2021
a44045b
Skip test if DBAL is missing
Jean85 Jun 23, 2021
32b1a12
Restore Middleware alias since it's totaly missing in DBAL v2
Jean85 Jul 7, 2021
7518646
Fix DBAL v2 detection in tests
Jean85 Jul 7, 2021
f9cf23d
Fix or ignore PHPStan errors
Jean85 Jul 7, 2021
0491c39
Remove TODO
Jean85 Jul 7, 2021
f261716
Add changelog entry
Jean85 Jul 7, 2021
84fdec5
Address CR comments
Jean85 Jul 9, 2021
afb77bc
Fix interface existence check
Jean85 Jul 9, 2021
e437dcc
Revert wrong change
Jean85 Jul 9, 2021
62b026e
Fix PHPStan baseline
Jean85 Jul 9, 2021
197de3d
Increase Psalm baseline
Jean85 Jul 9, 2021
2c773c9
Fix baselines, again
Jean85 Jul 9, 2021
b50e71c
Refactor DBAL version check
Jean85 Jul 9, 2021
88efd92
Fix exception message assertion
Jean85 Jul 9, 2021
4a1b4b0
Address CR comments
Jean85 Jul 27, 2021
18c02fb
Merge branch 'develop' into bump-dbal-support
Jean85 Jul 27, 2021
96b5765
Fix test expectation
Jean85 Jul 27, 2021
e90450e
Simplify assert method
Jean85 Jul 29, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,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);
}
}