Skip to content

Commit 406a0dc

Browse files
andrew-dembandrew-demb
authored
🐛 [thecodingmachine#711] Don't reuse cache for differently configured class finder instances (thecodingmachine#712)
1 parent d45b9b9 commit 406a0dc

17 files changed

+285
-54
lines changed

‎src/Discovery/Cache/HardClassFinderComputedCache.php

+4-2
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,13 @@
88
use ReflectionClass;
99
use TheCodingMachine\GraphQLite\Discovery\ClassFinder;
1010

11+
use function sprintf;
12+
1113
class HardClassFinderComputedCache implements ClassFinderComputedCache
1214
{
1315
public function __construct(
1416
private readonly CacheInterface $cache,
15-
)
16-
{
17+
) {
1718
}
1819

1920
/**
@@ -32,6 +33,7 @@ public function compute(
3233
callable $reduce,
3334
): mixed
3435
{
36+
$key = sprintf('%s.%s', $key, $classFinder->hash());
3537
$result = $this->cache->get($key);
3638

3739
if ($result !== null) {

‎src/Discovery/Cache/SnapshotClassFinderComputedCache.php

+3
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
use TheCodingMachine\GraphQLite\Cache\FilesSnapshot;
1010
use TheCodingMachine\GraphQLite\Discovery\ClassFinder;
1111

12+
use function sprintf;
13+
1214
/**
1315
* Provides cache for a {@see ClassFinder} based on a {@see filemtime()}.
1416
*
@@ -49,6 +51,7 @@ public function compute(
4951
callable $reduce,
5052
): mixed
5153
{
54+
$key = sprintf('%s.%s', $key, $classFinder->hash());
5255
$entries = $this->entries($classFinder, $key . '.entries', $map);
5356

5457
return $reduce($entries);

‎src/Discovery/ClassFinder.php

+5
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,9 @@
1111
interface ClassFinder extends IteratorAggregate
1212
{
1313
public function withPathFilter(callable $filter): self;
14+
15+
/**
16+
* Path filter does not affect the hash.
17+
*/
18+
public function hash(): string;
1419
}

‎src/Discovery/KcsClassFinder.php

+7-2
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ class KcsClassFinder implements ClassFinder
1212
{
1313
public function __construct(
1414
private FinderInterface $finder,
15-
)
16-
{
15+
private readonly string $hash,
16+
) {
1717
}
1818

1919
public function withPathFilter(callable $filter): ClassFinder
@@ -29,4 +29,9 @@ public function getIterator(): Traversable
2929
{
3030
return $this->finder->getIterator();
3131
}
32+
33+
public function hash(): string
34+
{
35+
return $this->hash;
36+
}
3237
}

‎src/Discovery/StaticClassFinder.php

+10
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,16 @@
77
use ReflectionClass;
88
use Traversable;
99

10+
use function md5;
11+
use function serialize;
12+
1013
class StaticClassFinder implements ClassFinder
1114
{
1215
/** @var (callable(string): bool)|null */
1316
private mixed $pathFilter = null;
1417

18+
private string|null $hash = null;
19+
1520
/** @param list<class-string> $classes */
1621
public function __construct(
1722
private readonly array $classes,
@@ -41,4 +46,9 @@ public function getIterator(): Traversable
4146
yield $class => $classReflection;
4247
}
4348
}
49+
50+
public function hash(): string
51+
{
52+
return $this->hash ??= md5(serialize($this->classes));
53+
}
4454
}

‎src/SchemaFactory.php

+4-1
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@
7070

7171
use function array_reverse;
7272
use function class_exists;
73+
use function implode;
7374
use function md5;
7475
use function substr;
7576
use function trigger_error;
@@ -565,6 +566,8 @@ private function createClassFinder(): ClassFinder
565566
$finder = $finder->inNamespace($namespace);
566567
}
567568

