diff --git a/src/app/Backends/Storage.php b/src/app/Backends/Storage.php --- a/src/app/Backends/Storage.php +++ b/src/app/Backends/Storage.php @@ -33,7 +33,25 @@ $disk->deleteDirectory($path); - $file->chunks()->delete(); + $file->forceDelete(); + } + + /** + * Delete a file chunk. + * + * @param \App\Fs\Chunk $chunk File chunk object + * + * @throws \Exception + */ + public static function fileChunkDelete(Chunk $chunk): void + { + $disk = LaravelStorage::disk('files'); + + $path = self::chunkLocation($chunk->chunk_id, $chunk->item); + + $disk->delete($path); + + $chunk->forceDelete(); } /** diff --git a/src/app/Console/Commands/Fs/ExpungeCommand.php b/src/app/Console/Commands/Fs/ExpungeCommand.php new file mode 100644 --- /dev/null +++ b/src/app/Console/Commands/Fs/ExpungeCommand.php @@ -0,0 +1,60 @@ +where('type', '&', Item::TYPE_FILE) + ->whereNotNull('deleted_at') + ->orderBy('deleted_at') + ->cursor(); + + // FIXME: Should we use an async job for each file? + + foreach ($files as $file) { + Storage::fileDelete($file); + } + + // We remove orphaned chunks after upload ttl (while marked as deleted some chunks may still be + // in the process of uploading a file). + $chunks = Chunk::withTrashed() + ->where('deleted_at', '<', now()->subSeconds(Storage::UPLOAD_TTL)) + ->orderBy('deleted_at') + ->cursor(); + + // FIXME: Should we use an async job for each chunk? + + foreach ($chunks as $chunk) { + Storage::fileChunkDelete($chunk); + } + } +} diff --git a/src/app/Console/Kernel.php b/src/app/Console/Kernel.php --- a/src/app/Console/Kernel.php +++ b/src/app/Console/Kernel.php @@ -28,6 +28,9 @@ // this is a laravel 8-ism //$schedule->command('wallet:charge')->everyFourHours(); + + // This command removes deleted storage files/file chunks from the filesystem + $schedule->command('fs:expunge')->hourly(); } /** diff --git a/src/app/Http/Controllers/API/V4/FilesController.php b/src/app/Http/Controllers/API/V4/FilesController.php --- a/src/app/Http/Controllers/API/V4/FilesController.php +++ b/src/app/Http/Controllers/API/V4/FilesController.php @@ -41,12 +41,8 @@ return $this->errorResponse($file); } - // FIXME: Here we're just deleting the file, but maybe it would be better/faster - // to mark the file (record in db) as deleted and invoke a job to - // delete it asynchronously? - - Storage::fileDelete($file); - + // Here we're just marking the file as deleted, it will be removed from the + // storage later with the fs:expunge command $file->delete(); return response()->json([ diff --git a/src/resources/vue/File/List.vue b/src/resources/vue/File/List.vue --- a/src/resources/vue/File/List.vue +++ b/src/resources/vue/File/List.vue @@ -23,7 +23,7 @@ - + {{ file.name }} diff --git a/src/tests/Feature/Controller/FilesTest.php b/src/tests/Feature/Controller/FilesTest.php --- a/src/tests/Feature/Controller/FilesTest.php +++ b/src/tests/Feature/Controller/FilesTest.php @@ -65,27 +65,9 @@ $this->assertSame('success', $json['status']); $this->assertSame("File deleted successfully.", $json['message']); - - // Test that the file has been removed from the filesystem? - $disk = LaravelStorage::disk('files'); - $this->assertFalse($disk->directoryExists($file->path . '/' . $file->id)); $this->assertSame(null, Item::find($file->id)); - // Test deletion of a chunked file - $file = $this->getTestFile($john, 'test.txt', ['T1', 'T2']); - - $response = $this->actingAs($john)->delete("api/v4/files/{$file->id}"); - $response->assertStatus(200); - - $json = $response->json(); - - $this->assertSame('success', $json['status']); - $this->assertSame("File deleted successfully.", $json['message']); - - // Test that the file has been removed from the filesystem? - $disk = LaravelStorage::disk('files'); - $this->assertFalse($disk->directoryExists($file->path . '/' . $file->id)); - $this->assertSame(null, Item::find($file->id)); + // Note: The file is expected to stay still in the filesystem, we're not testing this here. // TODO: Test acting as another user with permissions }