mirror of https://github.com/pixelfed/pixelfed
fix: delete superseded image/thumbnail files instead of orphaning them
Image::handleImageTransform derives the output filename from the current media_path and applies the encoder's output extension. When that differs from what is already stored (heic/avif -> jpg, or a thumbnail regenerated to a new extension), the new file landed at a different path and the previous file was left orphaned in the media directory — the source of the leftover _thumb files under public/m/_v2. Capture the path each transform supersedes and delete it after a successful write (only when the new output path differs, so we never delete what we just wrote). Remove the stale MediaDeleteLeafCleanupTest whose source change is not in the tree.pull/6988/head
Your Name
3 weeks ago
parent
fce75030e0
commit
b6d645a4d4
@ -0,0 +1,114 @@
|
||||
<?php
|
||||
|
||||
use App\Models\Media;
|
||||
use App\Models\User;
|
||||
use App\Util\Media\Image;
|
||||
use Illuminate\Foundation\Testing\LazilyRefreshDatabase;
|
||||
use Illuminate\Support\Facades\Config;
|
||||
use Illuminate\Support\Facades\Storage;
|
||||
|
||||
uses(LazilyRefreshDatabase::class);
|
||||
|
||||
/*
|
||||
|--------------------------------------------------------------------------
|
||||
| Image transform — supersede cleanup
|
||||
|--------------------------------------------------------------------------
|
||||
|
|
||||
| Regenerating a thumbnail whose output extension differs from the one on
|
||||
| disk used to leave the previous _thumb file orphaned in the media dir.
|
||||
| The transform now deletes the file it supersedes.
|
||||
|
|
||||
*/
|
||||
|
||||
beforeEach(function () {
|
||||
// Use a faked cloud disk so Image's transform goes through the Storage
|
||||
// facade end-to-end (the local branch reads/writes via raw storage_path(),
|
||||
// which Storage::fake does not intercept).
|
||||
Config::set('filesystems.default', 's3');
|
||||
Config::set('pixelfed.optimize_image', false);
|
||||
Storage::fake('s3', ['url' => 'https://cdn.test']);
|
||||
});
|
||||
|
||||
function seedPngMediaWithStaleThumb(): Media
|
||||
{
|
||||
$user = User::factory()->create();
|
||||
$user->refresh();
|
||||
$pid = $user->profile->id;
|
||||
|
||||
$leaf = 'public/m/_v2/'.$pid.'/aa/bb';
|
||||
$mediaPath = $leaf.'/photo.png';
|
||||
// A pre-existing thumbnail with a DIFFERENT extension than the png source
|
||||
// will produce on regeneration (png source -> photo_thumb.png).
|
||||
$staleThumb = $leaf.'/photo_thumb.jpeg';
|
||||
|
||||
// A real, decodable 4x4 PNG so the GD driver can process it.
|
||||
$im = imagecreatetruecolor(4, 4);
|
||||
ob_start();
|
||||
imagepng($im);
|
||||
$pngBytes = ob_get_clean();
|
||||
imagedestroy($im);
|
||||
|
||||
Storage::disk('s3')->put($mediaPath, $pngBytes);
|
||||
Storage::disk('s3')->put($staleThumb, 'OLD-THUMB-BYTES');
|
||||
|
||||
return Media::create([
|
||||
'profile_id' => $pid,
|
||||
'user_id' => $user->id,
|
||||
'media_path' => $mediaPath,
|
||||
'thumbnail_path' => $staleThumb,
|
||||
'mime' => 'image/png',
|
||||
'size' => strlen($pngBytes),
|
||||
'remote_media' => false,
|
||||
'order' => 0,
|
||||
]);
|
||||
}
|
||||
|
||||
it('deletes the superseded thumbnail when regeneration changes its extension', function () {
|
||||
$media = seedPngMediaWithStaleThumb();
|
||||
$staleThumb = $media->thumbnail_path;
|
||||
$disk = Storage::disk('s3');
|
||||
|
||||
expect($disk->exists($staleThumb))->toBeTrue();
|
||||
|
||||
(new Image)->resizeThumbnail($media);
|
||||
$media->refresh();
|
||||
|
||||
// thumbnail_path now points at the freshly generated file (png output).
|
||||
expect($media->thumbnail_path)->not->toBe($staleThumb);
|
||||
expect($disk->exists($media->thumbnail_path))->toBeTrue();
|
||||
|
||||
// The old, superseded thumbnail is gone rather than orphaned in the dir.
|
||||
expect($disk->exists($staleThumb))->toBeFalse();
|
||||
});
|
||||
|
||||
it('keeps the thumbnail when regeneration writes to the same path', function () {
|
||||
$user = User::factory()->create();
|
||||
$user->refresh();
|
||||
$pid = $user->profile->id;
|
||||
$leaf = 'public/m/_v2/'.$pid.'/cc/dd';
|
||||
$mediaPath = $leaf.'/photo.png';
|
||||
|
||||
$im = imagecreatetruecolor(4, 4);
|
||||
ob_start();
|
||||
imagepng($im);
|
||||
$pngBytes = ob_get_clean();
|
||||
imagedestroy($im);
|
||||
Storage::disk('s3')->put($mediaPath, $pngBytes);
|
||||
|
||||
$media = Media::create([
|
||||
'profile_id' => $pid,
|
||||
'user_id' => $user->id,
|
||||
'media_path' => $mediaPath,
|
||||
'mime' => 'image/png',
|
||||
'size' => strlen($pngBytes),
|
||||
'remote_media' => false,
|
||||
'order' => 0,
|
||||
]);
|
||||
|
||||
(new Image)->resizeThumbnail($media);
|
||||
$media->refresh();
|
||||
|
||||
// photo_thumb.png is generated and present.
|
||||
expect($media->thumbnail_path)->toBe($leaf.'/photo_thumb.png');
|
||||
expect(Storage::disk('s3')->exists($media->thumbnail_path))->toBeTrue();
|
||||
});
|
||||
@ -1,82 +0,0 @@
|
||||
<?php
|
||||
|
||||
use App\Jobs\MediaPipeline\MediaDeletePipeline;
|
||||
use App\Models\Media;
|
||||
use App\Models\User;
|
||||
use Illuminate\Foundation\Testing\LazilyRefreshDatabase;
|
||||
use Illuminate\Support\Facades\Config;
|
||||
use Illuminate\Support\Facades\Storage;
|
||||
|
||||
uses(LazilyRefreshDatabase::class);
|
||||
|
||||
/*
|
||||
|--------------------------------------------------------------------------
|
||||
| MediaDeletePipeline — in-flow leaf directory cleanup
|
||||
|--------------------------------------------------------------------------
|
||||
|
|
||||
| A media delete removes its own files AND the leaf directory it emptied
|
||||
| (public/m/_v2/{pid}/{month}/{random}), so the flow does not leave empty
|
||||
| folders behind for the scheduled sweep to find.
|
||||
|
|
||||
*/
|
||||
|
||||
beforeEach(function () {
|
||||
Config::set('filesystems.local', 'local');
|
||||
Config::set('pixelfed.cloud_storage', false);
|
||||
Storage::fake('local');
|
||||
});
|
||||
|
||||
function makeOrphanLocalMedia(): Media
|
||||
{
|
||||
$user = User::factory()->create();
|
||||
$user->refresh();
|
||||
$pid = $user->profile->id;
|
||||
|
||||
$leaf = 'public/m/_v2/'.$pid.'/aa-bb/rndrndrndrnd';
|
||||
$path = $leaf.'/file.jpg';
|
||||
$thumb = $leaf.'/file_thumb.jpeg';
|
||||
|
||||
Storage::disk('local')->put($path, 'PRIMARY');
|
||||
Storage::disk('local')->put($thumb, 'THUMB');
|
||||
|
||||
return Media::create([
|
||||
'status_id' => null,
|
||||
'profile_id' => $pid,
|
||||
'user_id' => $user->id,
|
||||
'media_path' => $path,
|
||||
'thumbnail_path' => $thumb,
|
||||
'mime' => 'image/jpeg',
|
||||
'size' => 7,
|
||||
'remote_media' => false,
|
||||
'order' => 0,
|
||||
]);
|
||||
}
|
||||
|
||||
it('deletes the media files and removes its now-empty leaf directory', function () {
|
||||
$media = makeOrphanLocalMedia();
|
||||
$leaf = implode('/', array_slice(explode('/', $media->media_path), 0, -1));
|
||||
$disk = Storage::disk('local');
|
||||
|
||||
expect($disk->exists($media->media_path))->toBeTrue();
|
||||
|
||||
(new MediaDeletePipeline($media))->handle();
|
||||
|
||||
expect($disk->exists($media->media_path))->toBeFalse();
|
||||
expect($disk->exists($media->thumbnail_path))->toBeFalse();
|
||||
expect($disk->directoryExists($leaf))->toBeFalse();
|
||||
});
|
||||
|
||||
it('leaves the leaf directory in place when another file still lives there', function () {
|
||||
$media = makeOrphanLocalMedia();
|
||||
$leaf = implode('/', array_slice(explode('/', $media->media_path), 0, -1));
|
||||
$disk = Storage::disk('local');
|
||||
|
||||
// A sibling file that this media does not own.
|
||||
$disk->put($leaf.'/sibling.jpg', 'KEEP');
|
||||
|
||||
(new MediaDeletePipeline($media))->handle();
|
||||
|
||||
expect($disk->exists($media->media_path))->toBeFalse();
|
||||
expect($disk->directoryExists($leaf))->toBeTrue();
|
||||
expect($disk->exists($leaf.'/sibling.jpg'))->toBeTrue();
|
||||
});
|
||||
Loading…
Reference in New Issue