Browse Source

fix(dispatch): clean up transient library upload from Direct-Print flow (#730)

  The "Print" button on a printer card (and drag-drop-onto-card) used
  FileUploadModal to persist the file as a LibraryFile, then dispatched
  through POST /library/files/{id}/print. The LibraryFile row + disk file
  were left behind after every one-off print, polluting File Manager with
  entries the user never asked to save.

  FilePrintRequest.cleanup_library_after_dispatch (default False) opts
  into post-dispatch cleanup. When set, _run_print_library_file stages
  db.delete(lib_file) in the same transaction as archive_print so a
  mid-flight FTP / start_print failure rolls both back cleanly, commits
  together, then unlinks the library disk file + thumbnail after commit
  succeeds. External library files (is_external=True) are never touched.

  Only the Printers-page Direct-Print PrintModal sets the flag. Every
  other api.printLibraryFile caller (File Manager Print, Project Detail
  Print) leaves it unset — their entries are there by user intent.

  Also moves formatPrintName out of PrintersPage.tsx into a new
  utils/printName.ts module — fa1c46d9 (#881) exported it inline so its
  test could import it, tripping react-refresh/only-export-components.
maziggy 1 month ago
parent
commit
1682b6956f

File diff suppressed because it is too large
+ 0 - 0
CHANGELOG.md


+ 2 - 1
backend/app/api/routes/library.py

@@ -2236,10 +2236,11 @@ async def print_library_file(
             filename=dispatch_source_name,
             printer_id=printer_id,
             printer_name=printer.name,
-            options=body.model_dump(exclude_none=True),
+            options=body.model_dump(exclude_none=True, exclude={"cleanup_library_after_dispatch"}),
             project_id=body.project_id,
             requested_by_user_id=current_user.id if current_user else None,
             requested_by_username=current_user.username if current_user else None,
+            cleanup_library_after_dispatch=body.cleanup_library_after_dispatch,
         )
     except DispatchEnqueueRejected as e:
         raise HTTPException(status_code=409, detail=str(e)) from e

+ 6 - 0
backend/app/schemas/library.py

@@ -210,6 +210,12 @@ class FilePrintRequest(BaseModel):
     use_ams: bool = True
     # Project to associate the resulting archive with
     project_id: int | None = None
+    # When true, delete the LibraryFile row + disk file after the archive has
+    # been created and the print has been dispatched. Used by the Printers-page
+    # Direct-Print flow (click / drag-drop a file onto a printer card) so the
+    # transient upload doesn't linger in File Manager. Cleanup is skipped on
+    # external library files.
+    cleanup_library_after_dispatch: bool = False
 
 
 class FileUploadResponse(BaseModel):

+ 25 - 0
backend/app/services/background_dispatch.py

@@ -55,6 +55,7 @@ class PrintDispatchJob:
     requested_by_user_id: int | None = None
     requested_by_username: str | None = None
     project_id: int | None = None
+    cleanup_library_after_dispatch: bool = False
 
 
 @dataclass(slots=True)
@@ -162,6 +163,7 @@ class BackgroundDispatchService:
         requested_by_user_id: int | None,
         requested_by_username: str | None,
         project_id: int | None = None,
+        cleanup_library_after_dispatch: bool = False,
     ) -> dict[str, Any]:
         return await self._dispatch(
             kind="print_library_file",
@@ -173,6 +175,7 @@ class BackgroundDispatchService:
             requested_by_user_id=requested_by_user_id,
             requested_by_username=requested_by_username,
             project_id=project_id,
+            cleanup_library_after_dispatch=cleanup_library_after_dispatch,
         )
 
     async def cancel_job(self, job_id: int) -> dict[str, Any]:
@@ -261,6 +264,7 @@ class BackgroundDispatchService:
         requested_by_user_id: int | None,
         requested_by_username: str | None,
         project_id: int | None = None,
+        cleanup_library_after_dispatch: bool = False,
     ) -> dict[str, Any]:
         async with self._lock:
             has_pending_for_printer = any(job.printer_id == printer_id for job in self._queued_jobs)
@@ -284,6 +288,7 @@ class BackgroundDispatchService:
                 requested_by_user_id=requested_by_user_id,
                 requested_by_username=requested_by_username,
                 project_id=project_id,
+                cleanup_library_after_dispatch=cleanup_library_after_dispatch,
             )
             self._next_job_id += 1
             self._batch_total += 1
