Files
projectsend/tests/Feature/Files/ZipDownloadsTest.php
T
ignacionelson 92a132d74f Give zip builds their own queue, so one archive cannot hold up the mail
The last piece of the #1687 follow-up. BuildZipDownloadJob allows itself
an hour, every shipped topology runs exactly one worker, and everything
shares the default queue -- so one large archive delayed every
notification email queued behind it. The size cap and the
one-build-per-person rule bounded that in July; they did not remove it.

onQueue('zips') in the constructor rather than at the dispatch site, so a
second caller cannot forget it. Both images grow a worker for it:
compose.yaml gains worker-zips, supervisord gains [program:queue-zips],
and the existing worker in each narrows to --queue=default. --tries=1
there matches the job, which records its own failure rather than being
retried.

The part that needs care is the manual install. A worker whose command
still says plain `queue:work` consumes `default` only, so it would send
email happily and never finish a single zip, with nothing in any log
saying why. INSTALL.md's unit now reads --queue=default,zips -- one
worker watching both, which is right for most installations -- and says
what happens if you leave it off, with the two-worker split offered for
anyone who would rather keep the two kinds of work apart. CHANGELOG
carries it as an upgrade note, since it is something to do rather than
something that was done.

Verified in the dev stack rather than only in a test: dispatched a build
and watched worker-zips take it while the default worker stayed idle.
2026-08-26 18:06:40 -03:00

561 lines
23 KiB
PHP

