Skip to content

Commit 426f45a

Browse files
Jean85Jean85
committed
Merge branch 'master' into develop
2 parents 162c36f + c5caac2 commit 426f45a

24 files changed

+467
-122
lines changed

‎.craft.yml

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,10 @@
1-
minVersion: 0.9.0
2-
github:
3-
owner: getsentry
4-
repo: sentry-symfony
1+
minVersion: 0.23.1
52
changelogPolicy: simple
6-
statusProvider:
7-
name: github
83
artifactProvider:
94
name: none
10-
preReleaseCommand: ""
5+
preReleaseCommand: ''
116
targets:
12-
- name: github
13-
- name: registry
14-
type: sdk
15-
config:
16-
canonical: 'composer:sentry/sentry-symfony'
7+
- name: github
8+
- name: registry
9+
sdks:
10+
composer:sentry/sentry-symfony:

‎.github/workflows/tests.yaml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,17 @@ jobs:
2323
- '7.3'
2424
- '7.2'
2525
sentry_constraint: [false]
26+
dbal_constraint: [false]
2627
symfony_constraint: ['']
2728
experimental: [false]
2829
include:
2930
# - description: 'sentry/sentry dev-develop'
3031
# php: '7.4'
3132
# sentry_constraint: 'dev-develop'
3233
# experimental: true
34+
- description: 'DBAL 2'
35+
php: '7.4'
36+
dbal_constraint: '^2.13'
3337
- description: 'Symfony 4.4'
3438
php: '7.3'
3539
symfony_constraint: 4.4.*
@@ -41,6 +45,7 @@ jobs:
4145
- description: 'prefer lowest'
4246
php: '7.2'
4347
composer_option: '--prefer-lowest'
48+
symfony_constraint: ^3.4.44
4449
env:
4550
SYMFONY_DEPRECATIONS_HELPER: disabled
4651

@@ -61,6 +66,8 @@ jobs:
6166
run: composer global require --no-progress --no-scripts --no-plugins symfony/flex
6267
- run: composer remove --dev symfony/messenger --no-update
6368
if: matrix.symfony_constraint == '3.4.*' || matrix.composer_option == '--prefer-lowest'
69+
- run: composer require --dev doctrine/dbal ${{ matrix.dbal_constraint }} --no-update
70+
if: matrix.dbal_constraint
6471
- run: composer update --no-progress --ansi ${{ matrix.composer_option }}
6572
- run: composer require sentry/sentry dev-develop
6673
if: matrix.sentry_constraint == 'dev-develop'

