API: Initial review pass of zip import/export endpoints

Review of #5592

diff --git a/app/Exports/Controllers/BookExportApiController.php b/app/Exports/Controllers/BookExportApiController.php
index 431afef143db45bccfa14994b040e2a00a1d44fb..e2d0addc30c9a6c0c3e3dda887f1e869aa50506f 100644 (file)
--- a/app/Exports/Controllers/BookExportApiController.php
+++ b/app/Exports/Controllers/BookExportApiController.php
@@ -65,18 +65,14 @@ class BookExportApiController extends ApiController
         return $this->download()->directly($markdown, $book->slug . '.md');
     }
 
-    
     /**
      * Export a book to a contained ZIP export file.
-     * @throws NotFoundException
      */
     public function exportZip(int $id, ZipExportBuilder $builder)
     {
         $book = $this->queries->findVisibleByIdOrFail($id);
-        $bookName= $book->getShortName();
-     
         $zip = $builder->buildForBook($book);
 
-        return $this->download()->streamedFileDirectly($zip, $bookName . '.zip', filesize($zip), true);
+        return $this->download()->streamedFileDirectly($zip, $book->slug . '.zip', true);
     }
-}
\ No newline at end of file
+}

diff --git a/app/Exports/Controllers/ChapterExportApiController.php b/app/Exports/Controllers/ChapterExportApiController.php
index 58df4c9b0872f2f7b026654834b745e604f7f977..66e2276b5ce2d6401b355b7d05992c494658fbbb 100644 (file)
--- a/app/Exports/Controllers/ChapterExportApiController.php
+++ b/app/Exports/Controllers/ChapterExportApiController.php
@@ -68,9 +68,8 @@ class ChapterExportApiController extends ApiController
     public function exportZip(int $id, ZipExportBuilder $builder)
     {
         $chapter = $this->queries->findVisibleByIdOrFail($id);
-        $chapterName= $chapter->getShortName();
         $zip = $builder->buildForChapter($chapter);
 
-        return $this->download()->streamedFileDirectly($zip, $chapterName . '.zip', filesize($zip), true);
+        return $this->download()->streamedFileDirectly($zip, $chapter->slug . '.zip', true);
     }
-}
\ No newline at end of file
+}

diff --git a/app/Exports/Controllers/ImportApiController.php b/app/Exports/Controllers/ImportApiController.php
index 682d340b3b4555ec12daf9700344118635c41abc..13bc9d83ea5e027ab03260103ef502006cac2695 100644 (file)
--- a/app/Exports/Controllers/ImportApiController.php
+++ b/app/Exports/Controllers/ImportApiController.php
@@ -7,12 +7,13 @@ namespace BookStack\Exports\Controllers;
 use BookStack\Exceptions\ZipImportException;
 use BookStack\Exceptions\ZipValidationException;
 use BookStack\Exports\ImportRepo;
-use BookStack\Http\Controller;
+use BookStack\Http\ApiController;
 use BookStack\Uploads\AttachmentService;
 use Illuminate\Http\Request;
 use Illuminate\Http\JsonResponse;
+use Illuminate\Http\Response;
 