<?php
declare(strict_types=1);
use App\Models\User;
use App\Modules\Audit\Action;
use App\Modules\Audit\ActivityLog;
use App\Modules\Files\Jobs\BuildZipDownloadJob;
use App\Modules\Files\Models\File;
use App\Modules\Files\Models\Folder;
use App\Modules\Files\Models\ZipDownload;
use App\Modules\Platform\Settings\Setting;
use App\Modules\Platform\Settings\Settings;
use Illuminate\Http\UploadedFile;
use Illuminate\Support\Facades\Queue;
use Illuminate\Support\Facades\Storage;
// QUEUE_CONNECTION=sync in phpunit.xml — BuildZipDownloadJob runs
// synchronously within the same request, so the zip is already built by
// the time a `postJson('/zip-downloads', ...)` call returns.
beforeEach(function () {
Storage::fake('files');
$this->admin = User::factory()->create();
// Settings are cached across tests (Cache::rememberForever survives
// the per-test DB rollback), so the cap every test below builds on is
// set rather than assumed — and reset here for the tests that lower it.
app(Settings::class)->set(Setting::MaxZipDownloadSizeMb, 2048);
});
function zipUploadFile(User $as, string $name, ?int $folderId = null): File
{
test()->actingAs($as)->post('/files', [
'file' => UploadedFile::fake()->create($name, 4, 'application/pdf'),
'name' => '',
'description' => '',
'folder_id' => $folderId,
]);
return File::query()->latest('id')->firstOrFail();
}
/**
* @return list<string>
*/
function zipEntryNames(ZipDownload $zipDownload): array
{
$zip = new ZipArchive;
$zip->open(Storage::disk('files')->path($zipDownload->refresh()->path));
$names = [];
for ($i = 0; $i < $zip->numFiles; $i++) {
$names[] = $zip->getNameIndex($i);
}
$zip->close();
return $names;
}
test('zipping a folder builds an archive with the hierarchy preserved', function () {
$parent = Folder::query()->create(['name' => 'Reports']);
$child = Folder::query()->create(['name' => 'Q1', 'parent_id' => $parent->id, 'path' => "/{$parent->id}/"]);
zipUploadFile($this->admin, 'summary.pdf', $parent->id);
zipUploadFile($this->admin, 'detail.pdf', $child->id);
$response = $this->actingAs($this->admin)->postJson('/zip-downloads', ['folder_ids' => [$parent->id]])->assertOk();
$zipDownload = ZipDownload::query()->findOrFail($response->json('id'));
expect($zipDownload->status)->toBe(ZipDownload::STATUS_READY)
->and($zipDownload->file_count)->toBe(2);
expect(zipEntryNames($zipDownload))->toContain('Reports/summary.pdf', 'Reports/Q1/detail.pdf');
});
test('filename collisions get a numbered suffix', function () {
$fileA = zipUploadFile($this->admin, 'notes.pdf');
$fileB = zipUploadFile($this->admin, 'notes.pdf');
$response = $this->actingAs($this->admin)->postJson('/zip-downloads', [
'file_ids' => [$fileA->id, $fileB->id],
])->assertOk();
$zipDownload = ZipDownload::query()->findOrFail($response->json('id'));
expect(zipEntryNames($zipDownload))->toContain('notes.pdf', 'notes (2).pdf');
});
test('a mixed loose-file and folder selection places files at the root and folders as subfolders', function () {
$folder = Folder::query()->create(['name' => 'Reports']);
$loose = zipUploadFile($this->admin, 'loose.pdf');
zipUploadFile($this->admin, 'inside.pdf', $folder->id);
$response = $this->actingAs($this->admin)->postJson('/zip-downloads', [
'file_ids' => [$loose->id],
'folder_ids' => [$folder->id],
])->assertOk();
$zipDownload = ZipDownload::query()->findOrFail($response->json('id'));
expect(zipEntryNames($zipDownload))->toContain('loose.pdf', 'Reports/inside.pdf');
});
test('inaccessible files are silently dropped, rejecting the request if nothing remains', function () {
$client = User::factory()->client()->create();
$secretFile = zipUploadFile($this->admin, 'secret.pdf');
$this->actingAs($client)->postJson('/zip-downloads', ['file_ids' => [$secretFile->id]])->assertStatus(422);
});
test('a client can zip a file assigned to them', function () {
$client = User::factory()->client()->create();
$file = zipUploadFile($this->admin, 'shared.pdf');
$this->actingAs($this->admin)->post("/files/{$file->id}/assignments", ['type' => 'client', 'id' => $client->id]);
$response = $this->actingAs($client)->postJson('/zip-downloads', ['file_ids' => [$file->id]])->assertOk();
$zipDownload = ZipDownload::query()->findOrFail($response->json('id'));
expect($zipDownload->file_ids)->toBe([$file->id]);
});
test('a client cannot zip a folder that is not shared with them', function () {
$client = User::factory()->client()->create();
$folder = Folder::query()->create(['name' => 'Private']);
zipUploadFile($this->admin, 'a.pdf', $folder->id);
$this->actingAs($client)->postJson('/zip-downloads', ['folder_ids' => [$folder->id]])->assertStatus(422);
});
// Holding a folder never implied being able to read everything inside it —
// expiry hides a file from a client while leaving it in place — so the job
// re-derives visibility per file instead of inheriting it from the folder.
test('zipping a shared folder excludes files the client may no longer see', function () {
$client = User::factory()->client()->create();
$folder = Folder::query()->create(['name' => 'Shared']);
$this->actingAs($this->admin)->post("/folders/{$folder->id}/assignments", ['type' => 'client', 'id' => $client->id]);
$live = zipUploadFile($this->admin, 'live.pdf', $folder->id);
$expired = zipUploadFile($this->admin, 'expired.pdf', $folder->id);
$expired->update(['expires_at' => now()->subDay()]);
$response = $this->actingAs($client)->postJson('/zip-downloads', ['folder_ids' => [$folder->id]])->assertOk();
$zipDownload = ZipDownload::query()->findOrFail($response->json('id'));
expect(zipEntryNames($zipDownload))->toBe(['Shared/live.pdf'])
->and($zipDownload->file_count)->toBe(1);
// Staff keep access to an expired file, so the same folder still zips
// both for them — the rule is per-viewer, not a property of the file.
$staffResponse = $this->actingAs($this->admin)->postJson('/zip-downloads', ['folder_ids' => [$folder->id]])->assertOk();
expect(zipEntryNames(ZipDownload::query()->findOrFail($staffResponse->json('id'))))
->toEqualCanonicalizing(['Shared/live.pdf', 'Shared/expired.pdf']);
expect($live->fresh())->not->toBeNull();
});
test('a client cannot zip a loose file that expired after it was shared', function () {
$client = User::factory()->client()->create();
$file = zipUploadFile($this->admin, 'a.pdf');
$this->actingAs($this->admin)->post("/files/{$file->id}/assignments", ['type' => 'client', 'id' => $client->id]);
$file->update(['expires_at' => now()->subDay()]);
$this->actingAs($client)->postJson('/zip-downloads', ['file_ids' => [$file->id]])->assertStatus(422);
});
test('the download endpoint logs a FileDownloaded entry for every bundled file', function () {
$folder = Folder::query()->create(['name' => 'Reports']);
$file = zipUploadFile($this->admin, 'a.pdf', $folder->id);
$response = $this->actingAs($this->admin)->postJson('/zip-downloads', ['folder_ids' => [$folder->id]])->assertOk();
$zipDownload = ZipDownload::query()->findOrFail($response->json('id'));
$this->actingAs($this->admin)->get("/zip-downloads/{$zipDownload->id}/download")
->assertOk()
->assertHeader('X-Accel-Redirect', '/protected-files/'.$zipDownload->path)
->assertHeader('Content-Disposition', 'attachment; filename="Reports.zip"');
expect(ActivityLog::query()->where('action', Action::FileDownloaded)->where('subject_id', $file->id)->exists())->toBeTrue();
});
test('a pending zip download 404s until ready', function () {
$zipDownload = ZipDownload::query()->create(['requested_by' => $this->admin->id, 'status' => ZipDownload::STATUS_PENDING]);
$this->actingAs($this->admin)->get("/zip-downloads/{$zipDownload->id}/download")->assertNotFound();
});
test('only the requester can poll or download their own zip', function () {
$other = User::factory()->create();
$zipDownload = ZipDownload::query()->create([
'requested_by' => $this->admin->id,
'status' => ZipDownload::STATUS_READY,
'path' => 'zips/whatever.zip',
]);
$this->actingAs($other)->getJson("/zip-downloads/{$zipDownload->id}")->assertNotFound();
$this->actingAs($other)->get("/zip-downloads/{$zipDownload->id}/download")->assertNotFound();
});
test('an oversized selection is rejected', function () {
$folder = Folder::query()->create(['name' => 'Huge']);
$now = now();
$rows = [];
for ($i = 0; $i < 10001; $i++) {
$rows[] = [
'folder_id' => $folder->id,
'name' => "file{$i}",
// Bulk insert() bypasses Eloquent's creating() event (the
// auto-slug fallback), so a unique one is supplied directly.
'slug' => "huge-file-{$i}",
'original_name' => "file{$i}.txt",
'path' => "huge/file{$i}.txt",
'mime_type' => 'text/plain',
'size' => 1,
'checksum' => str_repeat('a', 64),
'created_at' => $now,
'updated_at' => $now,
];
}
File::query()->insert($rows);
$this->actingAs($this->admin)->postJson('/zip-downloads', ['folder_ids' => [$folder->id]])->assertStatus(422);
});
test('the purge command removes zip downloads and files older than 24 hours', function () {
Storage::disk('files')->put('zips/old.zip', 'x');
Storage::disk('files')->put('zips/new.zip', 'x');
$old = ZipDownload::query()->create([
'requested_by' => $this->admin->id,
'status' => ZipDownload::STATUS_READY,
'path' => 'zips/old.zip',
]);
$old->forceFill(['created_at' => now()->subDays(2)])->save();
$new = ZipDownload::query()->create([
'requested_by' => $this->admin->id,
'status' => ZipDownload::STATUS_READY,
'path' => 'zips/new.zip',
]);
$this->artisan('projectsend:purge-zip-downloads')->assertSuccessful();
expect(ZipDownload::query()->find($old->id))->toBeNull()
->and(ZipDownload::query()->find($new->id))->not->toBeNull()
->and(Storage::disk('files')->exists('zips/old.zip'))->toBeFalse()
->and(Storage::disk('files')->exists('zips/new.zip'))->toBeTrue();
});
// The store guard rejects an empty selection, but a file selected and then
// removed before the queued job runs leaves nothing to add. libzip writes
// no file at all for a zero-entry archive, so a "ready" row would point the
// download controller at a path that does not exist.
test('a build with no available files is marked failed rather than ready over an empty archive', function () {
$file = zipUploadFile($this->admin, 'gone.pdf');
$zipDownload = ZipDownload::query()->create([
'requested_by' => $this->admin->id,
'status' => ZipDownload::STATUS_PENDING,
'file_ids' => [$file->id],
'folder_ids' => [],
]);
// Gone from the requester's view by the time the job builds the archive.
$file->delete();
(new BuildZipDownloadJob($zipDownload->id))->handle();
$zipDownload->refresh();
expect($zipDownload->status)->toBe(ZipDownload::STATUS_FAILED)
->and($zipDownload->path)->toBeNull()
->and(Storage::disk('files')->exists("zips/{$zipDownload->id}.zip"))->toBeFalse();
});
// ZipArchive reads each source only at close(); if the bytes vanish in
// between (a staff delete triggers FileDiskCleanup at once) close() returns
// false. The row must fail rather than go ready over an archive that was
// never actually written to disk.
test('a source file deleted after it was queued fails the build instead of serving a broken archive', function () {
$file = zipUploadFile($this->admin, 'vanishing.pdf');
$zipDownload = ZipDownload::query()->create([
'requested_by' => $this->admin->id,
'status' => ZipDownload::STATUS_PENDING,
'file_ids' => [$file->id],
'folder_ids' => [],
]);
// The row stays visible, but its bytes are gone before the build closes.
Storage::disk('files')->delete($file->path);
(new BuildZipDownloadJob($zipDownload->id))->handle();
$zipDownload->refresh();
expect($zipDownload->status)->toBe(ZipDownload::STATUS_FAILED)
->and($zipDownload->path)->toBeNull()
->and(Storage::disk('files')->exists("zips/{$zipDownload->id}.zip"))->toBeFalse();
});
// A build that blows the worker timeout is killed mid-run: handle()'s own
// catch never executes, so without this hook the row would poll as pending
// forever and the frontend would never stop.
test('the failed hook fails a still-pending row when the worker gives up on the job', function () {
$zipDownload = ZipDownload::query()->create([
'requested_by' => $this->admin->id,
'status' => ZipDownload::STATUS_PENDING,
]);
(new BuildZipDownloadJob($zipDownload->id))->failed(new RuntimeException('timed out'));
$zipDownload->refresh();
expect($zipDownload->status)->toBe(ZipDownload::STATUS_FAILED)
->and($zipDownload->error)->not->toBeNull();
});
// A late failure signal (a retry racing a build that already finished) must
// not overwrite a row that already delivered a ready archive.
test('the failed hook leaves a row that already finished alone', function () {
$zipDownload = ZipDownload::query()->create([
'requested_by' => $this->admin->id,
'status' => ZipDownload::STATUS_READY,
'path' => 'zips/whatever.zip',
]);
(new BuildZipDownloadJob($zipDownload->id))->failed(new RuntimeException('too late'));
expect($zipDownload->refresh()->status)->toBe(ZipDownload::STATUS_READY);
});
test('the build job runs once and allows enough time for a large archive', function () {
$job = new BuildZipDownloadJob(1);
expect($job->tries)->toBe(1)
->and($job->timeout)->toBeGreaterThan(60);
});
// A build killed before it finished (worker timeout, full disk) leaves a
// partial archive — and libzip's temp file beside it — on disk but never
// writes a path back to the row. Purge keys off the row id so it clears
// them anyway.
test('the purge command removes leftover archives even when the row never recorded a path', function () {
$row = ZipDownload::query()->create([
'requested_by' => $this->admin->id,
'status' => ZipDownload::STATUS_FAILED,
]);
$row->forceFill(['created_at' => now()->subDays(2)])->save();
Storage::disk('files')->put("zips/{$row->id}.zip", 'partial');
Storage::disk('files')->put("zips/{$row->id}.zip.tmp0a1b2c", 'libzip temp');
$this->artisan('projectsend:purge-zip-downloads')->assertSuccessful();
expect(ZipDownload::query()->find($row->id))->toBeNull()
->and(Storage::disk('files')->exists("zips/{$row->id}.zip"))->toBeFalse()
->and(Storage::disk('files')->exists("zips/{$row->id}.zip.tmp0a1b2c"))->toBeFalse();
});
// The cap is on bytes, not on file count: bytes are what the build
// actually costs — worker time, temp copies from a remote disk, and the
// archive itself. The message names both numbers so the person who asked
// knows how much to deselect.
test('a selection larger than the zip download limit is rejected, and says how large it was', function () {
app(Settings::class)->set(Setting::MaxZipDownloadSizeMb, 1);
$file = zipUploadFile($this->admin, 'huge.pdf');
$file->update(['size' => 5 * 1024 * 1024]);
$response = $this->actingAs($this->admin)
->postJson('/zip-downloads', ['file_ids' => [$file->id]])
->assertStatus(422);
expect($response->json('message'))->toContain('5.0 MB')->toContain('1 MB')
->and(ZipDownload::query()->count())->toBe(0);
});
test('a zip download limit of zero means no limit', function () {
app(Settings::class)->set(Setting::MaxZipDownloadSizeMb, 0);
$file = zipUploadFile($this->admin, 'huge.pdf');
$file->update(['size' => 50 * 1024 * 1024]);
$this->actingAs($this->admin)
->postJson('/zip-downloads', ['file_ids' => [$file->id]])
->assertOk();
});
// The controller checks what was asked for; the job checks again, because
// the selection is re-derived at build time and a folder can grow while
// the job waits in the queue.
test('a selection that grew past the limit after it was queued fails the build', function () {
app(Settings::class)->set(Setting::MaxZipDownloadSizeMb, 1);
$file = zipUploadFile($this->admin, 'grown.pdf');
$zipDownload = ZipDownload::query()->create([
'requested_by' => $this->admin->id,
'status' => ZipDownload::STATUS_PENDING,
'file_ids' => [$file->id],
'folder_ids' => [],
]);
$file->update(['size' => 5 * 1024 * 1024]);
(new BuildZipDownloadJob($zipDownload->id))->handle();
$zipDownload->refresh();
expect($zipDownload->status)->toBe(ZipDownload::STATUS_FAILED)
->and($zipDownload->path)->toBeNull()
->and(Storage::disk('files')->exists("zips/{$zipDownload->id}.zip"))->toBeFalse();
});
// A zip holds the single queue worker for as long as it takes to write,
// and every notification email waits behind it.
test('a build already in progress blocks a second request from the same person', function () {
ZipDownload::query()->create([
'requested_by' => $this->admin->id,
'status' => ZipDownload::STATUS_PENDING,
]);
$file = zipUploadFile($this->admin, 'later.pdf');
$this->actingAs($this->admin)
->postJson('/zip-downloads', ['file_ids' => [$file->id]])
->assertStatus(429);
});
test('one person\'s build in progress does not block anybody else', function () {
$other = User::factory()->create();
ZipDownload::query()->create([
'requested_by' => $other->id,
'status' => ZipDownload::STATUS_PENDING,
]);
$file = zipUploadFile($this->admin, 'mine.pdf');
$this->actingAs($this->admin)
->postJson('/zip-downloads', ['file_ids' => [$file->id]])
->assertOk();
});
// failed() resolves a row the queue gave up on, but a worker killed hard
// enough never runs it. Nobody should be locked out forever by a row that
// nothing is ever going to finish.
test('an abandoned pending build stops blocking after an hour', function () {
$stale = ZipDownload::query()->create([
'requested_by' => $this->admin->id,
'status' => ZipDownload::STATUS_PENDING,
]);
$stale->forceFill(['created_at' => now()->subHours(2)])->save();
$file = zipUploadFile($this->admin, 'again.pdf');
$this->actingAs($this->admin)
->postJson('/zip-downloads', ['file_ids' => [$file->id]])
->assertOk();
});
// "Nothing left to send" and "you have already had these as often as you
// were meant to" are different answers, and the list of what was left out
// belongs on the row either way.
test('a build where every file had already hit its download limit says so, and lists them', function () {
// Uploaded by somebody else: the uploader is exempt from their own
// file's limit, so the requester has to be a different person for the
// allowance to mean anything.
$file = zipUploadFile(User::factory()->create(), 'spent.pdf');
$file->update(['download_limit' => 1]);
ActivityLog::query()->create([
'actor_id' => $this->admin->id,
'actor_name' => $this->admin->name,
'actor_type' => $this->admin->type->value,
'action' => Action::FileDownloaded,
'subject_type' => $file->getMorphClass(),
'subject_id' => $file->id,
'created_at' => now(),
]);
$zipDownload = ZipDownload::query()->create([
'requested_by' => $this->admin->id,
'status' => ZipDownload::STATUS_PENDING,
'file_ids' => [$file->id],
'folder_ids' => [],
]);
(new BuildZipDownloadJob($zipDownload->id))->handle();
$zipDownload->refresh();
expect($zipDownload->status)->toBe(ZipDownload::STATUS_FAILED)
->and($zipDownload->error)->toContain('download limit')
->and($zipDownload->skipped_files)->toHaveCount(1)
->and($zipDownload->skipped_files[0]['id'])->toBe($file->id);
});
// zip_downloads.requested_by cascades on delete, so removing a user takes
// their rows with it and leaves every archive they built behind — nothing
// keyed on rows can ever see those again.
test('the purge command removes an archive whose row no longer exists', function () {
$this->admin; // setup complete
Storage::disk('files')->put('zips/4242.zip', 'stranded');
touch(Storage::disk('files')->path('zips/4242.zip'), now()->subDays(2)->getTimestamp());
// Younger than the grace period: this one could still belong to a
// build that is running right now.
Storage::disk('files')->put('zips/4243.zip', 'just built');
$this->artisan('projectsend:purge-zip-downloads')->assertSuccessful();
expect(Storage::disk('files')->exists('zips/4242.zip'))->toBeFalse()
->and(Storage::disk('files')->exists('zips/4243.zip'))->toBeTrue();
});
// original_name is uploader-chosen and validated only for length, so it
// must not be able to steer where an entry lands inside the archive.
test('a traversing filename cannot escape the archive as a zip entry', function () {
$folder = Folder::query()->create(['name' => 'Reports']);
$file = zipUploadFile($this->admin, 'ok.pdf', $folder->id);
// Set it directly: the sanitizer on the intake path would already have
// stripped this, and the point here is that the zip builder does not
// depend on that having happened.
$file->forceFill(['original_name' => '../../../.bashrc'])->save();
$response = $this->actingAs($this->admin)->postJson('/zip-downloads', ['folder_ids' => [$folder->id]])->assertOk();
$names = zipEntryNames(ZipDownload::query()->findOrFail($response->json('id')));
expect($names)->toHaveCount(1);
// Dots in a filename are fine — what must not exist is a path *segment*
// that walks up, which is what an extractor acts on.
$segments = explode('/', $names[0]);
expect($segments)->toHaveCount(2)
->and($segments)->not->toContain('..')
->and($names[0])->toStartWith('Reports/');
});
// Its own queue, so one hour-long build cannot hold up every notification
// email behind it. Both shipped topologies run a worker for it; a manual
// install is told to watch `default,zips` (INSTALL.md, and the upgrade
// note in CHANGELOG.md), because a worker that is not listening finishes
// no zips and says nothing about why.
test('a zip build is queued away from ordinary work', function () {
Queue::fake();
$file = zipUploadFile($this->admin, 'a.pdf');
$this->actingAs($this->admin)
->postJson('/zip-downloads', ['file_ids' => [$file->id]])
->assertOk();
Queue::assertPushed(BuildZipDownloadJob::class, fn (BuildZipDownloadJob $job): bool => $job->queue === 'zips');
});