@@ -863,7 +868,27 @@ class BackgroundDispatchService:
                         job.requested_by_username,
                     )
 
+                # Direct-Print flow only: archive_print copies, so deleting the
+                # transient library row + files here leaves archive intact. Disk
+                # deletes run only after commit so a rollback leaves no orphan.
+                cleanup_disk_paths: list[Path] = []
+                if job.cleanup_library_after_dispatch and not lib_file.is_external:
+                    cleanup_disk_paths.append(file_path)
+                    if lib_file.thumbnail_path:
+                        thumb_path = Path(lib_file.thumbnail_path)
+                        if not thumb_path.is_absolute():
+                            thumb_path = Path(settings.base_dir) / lib_file.thumbnail_path
+                        cleanup_disk_paths.append(thumb_path)
+                    await db.delete(lib_file)
+
                 await db.commit()
+
+                for cleanup_path in cleanup_disk_paths:
+                    try:
+                        if cleanup_path.exists():
+                            cleanup_path.unlink()
+                    except OSError as cleanup_err:
+                        logger.warning("Failed to delete transient library file %s: %s", cleanup_path, cleanup_err)
             except DispatchJobCancelled:
                 await db.rollback()
                 await self._set_active_message(job, f"Cancelled upload on {printer_name}.")

+ 63 - 0
backend/tests/integration/test_background_dispatch_api.py

@@ -179,6 +179,69 @@ class TestBackgroundDispatchLibraryAPI:
         assert response.status_code == 409
         assert "queue conflict" in response.json()["detail"]
 
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_library_print_cleanup_flag_defaults_false(
+        self, async_client: AsyncClient, library_file_factory, printer_factory, db_session, tmp_path
+    ):
+        """Absent cleanup_library_after_dispatch in the request body ⇒ False reaches the dispatcher.
+        Guards the File Manager / Project Detail paths from accidental deletion."""
+        printer = await printer_factory()
+        lib_file = await library_file_factory(filename="filemgr_part.gcode.3mf")
+
+        disk_path = tmp_path / lib_file.file_path
+        disk_path.parent.mkdir(parents=True, exist_ok=True)
+        disk_path.write_bytes(b"library data")
+
+        with (
+            patch("backend.app.api.routes.library.app_settings.base_dir", tmp_path),
+            patch("backend.app.services.printer_manager.printer_manager.is_connected", return_value=True),
+            patch(
+                "backend.app.services.background_dispatch.background_dispatch.dispatch_print_library_file",
+                new=AsyncMock(return_value={"dispatch_job_id": 30, "dispatch_position": 1}),
+            ) as mock_dispatch,
+        ):
+            response = await async_client.post(
+                f"/api/v1/library/files/{lib_file.id}/print?printer_id={printer.id}",
+                json={},
+            )
+
+        assert response.status_code == 200
+        mock_dispatch.assert_awaited_once()
+        assert mock_dispatch.await_args.kwargs["cleanup_library_after_dispatch"] is False
+        # cleanup flag must never leak into the print-option dict forwarded to MQTT
+        assert "cleanup_library_after_dispatch" not in mock_dispatch.await_args.kwargs["options"]
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_library_print_forwards_cleanup_flag_true(
+        self, async_client: AsyncClient, library_file_factory, printer_factory, db_session, tmp_path
+    ):
+        """Direct-Print flow sends cleanup_library_after_dispatch=True, which must reach the dispatcher."""
+        printer = await printer_factory()
+        lib_file = await library_file_factory(filename="transient_part.gcode.3mf")
+
+        disk_path = tmp_path / lib_file.file_path
+        disk_path.parent.mkdir(parents=True, exist_ok=True)
+        disk_path.write_bytes(b"library data")
+
+        with (
+            patch("backend.app.api.routes.library.app_settings.base_dir", tmp_path),
+            patch("backend.app.services.printer_manager.printer_manager.is_connected", return_value=True),
+            patch(
+                "backend.app.services.background_dispatch.background_dispatch.dispatch_print_library_file",
+                new=AsyncMock(return_value={"dispatch_job_id": 31, "dispatch_position": 1}),
+            ) as mock_dispatch,
+        ):
+            response = await async_client.post(
+                f"/api/v1/library/files/{lib_file.id}/print?printer_id={printer.id}",
+                json={"cleanup_library_after_dispatch": True},
+            )
+
+        assert response.status_code == 200
+        mock_dispatch.assert_awaited_once()
+        assert mock_dispatch.await_args.kwargs["cleanup_library_after_dispatch"] is True
+
 
 class TestBackgroundDispatchCancelAPI:
     """Tests for /background-dispatch cancel endpoint."""

