Browse Source

fix(dispatch): align SD cleanup with upload path so doubled-extension library rows don't leave ghost prints (#1542)

  A library row with archive.filename "Cube (1).gcode.3mf.gcode.3mf" uploaded
  to /Cube_(1).gcode.3mf.3mf (single-iteration strip + append). Post-print
  cleanup looked at /Cube_(1).3mf and /Cube_(1).gcode (subtask_name + ext),
  missed the on-card file, and A1 firmware re-ran it on next power-on.

  - New derive_remote_filename() helper in backend/app/utils/filename.py:
    iterative strip of .gcode.3mf/.3mf suffixes, append single .3mf,
    space->underscore. isinstance() guard raises TypeError on non-str
    input rather than entering the strip loop with a duck-typed
    object that returns truthy sentinels from endswith.
  - Three previously-duplicated upload sites (background_dispatch reprint +
    library, print_scheduler queue) now share the helper.
  - SD cleanup fetches archive.filename and tries the derived path first,
    with the legacy subtask_name + ext paths kept as fallbacks.
  - 10 new unit tests pin the reproducer + edge cases (doubled .gcode.3mf,
    doubled .3mf, raw .gcode preserved, idempotence, Unicode, plus the
    type guard against MagicMock / None / int inputs).
maziggy 7 giờ trước cách đây
mục cha
commit
6d316c5593

Những thai đổi đã bị hủy bỏ vì nó quá lớn
+ 0 - 0
CHANGELOG.md


+ 24 - 5
backend/app/main.py

@@ -3324,24 +3324,43 @@ async def on_print_complete(printer_id: int, data: dict):
                 if archive:
                     archive_id = archive.id
 
-    # Cleanup: delete uploaded file from printer SD card to prevent phantom prints (Issue #374)
-    # The print scheduler uploads files to the SD card root (/). Some printers (e.g. P1S)
+    # Cleanup: delete uploaded file from printer SD card to prevent phantom prints (Issue #374, #1542)
+    # The print scheduler uploads files to the SD card root (/). Some printers (e.g. P1S, A1)
     # auto-start files found in root on power cycle, causing ghost prints.
     # Must run before the archive_id early-return so it executes even when archiving is disabled.
     try:
         if subtask_name:
+            archive_filename: str | None = None
             async with async_session() as db:
+                from backend.app.models.archive import PrintArchive
                 from backend.app.models.printer import Printer
 
                 result = await db.execute(select(Printer).where(Printer.id == printer_id))
                 printer = result.scalar_one_or_none()
+                if archive_id:
+                    archive_row = await db.execute(select(PrintArchive.filename).where(PrintArchive.id == archive_id))
+                    archive_filename = archive_row.scalar_one_or_none()
 
             if printer:
                 from backend.app.services.bambu_ftp import delete_file_async
-
-                # Try both .3mf and .gcode extensions — the printer may have either
+                from backend.app.utils.filename import derive_remote_filename
+
+                # Primary candidate: the exact path the dispatcher uploaded to
+                # (derived from archive.filename via the same rule as upload).
+                # Without it, a library row that ended up with a doubled
+                # .gcode.3mf (#1542) leaves the real file behind because the
+                # subtask_name + ext fallbacks below don't match what's on the
+                # SD card. Fallbacks remain for archive-less prints (subtask
+                # never resolved to an archive) and for older naming variants.
+                candidate_paths: list[str] = []
+                if archive_filename:
+                    candidate_paths.append(f"/{derive_remote_filename(archive_filename)}")
                 for ext in (".3mf", ".gcode"):
-                    remote_path = f"/{subtask_name}{ext}"
+                    fallback = f"/{subtask_name}{ext}"
+                    if fallback not in candidate_paths:
+                        candidate_paths.append(fallback)
+
+                for remote_path in candidate_paths:
                     # Retry up to 3 times — the printer may still lock the filesystem briefly after a print ends
                     for attempt in range(1, 4):
                         try:

+ 3 - 16
backend/app/services/background_dispatch.py

@@ -32,6 +32,7 @@ from backend.app.services.bambu_ftp import (
     with_ftp_retry,
 )
 from backend.app.services.printer_manager import printer_manager
