From 9bc5b59c3fde9e93ecca749923905a80388159f6 Mon Sep 17 00:00:00 2001 From: Krystian Marcisz Date: Mon, 27 Nov 2023 15:35:29 +0100 Subject: [PATCH] [10.x][Cache] Fix handling of `false` values in apc (#49145) * [10.x][Cache] Fix handling of `false` values in apc This commit addresses an issue in the `ApcWrapper` and `ApcStore` components of the caching system, particularly when dealing with a cached value of `false`. Previously, the `ApcWrapper`'s default behavior was to simply retrieve and return values from the APC cache store. However, in `ApcStore::get()`, there was a check to determine if the returned value was not `false`. If it wasn't, the value would be returned. This posed a problem where a `false` value is legitimately stored in the cache, but the existing check led to unnecessary cache misses and repeated `get` & `store` operations. To resolve this, we have modified the implementation. Now, we leverage the second parameter of `apc_fetch`, which indicates whether the fetch operation was successful. This allows `ApcStore` to accurately differentiate between a successful fetch of a `false` value and a failed fetch operation. * Add missing test --- src/Illuminate/Cache/ApcStore.php | 6 +----- src/Illuminate/Cache/ApcWrapper.php | 4 +++- tests/Cache/CacheApcStoreTest.php | 8 ++++++++ 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/Illuminate/Cache/ApcStore.php b/src/Illuminate/Cache/ApcStore.php index 90132c16f45f..5779635b203b 100755 --- a/src/Illuminate/Cache/ApcStore.php +++ b/src/Illuminate/Cache/ApcStore.php @@ -41,11 +41,7 @@ public function __construct(ApcWrapper $apc, $prefix = '') */ public function get($key) { - $value = $this->apc->get($this->prefix.$key); - - if ($value !== false) { - return $value; - } + return $this->apc->get($this->prefix.$key); } /** diff --git a/src/Illuminate/Cache/ApcWrapper.php b/src/Illuminate/Cache/ApcWrapper.php index 6c129c633288..8f6ce488180f 100755 --- a/src/Illuminate/Cache/ApcWrapper.php +++ b/src/Illuminate/Cache/ApcWrapper.php @@ -29,7 +29,9 @@ public function __construct() */ public function get($key) { - return $this->apcu ? apcu_fetch($key) : apc_fetch($key); + $fetchedValue = $this->apcu ? apcu_fetch($key, $success) : apc_fetch($key, $success); + + return $success ? $fetchedValue : null; } /** diff --git a/tests/Cache/CacheApcStoreTest.php b/tests/Cache/CacheApcStoreTest.php index fed3a9dc8cc1..e13d2adf63ec 100755 --- a/tests/Cache/CacheApcStoreTest.php +++ b/tests/Cache/CacheApcStoreTest.php @@ -25,6 +25,14 @@ public function testAPCValueIsReturned() $this->assertSame('bar', $store->get('foo')); } + public function testAPCFalseValueIsReturned() + { + $apc = $this->getMockBuilder(ApcWrapper::class)->onlyMethods(['get'])->getMock(); + $apc->expects($this->once())->method('get')->willReturn(false); + $store = new ApcStore($apc); + $this->assertFalse($store->get('foo')); + } + public function testGetMultipleReturnsNullWhenNotFoundAndValueWhenFound() { $apc = $this->getMockBuilder(ApcWrapper::class)->onlyMethods(['get'])->getMock();