Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 815c45f

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
bug #40645 [FrameworkBundle] Dont store cache misses on warmup (Nyholm)
This PR was squashed before being merged into the 4.4 branch. Discussion ---------- [FrameworkBundle] Dont store cache misses on warmup | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #38694 | License | MIT | Doc PR | symfony/symfony-docs#15172 When we are warming the annotation cache, we are reading all annotation into an `ArrayAdapter`. When we are done we move the values to a `PhpArrayAdapter` to store them in a file. @Seldaek [found out](symfony/symfony#38694 (comment)) that when you are using a custom constraint with a `Symfony\Component\Validator\Constraints\Callback`, there is a strange error like: > Can use "yield from" only with arrays and Traversables That is because the `Closure` in the `Symfony\Component\Validator\Constraints\Callback` cannot be serialised and saved to cache. But since the `ArrayAdapter` is also [storing misses as null](symfony/symfony#35362), the null values are understood as real values. When all values are moved to the `PhpArrayAdapter` and we ask the cache for a value (via Doctrine's `CacheProvider`), it will return `null` as a value instead of `false` as a cache miss. And `null` is not something one could "yield from". Commits ------- 27a22b34af [FrameworkBundle] Dont store cache misses on warmup
2 parents 4feecba + b7ad105 commit 815c45f
Copy full SHA for 815c45f

File tree

3 files changed

+42
-1
lines changed
Filter options

3 files changed

+42
-1
lines changed

‎CacheWarmer/AnnotationsCacheWarmer.php

Copy file name to clipboardExpand all lines: CacheWarmer/AnnotationsCacheWarmer.php
+9Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Doctrine\Common\Annotations\Reader;
1717
use Psr\Cache\CacheItemPoolInterface;
1818
use Symfony\Component\Cache\Adapter\ArrayAdapter;
19+
use Symfony\Component\Cache\Adapter\PhpArrayAdapter;
1920
use Symfony\Component\Cache\DoctrineProvider;
2021

2122
/**
@@ -76,6 +77,14 @@ protected function doWarmUp($cacheDir, ArrayAdapter $arrayAdapter)
7677
return true;
7778
}
7879

80+
protected function warmUpPhpArrayAdapter(PhpArrayAdapter $phpArrayAdapter, array $values)
81+
{
82+
// make sure we don't cache null values
83+
$values = array_filter($values, function ($val) { return null !== $val; });
84+
85+
parent::warmUpPhpArrayAdapter($phpArrayAdapter, $values);
86+
}
87+
7988
private function readAllComponents(Reader $reader, string $class)
8089
{
8190
$reflectionClass = new \ReflectionClass($class);

‎CacheWarmer/ValidatorCacheWarmer.php

Copy file name to clipboardExpand all lines: CacheWarmer/ValidatorCacheWarmer.php
+3-1Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,9 @@ protected function doWarmUp($cacheDir, ArrayAdapter $arrayAdapter)
8080
protected function warmUpPhpArrayAdapter(PhpArrayAdapter $phpArrayAdapter, array $values)
8181
{
8282
// make sure we don't cache null values
83-
parent::warmUpPhpArrayAdapter($phpArrayAdapter, array_filter($values));
83+
$values = array_filter($values, function ($val) { return null !== $val; });
84+
85+
parent::warmUpPhpArrayAdapter($phpArrayAdapter, $values);
8486
}
8587

8688
/**

‎Tests/CacheWarmer/AnnotationsCacheWarmerTest.php

Copy file name to clipboardExpand all lines: Tests/CacheWarmer/AnnotationsCacheWarmerTest.php
+30Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use PHPUnit\Framework\MockObject\MockObject;
99
use Symfony\Bundle\FrameworkBundle\CacheWarmer\AnnotationsCacheWarmer;
1010
use Symfony\Bundle\FrameworkBundle\Tests\TestCase;
11+
use Symfony\Component\Cache\Adapter\ArrayAdapter;
1112
use Symfony\Component\Cache\Adapter\NullAdapter;
1213
use Symfony\Component\Cache\Adapter\PhpArrayAdapter;
1314
use Symfony\Component\Cache\DoctrineProvider;
@@ -120,6 +121,35 @@ public function testClassAutoloadExceptionWithUnrelatedException()
120121
spl_autoload_unregister($classLoader);
121122
}
122123

124+
public function testWarmupRemoveCacheMisses()
125+
{
126+
$cacheFile = tempnam($this->cacheDir, __FUNCTION__);
127+
$warmer = $this->getMockBuilder(AnnotationsCacheWarmer::class)
128+
->setConstructorArgs([new AnnotationReader(), $cacheFile])
129+
->setMethods(['doWarmUp'])
130+
->getMock();
131+
132+
$warmer->method('doWarmUp')->willReturnCallback(function ($cacheDir, ArrayAdapter $arrayAdapter) {
133+
$arrayAdapter->getItem('foo_miss');
134+
135+
$item = $arrayAdapter->getItem('bar_hit');
136+
$item->set('data');
137+
$arrayAdapter->save($item);
138+
139+
$item = $arrayAdapter->getItem('baz_hit_null');
140+
$item->set(null);
141+
$arrayAdapter->save($item);
142+
143+
return true;
144+
});
145+
146+
$warmer->warmUp($this->cacheDir);
147+
$data = include $cacheFile;
148+
149+
$this->assertCount(1, $data[0]);
150+
$this->assertTrue(isset($data[0]['bar_hit']));
151+
}
152+
123153
/**
124154
* @return MockObject|Reader
125155
*/

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.