+ 54 - 0
backend/tests/unit/services/test_background_dispatch.py

@@ -68,6 +68,60 @@ async def test_dispatch_enqueues_job_and_broadcasts_state():
     assert payload["data"]["recent_event"]["status"] == "dispatched"
 
 
+@pytest.mark.asyncio
+async def test_dispatch_library_file_defaults_cleanup_flag_false():
+    """cleanup_library_after_dispatch defaults to False when not passed — protects
+    File Manager / Project Detail / queued-library-file paths from surprise deletion."""
+    service = BackgroundDispatchService()
+
+    with (
+        patch("backend.app.services.background_dispatch.printer_manager.get_status", return_value=None),
+        patch("backend.app.services.background_dispatch.ws_manager.broadcast", new_callable=AsyncMock),
+    ):
+        await service.dispatch_print_library_file(
+            file_id=1,
+            filename="cube.gcode.3mf",
+            printer_id=1,
+            printer_name="Printer A",
+            options={},
+            requested_by_user_id=None,
+            requested_by_username=None,
+        )
+
+    assert len(service._queued_jobs) == 1
+    assert service._queued_jobs[0].cleanup_library_after_dispatch is False
+
+
+@pytest.mark.asyncio
+async def test_dispatch_library_file_propagates_cleanup_flag_true():
+    """cleanup_library_after_dispatch=True arrives on the queued job so the runner
+    can delete the transient LibraryFile after the print is accepted by the printer."""
+    service = BackgroundDispatchService()
+
+    with (
+        patch("backend.app.services.background_dispatch.printer_manager.get_status", return_value=None),
+        patch("backend.app.services.background_dispatch.ws_manager.broadcast", new_callable=AsyncMock),
+    ):
+        await service.dispatch_print_library_file(
+            file_id=1,
+            filename="cube.gcode.3mf",
+            printer_id=1,
+            printer_name="Printer A",
+            options={},
+            requested_by_user_id=42,
+            requested_by_username="alice",
+            cleanup_library_after_dispatch=True,
+        )
+
+    assert len(service._queued_jobs) == 1
+    job = service._queued_jobs[0]
+    assert job.cleanup_library_after_dispatch is True
+    # Sanity: other fields still wired correctly
+    assert job.requested_by_user_id == 42
+    assert job.requested_by_username == "alice"
+    assert job.kind == "print_library_file"
+
+
 @pytest.mark.asyncio
 async def test_cancel_queued_job_removes_it_and_broadcasts():
     """Cancelling queued job removes it immediately."""

+ 102 - 0
frontend/src/__tests__/components/PrintModal.test.tsx

@@ -1244,4 +1244,106 @@ describe('PrintModal', () => {
       });
     });
   });