568-
return new KcsClassFinder($finder);
569+
$hash = md5(implode(',', $this->namespaces));
570+
571+
return new KcsClassFinder($finder, $hash);
569572
}
570573
}

‎tests/AbstractQueryProvider.php

+7-2
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@
2929
use TheCodingMachine\GraphQLite\Discovery\Cache\ClassFinderComputedCache;
3030
use TheCodingMachine\GraphQLite\Discovery\Cache\HardClassFinderComputedCache;
3131
use TheCodingMachine\GraphQLite\Discovery\ClassFinder;
32-
use TheCodingMachine\GraphQLite\Discovery\StaticClassFinder;
3332
use TheCodingMachine\GraphQLite\Discovery\KcsClassFinder;
33+
use TheCodingMachine\GraphQLite\Discovery\StaticClassFinder;
3434
use TheCodingMachine\GraphQLite\Fixtures\Mocks\MockResolvableInputObjectType;
3535
use TheCodingMachine\GraphQLite\Fixtures\TestObject;
3636
use TheCodingMachine\GraphQLite\Fixtures\TestObject2;
@@ -67,6 +67,9 @@
6767
use TheCodingMachine\GraphQLite\Types\ResolvableMutableInputInterface;
6868
use TheCodingMachine\GraphQLite\Types\TypeResolver;
6969

70+
use function implode;
71+
use function md5;
72+
7073
abstract class AbstractQueryProvider extends TestCase
7174
{
7275
private $testObjectType;
@@ -481,7 +484,9 @@ protected function getClassFinder(array|string $namespaces): ClassFinder
481484

482485
$finder = $finder->withFileFinder(new CachedFileFinder(new DefaultFileFinder(), $arrayAdapter));
483486

484-
return new KcsClassFinder($finder);
487+
$hash = md5(implode(',', (array) $namespaces));
488+
489+
return new KcsClassFinder($finder, $hash);
485490
}
486491

487492
protected function getClassFinderComputedCache(): ClassFinderComputedCache

‎tests/Discovery/Cache/HardClassFinderComputedCacheTest.php

+51-8
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
11
<?php
22

3+
declare(strict_types=1);
4+
35
namespace TheCodingMachine\GraphQLite\Discovery\Cache;
46

57
use PHPUnit\Framework\Attributes\CoversClass;
68
use PHPUnit\Framework\TestCase;
9+
use ReflectionClass;
710
use Symfony\Component\Cache\Adapter\ArrayAdapter;
811
use Symfony\Component\Cache\Psr16Cache;
912
use TheCodingMachine\GraphQLite\Discovery\StaticClassFinder;
@@ -12,10 +15,12 @@
1215
use TheCodingMachine\GraphQLite\Fixtures\Types\FooType;
1316
use TheCodingMachine\GraphQLite\Loggers\ExceptionLogger;
1417

