Skip to content

Commit a940510

Browse files
daftspunkdaftspunk
authored
Merge pull request #677 from samuelpatro/perf/attachment-memo
Fix memoized attachment existence after generation and deletion
2 parents a2daf50 + cb96ec5 commit a940510

2 files changed

Lines changed: 222 additions & 3 deletions

File tree

‎src/Database/Attach/File.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -723,7 +723,7 @@ protected function makeThumbStorage($thumbFile, $thumbPath, $width, $height, $op
723723

724724
// Eagerly cache remote exists call
725725
if ($success) {
726-
Cache::forever($this->getCacheKey($thumbPath), true);
726+
Cache::memo()->forever($this->getCacheKey($thumbPath), true);
727727
}
728728
}
729729

@@ -752,7 +752,7 @@ public function deleteThumbs()
752752
$this->getDisk()->delete($collection);
753753

754754
foreach ($collection as $filePath) {
755-
Cache::forget($this->getCacheKey($filePath));
755+
Cache::memo()->forget($this->getCacheKey($filePath));
756756
}
757757
}
758758
}
@@ -863,7 +863,7 @@ protected function deleteFile($fileName = null)
863863

864864
// Clear remote storage cache
865865
if (!$this->isLocalStorage()) {
866-
Cache::forget($this->getCacheKey($filePath));
866+
Cache::memo()->forget($this->getCacheKey($filePath));
867867
}
868868

869869
$this->deleteEmptyDirectory($directory);

‎tests/Database/Attach/FileCacheTest.php‎