-class ImportApiController extends Controller
+class ImportApiController extends ApiController
 {
     public function __construct(
         protected ImportRepo $imports,
@@ -21,101 +22,94 @@ class ImportApiController extends Controller
     }
 
     /**
-     * List existing imports visible to the user.
+     * List existing ZIP imports visible to the user.
      */
     public function list(): JsonResponse
     {
-        $imports = $this->imports->getVisibleImports();
+        $imports = $this->imports->getVisibleImports()->all();
 
-        return response()->json([
-            'status' => 'success',
-            'imports' => $imports,
-        ]);
+        return response()->json($imports);
     }
 
     /**
-     * Upload, validate and store an import file.
+     * Upload, validate and store a ZIP import file.
+     * This does not run the import. That is performed via a separate endpoint.
      */
     public function upload(Request $request): JsonResponse
     {
-        $this->validate($request, [
-            'file' => ['required', ...AttachmentService::getFileValidationRules()]
-        ]);
+        $this->validate($request, $this->rules()['upload']);
 
         $file = $request->file('file');
 
         try {
             $import = $this->imports->storeFromUpload($file);
         } catch (ZipValidationException $exception) {
-            return response()->json([
-                'status' => 'error',
-                'message' => 'Validation failed',
-                'errors' => $exception->errors,
-            ], 422);
+            $message = "ZIP upload failed with the following validation errors: \n" . implode("\n", $exception->errors);
+            return $this->jsonError($message, 422);
         }
 
-        return response()->json([
-            'status' => 'success',
-            'import' => $import,
-        ], 201);
+        return response()->json($import);
     }
 
     /**
-     * Show details of a pending import.
+     * Read details of a pending ZIP import.
      */
     public function read(int $id): JsonResponse
     {
         $import = $this->imports->findVisible($id);
 
-        return response()->json([
-            'status' => 'success',
-            'import' => $import,
-            'data' => $import->decodeMetadata(),
-        ]);
+        return response()->json($import);
     }
 
     /**
-     * Run the import process.
+     * Run the import process for an uploaded ZIP import.
+     * The parent_id and parent_type parameters are required when the import type is 'chapter' or 'page'.
+     * On success, returns the imported item.
      */
-    public function create(int $id, Request $request): JsonResponse
+    public function run(int $id, Request $request): JsonResponse
     {
         $import = $this->imports->findVisible($id);
         $parent = null;
+        $rules = $this->rules()['run'];
 
         if ($import->type === 'page' || $import->type === 'chapter') {
-            $data = $this->validate($request, [
-                'parent' => ['required', 'string'],
-            ]);
-            $parent = $data['parent'];
+            $rules['parent_type'][] = 'required';
+            $rules['parent_id'][] = 'required';
+            $data = $this->validate($request, $rules);
+            $parent = "{$data['parent_type']}:{$data['parent_id']}";
         }
 
         try {
             $entity = $this->imports->runImport($import, $parent);
         } catch (ZipImportException $exception) {
-            return response()->json([
-                'status' => 'error',
-                'message' => 'Import failed',
-                'errors' => $exception->errors,
-            ], 500);
+            $message = "ZIP import failed with the following errors: \n" . implode("\n", $exception->errors);
+            return $this->jsonError($message);
         }
 
-        return response()->json([
-            'status' => 'success',
-            'entity' => $entity,
-        ]);
+        return response()->json($entity);
     }
 
     /**
-     * Delete a pending import.
+     * Delete a pending ZIP import.
      */
-    public function delete(int $id): JsonResponse
+    public function delete(int $id): Response
     {
         $import = $this->imports->findVisible($id);
         $this->imports->deleteImport($import);
 
-        return response()->json([
-            'status' => 'success',
-            'message' => 'Import deleted successfully',
-        ]);
+        return response('', 204);
     }
-}
\ No newline at end of file
+
+    protected function rules(): array
+    {
+        return [
+            'upload' => [
+                'file' => ['required', ...AttachmentService::getFileValidationRules()],
+            ],
+            'run' => [
+                'parent_type' => ['string', 'in:book,chapter'],
+                'parent_id' => ['int'],
+            ],
+        ];
+    }
+}

diff --git a/app/Exports/Controllers/PageExportApiController.php b/app/Exports/Controllers/PageExportApiController.php
index ef564da3e5c3c7ebab4773e2108f7a36a476709f..d6412614c45b6c1b0bc7fb3931c15bd201128d43 100644 (file)
--- a/app/Exports/Controllers/PageExportApiController.php
+++ b/app/Exports/Controllers/PageExportApiController.php
@@ -65,14 +65,11 @@ class PageExportApiController extends ApiController
         return $this->download()->directly($markdown, $page->slug . '.md');
     }
 
-
-
     public function exportZip(int $id, ZipExportBuilder $builder)
     {
         $page = $this->queries->findVisibleByIdOrFail($id);
-        $pageSlug = $page->slug;
         $zip = $builder->buildForPage($page);
 
-        return $this->download()->streamedFileDirectly($zip, $pageSlug . '.zip', filesize($zip), true);
+        return $this->download()->streamedFileDirectly($zip, $page->slug . '.zip', true);
     }
-}
\ No newline at end of file
+}

diff --git a/routes/api.php b/routes/api.php
index 5bdf5361130b32efd3c7db1ab51dda68aebb2e07..efb7b258c6fa78b75a34062b07c766ee8ebedea5 100644 (file)
--- a/routes/api.php
+++ b/routes/api.php
@@ -88,6 +88,12 @@ Route::get('roles/{id}', [RoleApiController::class, 'read']);
 Route::put('roles/{id}', [RoleApiController::class, 'update']);
 Route::delete('roles/{id}', [RoleApiController::class, 'delete']);
 
+Route::get('import', [ExportControllers\ImportApiController::class, 'list']);
+Route::post('import', [ExportControllers\ImportApiController::class, 'upload']);
+Route::get('import/{id}', [ExportControllers\ImportApiController::class, 'read']);
+Route::post('import/{id}', [ExportControllers\ImportApiController::class, 'run']);
+Route::delete('import/{id}', [ExportControllers\ImportApiController::class, 'delete']);
+
 Route::get('recycle-bin', [EntityControllers\RecycleBinApiController::class, 'list']);
 Route::put('recycle-bin/{deletionId}', [EntityControllers\RecycleBinApiController::class, 'restore']);
 Route::delete('recycle-bin/{deletionId}', [EntityControllers\RecycleBinApiController::class, 'destroy']);
@@ -98,9 +104,3 @@ Route::put('content-permissions/{contentType}/{contentId}', [ContentPermissionAp
 Route::get('audit-log', [AuditLogApiController::class, 'list']);
 
 Route::get('system', [SystemApiController::class, 'read']);
-
-Route::get('import', [ExportControllers\ImportApiController::class, 'list']);
-Route::post('import', [ExportControllers\ImportApiController::class, 'upload']);
-Route::get('import/{id}', [ExportControllers\ImportApiController::class, 'read']);
-Route::post('import/{id}/create', [ExportControllers\ImportApiController::class, 'create']);
-Route::delete('import/{id}', [ExportControllers\ImportApiController::class, 'destroy']);
\ No newline at end of file