Skip to content

Commit 2657892

Browse files
committed
Fix DeviceDetectorCache cache eviction flakiness
1 parent a92e044 commit 2657892

3 files changed

Lines changed: 26 additions & 4 deletions

File tree

CachedEntry.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ public static function getCached(string $userAgent): ?array
6060
if ($exists) {
6161
$values = @include($path);
6262
if (!empty($values) && is_array($values) && isset($values['os'])) {
63+
self::refreshAccessTime($path);
6364
return $values;
6465
}
6566
}
@@ -165,6 +166,16 @@ private static function getCacheFilesInCacheDir(): array
165166
});
166167
}
167168

169+
private static function refreshAccessTime(string $path): void
170+
{
171+
$modifiedTime = @filemtime($path);
172+
if ($modifiedTime === false) {
173+
return;
174+
}
175+
176+
@touch($path, $modifiedTime, time());
177+
}
178+
168179
public static function deleteLeastAccessedFiles(int $numFilesToDelete): void
169180
{
170181
if ($numFilesToDelete < 1) {

tests/Integration/CachedEntryTest.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,18 @@ public function test_writeToCache_GetCached()
190190
);
191191
}
192192

193+
public function testGetCached_refreshesAccessTime()
194+
{
195+
$filePath = CachedEntry::writeToCache('foo', []);
196+
$accessTimeBefore = fileatime($filePath);
197+
198+
sleep(1);
199+
CachedEntry::getCached('foo', []);
200+
clearstatcache(true, $filePath);
201+
202+
$this->assertGreaterThan($accessTimeBefore, fileatime($filePath));
203+
}
204+
193205
public function test_getCachePath()
194206
{
195207
$path1 = CachedEntry::getCachePath('foo');

tests/Integration/WarmDeviceDetectorCacheTest.php

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,8 @@ public function testDoesNotClearExistingFilesFromCacheByDefault()
106106

107107
public function testDoesClearExistingFilesFromCacheByDefaultWhenTooManyEntriesExist()
108108
{
109-
// was last accessed
110-
$userAgentKept = 'Mozilla/5.0 (Windows NT 6.1; WOW64; Trident/7.0; rv:11.0) like Gecko';
111-
$userAgentDeleted = 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/75.0.3770.142 Safari/537.36';
109+
$userAgentKept = 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/75.0.3770.142 Safari/537.36';
110+
$userAgentDeleted = 'Mozilla/5.0 (Windows NT 6.1; WOW64; Trident/7.0; rv:11.0) like Gecko';
112111
$cacheFilePathKept = CachedEntry::getCachePath($userAgentKept, true);
113112
$cacheFilePathDeleted = CachedEntry::getCachePath($userAgentDeleted, true);
114113

@@ -128,7 +127,7 @@ public function testDoesClearExistingFilesFromCacheByDefaultWhenTooManyEntriesEx
128127

129128
$this->setCountProcessNumEntries(1);
130129

131-
// now we run again and it should delete 2 of the entries
130+
// now we run again and it should refresh the most frequent entry and delete the other 2
132131
$this->applicationTester->run([
133132
'command' => WarmDeviceDetectorCache::COMMAND_NAME,
134133
]);

0 commit comments

Comments
 (0)