Skip to content

Commit

Permalink
[10.x][Cache] Fix handling of false values in apc (#49145)
Browse files Browse the repository at this point in the history
* [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
  • Loading branch information
simivar authored Nov 27, 2023
1 parent 6b01dc4 commit 9bc5b59
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 6 deletions.
6 changes: 1 addition & 5 deletions src/Illuminate/Cache/ApcStore.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
4 changes: 3 additions & 1 deletion src/Illuminate/Cache/ApcWrapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
8 changes: 8 additions & 0 deletions tests/Cache/CacheApcStoreTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 9bc5b59

Please sign in to comment.