18+
use function array_values;
19+
1520
#[CoversClass(HardClassFinderComputedCache::class)]
1621
class HardClassFinderComputedCacheTest extends TestCase
1722
{
18-
public function testCachesResultOfReduceFunction(): void
23+
public function testNotReusedCacheBecauseDifferentList(): void
1924
{
2025
$arrayAdapter = new ArrayAdapter();
2126
$arrayAdapter->setLogger(new ExceptionLogger());
@@ -30,8 +35,46 @@ public function testCachesResultOfReduceFunction(): void
3035
TestType::class,
3136
]),
3237
'key',
33-
fn (\ReflectionClass $reflection) => $reflection->getShortName(),
34-
fn (array $entries) => [array_values($entries)],
38+
static fn (ReflectionClass $reflection) => $reflection->getShortName(),
39+
static fn (array $entries) => [array_values($entries)],
40+
);
41+
42+
$this->assertSame([
43+
'FooType',
44+
'FooExtendType',
45+
'TestType',
46+
], $result);
47+
48+
// Class finder have different class list - result should not be reused from the cache.
49+
// This is necessary to avoid caching issues when there're multiple class finders shares the same cache pool.
50+
[$result] = $classFinderComputedCache->compute(
51+
new StaticClassFinder([FooType::class]),
52+
'key',
53+
static fn (ReflectionClass $reflection) => $reflection->getShortName(),
54+
static fn (array $entries) => [array_values($entries)],
55+
);
56+
57+
$this->assertSame(['FooType'], $result);
58+
}
59+
60+
public function testReusedCacheBecauseSameList(): void
61+
{
62+
$arrayAdapter = new ArrayAdapter();
63+
$arrayAdapter->setLogger(new ExceptionLogger());
64+
$cache = new Psr16Cache($arrayAdapter);
65+
66+
$classFinderComputedCache = new HardClassFinderComputedCache($cache);
67+
68+
$classList = [
69+
FooType::class,
70+
FooExtendType::class,
71+
TestType::class,
72+
];
73+
[$result] = $classFinderComputedCache->compute(
74+
new StaticClassFinder($classList),
75+
'key',
76+
static fn (ReflectionClass $reflection) => $reflection->getShortName(),
77+
static fn (array $entries) => [array_values($entries)],
3578
);
3679

3780
$this->assertSame([
@@ -40,13 +83,13 @@ public function testCachesResultOfReduceFunction(): void
4083
'TestType',
4184
], $result);
4285

43-
// Even though the class finder and both functions have changed - the result should still be cached.
86+
// Class finder have the same class list - even both functions have changed - the result should be cached.
4487
// This is useful in production, where code and file structure doesn't change.
4588
[$result] = $classFinderComputedCache->compute(
46-
new StaticClassFinder([]),
89+
new StaticClassFinder($classList),
4790
'key',
48-
fn (\ReflectionClass $reflection) => self::fail('Should not be called.'),
49-
fn (array $entries) => self::fail('Should not be called.'),
91+
static fn (ReflectionClass $reflection) => self::fail('Should not be called.'),
92+
static fn (array $entries) => self::fail('Should not be called.'),
5093
);
5194

5295
$this->assertSame([
@@ -55,4 +98,4 @@ public function testCachesResultOfReduceFunction(): void
5598
'TestType',
5699
], $result);
57100
}
58-
}
101+
}

‎tests/Discovery/Cache/SnapshotClassFinderComputedCacheTest.php

+24-27
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,46 @@
11
<?php
22

3+
declare(strict_types=1);
4+
35
namespace TheCodingMachine\GraphQLite\Discovery\Cache;
46

57
use PHPUnit\Framework\Attributes\CoversClass;
68
use PHPUnit\Framework\TestCase;
9+
use ReflectionClass;
710
use Symfony\Component\Cache\Adapter\ArrayAdapter;
811
use Symfony\Component\Cache\Psr16Cache;
912
use TheCodingMachine\GraphQLite\Discovery\StaticClassFinder;
1013
use TheCodingMachine\GraphQLite\Fixtures\TestType;
11-
use TheCodingMachine\GraphQLite\Fixtures\Types\EnumType;
1214
use TheCodingMachine\GraphQLite\Fixtures\Types\FooExtendType;
1315
use TheCodingMachine\GraphQLite\Fixtures\Types\FooType;
1416
use TheCodingMachine\GraphQLite\Loggers\ExceptionLogger;
1517

16-
use function Safe\touch;
18+
use function array_values;
19+
use function clearstatcache;
1720
use function Safe\filemtime;
21+
use function Safe\touch;
1822

