Files
projectsend/tests/Feature/Files/FileDiskCleanupTest.php
denkfabrik-li 4164678ebc Delete a file's renditions even when its own disk cannot be resolved
FileDiskCleanup wraps both deletions in one try. The first is the original
upload, on whatever disk the row names; the second is every cached
rendition, always on the local files disk. Storage::disk() throws outright
for a name with no configured driver -- which is the state the original's
disk is in whenever this fails at all -- so the catch swallowed it and the
renditions were never reached.

Nothing looks for them afterwards. OrphanFileScanner skips the rendition
directories on purpose (they are derived artifacts, never orphaned
uploads), so a file whose external disk had been removed or renamed kept
every cached copy of itself, indefinitely, on the disk that was working.

The two attempts are now separate, each with the same tolerance the class
was written for: a storage failure still never turns a delete click into a
500, and the warning is still the report.

While here, the comment in File::booted() that justifies deferring the
byte removal claimed "the worst case is bytes left on disk with no row,
which OrphanFileScanner already finds and reports". Not on this path: the
row is soft-deleted, and knownPaths() counts a trashed row's path as
claimed -- deliberately, so a scan never offers to double-adopt a file
still inside its erasure grace period. The comment now says what actually
happens.

One test: a file whose disk cannot be resolved loses its renditions. It
goes red without the fix, next to the existing test that the delete itself
still succeeds.
2026-08-28 06:40:48 +02:00

165 lines
5.8 KiB
PHP

<?php
declare(strict_types=1);
use App\Models\User;
use App\Modules\Files\Models\File;
use App\Modules\Files\Thumbnails\ImageAudience;
use App\Modules\Files\Thumbnails\ImageRendition;
use App\Modules\Files\Thumbnails\ThumbnailGenerator;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Storage;
use Illuminate\Support\Str;
beforeEach(function () {
Storage::fake('files');
Storage::fake('files_external');
$this->admin = User::factory()->create();
});
function makeStoredFile(array $overrides = []): File
{
$path = $overrides['path'] ?? '2026/07/'.Str::uuid()->toString().'.pdf';
$disk = $overrides['disk'] ?? 'files';
if ($disk !== 'nonexistent-disk') {
Storage::disk($disk)->put($path, 'hello-world');
}
return File::factory()->create([
'uploaded_by' => test()->admin->id,
'name' => 'doc',
'original_name' => 'doc.pdf',
'mime_type' => 'application/pdf',
'size' => 11,
...$overrides,
'path' => $path,
'disk' => $disk,
]);
}
test('deleting a file removes its bytes from disk while the row survives as trashed', function () {
$file = makeStoredFile();
$this->actingAs($this->admin)->delete("/files/{$file->id}")->assertRedirect();
Storage::disk('files')->assertMissing($file->path);
expect(File::withTrashed()->findOrFail($file->id)->trashed())->toBeTrue();
});
test('deleting a folder cascades disk cleanup to every file inside it', function () {
$folder = makeFolder('Reports');
$file = makeStoredFile(['folder_id' => $folder->id]);
$this->actingAs($this->admin)->delete("/folders/{$folder->id}")->assertRedirect();
Storage::disk('files')->assertMissing($file->path);
expect(File::withTrashed()->findOrFail($file->id)->trashed())->toBeTrue();
});
// Every rendition of every audience, not just the staff thumbnail — a
// file can easily have been viewed only by the client it was shared with,
// in which case the external copies are the only cached ones there are.
test('deleting a file also removes every cached rendition of it', function () {
$file = makeStoredFile(['mime_type' => 'image/jpeg']);
$paths = ThumbnailGenerator::pathsFor($file->id, 'image/jpeg');
expect($paths)->toHaveCount(count(ImageAudience::cases()) * count(ImageRendition::cases()));
foreach ($paths as $path) {
Storage::disk('files')->put($path, 'fake-thumbnail-bytes');
}
$this->actingAs($this->admin)->delete("/files/{$file->id}")->assertRedirect();
foreach ($paths as $path) {
Storage::disk('files')->assertMissing($path);
}
});
test('a file stored on the external disk is deleted from that disk, not the local one', function () {
$file = makeStoredFile(['disk' => 'files_external', 'path' => 'ext/doc.pdf']);
$this->actingAs($this->admin)->delete("/files/{$file->id}")->assertRedirect();
Storage::disk('files_external')->assertMissing($file->path);
});
// The row comes back on a rollback; the bytes have to still be under it.
// Nothing restores them, so this is the one failure in the whole cleanup
// path that cannot be repaired afterwards.
test('a transaction that rolls back leaves the bytes where they were', function () {
$file = makeStoredFile();
try {
DB::transaction(function () use ($file): void {
$file->delete();
throw new RuntimeException('something later in the transaction failed');
});
} catch (RuntimeException) {
// The point of the test is what survives it.
}
expect(File::query()->find($file->id))->not->toBeNull();
Storage::disk('files')->assertExists($file->path);
});
// The shape an account deletion has: content disposal runs in its own
// transaction, nested as a savepoint inside the caller's, and the write
// that fails afterwards belongs to the caller.
test('an outer rollback leaves the bytes even after the inner transaction committed', function () {
$file = makeStoredFile();
try {
DB::transaction(function () use ($file): void {
DB::transaction(function () use ($file): void {
$file->delete();
});
throw new RuntimeException('the account write after it failed');
});
} catch (RuntimeException) {
// Same.
}
expect(File::query()->find($file->id))->not->toBeNull();
Storage::disk('files')->assertExists($file->path);
});
test('a storage failure while cleaning up disk bytes never blocks the file from being deleted', function () {
// A disk with no configured driver at all throws immediately on
// resolution — proves a storage-layer failure (e.g. external storage
// misconfigured or its credentials rotated after upload) can't turn a
// routine delete into a 500.
$file = makeStoredFile(['disk' => 'nonexistent-disk', 'path' => 'whatever.pdf']);
$this->actingAs($this->admin)->delete("/files/{$file->id}")->assertRedirect();
expect(File::withTrashed()->findOrFail($file->id)->trashed())->toBeTrue();
});
// The renditions are always on the local disk, whatever the original's
// disk is — so a source disk that cannot even be resolved is no reason to
// keep them. Nothing else would: OrphanFileScanner skips the rendition
// directories on purpose, and the trashed row still claims its own path.
test('a source disk that cannot be resolved does not keep the renditions alive', function () {
$file = makeStoredFile([
'disk' => 'nonexistent-disk',
'path' => 'whatever.jpg',
'mime_type' => 'image/jpeg',
]);
$paths = ThumbnailGenerator::pathsFor($file->id, 'image/jpeg');
foreach ($paths as $path) {
Storage::disk('files')->put($path, 'fake-thumbnail-bytes');
}
$this->actingAs($this->admin)->delete("/files/{$file->id}")->assertRedirect();
foreach ($paths as $path) {
Storage::disk('files')->assertMissing($path);
}
});