From 5e8de1e21122eaf2a4e27fb202102d0d4b6a1f56 Mon Sep 17 00:00:00 2001 From: Jonathan Johnson Date: Fri, 17 Jun 2022 01:00:30 -0700 Subject: [PATCH 1/2] [Cache] Respect $save option in ArrayAdapter When $save is passed as the second option to the callback it should be respected, even in the ephemeral array adapter. --- .../Component/Cache/Adapter/ArrayAdapter.php | 5 +++- .../Cache/Tests/Adapter/AdapterTestCase.php | 25 +++++++++++++++++++ .../Tests/Adapter/PhpArrayAdapterTest.php | 1 + 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/Cache/Adapter/ArrayAdapter.php b/src/Symfony/Component/Cache/Adapter/ArrayAdapter.php index 157043abef18..20043dec18c5 100644 --- a/src/Symfony/Component/Cache/Adapter/ArrayAdapter.php +++ b/src/Symfony/Component/Cache/Adapter/ArrayAdapter.php @@ -60,7 +60,10 @@ public function get(string $key, callable $callback, float $beta = null, array & // ArrayAdapter works in memory, we don't care about stampede protection if (\INF === $beta || !$item->isHit()) { $save = true; - $this->save($item->set($callback($item, $save))); + $item->set($callback($item, $save)); + if ($save) { + $this->save($item); + } } return $item->get(); diff --git a/src/Symfony/Component/Cache/Tests/Adapter/AdapterTestCase.php b/src/Symfony/Component/Cache/Tests/Adapter/AdapterTestCase.php index 123cda89b872..da5527034822 100644 --- a/src/Symfony/Component/Cache/Tests/Adapter/AdapterTestCase.php +++ b/src/Symfony/Component/Cache/Tests/Adapter/AdapterTestCase.php @@ -98,6 +98,31 @@ public function testRecursiveGet() $this->assertSame(1, $cache->get('k2', function () { return 2; })); } + public function testDontSaveWhenAskedNotTo() + { + if (isset($this->skippedTests[__FUNCTION__])) { + $this->markTestSkipped($this->skippedTests[__FUNCTION__]); + } + + $cache = $this->createCachePool(0, __FUNCTION__); + + $v1 = $cache->get('some-key', function($item, &$save){ + $save = false; + return 1; + }); + $this->assertSame($v1, 1); + + $v2 = $cache->get('some-key', function(){ + return 2; + }); + $this->assertSame($v2, 2, 'First value was cached and should not have been'); + + $v3 = $cache->get('some-key', function(){ + $this->fail('Value should have come from cache'); + }); + $this->assertSame($v3, 2); + } + public function testGetMetadata() { if (isset($this->skippedTests[__FUNCTION__])) { diff --git a/src/Symfony/Component/Cache/Tests/Adapter/PhpArrayAdapterTest.php b/src/Symfony/Component/Cache/Tests/Adapter/PhpArrayAdapterTest.php index 58318f6a2529..b7ced48abf7c 100644 --- a/src/Symfony/Component/Cache/Tests/Adapter/PhpArrayAdapterTest.php +++ b/src/Symfony/Component/Cache/Tests/Adapter/PhpArrayAdapterTest.php @@ -24,6 +24,7 @@ class PhpArrayAdapterTest extends AdapterTestCase { protected $skippedTests = [ 'testGet' => 'PhpArrayAdapter is read-only.', + 'testDontSaveWhenAskedNotTo' => 'PhpArrayAdapter is read-only.', 'testRecursiveGet' => 'PhpArrayAdapter is read-only.', 'testBasicUsage' => 'PhpArrayAdapter is read-only.', 'testBasicUsageWithLongKey' => 'PhpArrayAdapter is read-only.', From b14aa77e4debf903507899bb85ec6015a7382cad Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Sun, 19 Jun 2022 13:27:12 +0200 Subject: [PATCH 2/2] [Cache] Respect $save option in ChainAdapter --- src/Symfony/Component/Cache/Adapter/ChainAdapter.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/Cache/Adapter/ChainAdapter.php b/src/Symfony/Component/Cache/Adapter/ChainAdapter.php index 39d9afd0e057..257b0734095d 100644 --- a/src/Symfony/Component/Cache/Adapter/ChainAdapter.php +++ b/src/Symfony/Component/Cache/Adapter/ChainAdapter.php @@ -91,9 +91,17 @@ static function ($sourceItem, $item, $sourceMetadata = null) use ($defaultLifeti */ public function get(string $key, callable $callback, float $beta = null, array &$metadata = null) { + $doSave = true; + $callback = static function (CacheItem $item, bool &$save) use ($callback, &$doSave) { + $value = $callback($item, $save); + $doSave = $save; + + return $value; + }; + $lastItem = null; $i = 0; - $wrap = function (CacheItem $item = null) use ($key, $callback, $beta, &$wrap, &$i, &$lastItem, &$metadata) { + $wrap = function (CacheItem $item = null, bool &$save = true) use ($key, $callback, $beta, &$wrap, &$i, &$doSave, &$lastItem, &$metadata) { $adapter = $this->adapters[$i]; if (isset($this->adapters[++$i])) { $callback = $wrap; @@ -107,6 +115,7 @@ public function get(string $key, callable $callback, float $beta = null, array & if (null !== $item) { ($this->syncItem)($lastItem = $lastItem ?? $item, $item, $metadata); } + $save = $doSave; return $value; };