Bläddra i källkod

Check filament deficit for Library-backed queue items (#2779)

    A job needing 20.5 g was dispatched onto a spool holding 9 g and the printer
    started. _resolve_source_3mf returned LibraryFile.file_path verbatim, but that
    column stores a path relative to base_dir -- so it resolved against the process
    working directory, found nothing, and compute_deficit_for_queue_item treated a
    missing source as "nothing to verify" and returned no deficit.

    Every library-backed queue item was affected: Slicer Pipeline jobs, which are
    always library-backed, and everything added through the Library's bulk Add to
    queue. Both callers share the resolver, so the Play button on the queue was as
    blind as the auto-dispatcher. Archive-backed items (print history, VP intake)
    resolved correctly and were never affected, and neither was PrintModal, which
    resolves the file on its own path.

    The library branch now uses the same idiom as the eleven other readers of
    file_path -- absolute stays, relative joins base_dir. The join carries a
    SEC-PATH-OK marker: the value is DB-stored and generated by the Library ingest,
    and it is already what resolves the file for upload, so the check has to
    resolve it identically or it is not checking what gets printed.

    A source that is configured but absent now logs a warning naming the item and
    the resolved path. It still dispatches, because the upload needs the same file
    seconds later and fails there, where blocking would strand a queue on a moved
    file -- but a safety check that skips itself must not do so in silence, which
    is what hid this for every library-backed item.

    Tests cover the relative path (the reporter's 20.5 g against 9 g), the absolute
    path against a base_dir the file is not under, and the missing-source warning.
    The existing cases all used archives with absolute paths, which is the gap the
    bug lived in.
maziggy 3 veckor sedan
förälder
incheckning
7dcfd0921d

+ 32 - 3
backend/app/services/filament_deficit.py

@@ -79,11 +79,28 @@ def _global_to_ams_key(global_tray_id: int) -> tuple[int, int]:
 
 
 
 
 def _resolve_source_3mf(item: PrintQueueItem) -> Path | None:
 def _resolve_source_3mf(item: PrintQueueItem) -> Path | None:
-    """Locate the 3MF file backing this queue item (archive or library)."""
+    """Locate the 3MF file backing this queue item (archive or library).
+
+    ``LibraryFile.file_path`` is stored relative to ``base_dir`` (rows written
+    before that convention hold absolute paths, which is why every reader
+    guards on ``is_absolute``). Resolving a relative one against the process
+    working directory finds nothing, and a source that cannot be found is
+    treated as "nothing to verify" — so this check silently passed every
+    library-backed item, which is every Slicer Pipeline job and everything
+    queued from the Library page (#2779).
+    """
     if item.archive is not None and item.archive.file_path:
     if item.archive is not None and item.archive.file_path:
         return app_settings.base_dir / item.archive.file_path
         return app_settings.base_dir / item.archive.file_path
     if item.library_file is not None and item.library_file.file_path:
     if item.library_file is not None and item.library_file.file_path:
-        return Path(item.library_file.file_path)
+        library_path = Path(item.library_file.file_path)
+        if library_path.is_absolute():
+            return library_path
+        # SEC-PATH-OK: file_path is DB-stored and generated by the Library
+        # ingest (archive/library/files/<uuid>.<ext>), never request input. The
+        # same value already resolves the file for upload in print_queue.py and
+        # print_scheduler.py — this check reads what the printer is about to be
+        # sent, so it must resolve it identically.
+        return app_settings.base_dir / item.library_file.file_path
     return None
     return None
 
 
 
 
@@ -316,7 +333,19 @@ async def compute_deficit_for_queue_item(
     item = refreshed.scalar_one_or_none() or item
     item = refreshed.scalar_one_or_none() or item
 
 
     source_path = _resolve_source_3mf(item)
     source_path = _resolve_source_3mf(item)
-    if source_path is None or not source_path.exists():
+    if source_path is None:
+        # No archive and no library file — nothing was ever attached to check.
+        return []
+    if not source_path.exists():
+        # Dispatch is not blocked: the upload that follows needs the same file
+        # and fails within seconds, where wedging the queue here would strand
+        # it. But skipping a safety check must leave a trace — a silent skip is
+        # what hid #2779 for every library-backed item.
+        logger.warning(
+            "Filament check skipped for queue item %s: source 3MF not found at %s",
+            item.id,
+            source_path,
+        )
         return []
         return []
 
 
     requirements = extract_filament_requirements(source_path, item.plate_id)
     requirements = extract_filament_requirements(source_path, item.plate_id)

+ 111 - 0
backend/tests/unit/services/test_filament_deficit.py

@@ -13,6 +13,7 @@ the contract for the cases that matter:
 from __future__ import annotations
 from __future__ import annotations
 
 
 import json
 import json
+import logging
 import zipfile
 import zipfile
 from pathlib import Path
 from pathlib import Path
 from unittest.mock import patch
 from unittest.mock import patch
@@ -63,6 +64,32 @@ async def _setup_archive_3mf(db_session, tmp_path: Path, filaments: list[dict])
     return archive
     return archive
 
 
 
 
+async def _setup_library_3mf(db_session, base_dir: Path, filaments: list[dict], *, absolute: bool = False):
+    """Create a 3MF under ``base_dir`` and a LibraryFile row pointing at it.
+
+    Mirrors production storage: the file lands in
+    ``<base_dir>/archive/library/files/`` and the row stores the path
+    *relative* to base_dir, exactly as ``library.py`` writes it (#2779).
+    """
+    from backend.app.models.library import LibraryFile
+
+    rel_path = Path("archive/library/files/deficit_probe.gcode.3mf")
+    abs_path = base_dir / rel_path
+    abs_path.parent.mkdir(parents=True, exist_ok=True)
+    _write_3mf(abs_path, filaments)
+
+    lib_file = LibraryFile(
+        filename="deficit_probe.gcode.3mf",
+        file_path=str(abs_path) if absolute else str(rel_path),
+        file_type="3mf",
+        file_size=abs_path.stat().st_size,
+    )
+    db_session.add(lib_file)
+    await db_session.commit()
+    await db_session.refresh(lib_file)
+    return lib_file
+
+
 async def _spool(
 async def _spool(
     db_session,
     db_session,
     *,
     *,
@@ -103,10 +130,12 @@ async def _queue_item(
     archive: PrintArchive | None,
     archive: PrintArchive | None,
     ams_mapping: list[int] | None,
     ams_mapping: list[int] | None,
     plate_id: int | None = None,
     plate_id: int | None = None,
+    library_file=None,
 ) -> PrintQueueItem:
 ) -> PrintQueueItem:
     item = PrintQueueItem(
     item = PrintQueueItem(
         printer_id=printer_id,
         printer_id=printer_id,
         archive_id=archive.id if archive else None,
         archive_id=archive.id if archive else None,
+        library_file_id=library_file.id if library_file else None,
         ams_mapping=json.dumps(ams_mapping) if ams_mapping is not None else None,
         ams_mapping=json.dumps(ams_mapping) if ams_mapping is not None else None,
         plate_id=plate_id,
         plate_id=plate_id,
         status="pending",
         status="pending",
@@ -226,6 +255,88 @@ class TestFilamentDeficit:
 
 
         assert deficit == []
         assert deficit == []
 
 
+    @pytest.mark.asyncio
+    async def test_library_file_with_relative_path_is_checked(self, db_session, printer_factory, tmp_path):
+        """#2779: a Library-backed item stores its path relative to base_dir.
+
+        Resolving it against the process working directory finds nothing, and
+        "no source" is treated as "nothing to verify" — so the check returned
+        no deficit and the scheduler dispatched onto a spool that could not
+        finish the print. Every Slicer Pipeline item and everything queued via
+        the Library's Add to queue is library-backed, so the guard was absent
+        for all of them. Numbers are the reporter's: 20.5 g needed, 9 g left.
+        """
+        printer = await printer_factory()
+        lib_file = await _setup_library_3mf(
+            db_session,
+            tmp_path,
+            [{"id": "1", "type": "PLA", "color": "#FFFFFF", "used_g": "20.5"}],
+        )
+        assert not Path(lib_file.file_path).is_absolute()  # the shape that broke
+
+        spool = await _spool(db_session, label_weight=1000, weight_used=991.0)  # 9g left
+        await _assign(db_session, printer_id=printer.id, spool_id=spool.id, ams_id=0, tray_id=0)
+        item = await _queue_item(
+            db_session, printer_id=printer.id, archive=None, library_file=lib_file, ams_mapping=[0]
+        )
+
+        with patch("backend.app.services.filament_deficit.app_settings.base_dir", tmp_path):
+            deficit = await compute_deficit_for_queue_item(db_session, item)
+
+        assert len(deficit) == 1
+        assert deficit[0].required_grams == 20.5
+        assert deficit[0].remaining_grams == 9.0
+
+    @pytest.mark.asyncio
+    async def test_library_file_with_absolute_path_is_checked(self, db_session, printer_factory, tmp_path):
+        """The other half of the resolver: a row that already holds an absolute
+        path must not be joined onto base_dir a second time."""
+        printer = await printer_factory()
+        lib_file = await _setup_library_3mf(
+            db_session,
+            tmp_path,
+            [{"id": "1", "type": "PLA", "color": "#FFFFFF", "used_g": "100.0"}],
+            absolute=True,
+        )
+        spool = await _spool(db_session, label_weight=1000, weight_used=970.0)  # 30g left
+        await _assign(db_session, printer_id=printer.id, spool_id=spool.id, ams_id=0, tray_id=0)
+        item = await _queue_item(
+            db_session, printer_id=printer.id, archive=None, library_file=lib_file, ams_mapping=[0]
+        )
+
+        # A base_dir the file is NOT under — joining it on would break the path.
+        with patch("backend.app.services.filament_deficit.app_settings.base_dir", tmp_path / "elsewhere"):
+            deficit = await compute_deficit_for_queue_item(db_session, item)
+
+        assert len(deficit) == 1
+        assert deficit[0].required_grams == 100.0
+
+    @pytest.mark.asyncio
+    async def test_missing_source_is_logged_not_just_skipped(self, db_session, printer_factory, caplog):
+        """A source that is configured but absent still dispatches — the upload
+        would fail seconds later anyway, and wedging the queue on a missing
+        file is the worse trade. But it must not pass silently: skipping the
+        check without a trace is what let #2779 go unnoticed for every
+        library-backed item.
+        """
+        printer = await printer_factory()
+        archive = PrintArchive(
+            filename="ghost.3mf",
+            file_path="/nonexistent/ghost.3mf",
+            file_size=0,
+            status="completed",
+        )
+        db_session.add(archive)
+        await db_session.commit()
+        await db_session.refresh(archive)
+        item = await _queue_item(db_session, printer_id=printer.id, archive=archive, ams_mapping=[0])
+
+        with caplog.at_level(logging.WARNING, logger="backend.app.services.filament_deficit"):
+            deficit = await compute_deficit_for_queue_item(db_session, item)
+
+        assert deficit == []
+        assert any("ghost.3mf" in r.getMessage() for r in caplog.records)
+
     @pytest.mark.asyncio
     @pytest.mark.asyncio
     async def test_returns_empty_when_3mf_missing(self, db_session, printer_factory):
     async def test_returns_empty_when_3mf_missing(self, db_session, printer_factory):
         printer = await printer_factory()
         printer = await printer_factory()