Skip to content

Commit 27f602b

Browse files
ste93cryste93cry
committed
Fix CR issues
1 parent bcab5d9 commit 27f602b

23 files changed

+191
-65
lines changed

‎.github/workflows/static-analysis.yaml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@ jobs:
1717

1818
- name: Setup PHP
1919
uses: shivammathur/setup-php@v2
20-
with:
21-
php-version: '7.4'
2220

2321
- name: Install dependencies
2422
run: composer update --no-progress --no-interaction --prefer-dist

‎.github/workflows/tests.yaml

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,3 +65,59 @@ jobs:
6565
with:
6666
file: './coverage.xml'
6767
fail_ci_if_error: true
68+
69+
missing-optional-packages-tests:
70+
name: Tests without optional packages
71+
runs-on: ubuntu-latest
72+
strategy:
73+
fail-fast: false
74+
matrix:
75+
php:
76+
- '7.2'
77+
- '8.0'
78+
dependencies:
79+
- lowest
80+
- highest
81+
82+
steps:
83+
- name: Checkout
84+
uses: actions/checkout@v2
85+
86+
- name: Setup PHP
87+
uses: shivammathur/setup-php@v2
88+
with:
89+
php-version: ${{ matrix.php }}
90+
coverage: xdebug
91+
92+
- name: Setup Problem Matchers for PHPUnit
93+
run: echo "::add-matcher::${{ runner.tool_cache }}/phpunit.json"
94+
95+
- name: Determine Composer cache directory
96+
id: composer-cache
97+
run: echo "::set-output name=directory::$(composer config cache-dir)"
98+
99+
- name: Cache Composer dependencies
100+
uses: actions/cache@v2
101+
with:
102+
path: ${{ steps.composer-cache.outputs.directory }}
103+
key: ${{ runner.os }}-${{ matrix.php }}-composer-${{ matrix.dependencies }}-${{ hashFiles('**/composer.lock') }}
104+
restore-keys: ${{ runner.os }}-${{ matrix.php }}-composer-${{ matrix.dependencies }}-
105+
106+
- name: Remove optional packages
107+
run: composer remove doctrine/dbal doctrine/doctrine-bundle symfony/messenger --dev --no-update
108+
109+
- name: Install highest dependencies
110+
run: composer update --no-progress --no-interaction --prefer-dist
111+
if: ${{ matrix.dependencies == 'highest' }}
112+
113+
- name: Install lowest dependencies
114+
run: composer update --no-progress --no-interaction --prefer-dist --prefer-lowest
115+
if: ${{ matrix.dependencies == 'lowest' }}
116+
117+
- name: Run tests
118+
run: vendor/bin/phpunit --coverage-clover=build/coverage-report.xml
119+
120+
- name: Upload code coverage
121+
uses: codecov/codecov-action@v1
122+
with:
123+
file: build/coverage-report.xml