+from backend.app.utils.filename import derive_remote_filename
 
 logger = logging.getLogger(__name__)
 
@@ -580,14 +581,7 @@ class BackgroundDispatchService:
             if not file_path.exists():
                 raise RuntimeError("Archive file not found")
 
-            base_name = archive.filename
-            if base_name.endswith(".gcode.3mf"):
-                base_name = base_name[:-10]
-            elif base_name.endswith(".3mf"):
-                base_name = base_name[:-4]
-            remote_filename = f"{base_name}.3mf"
-            # Sanitize: firmware parses ftp://{filename} as a URL, spaces break it
-            remote_filename = remote_filename.replace(" ", "_")
+            remote_filename = derive_remote_filename(archive.filename)
             remote_path = f"/{remote_filename}"
 
             ftp_retry_enabled, ftp_retry_count, ftp_retry_delay, ftp_timeout = await get_ftp_retry_settings()
@@ -784,14 +778,7 @@ class BackgroundDispatchService:
 
             await db.flush()
 
-            base_name = lib_file.filename
-            if base_name.endswith(".gcode.3mf"):
-                base_name = base_name[:-10]
-            elif base_name.endswith(".3mf"):
-                base_name = base_name[:-4]
-            remote_filename = f"{base_name}.3mf"
-            # Sanitize: firmware parses ftp://{filename} as a URL, spaces break it
-            remote_filename = remote_filename.replace(" ", "_")
+            remote_filename = derive_remote_filename(lib_file.filename)
             remote_path = f"/{remote_filename}"
 
             ftp_retry_enabled, ftp_retry_count, ftp_retry_delay, ftp_timeout = await get_ftp_retry_settings()

+ 2 - 10
backend/app/services/print_scheduler.py

@@ -32,6 +32,7 @@ from backend.app.services.filament_deficit import compute_deficit_for_queue_item
 from backend.app.services.notification_service import notification_service
 from backend.app.services.printer_manager import printer_manager, supports_drying
 from backend.app.services.smart_plug_manager import smart_plug_manager
+from backend.app.utils.filename import derive_remote_filename
 from backend.app.utils.printer_models import normalize_printer_model
 
 logger = logging.getLogger(__name__)
@@ -2013,18 +2014,9 @@ class PrintScheduler:
             except Exception as e:
                 logger.warning("Queue item %s: G-code injection failed, using original: %s", item.id, e)
 
-        # Upload file to printer via FTP
-        # Use a clean filename to avoid issues with double extensions like .gcode.3mf
-        base_name = filename
-        if base_name.endswith(".gcode.3mf"):
-            base_name = base_name[:-10]  # Remove .gcode.3mf
-        elif base_name.endswith(".3mf"):
-            base_name = base_name[:-4]  # Remove .3mf
-        remote_filename = f"{base_name}.3mf"
-        # Sanitize: firmware parses ftp://{filename} as a URL, spaces break it
-        remote_filename = remote_filename.replace(" ", "_")
         # Upload to root directory (not /cache/) - the start_print command references
         # files by name only (ftp://{filename}), so they must be in the root
+        remote_filename = derive_remote_filename(filename)
         remote_path = f"/{remote_filename}"
 
         # Get FTP retry settings

+ 33 - 0
backend/app/utils/filename.py

@@ -55,3 +55,36 @@ def validate_print_filename(name: str) -> None:
 
     if len(name.encode("utf-8")) > MAX_FILENAME_BYTES:
         raise InvalidFilenameError(f"Filename exceeds {MAX_FILENAME_BYTES} bytes")