+
+  describe('cleanup_library_after_dispatch forwarding (#730)', () => {
+    // The Printers-page Direct-Print flow passes cleanupLibraryAfterDispatch so the
+    // transient LibraryFile created by FileUploadModal is deleted once the archive
+    // owns its own copy. File Manager / Project Detail flows leave the prop unset so
+    // their deliberately-added library entries survive the print.
+    beforeEach(() => {
+      server.use(
+        http.get('/api/v1/library/files/:id', () => {
+          return HttpResponse.json({
+            id: 5,
+            filename: 'benchy.gcode.3mf',
+            file_type: '3mf',
+            folder_id: null,
+            project_id: null,
+            file_hash: null,
+            file_size_bytes: 1024,
+            thumbnail_path: null,
+            created_at: '2024-01-01T00:00:00Z',
+            updated_at: '2024-01-01T00:00:00Z',
+          });
+        }),
+        http.get('/api/v1/library/files/:id/plates', () => {
+          return HttpResponse.json({ is_multi_plate: false, plates: [] });
+        }),
+        http.get('/api/v1/library/files/:id/filament-requirements', () => {
+          return HttpResponse.json({ file_id: 5, filename: 'benchy.gcode.3mf', filaments: [] });
+        }),
+        http.get('/api/v1/printers/:id/status', () => {
+          return HttpResponse.json({ connected: true, state: 'IDLE', ams: [], vt_tray: [] });
+        }),
+      );
+    });
+
+    it('forwards cleanup_library_after_dispatch=true when the Direct-Print prop is set', async () => {
+      let capturedBody: Record<string, unknown> | null = null;
+      server.use(
+        http.post('/api/v1/library/files/:id/print', async ({ request }) => {
+          capturedBody = (await request.json()) as Record<string, unknown>;
+          return HttpResponse.json({ status: 'dispatched', dispatch_job_id: 'abc', dispatch_position: 0 });
+        })
+      );
+      const user = userEvent.setup();
+
+      render(
+        <PrintModal
+          mode="reprint"
+          libraryFileId={5}
+          archiveName="Benchy"
+          cleanupLibraryAfterDispatch
+          initialSelectedPrinterIds={[1]}
+          onClose={mockOnClose}
+          onSuccess={mockOnSuccess}
+        />
+      );
+
+      await waitFor(() => {
+        expect(screen.getByRole('button', { name: /^print$/i })).toBeInTheDocument();
+      });
+
+      await user.click(screen.getByRole('button', { name: /^print$/i }));
+
+      await waitFor(() => {
+        expect(capturedBody).not.toBeNull();
+        expect(capturedBody?.cleanup_library_after_dispatch).toBe(true);
+      });
+    });
+
+    it('defaults to omitting cleanup_library_after_dispatch (File Manager / Project flows survive)', async () => {
+      let capturedBody: Record<string, unknown> | null = null;
+      server.use(
+        http.post('/api/v1/library/files/:id/print', async ({ request }) => {
+          capturedBody = (await request.json()) as Record<string, unknown>;
+          return HttpResponse.json({ status: 'dispatched', dispatch_job_id: 'abc', dispatch_position: 0 });
+        })
+      );
+      const user = userEvent.setup();
+
+      render(
+        <PrintModal
+          mode="reprint"
+          libraryFileId={5}
+          archiveName="Benchy"
+          initialSelectedPrinterIds={[1]}
+          onClose={mockOnClose}
+          onSuccess={mockOnSuccess}
+        />
+      );
+
+      await waitFor(() => {
+        expect(screen.getByRole('button', { name: /^print$/i })).toBeInTheDocument();
+      });
+
+      await user.click(screen.getByRole('button', { name: /^print$/i }));
+
+      await waitFor(() => {
+        expect(capturedBody).not.toBeNull();
+      });
+      // Either omitted entirely or explicitly undefined — both interpret as "keep file"
+      expect(capturedBody?.cleanup_library_after_dispatch).toBeUndefined();
+    });
+  });
 });

+ 1 - 1
frontend/src/__tests__/pages/PrintersPageFormatPrintName.test.ts

@@ -8,7 +8,7 @@
  */
 
 import { describe, it, expect } from 'vitest';
-import { formatPrintName } from '../../pages/PrintersPage';
+import { formatPrintName } from '../../utils/printName';
 
 // Minimal translator stub: returns the fallback with the plate number interpolated
 // the same way i18next would. Keeps these tests independent of the i18n setup.

+ 1 - 0
frontend/src/api/client.ts