‎composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@
5454
},
5555
"suggest": {
5656
"monolog/monolog": "Allow sending log messages to Sentry by using the included Monolog handler.",
57-
"doctrine/dbal": "Allow distributed tracing of SQL queries using Sentry."
57+
"doctrine/doctrine-bundle": "Allow distributed tracing of database queries using Sentry."
5858
},
5959
"autoload": {
6060
"files": [

‎phpstan-baseline.neon

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,17 @@ parameters:
3636
path: src/Tracing/Doctrine/DBAL/TracingDriver.php
3737

3838
-
39-
message: "#^Method Doctrine\\\\DBAL\\\\Driver\\:\\:connect\\(\\) invoked with 4 parameters, 1 required\\.$#"
39+
message: "#^Call to an undefined method Doctrine\\\\DBAL\\\\Driver\\|Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\Compatibility\\\\ExceptionConverterDriverInterface\\:\\:connect\\(\\)\\.$#"
40+
count: 1
41+
path: src/Tracing/Doctrine/DBAL/TracingDriver.php
42+
43+
-
44+
message: "#^Call to an undefined method Doctrine\\\\DBAL\\\\Driver\\|Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\Compatibility\\\\ExceptionConverterDriverInterface\\:\\:getDatabasePlatform\\(\\)\\.$#"
45+
count: 2
46+
path: src/Tracing/Doctrine/DBAL/TracingDriver.php
47+
48+
-
49+
message: "#^Call to an undefined method Doctrine\\\\DBAL\\\\Driver\\|Sentry\\\\SentryBundle\\\\Tracing\\\\Doctrine\\\\DBAL\\\\Compatibility\\\\ExceptionConverterDriverInterface\\:\\:getSchemaManager\\(\\)\\.$#"
4050
count: 1
4151
path: src/Tracing/Doctrine/DBAL/TracingDriver.php
4252

@@ -206,7 +216,7 @@ parameters:
206216
path: tests/Tracing/Doctrine/DBAL/TracingDriverTest.php
207217

208218
-
209-
message: "#^Trying to mock an undefined method convertException\\(\\) on class Sentry\\\\SentryBundle\\\\Tests\\\\Tracing\\\\Doctrine\\\\DBAL\\\\ExceptionConverterDriverInterface\\.$#"
219+
message: "#^Trying to mock an undefined method convertException\\(\\) on class Sentry\\\\SentryBundle\\\\Tests\\\\Tracing\\\\Doctrine\\\\DBAL\\\\StubExceptionConverterDriverInterface\\.$#"
210220
count: 1
211221
path: tests/Tracing/Doctrine/DBAL/TracingDriverTest.php
212222

‎src/DependencyInjection/SentryExtension.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use Sentry\SentryBundle\EventListener\ErrorListener;
1818
use Sentry\SentryBundle\EventListener\MessengerListener;
1919
use Sentry\SentryBundle\SentryBundle;
20+
use Sentry\SentryBundle\Tracing\Doctrine\DBAL\ConnectionConfigurator;
2021
use Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverMiddleware;
2122
use Sentry\Serializer\RepresentationSerializer;
2223
use Sentry\Serializer\Serializer;
@@ -167,6 +168,7 @@ private function registerTracingConfiguration(ContainerBuilder $container, array
167168
$container->setParameter('sentry.tracing.dbal.connections', $isConfigEnabled ? $config['dbal']['connections'] : []);
168169

169170
if (!$isConfigEnabled) {
171+
$container->removeDefinition(ConnectionConfigurator::class);
170172
$container->removeDefinition(TracingDriverMiddleware::class);
171173
}
172174
}

‎src/Tracing/Doctrine/DBAL/Compatibility/DriverInterface.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Sentry\SentryBundle\Tracing\Doctrine\DBAL\Compatibility;
6+
7+
/**
8+
* @internal
9+
*/
10+
interface DriverInterface
11+
{
12+
}

‎src/Tracing/Doctrine/DBAL/Compatibility/ExceptionConverterDriverInterface.php

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,9 @@
44

55
namespace Sentry\SentryBundle\Tracing\Doctrine\DBAL\Compatibility;
66

7-
use Doctrine\DBAL\Driver as DriverInterface;
8-
97
/**
108
* @internal
119
*/
12-
interface ExceptionConverterDriverInterface extends DriverInterface
10+
interface ExceptionConverterDriverInterface
1311
{
1412
}

‎src/Tracing/Doctrine/DBAL/TracingDriver.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
* This is a simple implementation of the {@see DriverInterface} interface that
2020
* decorates an existing driver to support distributed tracing capabilities.
2121
* This implementation IS and MUST be compatible with all versions of Doctrine
22-
* DBAL >= 2.11.
22+
* DBAL >= 2.10.
2323
*
2424
* @internal
2525
*/

‎src/Tracing/Doctrine/DBAL/TracingDriverConnection.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@
1313

1414
/**
1515
* This implementation wraps a driver connection and adds distributed tracing
16-
* capabilities to Doctrine DBAL.
16+
* capabilities to Doctrine DBAL. This implementation IS and MUST be compatible
17+
* with all versions of Doctrine DBAL >= 2.10.
1718
*/
1819
final class TracingDriverConnection implements DriverConnectionInterface
1920
{
@@ -45,7 +46,7 @@ final class TracingDriverConnection implements DriverConnectionInterface
4546
/**
4647
* @internal
4748
*/
48-
public const SPAN_OP_TRANSACTION_ROLLBACK = 'sql.transaciton.rollback';
49+
public const SPAN_OP_TRANSACTION_ROLLBACK = 'sql.transaction.rollback';
4950

5051
/**
5152
* @var HubInterface The current hub

‎src/aliases.php

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
namespace Sentry\SentryBundle;
66

7-
use Doctrine\DBAL\Connection;
7+
use Doctrine\DBAL\Driver as DoctrineDriverInterface;
88
use Doctrine\DBAL\Driver\DriverException as LegacyDriverExceptionInterface;
99
use Doctrine\DBAL\Driver\Exception as DriverExceptionInterface;
1010
use Doctrine\DBAL\Driver\ExceptionConverterDriver as LegacyExceptionConverterDriverInterface;
@@ -15,6 +15,7 @@
1515
use Sentry\SentryBundle\EventListener\RequestListenerControllerEvent;
1616
use Sentry\SentryBundle\EventListener\RequestListenerRequestEvent;
1717
use Sentry\SentryBundle\EventListener\SubRequestListenerRequestEvent;
18+
use Sentry\SentryBundle\Tracing\Doctrine\DBAL\Compatibility\DriverInterface;
1819
use Sentry\SentryBundle\Tracing\Doctrine\DBAL\Compatibility\ExceptionConverterDriverInterface;
1920
use Sentry\SentryBundle\Tracing\Doctrine\DBAL\Compatibility\MiddlewareInterface;
2021
use Symfony\Component\HttpKernel\Event\ControllerEvent;
@@ -67,24 +68,27 @@ class_alias(GetResponseEvent::class, SubRequestListenerRequestEvent::class);
6768
}
6869
}
6970

70-
if (class_exists(Connection::class)) {
71-
if (!interface_exists(Result::class)) {
72-
/** @psalm-suppress UndefinedClass */
73-
class_alias(Statement::class, Result::class);
74-
}
71+
if (interface_exists(Statement::class) && !interface_exists(Result::class)) {
72+
/** @psalm-suppress UndefinedClass */
73+
class_alias(Statement::class, Result::class);
74+
}
7575

76-
if (!interface_exists(DoctrineMiddlewareInterface::class)) {
77-
/** @psalm-suppress UndefinedClass */
78-
class_alias(MiddlewareInterface::class, DoctrineMiddlewareInterface::class);
79-
}
76+
if (interface_exists(DriverExceptionInterface::class) && !interface_exists(LegacyDriverExceptionInterface::class)) {
77+
/** @psalm-suppress UndefinedClass */
78+
class_alias(DriverExceptionInterface::class, LegacyDriverExceptionInterface::class);
79+
}
8080

81-
if (!interface_exists(LegacyExceptionConverterDriverInterface::class)) {
82-
/** @psalm-suppress UndefinedClass */
83-
class_alias(ExceptionConverterDriverInterface::class, LegacyExceptionConverterDriverInterface::class);
84-
}
81+
if (!interface_exists(DoctrineMiddlewareInterface::class)) {
82+
/** @psalm-suppress UndefinedClass */
83+
class_alias(MiddlewareInterface::class, DoctrineMiddlewareInterface::class);
84+
}
8585

86-
if (!interface_exists(LegacyDriverExceptionInterface::class)) {
87-
/** @psalm-suppress UndefinedClass */
88-
class_alias(DriverExceptionInterface::class, LegacyDriverExceptionInterface::class);
89-
}
86+
if (!interface_exists(DoctrineDriverInterface::class)) {
87+
/** @psalm-suppress UndefinedClass */
88+
class_alias(DriverInterface::class, DoctrineDriverInterface::class);
89+
}
90+
91+
if (!interface_exists(LegacyExceptionConverterDriverInterface::class)) {
92+
/** @psalm-suppress UndefinedClass */
93+
class_alias(ExceptionConverterDriverInterface::class, LegacyExceptionConverterDriverInterface::class);
9094
}

‎tests/DependencyInjection/Compiler/DbalTracingPassTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ final class DbalTracingPassTest extends DoctrineTestCase
1818
{
1919
public function testProcessWithDoctrineDBALVersionAtLeast30(): void
2020
{
21-
if (!$this->isDoctrineDBALVersion3Installed()) {
21+
if (!self::isDoctrineDBALVersion3Installed()) {
2222
$this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be >= 3.0.');
2323
}
2424

@@ -84,7 +84,7 @@ public function testProcessWithDoctrineDBALVersionAtLeast30(): void
8484

8585
public function testProcessWithDoctrineDBALVersionLowerThan30(): void
8686
{
87-
if ($this->isDoctrineDBALVersion3Installed()) {
87+
if (self::isDoctrineDBALVersion3Installed()) {
8888
$this->markTestSkipped('This test requires the version of the "doctrine/dbal" Composer package to be < 3.0.');
8989
}
9090

‎tests/DependencyInjection/ConfigurationTest.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
namespace Sentry\SentryBundle\Tests\DependencyInjection;
66

7+
use Doctrine\Bundle\DoctrineBundle\DoctrineBundle;
78
use Jean85\PrettyVersions;
89
use PHPUnit\Framework\TestCase;
910
use Sentry\SentryBundle\DependencyInjection\Configuration;
@@ -38,7 +39,7 @@ public function testProcessConfigurationWithDefaultConfiguration(): void
3839
'tracing' => [
3940
'dbal' => [
4041
'enabled' => false,
41-
'connections' => ['%doctrine.default_connection%'],
42+
'connections' => class_exists(DoctrineBundle::class) ? ['%doctrine.default_connection%'] : [],
4243
],
4344
],
4445
];

‎tests/DependencyInjection/Fixtures/php/dbal_tracing_disabled.php renamed to ‎tests/DependencyInjection/Fixtures/php/dbal_tracing_enabled.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
$container->loadFromExtension('sentry', [
99
'tracing' => [
1010
'dbal' => [
11-
'enabled' => false,
11+
'enabled' => true,
1212
'connections' => ['default'],
1313
],
1414
],

‎tests/DependencyInjection/Fixtures/php/full.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@
4444
],
4545
'tracing' => [
4646
'dbal' => [
47-
'enabled' => true,
47+
'enabled' => false,
48+
'connections' => ['default'],
4849
],
4950
],
5051
]);

‎tests/DependencyInjection/Fixtures/xml/dbal_tracing_disabled.xml renamed to ‎tests/DependencyInjection/Fixtures/xml/dbal_tracing_enabled.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
<sentry:config>
1010
<sentry:tracing>
11-
<sentry:dbal enabled="false">
11+
<sentry:dbal enabled="true">
1212
<sentry:connection>default</sentry:connection>
1313
</sentry:dbal>
1414
</sentry:tracing>

‎tests/DependencyInjection/Fixtures/xml/full.xml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,9 @@
3838
</sentry:options>
3939
<sentry:messenger enabled="true" capture-soft-fails="false" />
4040
<sentry:tracing>
41-
<sentry:dbal enabled="true" />
41+
<sentry:dbal enabled="false">
42+
<sentry:connection>default</sentry:connection>
43+
</sentry:dbal>
4244
</sentry:tracing>
4345
</sentry:config>
4446
</container>

‎tests/DependencyInjection/Fixtures/yml/dbal_tracing_disabled.yml renamed to ‎tests/DependencyInjection/Fixtures/yml/dbal_tracing_enabled.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
sentry:
22
tracing:
33
dbal:
4-
enabled: false
4+
enabled: true
55
connections:
66
- default

‎tests/DependencyInjection/Fixtures/yml/full.yml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,6 @@ sentry:
3939
capture_soft_fails: false
4040
tracing:
4141
dbal:
42-
enabled: true
42+
enabled: false
43+
connections:
44+
- enabled

‎tests/DependencyInjection/SentryExtensionTest.php

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Sentry\SentryBundle\EventListener\RequestListener;
1717
use Sentry\SentryBundle\EventListener\SubRequestListener;
1818
use Sentry\SentryBundle\SentryBundle;
19+
use Sentry\SentryBundle\Tracing\Doctrine\DBAL\ConnectionConfigurator;
1920
use Sentry\SentryBundle\Tracing\Doctrine\DBAL\TracingDriverMiddleware;
2021
use Sentry\Serializer\RepresentationSerializer;
2122
use Sentry\Serializer\Serializer;
@@ -264,16 +265,19 @@ public function testIgnoreErrorsIntegrationIsNotAddedTwiceIfAlreadyConfigured():
264265

265266
public function testTracingDriverMiddlewareIsConfiguredWhenDbalTracingIsEnable(): void
266267
{
267-
$container = $this->createContainerFromFixture('full');
268+
$container = $this->createContainerFromFixture('dbal_tracing_enabled');
268269

269270
$this->assertTrue($container->hasDefinition(TracingDriverMiddleware::class));
271+
$this->assertTrue($container->hasDefinition(ConnectionConfigurator::class));
272+
$this->assertNotEmpty($container->getParameter('sentry.tracing.dbal.connections'));
270273
}
271274

272275
public function testTracingDriverMiddlewareIsRemovedWhenDbalTracingIsDisabled(): void
273276
{
274-
$container = $this->createContainerFromFixture('dbal_tracing_disabled');
277+
$container = $this->createContainerFromFixture('full');
275278

276279
$this->assertFalse($container->hasDefinition(TracingDriverMiddleware::class));
280+
$this->assertFalse($container->hasDefinition(ConnectionConfigurator::class));
277281
$this->assertEmpty($container->getParameter('sentry.tracing.dbal.connections'));
278282
}
279283

‎tests/DoctrineTestCase.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,19 @@
44

55
namespace Sentry\SentryBundle\Tests;
66

7+
use Doctrine\Bundle\DoctrineBundle\DoctrineBundle;
78
use Doctrine\DBAL\Driver\ResultStatement;
89
use PHPUnit\Framework\TestCase;
910

1011
abstract class DoctrineTestCase extends TestCase
1112
{
12-
protected function isDoctrineDBALVersion3Installed(): bool
13+
protected static function isDoctrineDBALVersion3Installed(): bool
1314
{
1415
return !interface_exists(ResultStatement::class);
1516
}
17+
18+
protected static function isDoctrineBundlePackageInstalled(): bool
19+
{
20+
return class_exists(DoctrineBundle::class);
21+
}
1622
}

0 commit comments

Comments
 (0)