+
+
+def derive_remote_filename(filename: str) -> str:
+    """Compute the SD-card filename used when uploading a sliced print file.
+
+    Strips repeated trailing ``.gcode.3mf`` / ``.3mf`` suffixes until the
+    bare stem remains, then appends a single ``.3mf``; spaces are
+    replaced with underscores because the firmware parses
+    ``ftp://{filename}`` as a URL.
+
+    Canonical for both the dispatch uploader and the post-print SD
+    cleanup — when the two drift apart the cleanup misses, and a
+    library row whose stored filename ended up with a doubled
+    ``.gcode.3mf`` (#1542) leaves the real file on the SD card. On A1
+    firmware that lingering file becomes a ghost print on the next
+    power-on (same family as the P1S behaviour in #374).
+
+    Raises ``TypeError`` on non-string input rather than entering the
+    strip loop, because a duck-typed object that returns truthy
+    sentinels from ``endswith`` would never escape and the resulting
+    unbounded allocation has cgroup-OOM'd the test runner under mocks.
+    """
+    if not isinstance(filename, str):
+        raise TypeError(f"derive_remote_filename requires str, got {type(filename).__name__}")
+    stem = filename
+    while True:
+        if stem.endswith(".gcode.3mf"):
+            stem = stem[:-10]
+        elif stem.endswith(".3mf"):
+            stem = stem[:-4]
+        else:
+            break
+    return f"{stem}.3mf".replace(" ", "_")

+ 54 - 0
backend/tests/unit/test_filename_validation.py

@@ -5,6 +5,7 @@ import pytest
 from backend.app.utils.filename import (
     INVALID_FILENAME_CHARS,
     InvalidFilenameError,
+    derive_remote_filename,
     validate_print_filename,
 )
 
@@ -64,3 +65,56 @@ class TestValidatePrintFilename:
         # 'ä' is 2 bytes in UTF-8
         with pytest.raises(InvalidFilenameError, match="bytes"):
             validate_print_filename("ä" * 200)
+
+
+class TestDeriveRemoteFilename:
+    """SD-card upload-name derivation must match what the cleanup deletes (#1542)."""
+
+    def test_strips_gcode_3mf(self) -> None:
+        assert derive_remote_filename("Cube.gcode.3mf") == "Cube.3mf"
+
+    def test_strips_3mf(self) -> None:
+        assert derive_remote_filename("Cube.3mf") == "Cube.3mf"
+
+    def test_bare_stem_appends_3mf(self) -> None:
+        assert derive_remote_filename("Cube") == "Cube.3mf"
+
+    def test_replaces_spaces_with_underscores(self) -> None:
+        # firmware parses ftp://{filename} as a URL, spaces break it
+        assert derive_remote_filename("Cube (1).gcode.3mf") == "Cube_(1).3mf"
+
+    def test_doubled_gcode_3mf_fully_stripped(self) -> None:
+        # The literal reproducer from #1542: library row had .gcode.3mf appended twice
+        assert derive_remote_filename("Cube (1).gcode.3mf.gcode.3mf") == "Cube_(1).3mf"
+
+    def test_doubled_3mf_fully_stripped(self) -> None:
+        assert derive_remote_filename("Cube.3mf.3mf") == "Cube.3mf"
+
+    def test_mixed_double_extensions_fully_stripped(self) -> None:
+        assert derive_remote_filename("Cube.gcode.3mf.3mf") == "Cube.3mf"
+
+    def test_raw_gcode_unchanged_stem(self) -> None:
+        # Bare .gcode (no .3mf wrapper) is a valid sliced file — only the
+        # .3mf wrapper gets stripped; .gcode survives and the result is
+        # the printer's expected ftp:// target.
+        assert derive_remote_filename("Cube.gcode") == "Cube.gcode.3mf"
+
+    def test_idempotent(self) -> None:
+        once = derive_remote_filename("Cube (1).gcode.3mf.gcode.3mf")
+        assert derive_remote_filename(once) == once
+
+    def test_unicode_stem_preserved(self) -> None:
+        assert derive_remote_filename("プリント.gcode.3mf") == "プリント.3mf"
+
+    def test_non_string_input_raises_typeerror(self) -> None:
+        """A duck-typed object whose endswith always returns truthy must not be
+        allowed to enter the strip loop — that's how a test mock OOM'd the
+        container at 61 GB before the type guard was added."""
+        from unittest.mock import MagicMock
+
+        with pytest.raises(TypeError, match="requires str"):
+            derive_remote_filename(MagicMock())
+        with pytest.raises(TypeError, match="requires str"):
+            derive_remote_filename(None)  # type: ignore[arg-type]
+        with pytest.raises(TypeError, match="requires str"):
+            derive_remote_filename(123)  # type: ignore[arg-type]

Một số tệp đã không được hiển thị bởi vì quá nhiều tập tin thay đổi trong này khác