1923
#[CoversClass(SnapshotClassFinderComputedCache::class)]
2024
class SnapshotClassFinderComputedCacheTest extends TestCase
2125
{
22-
public function testCachesIndividualEntries(): void
26+
public function testCachesIndividualEntriesSameList(): void
2327
{
2428
$arrayAdapter = new ArrayAdapter();
2529
$arrayAdapter->setLogger(new ExceptionLogger());
2630
$cache = new Psr16Cache($arrayAdapter);
2731

2832
$classFinderComputedCache = new SnapshotClassFinderComputedCache($cache);
2933

34+
$classList = [
35+
FooType::class,
36+
FooExtendType::class,
37+
TestType::class,
38+
];
3039
[$result] = $classFinderComputedCache->compute(
31-
new StaticClassFinder([
32-
FooType::class,
33-
FooExtendType::class,
34-
TestType::class,
35-
]),
40+
new StaticClassFinder($classList),
3641
'key',
37-
fn (\ReflectionClass $reflection) => $reflection->getShortName(),
38-
fn (array $entries) => [array_values($entries)],
42+
static fn (ReflectionClass $reflection) => $reflection->getShortName(),
43+
static fn (array $entries) => [array_values($entries)],
3944
);
4045

4146
$this->assertSame([
@@ -45,14 +50,10 @@ public function testCachesIndividualEntries(): void
4550
], $result);
4651

4752
[$result] = $classFinderComputedCache->compute(
48-
new StaticClassFinder([
49-
FooType::class,
50-
FooExtendType::class,
51-
TestType::class,
52-
]),
53+
new StaticClassFinder($classList),
5354
'key',
54-
fn (\ReflectionClass $reflection) => self::fail('Should not be called.'),
55-
fn (array $entries) => [array_values($entries)],
55+
static fn (ReflectionClass $reflection) => self::fail('Should not be called.'),
56+
static fn (array $entries) => [array_values($entries)],
5657
);
5758

5859
$this->assertSame([
@@ -61,23 +62,19 @@ public function testCachesIndividualEntries(): void
6162
'TestType',
6263
], $result);
6364

64-
$this->touch((new \ReflectionClass(FooType::class))->getFileName());
65+
$this->touch((new ReflectionClass(FooType::class))->getFileName());
6566

6667
[$result] = $classFinderComputedCache->compute(
67-
new StaticClassFinder([
68-
FooType::class,
69-
TestType::class,
70-
EnumType::class,
71-
]),
68+
new StaticClassFinder($classList),
7269
'key',
73-
fn (\ReflectionClass $reflection) => $reflection->getShortName() . ' Modified',
74-
fn (array $entries) => [array_values($entries)],
70+
static fn (ReflectionClass $reflection) => $reflection->getShortName() . ' Modified',
71+
static fn (array $entries) => [array_values($entries)],
7572
);
7673

7774
$this->assertSame([
7875
'FooType Modified',
76+
'FooExtendType',
7977
'TestType',
80-
'EnumType Modified',
8178
], $result);
8279
}
8380

@@ -86,4 +83,4 @@ private function touch(string $fileName): void
8683
touch($fileName, filemtime($fileName) + 1);
8784
clearstatcache();
8885
}
89-
}
86+
}

‎tests/Discovery/KcsClassFinderTest.php

+4-3
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,10 @@ class KcsClassFinderTest extends TestCase
2020
{
2121
public function testYieldsGivenClasses(): void
2222
{
23+
$namespaces = 'TheCodingMachine\GraphQLite\Fixtures\Types';
2324
$finder = new KcsClassFinder(
24-
(new ComposerFinder())
25-
->inNamespace('TheCodingMachine\GraphQLite\Fixtures\Types')
25+
(new ComposerFinder())->inNamespace($namespaces),
26+
md5($namespaces)
2627
);
2728

2829
$finderWithPath = $finder->withPathFilter(fn (string $path) => str_contains($path, 'FooExtendType.php'));
@@ -50,4 +51,4 @@ private function assertFoundClasses(array $expectedClasses, ClassFinder $classFi
5051
$this->assertContainsOnlyInstancesOf(\ReflectionClass::class, $result);
5152
$this->assertEqualsCanonicalizing($expectedClasses, array_keys($result));
5253
}
53-
}
54+
}

0 commit comments

Comments
 (0)