diff --git a/app/Modules/Files/Http/Controllers/OrphanFilesController.php b/app/Modules/Files/Http/Controllers/OrphanFilesController.php index f6ef586f..ff4d4a0b 100644 --- a/app/Modules/Files/Http/Controllers/OrphanFilesController.php +++ b/app/Modules/Files/Http/Controllers/OrphanFilesController.php @@ -5,20 +5,24 @@ declare(strict_types=1); namespace App\Modules\Files\Http\Controllers; use App\Http\Controllers\Controller; +use App\Models\User; use App\Modules\Audit\Action; use App\Modules\Audit\ActivityLogger; +use App\Modules\Files\Jobs\ImportOrphanFilesJob; use App\Modules\Files\Models\File; +use App\Modules\Files\OrphanFileImporter; use App\Modules\Files\OrphanFileScanner; +use App\Modules\Files\OrphanImportProgress; use App\Modules\Files\Scanning\ScanStatus; -use App\Modules\Files\Uploads\StoreUploadedFile; use App\Support\Pagination; -use Illuminate\Contracts\Filesystem\Filesystem; +use Illuminate\Http\JsonResponse; use Illuminate\Http\RedirectResponse; use Illuminate\Http\Request; use Illuminate\Pagination\LengthAwarePaginator; use Illuminate\Pagination\Paginator; use Illuminate\Support\Facades\Storage; use Illuminate\Validation\Rule; +use Illuminate\Validation\ValidationException; use Inertia\Inertia; use Inertia\Response; @@ -33,7 +37,8 @@ class OrphanFilesController extends Controller { public function __construct( private readonly OrphanFileScanner $scanner, - private readonly StoreUploadedFile $storeFile, + private readonly OrphanFileImporter $importer, + private readonly OrphanImportProgress $progress, private readonly ActivityLogger $activity, ) {} @@ -91,6 +96,7 @@ class OrphanFilesController extends Controller 'search' => $search, 'scanned_disks' => $this->scanner->scannedDisks(), 'missing_count' => File::query()->where('scan_status', ScanStatus::Missing)->count(), + 'import_run' => $this->progress->current(), ]); } @@ -138,14 +144,20 @@ class OrphanFilesController extends Controller $user = $request->user(); assert($user !== null); - $validated = $this->validateItems($request); + if ($request->boolean('all')) { + return $this->importAll($request, $user); + } + + // While a background run is adopting files, a second importer + // could adopt the same path twice. + if ($this->progress->isActive()) { + $this->alreadyRunning(); + } $imported = 0; $importedFile = null; - foreach ($validated['items'] as $item) { - $disk = Storage::disk($item['disk']); - + foreach ($this->validateItems($request)['items'] as $item) { // Re-validate against a fresh scan — never trust a // client-supplied disk/path just because an earlier scan // listed it. @@ -153,18 +165,7 @@ class OrphanFilesController extends Controller continue; } - $importedFile = $this->storeFile->create( - uploader: $user, - originalName: basename($item['path']), - path: $item['path'], - mimeType: $disk->mimeType($item['path']) ?: 'application/octet-stream', - size: $disk->size($item['path']), - checksum: $this->checksumOf($disk, $item['path']), - folderId: null, - disk: $item['disk'], - action: Action::FileImported, - ); - + $importedFile = $this->importer->import($user, $item['disk'], $item['path']); $imported++; } @@ -182,6 +183,49 @@ class OrphanFilesController extends Controller )); } + /** + * Every orphan the search matches, on every page. Handed to a queued + * job: thousands of files hashed in full take minutes, and a request + * is cut off after 30s of CPU — part-way through, with an error page + * for an import that was in fact half done. See ImportOrphanFilesJob. + */ + private function importAll(Request $request, User $user): RedirectResponse + { + $search = trim($request->validate(['search' => ['nullable', 'string', 'max:255']])['search'] ?? ''); + $search = $search !== '' ? $search : null; + + $total = count($this->scanner->importable($user, $search)); + + if ($total === 0) { + return back()->with('success', trans_choice(':count file imported.|:count files imported.', 0, ['count' => '0'])); + } + + if (! $this->progress->tryStart($total)) { + $this->alreadyRunning(); + } + + ImportOrphanFilesJob::dispatch($user->id, $search); + + return back()->with('success', trans_choice( + 'Importing :count file in the background.|Importing :count files in the background.', + $total, + ['count' => (string) $total], + )); + } + + private function alreadyRunning(): never + { + throw ValidationException::withMessages(['items' => __('An import is already running. Wait for it to finish.')]); + } + + /** + * Polled by the orphans screen while a background run is going. + */ + public function importStatus(): JsonResponse + { + return response()->json($this->progress->current()); + } + public function destroy(Request $request): RedirectResponse { $validated = $this->validateItems($request); @@ -218,24 +262,4 @@ class OrphanFilesController extends Controller 'items.*.path' => ['required', 'string'], ]); } - - /** - * Streamed rather than hash_file() on a local path — the only way to - * checksum a file that might live on a non-local disk (S3 has no - * local filesystem path to hand hash_file()). - */ - private function checksumOf(Filesystem $disk, string $path): string - { - $stream = $disk->readStream($path); - - if ($stream === null) { - return ''; - } - - $context = hash_init('sha256'); - hash_update_stream($context, $stream); - fclose($stream); - - return hash_final($context); - } } diff --git a/app/Modules/Files/Jobs/ImportOrphanFilesJob.php b/app/Modules/Files/Jobs/ImportOrphanFilesJob.php new file mode 100644 index 00000000..46231a8e --- /dev/null +++ b/app/Modules/Files/Jobs/ImportOrphanFilesJob.php @@ -0,0 +1,79 @@ +find($this->userId); + + if ($user === null) { + $progress->fail('The account that started the import no longer exists.'); + + return; + } + + $deadline = microtime(true) + $this->budgetSeconds; + + foreach ($scanner->importable($user, $this->search) as $i => $item) { + // At least one file per chunk, so a run always moves forward. + if ($i > 0 && microtime(true) >= $deadline) { + self::dispatch($this->userId, $this->search, $this->budgetSeconds); + + return; + } + + $importer->import($user, $item['disk'], $item['path']); + $progress->advance(); + } + + $progress->finish(); + } + + public function failed(Throwable $exception): void + { + app(OrphanImportProgress::class)->fail($exception->getMessage()); + } +} diff --git a/app/Modules/Files/OrphanFileImporter.php b/app/Modules/Files/OrphanFileImporter.php new file mode 100644 index 00000000..1fa65dd0 --- /dev/null +++ b/app/Modules/Files/OrphanFileImporter.php @@ -0,0 +1,62 @@ +storeFile->create( + uploader: $user, + originalName: basename($path), + path: $path, + mimeType: $disk->mimeType($path) ?: 'application/octet-stream', + size: $disk->size($path), + checksum: $this->checksumOf($disk, $path), + folderId: null, + disk: $diskName, + action: Action::FileImported, + ); + } + + /** + * Streamed rather than hash_file() on a local path — the only way to + * checksum a file that might live on a non-local disk (S3 has no + * local filesystem path to hand hash_file()). + */ + private function checksumOf(Filesystem $disk, string $path): string + { + $stream = $disk->readStream($path); + + if ($stream === null) { + return ''; + } + + $context = hash_init('sha256'); + hash_update_stream($context, $stream); + fclose($stream); + + return hash_final($context); + } +} diff --git a/app/Modules/Files/OrphanFileScanner.php b/app/Modules/Files/OrphanFileScanner.php index fddc6e80..d97cd19f 100644 --- a/app/Modules/Files/OrphanFileScanner.php +++ b/app/Modules/Files/OrphanFileScanner.php @@ -100,6 +100,26 @@ class OrphanFileScanner return $orphans; } + /** + * Every orphan the search matches that $viewer may import: the same + * rule as isImportable(), applied to a whole scan. Taken from a fresh + * scan, so unlike a client-supplied list it needs no per-path re-check. + * + * @return list + */ + public function importable(User $viewer, ?string $search = null): array + { + $importable = []; + + foreach ($this->scan($viewer, $search) as $orphan) { + if ($orphan['allowed'] && $orphan['size'] > 0) { + $importable[] = ['disk' => $orphan['disk'], 'path' => $orphan['path']]; + } + } + + return $importable; + } + /** * Re-validated at import/delete time — never trust a client-supplied * disk/path just because it was in an earlier scan response. $disk diff --git a/app/Modules/Files/OrphanImportProgress.php b/app/Modules/Files/OrphanImportProgress.php new file mode 100644 index 00000000..c278a4a1 --- /dev/null +++ b/app/Modules/Files/OrphanImportProgress.php @@ -0,0 +1,131 @@ +get(function () use ($total): bool { + if ($this->isActive()) { + return false; + } + + $this->put([ + 'status' => 'running', + 'total' => $total, + 'imported' => 0, + 'error' => null, + 'started_at' => now()->getTimestamp(), + ]); + + return true; + }); + } + + public function advance(): void + { + $run = $this->raw(); + + if ($run !== null) { + $run['imported']++; + $this->put($run); + } + } + + public function finish(): void + { + $this->end('finished'); + } + + public function fail(string $error): void + { + $this->end('failed', $error); + } + + public function isActive(): bool + { + return ($this->current()['status'] ?? null) === 'running'; + } + + /** + * The run as the page shows it. A running run that has gone quiet is + * reported as stalled. + * + * @return array{status: string, total: int, imported: int, error: ?string, started_at: int}|null + */ + public function current(): ?array + { + $run = $this->raw(); + + if ($run === null) { + return null; + } + + if ($run['status'] === 'running' && now()->getTimestamp() - $run['updated_at'] > self::STALL_SECONDS) { + $run['status'] = 'stalled'; + } + + unset($run['updated_at']); + + return $run; + } + + private function end(string $status, ?string $error = null): void + { + $run = $this->raw(); + + if ($run !== null) { + $this->put([...$run, 'status' => $status, 'error' => $error]); + } + } + + /** + * @return Run|null + */ + private function raw(): ?array + { + return Cache::get(self::KEY); + } + + /** + * @param array $run + */ + private function put(array $run): void + { + Cache::put(self::KEY, [...$run, 'updated_at' => now()->getTimestamp()], self::TTL_SECONDS); + } +} diff --git a/resources/js/pages/files/orphans.tsx b/resources/js/pages/files/orphans.tsx index 8a836c13..f4b7edae 100644 Binary files a/resources/js/pages/files/orphans.tsx and b/resources/js/pages/files/orphans.tsx differ diff --git a/routes/web.php b/routes/web.php index 38138e0e..1407cb4c 100644 --- a/routes/web.php +++ b/routes/web.php @@ -126,6 +126,7 @@ Route::middleware(['auth'])->group(function () { Route::middleware(['staff', 'can:import_orphans'])->group(function () { Route::get('files/orphans', [OrphanFilesController::class, 'index'])->name('orphan-files.index'); Route::post('files/orphans/import', [OrphanFilesController::class, 'import'])->name('orphan-files.import'); + Route::get('files/orphans/import-status', [OrphanFilesController::class, 'importStatus'])->name('orphan-files.import-status'); Route::post('files/orphans/delete', [OrphanFilesController::class, 'destroy'])->name('orphan-files.delete'); }); diff --git a/tests/Feature/Files/OrphanFilesTest.php b/tests/Feature/Files/OrphanFilesTest.php index 0738673b..235a063e 100644 --- a/tests/Feature/Files/OrphanFilesTest.php +++ b/tests/Feature/Files/OrphanFilesTest.php @@ -5,9 +5,12 @@ declare(strict_types=1); use App\Models\User; use App\Modules\Audit\Action; use App\Modules\Audit\ActivityLog; +use App\Modules\Files\Jobs\ImportOrphanFilesJob; use App\Modules\Files\Models\File; +use App\Modules\Files\OrphanImportProgress; use App\Modules\Platform\Settings\Setting; use App\Modules\Platform\Settings\Settings; +use Illuminate\Support\Facades\Queue; use Illuminate\Support\Facades\Storage; use Illuminate\Testing\TestResponse; use Inertia\Testing\AssertableInertia; @@ -31,6 +34,23 @@ function makeAdoptedFile(User $uploader, string $path, string $disk = 'files'): ]); } +/** + * 30 importable .txt files under 2026/07/batch, plus one empty and one + * restricted file there and one importable file outside it. + */ +function makeBatchOrphans(): void +{ + app(Settings::class)->set(Setting::UploadTypeRestriction, 'all'); + app(Settings::class)->set(Setting::AllowedUploadExtensions, ['txt']); + + foreach (range(1, 30) as $i) { + makeOrphanFile(sprintf('2026/07/batch/%02d.txt', $i)); + } + makeOrphanFile('2026/07/batch/empty.txt', ''); + makeOrphanFile('2026/07/batch/shell.php'); + makeOrphanFile('2026/07/elsewhere.txt'); +} + function orphanImport(array $items): TestResponse { return test()->postJson('/files/orphans/import', ['items' => $items]); @@ -215,6 +235,86 @@ test('importing multiple orphans at once stays on the list rather than picking o } }); +test('import all queues a background run for every importable match instead of importing in the request', function () { + Queue::fake(); + makeBatchOrphans(); + + $this->actingAs($this->admin) + ->postJson('/files/orphans/import', ['all' => true, 'search' => 'batch']) + ->assertRedirect(); + + expect(File::query()->count())->toBe(0); + Queue::assertPushed(ImportOrphanFilesJob::class, 1); + + $this->getJson('/files/orphans/import-status') + ->assertOk() + ->assertJson(['status' => 'running', 'total' => 30, 'imported' => 0]); +}); + +test('the background run imports every match across as many chunks as it takes, then reports finished', function () { + makeBatchOrphans(); + + app(OrphanImportProgress::class)->tryStart(30); + + // A zero budget makes every chunk stop after one file, so the run has + // to hand over to the next chunk 29 times to finish. + ImportOrphanFilesJob::dispatch($this->admin->id, 'batch', budgetSeconds: 0); + + expect(File::query()->count())->toBe(30) + ->and(File::query()->where('path', '2026/07/elsewhere.txt')->exists())->toBeFalse() + ->and(File::query()->where('uploaded_by', $this->admin->id)->count())->toBe(30); + + $this->actingAs($this->admin)->getJson('/files/orphans/import-status') + ->assertJson(['status' => 'finished', 'total' => 30, 'imported' => 30]); +}); + +test('while a run is in progress no other import is accepted, so nothing is adopted twice', function () { + Queue::fake(); + makeOrphanFile('2026/07/one.txt'); + + app(OrphanImportProgress::class)->tryStart(1); + + $this->actingAs($this->admin); + orphanImport([['disk' => 'files', 'path' => '2026/07/one.txt']])->assertUnprocessable(); + $this->postJson('/files/orphans/import', ['all' => true])->assertUnprocessable(); + + expect(File::query()->count())->toBe(0); + Queue::assertNothingPushed(); +}); + +test('a run that stops making progress is reported as stalled and no longer blocks a new one', function () { + Queue::fake(); + makeOrphanFile('2026/07/one.txt'); + + app(OrphanImportProgress::class)->tryStart(5); + $this->travel(6)->minutes(); + + $this->actingAs($this->admin)->getJson('/files/orphans/import-status') + ->assertJson(['status' => 'stalled', 'total' => 5, 'imported' => 0]); + + $this->postJson('/files/orphans/import', ['all' => true])->assertRedirect(); + Queue::assertPushed(ImportOrphanFilesJob::class, 1); +}); + +test('a run whose job fails is reported as failed with the reason', function () { + app(OrphanImportProgress::class)->tryStart(5); + + (new ImportOrphanFilesJob($this->admin->id, null))->failed(new RuntimeException('Disk unreachable')); + + $this->actingAs($this->admin)->getJson('/files/orphans/import-status') + ->assertJson(['status' => 'failed', 'error' => 'Disk unreachable']); +}); + +test('import all with nothing importable queues nothing', function () { + Queue::fake(); + makeOrphanFile('2026/07/empty.txt', ''); + + $this->actingAs($this->admin)->postJson('/files/orphans/import', ['all' => true])->assertRedirect(); + + Queue::assertNothingPushed(); + $this->getJson('/files/orphans/import-status')->assertOk()->assertExactJson([]); +}); + test('a 0-byte orphan cannot be imported but can still be deleted', function () { makeOrphanFile('2026/07/empty.txt', '');