Lines changed: 219 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,219 @@
1+
<?php
2+
3+
use Illuminate\Cache\CacheManager;
4+
use Illuminate\Config\Repository;
5+
use Illuminate\Container\Container;
6+
use Illuminate\Database\Capsule\Manager as Capsule;
7+
use Illuminate\Filesystem\Filesystem;
8+
use Illuminate\Support\Facades\Facade;
9+
use October\Rain\Database\Attach\File as Attachment;
10+
11+
class FileCacheTest extends TestCase
12+
{
13+
protected $savedFacadeApp;
14+
protected $savedFacadeInstances;
15+
protected $savedResolver;
16+
protected $savedDispatcher;
17+
protected $cache;
18+
protected $file;
19+
protected $tempPath;
20+
21+
public function setUp(): void
22+
{
23+
parent::setUp();
24+
25+
$this->savedFacadeApp = Facade::getFacadeApplication();
26+
$this->savedFacadeInstances = (new ReflectionProperty(Facade::class, 'resolvedInstance'))->getValue();
27+
$this->savedResolver = Attachment::getConnectionResolver();
28+
$this->savedDispatcher = Attachment::getEventDispatcher();
29+
30+
$app = new Container;
31+
$app->instance('config', new Repository([
32+
'cache' => [
33+
'default' => 'array',
34+
'stores' => ['array' => ['driver' => 'array']],
35+
],
36+
]));
37+
$this->cache = new CacheManager($app);
38+
$app->instance('cache', $this->cache);
39+
$app->instance('files', new Filesystem);
40+
Facade::clearResolvedInstances();
41+
Facade::setFacadeApplication($app);
42+
43+
$capsule = new Capsule($app);
44+
$capsule->addConnection(['driver' => 'sqlite', 'database' => ':memory:', 'prefix' => '']);
45+
$capsule->bootEloquent();
46+
47+
$this->tempPath = sys_get_temp_dir() . '/october-file-cache-test-' . uniqid();
48+
mkdir($this->tempPath);
49+
50+
$this->file = new FileCacheTestAttachment;
51+
$this->file->tempPath = $this->tempPath;
52+
$this->file->setRawAttributes([
53+
'id' => 42,
54+
'file_name' => 'photo.jpg',
55+
'disk_name' => 'abcdef123.jpg',
56+
'is_public' => true,
57+
]);
58+
$this->file->disk = new FileCacheTestDisk;
59+
$this->file->disk->paths[$this->file->getDiskPath()] = true;
60+
}
61+
62+
public function tearDown(): void
63+
{
64+
array_map('unlink', glob($this->tempPath . '/*'));
65+
rmdir($this->tempPath);
66+
67+
if ($this->savedResolver) {
68+
Attachment::setConnectionResolver($this->savedResolver);
69+
}
70+
else {
71+
Attachment::unsetConnectionResolver();
72+
}
73+
if ($this->savedDispatcher) {
74+
Attachment::setEventDispatcher($this->savedDispatcher);
75+
}
76+
else {
77+
Attachment::unsetEventDispatcher();
78+
}
79+
80+
Facade::clearResolvedInstances();
81+
Facade::setFacadeApplication($this->savedFacadeApp);
82+
(new ReflectionProperty(Facade::class, 'resolvedInstance'))->setValue(null, $this->savedFacadeInstances);
83+
84+
parent::tearDown();
85+
}
86+
87+
public function testPersistedMissingThumbnailIsGeneratedOnce()
88+
{
89+
$thumb = $this->file->getThumbFilename(100, 100, []);
90+
$key = $this->file->getCacheKey($this->file->getDiskPath($thumb));
91+
$this->cache->forever($key, false);
92+
93+
$this->assertSame($this->file->getPath($thumb), $this->file->getThumb(100, 100));
94+
$this->assertSame($this->file->getPath($thumb), $this->file->getThumb(100, 100));
95+
$this->assertSame(1, $this->file->publications);
96+
$this->assertTrue($this->cache->get($key));
97+
}
98+
99+
public function testColdMissingThumbnailIsGeneratedOnce()
100+
{
101+
$thumb = $this->file->getThumbFilename(100, 100, []);
102+
103+
$this->assertSame($this->file->getPath($thumb), $this->file->getThumb(100, 100));
104+
$this->assertSame($this->file->getPath($thumb), $this->file->getThumb(100, 100));
105+
$this->assertSame(1, $this->file->publications);
106+
}
107+
108+
public function testDeletingThumbnailInvalidatesMemoizedExistence()
109+
{
110+
$thumb = $this->file->getThumbFilename(100, 100, []);
111+
$path = $this->file->getDiskPath($thumb);
112+
$key = $this->file->getCacheKey($path);
113+
$this->file->disk->paths[$path] = true;
114+
$this->cache->forever($key, true);
115+
116+
$this->assertSame($this->file->getPath($thumb), $this->file->getThumb(100, 100));
117+
$this->assertSame(0, $this->file->publications);
118+
119+
$this->file->deleteThumbs();
120+
121+
$this->assertArrayNotHasKey($path, $this->file->disk->paths);
122+
$this->assertNull($this->cache->get($key));
123+
$this->assertSame($this->file->getPath($thumb), $this->file->getThumb(100, 100));
124+
$this->assertSame(1, $this->file->publications);
125+
}
126+
127+
public function testDeletingOriginalFileInvalidatesMemoizedExistence()
128+
{
129+
$key = $this->file->getCacheKey();
130+
$this->cache->forever($key, true);
131+
$this->assertTrue(self::callProtectedMethod($this->file, 'hasFile'));
132+
133+
self::callProtectedMethod($this->file, 'deleteFile');
134+
135+
$this->assertArrayNotHasKey($this->file->getDiskPath(), $this->file->disk->paths);
136+
$this->assertNull($this->cache->get($key));
137+
$this->assertFalse(self::callProtectedMethod($this->file, 'hasFile'));
138+
$this->assertSame($this->file->getUrl(), $this->file->getThumb(100, 100));
139+
$this->assertSame(0, $this->file->publications);
140+
}
141+
}
142+
143+
/**
144+
* FileCacheTestAttachment replaces remote storage transfers with an in-memory disk.
145+
* Thumbnails are still generated by the real resizer from a small generated image.
146+
*/
147+
class FileCacheTestAttachment extends Attachment
148+
{
149+
public $disk;
150+
public $tempPath;
151+
public $publications = 0;
152+
153+
public function getDisk()
154+
{
155+
return $this->disk;
156+
}
157+
158+
protected function isLocalStorage()
159+
{
160+
return false;
161+
}
162+
163+
public function getTempPath()
164+
{
165+
return $this->tempPath;
166+
}
167+
168+
protected function copyStorageToLocal($storagePath, $localPath)
169+
{
170+
return imagejpeg(imagecreatetruecolor(4, 4), $localPath);
171+
}
172+
173+
protected function copyLocalToStorage($localPath, $storagePath)
174+
{
175+
$this->publications++;
176+
$this->disk->paths[$storagePath] = true;
177+
178+
return true;
179+
}
180+
}
181+
182+
/**
183+
* FileCacheTestDisk keeps the remote file listing in memory.
184+
*/
185+
class FileCacheTestDisk
186+
{
187+
public $paths = [];
188+
189+
public function exists($path)
190+
{
191+
return isset($this->paths[$path]);
192+
}
193+
194+
public function files($directory)
195+
{
196+
return array_values(array_filter(array_keys($this->paths), function ($path) use ($directory) {
197+
return str_starts_with($path, $directory);
198+
}));
199+
}
200+
201+
public function allFiles($directory)
202+
{
203+
return $this->files($directory);
204+
}
205+
206+
public function delete($paths)
207+
{
208+
foreach ((array) $paths as $path) {
209+
unset($this->paths[$path]);
210+
}
211+
212+
return true;
213+
}
214+
215+
public function deleteDirectory($directory)
216+
{
217+
return true;
218+
}
219+
}

0 commit comments

Comments
 (0)