Selaa lähdekoodia

fix(archive): never delete persistent files in 3MF cache cleanup (#1212)

  Daily builds since 889c8bd8 (Apr 29) silently destroyed archive copies
  and library file bytes on every print completion. Reprint / View G-code
  later returned 404 with no log line explaining why; the DB row was
  intact and the archive grid kept showing the entry, but the file
  behind archive.file_path no longer existed on disk.

  Root cause: #1166 added three dispatch sites that cache the live
  archive copy (and library file bytes for Direct-Print) in the shared
  3MF download cache, so /cover could skip a redundant FTP transfer
  mid-print. The cache was originally designed for transient downloads
  under archive_dir/temp/, and clear_3mf_cache(printer_id) — called
  from on_print_complete to keep that temp dir from accumulating —
  happily unlink()'d every cached path. Path.exists() guarded the
  unlink, so no exception, no warning, just silent destruction. Listing
  didn't change; only acting on the archive surfaced the 404.

  Fix: clear_3mf_cache._maybe_unlink refuses to unlink any path outside
  archive_dir/temp. Cache dict is still cleared (so re-cache continues
  to work and /cover hits a fresh path next print), only the on-disk
  delete is gated. Persistent locations — archive/<printer_id>/...,
  archive/unassigned/... (VP-archived prints with printer_id=None),
  library_files/..., is_external library mounts — all survive.

  Regression test test_clear_does_not_delete_persistent_files pins the
  contract end-to-end: archive 3mf, library 3mf, and temp 3mf all
  cached for the same printer; after clear, all three cache entries
  are dropped from the dict, but only the temp file is unlinked from
  disk. Two existing tests updated to put fixtures under
  archive_dir/temp.
maziggy 3 viikkoa sitten
vanhempi
sitoutus
3bb99759d2
3 muutettua tiedostoa jossa 81 lisäystä ja 11 poistoa
  1. 0 0
      CHANGELOG.md
  2. 23 5
      backend/app/services/bambu_ftp.py
  3. 58 6
      backend/tests/unit/services/test_bambu_ftp.py

Tiedoston diff-näkymää rajattu, sillä se on liian suuri
+ 0 - 0
CHANGELOG.md


+ 23 - 5
backend/app/services/bambu_ftp.py

@@ -670,14 +670,32 @@ def clear_3mf_cache(printer_id: int | None = None, delete_files: bool = True) ->
     When ``delete_files`` is True (default) the on-disk 3MF is removed as well
     — called from on_print_complete so temp files don't accumulate across
     prints. Tests that want to inspect the cache contents disable this.
+
+    Only paths inside ``archive_dir/temp`` are unlinked. The dispatch sites
+    added in #1166 also cache the live archive copy and library file bytes
+    so /cover can skip FTP — those are *user data*, never the cache's to
+    delete. Pre-fix this branch silently removed archive 3mfs on every print
+    completion (#1212 + private reports of "file disappeared overnight").
     """
+    from backend.app.core.config import settings as _config_settings
+
+    temp_root = _config_settings.archive_dir / "temp"
+
+    def _is_temp_path(path: Path) -> bool:
+        try:
+            return path.is_relative_to(temp_root)
+        except (OSError, ValueError):
+            return False
 
     def _maybe_unlink(path: Path) -> None:
-        if delete_files and path.exists():
-            try:
-                path.unlink()
-            except OSError as exc:
-                logger.debug("3MF cache cleanup skipped %s: %s", path, exc)
+        if not delete_files or not path.exists():
+            return
+        if not _is_temp_path(path):
+            return
+        try:
+            path.unlink()
+        except OSError as exc:
+            logger.debug("3MF cache cleanup skipped %s: %s", path, exc)
 
     if printer_id is None:
         for path in list(_threemf_path_cache.values()):

+ 58 - 6
backend/tests/unit/services/test_bambu_ftp.py

@@ -1198,26 +1198,78 @@ class TestThreeMFCache:
         cache_3mf_download(1, "A.3mf", f)
         assert get_cached_3mf(1, "A.3mf") == f
 
-    def test_clear_by_printer_scoped(self, tmp_path):
+    def test_clear_by_printer_scoped(self, tmp_path, monkeypatch):
         """Clearing one printer leaves the other untouched."""
-        f1 = tmp_path / "one.3mf"
+        from backend.app.core import config as _config
+
+        monkeypatch.setattr(_config.settings, "archive_dir", tmp_path)
+        temp_dir = tmp_path / "temp"
+        temp_dir.mkdir()
+        f1 = temp_dir / "one.3mf"
         f1.write_bytes(b"1")
-        f2 = tmp_path / "two.3mf"
+        f2 = temp_dir / "two.3mf"
         f2.write_bytes(b"2")
         cache_3mf_download(1, "one.3mf", f1)
         cache_3mf_download(2, "two.3mf", f2)
         clear_3mf_cache(1)
         assert get_cached_3mf(1, "one.3mf") is None
         assert get_cached_3mf(2, "two.3mf") == f2
-        # clear_3mf_cache defaulted to delete_files=True, so the file is gone
+        # clear_3mf_cache defaulted to delete_files=True, so the temp file is gone
         assert not f1.exists()
         assert f2.exists()
 
-    def test_clear_without_deleting_files(self, tmp_path):
+    def test_clear_without_deleting_files(self, tmp_path, monkeypatch):
         """delete_files=False leaves files on disk — used by tests."""
-        f = tmp_path / "keep.3mf"
+        from backend.app.core import config as _config
+
+        monkeypatch.setattr(_config.settings, "archive_dir", tmp_path)
+        temp_dir = tmp_path / "temp"
+        temp_dir.mkdir()
+        f = temp_dir / "keep.3mf"
         f.write_bytes(b"x")
         cache_3mf_download(1, "keep.3mf", f)
         clear_3mf_cache(1, delete_files=False)
         assert get_cached_3mf(1, "keep.3mf") is None
         assert f.exists()
+
+    def test_clear_does_not_delete_persistent_files(self, tmp_path, monkeypatch):
+        """Regression for #1212 / "file disappeared overnight" reports.
+
+        Dispatch sites added in #1166 cache the live archive copy and library
+        file bytes — paths outside ``archive_dir/temp`` — so /cover can skip
+        FTP. Those files are user data; the cache cleanup must never unlink
+        them. Pre-fix, ``clear_3mf_cache(printer_id, delete_files=True)`` ran
+        on every ``on_print_complete`` and silently destroyed them, leaving a
+        DB row whose ``file_path`` pointed at nothing — breaking Reprint and
+        View G-code with a 404.
+        """
+        from backend.app.core import config as _config
+
+        monkeypatch.setattr(_config.settings, "archive_dir", tmp_path / "archive")
+        (tmp_path / "archive" / "temp").mkdir(parents=True)
+
+        archive_file = tmp_path / "archive" / "1" / "20260504_wallhooks" / "wallhooks.gcode.3mf"
+        archive_file.parent.mkdir(parents=True)
+        archive_file.write_bytes(b"archive bytes")
+
+        library_file = tmp_path / "library_files" / "abcd.3mf"
+        library_file.parent.mkdir(parents=True)
+        library_file.write_bytes(b"library bytes")
+
+        temp_file = tmp_path / "archive" / "temp" / "cover_1_x.3mf"
+        temp_file.write_bytes(b"temp bytes")
+
+        cache_3mf_download(1, "wallhooks.gcode.3mf", archive_file)
+        cache_3mf_download(1, "library.3mf", library_file)
+        cache_3mf_download(1, "cover_1_x.3mf", temp_file)
+
+        clear_3mf_cache(1)
+
+        # All three cache entries are dropped from the dict.
+        assert get_cached_3mf(1, "wallhooks.gcode.3mf") is None
+        assert get_cached_3mf(1, "library.3mf") is None
+        assert get_cached_3mf(1, "cover_1_x.3mf") is None
+        # But only the temp file is unlinked — user data survives.
+        assert archive_file.exists(), "archive 3mf must not be deleted by cache cleanup"
+        assert library_file.exists(), "library 3mf must not be deleted by cache cleanup"
+        assert not temp_file.exists(), "temp file should still be cleaned up"

Kaikkia tiedostoja ei voida näyttää, sillä liian monta tiedostoa muuttui tässä diffissä