Просмотр исходного кода

fix(archives): validate downloaded 3MF plate against gcode_file (#1204)

Two consecutive plates of the same model would create the second print's
archive with the first plate's metadata: subtask_name lags across the
boundary while gcode_file is fresh, so the FTP candidate list (built
from subtask_name first) lands on the previous plate's still-resident
upload. The 3MF parser then locks the wrong _plate_index, name, time
estimate, and per-slot filament data into the archive at creation.

Fix peeks the downloaded 3MF's slice_info plate index, compares against
parse_plate_id(filename) (the plate parsed from /Metadata/plate_N.gcode,
which always reflects what's running), and on mismatch retries FTP with
swap_plate_suffix(subtask_name, expected_plate) — handling both the
spaced "Plate N" and underscored "_plate_N" suffix forms seen in real
subtask_names. If the retry finds a matching 3MF, the wrong file is
dropped and the corrected one feeds the archive; if no match is found
(or no swap is possible) the wrong file is dropped and the existing
no-3MF fallback creates an archive whose name reflects the right plate.

The validation only runs when parse_plate_id() returns a value, so
single-plate / cloud-named / non-Bambu jobs are unaffected.

17 new unit tests in test_archive_plate_validation.py cover both helpers:
plate-index peek across malformed / missing / non-integer / non-zip
inputs, and the suffix swap across both casings, the underscored form,
case-insensitive matching, and rejection of names without a recognised
suffix.
maziggy 3 недель назад
Родитель
Сommit
713b85387a

Разница между файлами не показана из-за своего большого размера
+ 0 - 0
CHANGELOG.md


+ 110 - 1
backend/app/main.py

@@ -69,7 +69,7 @@ from backend.app.core.config import APP_VERSION, settings as app_settings
 from backend.app.core.database import async_session, engine, init_db
 from backend.app.core.websocket import ws_manager
 from backend.app.models.smart_plug import SmartPlug
-from backend.app.services.archive import ArchiveService
+from backend.app.services.archive import ArchiveService, peek_plate_index_in_3mf, swap_plate_suffix
 from backend.app.services.archive_purge import archive_purge_service
 from backend.app.services.background_dispatch import background_dispatch
 from backend.app.services.bambu_ftp import (
@@ -93,6 +93,7 @@ from backend.app.services.obico_detection import obico_detection_service
 from backend.app.services.print_scheduler import scheduler as print_scheduler
 from backend.app.services.printer_manager import (
     init_printer_connections,
+    parse_plate_id,
     printer_manager,
     printer_state_to_dict,
 )
@@ -2151,6 +2152,114 @@ async def on_print_start(printer_id: int, data: dict):
                 except Exception as e:
                     logger.debug("Failed to list %s: %s", search_dir, e)
 
+        # Validate the downloaded 3MF actually matches the plate that's running
+        # (#1204): subtask_name lags across consecutive plates of the same model,
+        # so the first FTP candidate (built from subtask_name) can land on the
+        # previous plate's still-resident upload. Cross-check the slice_info
+        # plate index against the plate parsed from gcode_file (always fresh —
+        # it's the field whose change triggered this callback).
+        if downloaded_filename and temp_path:
+            expected_plate = parse_plate_id(filename)
+            actual_plate = peek_plate_index_in_3mf(temp_path) if expected_plate is not None else None
+            if expected_plate is not None and actual_plate is not None and actual_plate != expected_plate:
+                logger.warning(
+                    "[CALLBACK] 3MF plate mismatch: downloaded %s reports plate %s but printer is "
+                    "running plate %s — subtask_name=%r appears stale, retrying with corrected name",
+                    downloaded_filename,
+                    actual_plate,
+                    expected_plate,
+                    subtask_name,
+                )
+                corrected_subtask = swap_plate_suffix(subtask_name, expected_plate)
+                retry_succeeded = False
+                if corrected_subtask and corrected_subtask != subtask_name:
+                    for try_filename in (f"{corrected_subtask}.gcode.3mf", f"{corrected_subtask}.3mf"):
+                        retry_temp_path = app_settings.archive_dir / "temp" / try_filename
+                        retry_temp_path.parent.mkdir(parents=True, exist_ok=True)
+                        for remote_path in (
+                            f"/{try_filename}",
+                            f"/cache/{try_filename}",
+                            f"/model/{try_filename}",
+                            f"/data/{try_filename}",
+                            f"/data/Metadata/{try_filename}",
+                        ):
+                            try:
+                                if ftp_retry_enabled:
+                                    downloaded = await with_ftp_retry(
+                                        download_file_async,
+                                        printer.ip_address,
+                                        printer.access_code,
+                                        remote_path,
+                                        retry_temp_path,
+                                        timeout=ftp_timeout,
+                                        socket_timeout=ftp_timeout,
+                                        printer_model=printer.model,
+                                        max_retries=ftp_retry_count,
+                                        retry_delay=ftp_retry_delay,
+                                        operation_name=f"Re-download 3MF from {remote_path}",
+                                        non_retry_exceptions=(FileNotOnPrinterError,),
+                                    )
+                                else:
+                                    downloaded = await download_file_async(
+                                        printer.ip_address,
+                                        printer.access_code,
+                                        remote_path,
+                                        retry_temp_path,
+                                        timeout=ftp_timeout,
+                                        socket_timeout=ftp_timeout,
+                                        printer_model=printer.model,
+                                    )
+                                if downloaded and peek_plate_index_in_3mf(retry_temp_path) == expected_plate:
+                                    logger.info(
+                                        "[CALLBACK] Re-download succeeded with corrected name %s "
+                                        "(plate %s) — replacing wrong file",
+                                        try_filename,
+                                        expected_plate,
+                                    )
+                                    try:
+                                        temp_path.unlink(missing_ok=True)
+                                    except OSError:
+                                        pass
+                                    temp_path = retry_temp_path
+                                    downloaded_filename = try_filename
+                                    subtask_name = corrected_subtask
+                                    cache_3mf_download(printer_id, try_filename, temp_path)
+                                    retry_succeeded = True
+                                    break
+                                elif downloaded:
+                                    # Wrong plate again — discard and keep trying
+                                    try:
+                                        retry_temp_path.unlink(missing_ok=True)
+                                    except OSError:
+                                        pass
+                            except FileNotOnPrinterError:
+                                continue
+                            except Exception as e:
+                                logger.debug("Re-download failed for %s: %s", remote_path, e)
+                        if retry_succeeded:
+                            break
+                # If the retry didn't find a matching file, drop the wrong 3MF
+                # so the no-3MF fallback below creates an archive whose name
+                # at least reflects the right plate.
+                if not retry_succeeded:
+                    logger.warning(
+                        "[CALLBACK] Could not re-download correct plate %s — falling back to no-3MF archive",
+                        expected_plate,
+                    )
+                    try:
+                        temp_path.unlink(missing_ok=True)
+                    except OSError:
+                        pass
+                    temp_path = None
+                    downloaded_filename = None
+                    # Override the stale subtask_name so the fallback archive's
+                    # print_name reflects the correct plate. Prefer the swapped
+                    # name when we have one; otherwise let filename win.
+                    if corrected_subtask:
+                        subtask_name = corrected_subtask
+                    else:
+                        subtask_name = ""
+
         if not downloaded_filename or not temp_path:
             logger.warning("Could not find 3MF file for print: %s", filename or subtask_name)
             # Create a fallback archive without 3MF data so the print is still tracked

+ 52 - 0
backend/app/services/archive.py

@@ -66,6 +66,58 @@ def resolve_display_stem(filename: str) -> str:
     return Path(name).stem
 
 
+def peek_plate_index_in_3mf(file_path: Path) -> int | None:
+    """Return the plate index recorded inside a Bambu 3MF, or None.
+
+    Reads only ``Metadata/slice_info.config`` to keep this cheap — used by
+    the print-start callback to verify that the 3MF we just downloaded over
+    FTP actually matches the plate the printer is running (#1204). The full
+    ThreeMFParser does much more work and runs later inside ArchiveService.
+    """
+    try:
+        with zipfile.ZipFile(file_path, "r") as zf:
+            if "Metadata/slice_info.config" not in zf.namelist():
+                return None
+            content = zf.read("Metadata/slice_info.config").decode()
+            root = ET.fromstring(content)
+            plate = root.find(".//plate")
+            if plate is None:
+                return None
+            for meta in plate.findall("metadata"):
+                if meta.get("key") == "index":
+                    value = meta.get("value")
+                    if value:
+                        try:
+                            return int(value)
+                        except ValueError:
+                            return None
+    except Exception:
+        return None
+    return None
+
+
+_PLATE_SUFFIX_RE = re.compile(r"^(.*?)(\s*-\s*Plate\s+|_plate_)(\d+)$", re.IGNORECASE)
+
+
+def swap_plate_suffix(name: str | None, target_plate: int) -> str | None:
+    """Return ``name`` with its trailing plate number replaced, or None.
+
+    Bambu Studio names multi-plate uploads ``"<Project> - Plate <N>"`` (and
+    a lowercase ``"_plate_<N>"`` variant exists too — see
+    test_print_start_expected_promotion). When MQTT subtask_name lags
+    across consecutive plates of the same model (#1204) the suffix points
+    at the previous plate; swapping it gives us the correct upload to
+    re-fetch from FTP. Returns None if no recognised suffix is present.
+    """
+    if not name:
+        return None
+    m = _PLATE_SUFFIX_RE.match(name)
+    if not m:
+        return None
+    base, separator, _ = m.groups()
+    return f"{base}{separator}{target_plate}"
+
+
 class ThreeMFParser:
     """Parser for Bambu Lab 3MF files."""
 

+ 115 - 0
backend/tests/unit/services/test_archive_plate_validation.py

@@ -0,0 +1,115 @@
+"""Tests for the plate-validation helpers used by the print_start callback.
+
+Covers the #1204 fix: when two plates of the same model are printed back to
+back, MQTT subtask_name can lag and the FTP candidate built from it lands on
+the previous plate's still-resident upload. The fix peeks the slice_info
+plate index, compares it to the plate parsed from gcode_file, and (on
+mismatch) re-fetches with a corrected name.
+
+Pinned here:
+- ``peek_plate_index_in_3mf`` reads ONLY ``Metadata/slice_info.config`` and
+  returns the integer plate index, or None on any failure (missing entry,
+  malformed XML, unreadable zip, etc.). Cheap by design — the full parse
+  runs later inside ArchiveService.
+- ``swap_plate_suffix`` rewrites the trailing plate number in a Bambu-style
+  job name. Covers both the spaced "Project - Plate N" form and the
+  underscored "project_plate_N" variant seen in real subtask_names.
+"""
+
+import zipfile
+
+import pytest
+
+from backend.app.services.archive import peek_plate_index_in_3mf, swap_plate_suffix
+
+
+def _make_3mf(tmp_path, *, plate_index: int | None = None, malformed: bool = False):
+    """Build a minimal 3MF with a single ``<plate>`` whose ``index`` metadata is set."""
+    path = tmp_path / "test.3mf"
+    if malformed:
+        path.write_bytes(b"not a zip")
+        return path
+    with zipfile.ZipFile(path, "w") as zf:
+        if plate_index is None:
+            # plate present but with no index metadata — exercise the "no index" branch
+            zf.writestr("Metadata/slice_info.config", "<config><plate></plate></config>")
+        else:
+            zf.writestr(
+                "Metadata/slice_info.config",
+                f'<config><plate><metadata key="index" value="{plate_index}" /></plate></config>',
+            )
+    return path
+
+
+class TestPeekPlateIndexIn3mf:
+    def test_returns_index_for_valid_3mf(self, tmp_path):
+        path = _make_3mf(tmp_path, plate_index=2)
+        assert peek_plate_index_in_3mf(path) == 2
+
+    def test_returns_none_when_index_missing(self, tmp_path):
+        path = _make_3mf(tmp_path, plate_index=None)
+        assert peek_plate_index_in_3mf(path) is None
+
+    def test_returns_none_when_slice_info_absent(self, tmp_path):
+        path = tmp_path / "noslice.3mf"
+        with zipfile.ZipFile(path, "w") as zf:
+            zf.writestr("3D/3dmodel.model", "<model/>")
+        assert peek_plate_index_in_3mf(path) is None
+
+    def test_returns_none_on_non_zip_file(self, tmp_path):
+        path = _make_3mf(tmp_path, malformed=True)
+        assert peek_plate_index_in_3mf(path) is None
+
+    def test_returns_none_on_missing_file(self, tmp_path):
+        assert peek_plate_index_in_3mf(tmp_path / "does-not-exist.3mf") is None
+
+    def test_returns_none_on_non_integer_index(self, tmp_path):
+        path = tmp_path / "bad.3mf"
+        with zipfile.ZipFile(path, "w") as zf:
+            zf.writestr(
+                "Metadata/slice_info.config",
+                '<config><plate><metadata key="index" value="abc" /></plate></config>',
+            )
+        assert peek_plate_index_in_3mf(path) is None
+
+
+class TestSwapPlateSuffix:
+    @pytest.mark.parametrize(
+        ("name", "target", "expected"),
+        [
+            # Bambu Studio's default form (spaces around hyphen, capitalised "Plate").
+            ("MyModel - Plate 2", 1, "MyModel - Plate 1"),
+            ("MyModel - Plate 1", 5, "MyModel - Plate 5"),
+            # Hyphen variants without surrounding spaces should still match (regex
+            # uses \s* — slicer output occasionally normalises spacing).
+            ("Tight-Plate 3", 7, "Tight-Plate 7"),
+            # Underscored form seen in real subtask_names (see
+            # test_print_start_expected_promotion fixture "Box3.0_(2)_plate_5").
+            ("Box3.0_(2)_plate_5", 1, "Box3.0_(2)_plate_1"),
+            # Case-insensitive match — older exports occasionally use lowercase.
+            ("model - plate 4", 2, "model - plate 2"),
+        ],
+    )
+    def test_swaps_plate_number(self, name, target, expected):
+        assert swap_plate_suffix(name, target) == expected
+
+    @pytest.mark.parametrize(
+        "name",
+        [
+            "JustAModelName",  # No plate suffix at all — single-plate project.
+            "Model with - Plate in middle of name",  # "Plate" not at the end.
+            "Plate 2",  # Bare "Plate N" with no base — refuse rather than guess.
+            "",  # Empty string.
+        ],
+    )
+    def test_returns_none_when_no_recognised_suffix(self, name):
+        assert swap_plate_suffix(name, 1) is None
+
+    def test_returns_none_for_none_input(self):
+        assert swap_plate_suffix(None, 1) is None
+
+    def test_preserves_separator_casing(self):
+        # The replacement must not normalise " - Plate " to "_plate_" or vice versa.
+        # Otherwise the corrected name won't match what BambuStudio actually uploaded.
+        assert swap_plate_suffix("Model - Plate 1", 2) == "Model - Plate 2"
+        assert swap_plate_suffix("model_plate_1", 2) == "model_plate_2"

Некоторые файлы не были показаны из-за большого количества измененных файлов