Skip to content

Commit 551b0b6

Browse files
ste93cryste93cry
authoredJun 18, 2021
Fix decoration of cache adapters inheriting parent service (#525)
* Fix decoration of cache adapters inheriting parent service * Fix CR issues
·
5.2.04.1.4
1 parent 5f5fe82 commit 551b0b6

File tree

7 files changed

+166
-31
lines changed

7 files changed

+166
-31
lines changed
 

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
## Unreleased
44

5+
- Fix decoration of cache adapters inheriting parent service (#525)
56
- Fix extraction of the username of the logged-in user in Symfony 5.3 (#518)
67

78
## 4.1.3 (2021-05-31)

‎phpstan-baseline.neon

+12-2
Original file line numberDiff line numberDiff line change
@@ -226,9 +226,19 @@ parameters:
226226
path: tests/Tracing/Cache/AbstractTraceableCacheAdapterTest.php
227227

228228
-
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\\.$#"
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\\\\CacheInterface given\\.$#"
230+
count: 2
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\\\\PruneableCacheAdapterInterface given\\.$#"
230235
count: 1
231-
path: tests/Tracing/Cache/TraceableTagAwareCacheAdapterTest.php
236+
path: tests/Tracing/Cache/AbstractTraceableCacheAdapterTest.php
237+
238+
-
239+
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\\.$#"
240+
count: 1
241+
path: tests/Tracing/Cache/AbstractTraceableCacheAdapterTest.php
232242

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

‎src/DependencyInjection/Compiler/CacheTracingPass.php

+20-1
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
}

‎tests/DependencyInjection/Compiler/CacheTracingPassTest.php

+96-22
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/Tracing/Cache/AbstractTraceableCacheAdapterTest.php

+5-2
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
/**
2020
* @phpstan-template TCacheAdapter of AdapterInterface
21+
* @phpstan-template TDecoratedCacheAdapter of AdapterInterface
2122
*/
2223
abstract class AbstractTraceableCacheAdapterTest extends TestCase
2324
{
@@ -410,12 +411,14 @@ private static function isCachePackageInstalled(): bool
410411
}
411412

412413
/**
414+
* @phpstan-param TDecoratedCacheAdapter $decoratedAdapter
415+
*
413416
* @phpstan-return TCacheAdapter
414417
*/
415-
abstract protected function createCacheAdapter(AdapterInterface $decoratedAdapter): AdapterInterface;
418+
abstract protected function createCacheAdapter(AdapterInterface $decoratedAdapter);
416419

417420
/**
418-
* @return class-string<AdapterInterface>
421+
* @return class-string<TDecoratedCacheAdapter>
419422
*/
420423
abstract protected static function getAdapterClassFqcn(): string;
421424
}

‎tests/Tracing/Cache/TraceableCacheAdapterTest.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,14 @@
88
use Symfony\Component\Cache\Adapter\AdapterInterface;
99

1010
/**
11-
* @phpstan-extends AbstractTraceableCacheAdapterTest<TraceableCacheAdapter>
11+
* @phpstan-extends AbstractTraceableCacheAdapterTest<TraceableCacheAdapter, AdapterInterface>
1212
*/
1313
final class TraceableCacheAdapterTest extends AbstractTraceableCacheAdapterTest
1414
{
1515
/**
1616
* {@inheritdoc}
1717
*/
18-
protected function createCacheAdapter(AdapterInterface $decoratedAdapter): AdapterInterface
18+
protected function createCacheAdapter(AdapterInterface $decoratedAdapter): TraceableCacheAdapter
1919
{
2020
return new TraceableCacheAdapter($this->hub, $decoratedAdapter);
2121
}

‎tests/Tracing/Cache/TraceableTagAwareCacheAdapterTest.php

+30-2
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,46 @@
55
namespace Sentry\SentryBundle\Tests\Tracing\Cache;
66

77
use Sentry\SentryBundle\Tracing\Cache\TraceableTagAwareCacheAdapter;
8+
use Sentry\Tracing\Transaction;
9+
use Sentry\Tracing\TransactionContext;
810
use Symfony\Component\Cache\Adapter\AdapterInterface;
911
use Symfony\Component\Cache\Adapter\TagAwareAdapterInterface;
1012

1113
/**
12-
* @phpstan-extends AbstractTraceableCacheAdapterTest<TraceableTagAwareCacheAdapter>
14+
* @phpstan-extends AbstractTraceableCacheAdapterTest<TraceableTagAwareCacheAdapter, TagAwareAdapterInterface>
1315
*/
1416
final class TraceableTagAwareCacheAdapterTest extends AbstractTraceableCacheAdapterTest
1517
{
18+
public function testInvalidateTags(): void
19+
{
20+
$transaction = new Transaction(new TransactionContext(), $this->hub);
21+
$transaction->initSpanRecorder();
22+
23+
$this->hub->expects($this->once())
24+
->method('getSpan')
25+
->willReturn($transaction);
26+
27+
$decoratedAdapter = $this->createMock(self::getAdapterClassFqcn());
28+
$decoratedAdapter->expects($this->once())
29+
->method('invalidateTags')
30+
->with(['foo'])
31+
->willReturn(true);
32+
33+
$adapter = $this->createCacheAdapter($decoratedAdapter);
34+
35+
$this->assertTrue($adapter->invalidateTags(['foo']));
36+
37+
$spans = $transaction->getSpanRecorder()->getSpans();
38+
39+
$this->assertCount(2, $spans);
40+
$this->assertSame('cache.invalidate_tags', $spans[1]->getOp());
41+
$this->assertNotNull($spans[1]->getEndTimestamp());
42+
}
43+
1644
/**
1745
* {@inheritdoc}
1846
*/
19-
protected function createCacheAdapter(AdapterInterface $decoratedAdapter): AdapterInterface
47+
protected function createCacheAdapter(AdapterInterface $decoratedAdapter): TraceableTagAwareCacheAdapter
2048
{
2149
return new TraceableTagAwareCacheAdapter($this->hub, $decoratedAdapter);
2250
}

0 commit comments

Comments
 (0)
Please sign in to comment.