‎CHANGELOG.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,14 @@
66
- Add the `sentry_trace_meta()` Twig function to print the `sentry-trace` HTML meta tag (#510)
77
- Make the list of commands for which distributed tracing is active configurable (#515)
88

9+
## 4.1.4 (2021-06-18)
10+
11+
- Fix decoration of cache adapters inheriting parent service (#525)
12+
- Fix extraction of the username of the logged-in user in Symfony `5.3` (#518)
13+
914
## 4.1.3 (2021-05-31)
1015

11-
- Fix missing require of the `symfony/cache-contracts` package (#506)
16+
- Fix missing require of the `symfony/cache-contracts` package (#506)
1217

1318
## 4.1.2 (2021-05-17)
1419

‎phpstan-baseline.neon

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -115,11 +115,6 @@ parameters:
115115
count: 1
116116
path: src/aliases.php
117117

118-
-
119-
message: "#^Class Symfony\\\\Bundle\\\\FrameworkBundle\\\\Client not found\\.$#"
120-
count: 1
121-
path: tests/End2End/End2EndTest.php
122-
123118
-
124119
message: "#^Call to method getException\\(\\) on an unknown class Symfony\\\\Component\\\\HttpKernel\\\\Event\\\\GetResponseForExceptionEvent\\.$#"
125120
count: 1
@@ -226,9 +221,19 @@ parameters:
226221
path: tests/Tracing/Cache/AbstractTraceableCacheAdapterTest.php
227222

228223
-
229-
message: "#^Parameter \\#2 \\$decoratedAdapter of class Sentry\\\\SentryBundle\\\\Tracing\\\\Cache\\\\TraceableTagAwareCacheAdapter constructor expects Symfony\\\\Component\\\\Cache\\\\Adapter\\\\TagAwareAdapterInterface, Symfony\\\\Component\\\\Cache\\\\Adapter\\\\AdapterInterface given\\.$#"
224+
message: "#^Parameter \\#1 \\$decoratedAdapter of method Sentry\\\\SentryBundle\\\\Tests\\\\Tracing\\\\Cache\\\\AbstractTraceableCacheAdapterTest\\<TCacheAdapter of Symfony\\\\Component\\\\Cache\\\\Adapter\\\\AdapterInterface,TDecoratedCacheAdapter of Symfony\\\\Component\\\\Cache\\\\Adapter\\\\AdapterInterface\\>\\:\\:createCacheAdapter\\(\\) expects TDecoratedCacheAdapter of Symfony\\\\Component\\\\Cache\\\\Adapter\\\\AdapterInterface, PHPUnit\\\\Framework\\\\MockObject\\\\MockObject&Sentry\\\\SentryBundle\\\\Tests\\\\Tracing\\\\Cache\\\\CacheInterface given\\.$#"
225+
count: 2
226+
path: tests/Tracing/Cache/AbstractTraceableCacheAdapterTest.php
227+
228+
-
229+
message: "#^Parameter \\#1 \\$decoratedAdapter of method Sentry\\\\SentryBundle\\\\Tests\\\\Tracing\\\\Cache\\\\AbstractTraceableCacheAdapterTest\\<TCacheAdapter of Symfony\\\\Component\\\\Cache\\\\Adapter\\\\AdapterInterface,TDecoratedCacheAdapter of Symfony\\\\Component\\\\Cache\\\\Adapter\\\\AdapterInterface\\>\\:\\:createCacheAdapter\\(\\) expects TDecoratedCacheAdapter of Symfony\\\\Component\\\\Cache\\\\Adapter\\\\AdapterInterface, PHPUnit\\\\Framework\\\\MockObject\\\\MockObject&Sentry\\\\SentryBundle\\\\Tests\\\\Tracing\\\\Cache\\\\PruneableCacheAdapterInterface given\\.$#"
230+
count: 1
231+
path: tests/Tracing/Cache/AbstractTraceableCacheAdapterTest.php
232+
233+
-
234+
message: "#^Parameter \\#1 \\$decoratedAdapter of method Sentry\\\\SentryBundle\\\\Tests\\\\Tracing\\\\Cache\\\\AbstractTraceableCacheAdapterTest\\<TCacheAdapter of Symfony\\\\Component\\\\Cache\\\\Adapter\\\\AdapterInterface,TDecoratedCacheAdapter of Symfony\\\\Component\\\\Cache\\\\Adapter\\\\AdapterInterface\\>\\:\\:createCacheAdapter\\(\\) expects TDecoratedCacheAdapter of Symfony\\\\Component\\\\Cache\\\\Adapter\\\\AdapterInterface, PHPUnit\\\\Framework\\\\MockObject\\\\MockObject&Sentry\\\\SentryBundle\\\\Tests\\\\Tracing\\\\Cache\\\\ResettableCacheAdapterInterface given\\.$#"
230235
count: 1
231-
path: tests/Tracing/Cache/TraceableTagAwareCacheAdapterTest.php
236+
path: tests/Tracing/Cache/AbstractTraceableCacheAdapterTest.php
232237

233238
-
234239
message: "#^Parameter \\#1 \\$driverException of class Doctrine\\\\DBAL\\\\Exception\\\\DriverException constructor expects Doctrine\\\\DBAL\\\\Driver\\\\Exception, string given\\.$#"

‎phpstan.neon

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ parameters:
1010
- tests/End2End/App
1111
dynamicConstantNames:
1212
- Symfony\Component\HttpKernel\Kernel::VERSION
13+
- Symfony\Component\HttpKernel\Kernel::VERSION_ID
1314
- Doctrine\DBAL\Version::VERSION
1415
stubFiles:
1516
- tests/Stubs/Profile.phpstub

‎src/DependencyInjection/Compiler/CacheTracingPass.php

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use Symfony\Component\DependencyInjection\ChildDefinition;
99
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
1010
use Symfony\Component\DependencyInjection\ContainerBuilder;
11+
use Symfony\Component\DependencyInjection\Definition;
1112
use Symfony\Component\DependencyInjection\Reference;
1213

1314
final class CacheTracingPass implements CompilerPassInterface
@@ -28,7 +29,13 @@ public function process(ContainerBuilder $container): void
2829
continue;
2930
}
3031

31-
if (is_subclass_of($cachePoolDefinition->getClass(), TagAwareAdapterInterface::class)) {
32+
$definitionClass = $this->resolveDefinitionClass($container, $cachePoolDefinition);
33+
34+
if (null === $definitionClass) {
35+
continue;
36+
}
37+
38+
if (is_subclass_of($definitionClass, TagAwareAdapterInterface::class)) {
3239
$traceableCachePoolDefinition = new ChildDefinition('sentry.tracing.traceable_tag_aware_cache_adapter');
3340
} else {
3441
$traceableCachePoolDefinition = new ChildDefinition('sentry.tracing.traceable_cache_adapter');
@@ -40,4 +47,16 @@ public function process(ContainerBuilder $container): void
4047
$container->setDefinition($serviceId . '.traceable', $traceableCachePoolDefinition);
4148
}
4249
}
50+
51+
private function resolveDefinitionClass(ContainerBuilder $container, Definition $definition): ?string
52+
{
53+
$class = $definition->getClass();
54+
55+
while (null === $class && $definition instanceof ChildDefinition) {
56+
$definition = $container->findDefinition($definition->getParent());
57+
$class = $definition->getClass();
58+
}
59+
60+
return $class;
61+
}
4362
}

‎src/EventListener/RequestListener.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,13 @@ public function handleKernelControllerEvent(RequestListenerControllerEvent $even
104104
private function getUsername($user): ?string
105105
{
106106
if ($user instanceof UserInterface) {
107-
return method_exists($user, 'getUserIdentifier') ? $user->getUserIdentifier() : $user->getUsername();
107+
if (method_exists($user, 'getUserIdentifier')) {
108+
return $user->getUserIdentifier();
109+
}
110+
111+
if (method_exists($user, 'getUsername')) {
112+
return $user->getUsername();
113+
}
108114
}
109115

110116
if (\is_string($user)) {

‎tests/Command/SentryTestCommandTest.php

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

1818
class SentryTestCommandTest extends BaseTestCase
1919
{
20+
protected function tearDown(): void
21+
{
22+
parent::tearDown();
23+
24+
// reset current Hub to avoid leaking the mock outside of this tests
25+
SentrySdk::init();
26+
}
27+
2028
public function testExecuteSuccessfully(): void
2129
{
2230
$options = new Options(['dsn' => 'http://public:[email protected]/sentry/1']);

‎tests/DependencyInjection/Compiler/CacheTracingPassTest.php

Lines changed: 96 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -11,46 +11,120 @@
1111
use Sentry\State\HubInterface;
1212
use Symfony\Component\Cache\Adapter\AdapterInterface;
1313
use Symfony\Component\Cache\Adapter\TagAwareAdapterInterface;
14+
use Symfony\Component\DependencyInjection\ChildDefinition;
1415
use Symfony\Component\DependencyInjection\ContainerBuilder;
1516
use Symfony\Component\DependencyInjection\Definition;
1617
use Symfony\Component\DependencyInjection\Reference;
1718

1819
final class CacheTracingPassTest extends TestCase
1920
{
20-
public function testProcess(): void
21+
/**
22+
* @dataProvider processDataProvider
23+
*
24+
* @param array<string, Definition> $definitions
25+
*/
26+
public function testProcess(array $definitions, string $expectedDefinitionClass, string $expectedInnerDefinitionClass): void
27+
{
28+
$container = $this->createContainerBuilder(true);
29+
$container->addDefinitions($definitions);
30+
$container->compile();
31+
32+
$cacheTraceableDefinition = $container->findDefinition('app.cache');
33+
34+
$this->assertSame($expectedDefinitionClass, $cacheTraceableDefinition->getClass());
35+
$this->assertInstanceOf(Definition::class, $cacheTraceableDefinition->getArgument(1));
36+
$this->assertSame($expectedInnerDefinitionClass, $cacheTraceableDefinition->getArgument(1)->getClass());
37+
}
38+
39+
/**
40+
* @return \Generator<mixed>
41+
*/
42+
public function processDataProvider(): \Generator
2143
{
2244
$cacheAdapter = $this->createMock(AdapterInterface::class);
2345
$tagAwareCacheAdapter = $this->createMock(TagAwareAdapterInterface::class);
24-
$container = $this->createContainerBuilder(true);
2546

26-
$container->register('app.cache.foo', \get_class($tagAwareCacheAdapter))
27-
->setPublic(true)
28-
->addTag('cache.pool');
47+
yield 'Cache pool adapter service' => [
48+
[
49+
'app.cache' => (new Definition(\get_class($cacheAdapter)))
50+
->setPublic(true)
51+
->addTag('cache.pool'),
52+
],
53+
TraceableCacheAdapter::class,
54+
\get_class($cacheAdapter),
55+
];
56+
57+
yield 'Tag-aware cache adapter service' => [
58+
[
59+
'app.cache' => (new Definition(\get_class($tagAwareCacheAdapter)))
60+
->setPublic(true)
61+
->addTag('cache.pool'),
62+
],
63+
TraceableTagAwareCacheAdapter::class,
64+
\get_class($tagAwareCacheAdapter),
65+
];
66+
67+
yield 'Cache pool adapter service inheriting parent service' => [
68+
[
69+
'app.cache.parent' => new Definition(\get_class($cacheAdapter)),
70+
'app.cache' => (new ChildDefinition('app.cache.parent'))
71+
->setPublic(true)
72+
->addTag('cache.pool'),
73+
],
74+
TraceableCacheAdapter::class,
75+
\get_class($cacheAdapter),
76+
];
77+
78+
yield 'Tag-aware cache pool adapter service inheriting parent service and overriding class' => [
79+
[
80+
'app.cache.parent' => new Definition(\get_class($cacheAdapter)),
81+
'app.cache' => (new ChildDefinition('app.cache.parent'))
82+
->setClass(\get_class($tagAwareCacheAdapter))
83+
->setPublic(true)
84+
->addTag('cache.pool'),
85+
],
86+
TraceableTagAwareCacheAdapter::class,
87+
\get_class($tagAwareCacheAdapter),
88+
];
89+
90+
yield 'Tag-aware cache pool adapter service inheriting multiple parent services' => [
91+
[
92+
'app.cache.parent_1' => new Definition(\get_class($cacheAdapter)),
93+
'app.cache.parent_2' => (new ChildDefinition('app.cache.parent_1'))
94+
->setClass(\get_class($tagAwareCacheAdapter)),
95+
'app.cache' => (new ChildDefinition('app.cache.parent_2'))
96+
->setPublic(true)
97+
->addTag('cache.pool'),
98+
],
99+
TraceableTagAwareCacheAdapter::class,
100+
\get_class($tagAwareCacheAdapter),
101+
];
102+
103+
yield 'Tag-aware cache pool adapter service inheriting parent service' => [
104+
[
105+
'app.cache.parent' => new Definition(\get_class($tagAwareCacheAdapter)),
106+
'app.cache' => (new ChildDefinition('app.cache.parent'))
107+
->setPublic(true)
108+
->addTag('cache.pool'),
109+
],
110+
TraceableTagAwareCacheAdapter::class,
111+
\get_class($tagAwareCacheAdapter),
112+
];
113+
}
29114

30-
$container->register('app.cache.bar', \get_class($cacheAdapter))
31-
->setPublic(true)
32-
->addTag('cache.pool');
115+
public function testProcessDoesNothingIfCachePoolServiceDefinitionIsAbstract(): void
116+
{
117+
$cacheAdapter = $this->createMock(AdapterInterface::class);
118+
$container = $this->createContainerBuilder(true);
33119

34-
$container->register('app.cache.baz')
120+
$container->register('app.cache', \get_class($cacheAdapter))
35121
->setPublic(true)
36122
->setAbstract(true)
37123
->addTag('cache.pool');
38124

39125
$container->compile();
40126

41-
$cacheTraceableDefinition = $container->findDefinition('app.cache.foo');
42-
43-
$this->assertSame(TraceableTagAwareCacheAdapter::class, $cacheTraceableDefinition->getClass());
44-
$this->assertInstanceOf(Definition::class, $cacheTraceableDefinition->getArgument(1));
45-
$this->assertSame(\get_class($tagAwareCacheAdapter), $cacheTraceableDefinition->getArgument(1)->getClass());
46-
47-
$cacheTraceableDefinition = $container->findDefinition('app.cache.bar');
48-
49-
$this->assertSame(TraceableCacheAdapter::class, $cacheTraceableDefinition->getClass());
50-
$this->assertInstanceOf(Definition::class, $cacheTraceableDefinition->getArgument(1));
51-
$this->assertSame(\get_class($cacheAdapter), $cacheTraceableDefinition->getArgument(1)->getClass());
52-
53-
$this->assertFalse($container->hasDefinition('app.cache.baz'));
127+
$this->assertFalse($container->hasDefinition('app.cache'));
54128
}
55129

56130
public function testProcessDoesNothingIfConditionsForEnablingTracingAreMissing(): void

‎tests/End2End/App/Controller/TracingController.php

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Sentry\SentryBundle\Tests\End2End\App\Controller;
6+
7+
use Doctrine\DBAL\Connection;
8+
use Sentry\State\HubInterface;
9+
use Sentry\Tracing\SpanContext;
10+
use Symfony\Component\HttpFoundation\Response;
11+
12+
class TracingController
13+
{
14+
/**
15+
* @var HubInterface
16+
*/
17+
private $hub;
18+
19+
/**
20+
* @var Connection|null
21+
*/
22+
private $connection;
23+
24+
public function __construct(HubInterface $hub, Connection $connection = null)
25+
{
26+
$this->hub = $hub;
27+
$this->connection = $connection;
28+
}
29+
30+
public function pingDatabase(): Response
31+
{
32+
$this->hub->setSpan(
33+
$this->hub->getSpan()
34+
->startChild($this->createSpan())
35+
);
36+
37+
if ($this->connection) {
38+
$this->connection->executeQuery('SELECT 1');
39+
}
40+
41+
return new Response('Success');
42+
}
43+
44+
private function createSpan(): SpanContext
45+
{
46+
$spanContext = new SpanContext();
47+
$spanContext->setOp('mock.span');
48+
$spanContext->setDescription('mocked subspan');
49+
50+
return $spanContext;
51+
}
52+
}

0 commit comments

Comments
 (0)