@@ -4645,6 +4645,7 @@ export const api = {
       timelapse?: boolean;
       use_ams?: boolean;
       project_id?: number;
+      cleanup_library_after_dispatch?: boolean;
     }
   ) =>
     request<BackgroundDispatchResponse>(

+ 2 - 0
frontend/src/components/PrintModal/index.tsx

@@ -49,6 +49,7 @@ export function PrintModal({
   onClose,
   onSuccess,
   projectId,
+  cleanupLibraryAfterDispatch,
 }: PrintModalProps) {
   const { t } = useTranslation();
   const queryClient = useQueryClient();
@@ -737,6 +738,7 @@ export function PrintModal({
                   ams_mapping: printerMapping,
                   ...printOptions,
                   project_id: projectId,
+                  cleanup_library_after_dispatch: cleanupLibraryAfterDispatch,
                 });
               } else {
                 // project_id is intentionally omitted here: reprintArchive targets an existing

+ 3 - 0
frontend/src/components/PrintModal/types.ts

@@ -34,6 +34,9 @@ export interface PrintModalProps {
   onSuccess?: () => void;
   /** Project ID to associate the resulting archive with (only when triggered from project view) */
   projectId?: number;
+  /** Delete the LibraryFile after dispatch — used by the Printers-page Direct-Print flow
+   *  so transient uploads don't linger in File Manager. Only applies to library-file prints. */
+  cleanupLibraryAfterDispatch?: boolean;
 }
 
 /**

+ 2 - 22
frontend/src/pages/PrintersPage.tsx

@@ -1,5 +1,6 @@
 import { useState, useEffect, useMemo, useRef, useCallback } from 'react';
 import { compareFwVersions } from '../utils/firmwareVersion';
+import { formatPrintName } from '../utils/printName';
 import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query';
 import { useTranslation } from 'react-i18next';
 import { useTheme } from '../contexts/ThemeContext';
@@ -90,28 +91,6 @@ import { getColorName, parseFilamentColor, isLightColor } from '../utils/colors'
 // Color names resolve via getColorName() which reads the backend color_catalog
 // (loaded once by ColorCatalogProvider). No hardcoded tables here — see #857.
 
-// Append a plate label to the print name. When `plateLabel` is provided (resolved
-// by the caller from the linked archive's plate list — see #881 follow-up), it
-// is used verbatim, including the explicit "Plate 1" case on multi-plate 3MFs.
-// Falls back to parsing `plate_N.gcode` from the MQTT gcode_file path, and in
-// that fallback we only show N > 1 because we can't tell from the path alone
-// whether the 3MF is multi-plate.
-export function formatPrintName(
-  name: string | null,
-  gcodeFile: string | null | undefined,
-  t: (key: string, fallback: string, opts?: Record<string, unknown>) => string,
-  plateLabel?: string | null,
-): string {
-  if (!name) return '';
-  if (plateLabel) return `${name} — ${plateLabel}`;
-  if (!gcodeFile) return name;
-  const match = gcodeFile.match(/plate_(\d+)\.gcode/);
-  if (match && parseInt(match[1], 10) > 1) {
-    return `${name} — ${t('printers.plateNumber', 'Plate {{number}}', { number: match[1] })}`;
-  }
-  return name;
-}
-
 // Format K value with 3 decimal places, default to 0.020 if null
 function formatKValue(k: number | null | undefined): string {
   const value = k ?? 0.020;
@@ -4419,6 +4398,7 @@ function PrinterCard({
           initialSelectedPrinterIds={[printer.id]}
           onClose={() => setPrintAfterUpload(null)}
           onSuccess={() => setPrintAfterUpload(null)}
+          cleanupLibraryAfterDispatch
         />
       )}
 

+ 23 - 0
frontend/src/utils/printName.ts

@@ -0,0 +1,23 @@
+/**
+ * Append a plate label to the print name. When `plateLabel` is provided (resolved
+ * by the caller from the linked archive's plate list — see #881 follow-up), it
+ * is used verbatim, including the explicit "Plate 1" case on multi-plate 3MFs.
+ * Falls back to parsing `plate_N.gcode` from the MQTT gcode_file path, and in
+ * that fallback we only show N > 1 because we can't tell from the path alone
+ * whether the 3MF is multi-plate.
+ */
+export function formatPrintName(
+  name: string | null,
+  gcodeFile: string | null | undefined,
+  t: (key: string, fallback: string, opts?: Record<string, unknown>) => string,
+  plateLabel?: string | null,
+): string {
+  if (!name) return '';
+  if (plateLabel) return `${name} — ${plateLabel}`;
+  if (!gcodeFile) return name;
+  const match = gcodeFile.match(/plate_(\d+)\.gcode/);
+  if (match && parseInt(match[1], 10) > 1) {
+    return `${name} — ${t('printers.plateNumber', 'Plate {{number}}', { number: match[1] })}`;
+  }
+  return name;
+}

File diff suppressed because it is too large
+ 0 - 0
static/assets/index-BskdRxdi.js


+ 1 - 1
static/index.html

@@ -26,7 +26,7 @@
 
     <!-- Splash screens for iOS -->
     <link rel="apple-touch-startup-image" href="/img/android-chrome-512x512.png" />
-    <script type="module" crossorigin src="/assets/index-06WvZfP-.js"></script>
+    <script type="module" crossorigin src="/assets/index-BskdRxdi.js"></script>
     <link rel="stylesheet" crossorigin href="/assets/index-CkAOuJaW.css">
   </head>
   <body>

Some files were not shown because too many files changed in this diff