diff --git a/app/Modules/Files/Uploads/LocalPartStore.php b/app/Modules/Files/Uploads/LocalPartStore.php index 31077df1..ec70e623 100644 --- a/app/Modules/Files/Uploads/LocalPartStore.php +++ b/app/Modules/Files/Uploads/LocalPartStore.php @@ -255,7 +255,26 @@ class LocalPartStore private function directory(UploadSession $session): string { - return storage_path('app/uploads-tmp/'.$session->id); + return $this->root().'/'.$session->id; + } + + /** + * Where part files live while a transfer is in progress. + * + * Configurable only so the test suite can hold it apart per parallel + * worker. This is a real directory rather than a faked disk, and each + * worker's database restarts session ids at 1, so two workers writing + * parts land in the same place — and ChunkedUploadsTest's afterEach + * deletes the whole tree, for everybody. Unset, which is every + * installation, the path is what it has always been. + */ + private function root(): string + { + $configured = config('projectsend.uploads.parts_path'); + + return is_string($configured) && $configured !== '' + ? rtrim($configured, '/') + : storage_path('app/uploads-tmp'); } private function partPath(UploadSession $session, int $partNumber): string diff --git a/config/projectsend.php b/config/projectsend.php index 364e9a7e..a9910024 100644 --- a/config/projectsend.php +++ b/config/projectsend.php @@ -19,6 +19,23 @@ return [ 'edition' => Edition::from((string) env('PROJECTSEND_EDITION', 'community')), + /* + |-------------------------------------------------------------------------- + | Uploads + |-------------------------------------------------------------------------- + | + | Where a chunked upload's parts wait while the transfer is running, + | before they are assembled onto the storage disk. Leave this unset: + | it exists so the test suite can give each parallel worker its own + | directory, since parts are real files on a real path rather than a + | faked disk, and session ids restart at 1 in every worker's database. + | + */ + + 'uploads' => [ + 'parts_path' => env('UPLOAD_PARTS_PATH'), + ], + /* |-------------------------------------------------------------------------- | Chunked upload part size (MB) diff --git a/tests/Feature/Files/ChunkedUploadsTest.php b/tests/Feature/Files/ChunkedUploadsTest.php index 7e6d18b4..4b672500 100644 --- a/tests/Feature/Files/ChunkedUploadsTest.php +++ b/tests/Feature/Files/ChunkedUploadsTest.php @@ -15,6 +15,17 @@ use App\Modules\Platform\Settings\Settings; use Illuminate\Support\Facades\Storage; use Illuminate\Testing\TestResponse; +/** + * Where this worker's parts live. Not storage_path('app/uploads-tmp') + * directly: each parallel worker gets its own root (see Tests\TestCase), + * because session ids restart at 1 in every worker's database and the + * cleanup below would otherwise delete the others' parts mid-test. + */ +function partsRoot(): string +{ + return (string) config('projectsend.uploads.parts_path'); +} + function grantChunkedUploadPermission(User $user): void { RolePermission::query()->firstOrCreate(['role_id' => $user->role_id, 'permission' => Permission::Upload->value]); @@ -26,7 +37,7 @@ beforeEach(function () { }); afterEach(function () { - Illuminate\Support\Facades\File::deleteDirectory(storage_path('app/uploads-tmp')); + Illuminate\Support\Facades\File::deleteDirectory(partsRoot()); }); function createSession(int $size = 1024, string $filename = 'big.zip'): string @@ -103,7 +114,7 @@ test('complete assembles parts in order into a verified File record', function ( ->and($file->checksum)->toBe(hash('sha256', 'hello-world')) ->and(Storage::disk('files')->get($file->path))->toBe('hello-world') ->and(UploadSession::query()->find($sessionId))->toBeNull() - ->and(is_dir(storage_path('app/uploads-tmp/'.$sessionId)))->toBeFalse() + ->and(is_dir(partsRoot().'/'.$sessionId))->toBeFalse() ->and(ActivityLog::query()->where('action', Action::FileUploaded)->where('subject_name', 'assembled')->exists())->toBeTrue(); }); @@ -192,7 +203,7 @@ test('abort deletes parts and the purge command clears only stale sessions', fun $aborted = createSession(); putPart($aborted, 1, 'data'); $this->deleteJson("/uploads/{$aborted}")->assertNoContent(); - expect(is_dir(storage_path('app/uploads-tmp/'.$aborted)))->toBeFalse() + expect(is_dir(partsRoot().'/'.$aborted))->toBeFalse() ->and(UploadSession::query()->find($aborted))->toBeNull(); $fresh = createSession(100, 'fresh.zip'); @@ -332,3 +343,21 @@ test('a folder that survives the upload still receives the file', function () { expect(File::query()->whereKey($fileId)->value('folder_id'))->toBe($folder->id); }); + +// The isolation itself cannot be observed from inside one test, but the +// mechanism it rests on can: parts go where the configured root says, so +// giving each worker its own root actually holds them apart. +test('parts are written under the configured root', function () { + $this->actingAs($this->admin); + + $custom = storage_path('app/uploads-tmp/somewhere-else'); + config(['projectsend.uploads.parts_path' => $custom]); + + $sessionId = createSession(1024); + putPart($sessionId, 1, 'hello')->assertOk(); + + expect(is_file($custom.'/'.$sessionId.'/1.part'))->toBeTrue() + ->and(is_dir(storage_path('app/uploads-tmp/'.$sessionId)))->toBeFalse(); + + Illuminate\Support\Facades\File::deleteDirectory($custom); +}); diff --git a/tests/TestCase.php b/tests/TestCase.php index fe1ffc2f..e7123e9d 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -3,8 +3,37 @@ namespace Tests; use Illuminate\Foundation\Testing\TestCase as BaseTestCase; +use Illuminate\Support\Facades\File; +use Illuminate\Support\Facades\ParallelTesting; abstract class TestCase extends BaseTestCase { - // + /** + * Give each parallel worker its own directory for upload parts. + * + * Parts are real files under a real path, not a faked disk, and every + * worker's database restarts session ids at 1 — so two workers writing + * parts land in the same directory, and ChunkedUploadsTest's afterEach + * deletes the whole tree for all of them. That showed up as a + * one-run-in-three failure, which is worse than a steady one: it + * trains you to re-run rather than look. + * + * Outside a parallel run the token is null and everything lands under + * w0, which is still held apart from whatever a previous run left. + */ + protected function setUp(): void + { + parent::setUp(); + + $root = storage_path('app/uploads-tmp/w'.(ParallelTesting::token() ?: '0')); + + config(['projectsend.uploads.parts_path' => $root]); + + // Emptied per test, not just per file. RefreshDatabase rolls back, + // so session ids restart at 1 in every test — two tests in the + // same worker reuse the same directory name, and a run that + // crashed before its cleanup leaves the previous one's parts + // sitting there under the id the next test is about to claim. + File::deleteDirectory($root); + } }