fix(obico): POST image bytes directly to ML API instead of callback URL (#1003)

The ML API previously called back into Bambuddy to fetch snapshots,
  which failed behind reverse proxies with external auth (Authelia, etc.).
  Now the detection loop captures the JPEG locally and POSTs it directly
  as multipart form data — no callback URL, no nonce cache, no
  external_url dependency.

backend/tests/integration/test_obico_api.py

@@ -1,71 +0,0 @@
-"""Integration tests for Obico API endpoints (#172 follow-up).
-
-Verifies the /obico/cached-frame/{nonce} endpoint used by Obico's ML API to fetch
-pre-captured JPEG frames. This endpoint lets the detection loop sidestep Obico's
-hardcoded 5s read timeout by pre-populating a cache before issuing the ML call.
-"""
-
-import pytest
-from httpx import AsyncClient
-
-from backend.app.services.obico_detection import _frame_cache, stash_frame
-
-FAKE_JPEG = b"\xff\xd8\xff\xe0\x00\x10JFIF\x00\x01\x01\x00\x00\x01\x00\x01\x00\x00\xff\xd9"
-
-
-@pytest.fixture(autouse=True)
-def clear_cache():
-    _frame_cache.clear()
-    yield
-    _frame_cache.clear()
-
-
-class TestObicoCachedFrame:
-    @pytest.mark.asyncio
-    @pytest.mark.integration
-    async def test_valid_nonce_returns_jpeg(self, async_client: AsyncClient):
-        """A stashed nonce returns the stored JPEG bytes with image/jpeg."""
-        nonce = await stash_frame(FAKE_JPEG)
-        response = await async_client.get(f"/api/v1/obico/cached-frame/{nonce}")
-        assert response.status_code == 200
-        assert response.headers["content-type"] == "image/jpeg"
-        assert response.content == FAKE_JPEG
-
-    @pytest.mark.asyncio
-    @pytest.mark.integration
-    async def test_unknown_nonce_is_404(self, async_client: AsyncClient):
-        """An unguessable URL must not leak that the endpoint exists — return 404."""
-        response = await async_client.get("/api/v1/obico/cached-frame/definitely-not-a-real-nonce")
-        assert response.status_code == 404
-
-    @pytest.mark.asyncio
-    @pytest.mark.integration
-    async def test_nonce_is_single_use(self, async_client: AsyncClient):
-        """A second fetch with the same nonce returns 404 — prevents replay."""
-        nonce = await stash_frame(FAKE_JPEG)
-        first = await async_client.get(f"/api/v1/obico/cached-frame/{nonce}")
-        assert first.status_code == 200
-        second = await async_client.get(f"/api/v1/obico/cached-frame/{nonce}")
-        assert second.status_code == 404
-
-    @pytest.mark.asyncio
-    @pytest.mark.integration
-    async def test_endpoint_is_public(self, async_client: AsyncClient):
-        """Obico's ML API can't send auth headers, so the nonce IS the credential.
-        The path must be in PUBLIC_API_PATTERNS (no auth wall)."""
-        nonce = await stash_frame(FAKE_JPEG)
-        # Intentionally omit any auth headers even if the fixture would normally inject them
-        response = await async_client.get(
-            f"/api/v1/obico/cached-frame/{nonce}",
-            headers={},  # no Authorization header
-        )
-        assert response.status_code == 200
-
-    @pytest.mark.asyncio
-    @pytest.mark.integration
-    async def test_response_is_not_cached(self, async_client: AsyncClient):
-        """Browsers/proxies must not hold onto the image after Obico consumes it."""
-        nonce = await stash_frame(FAKE_JPEG)
-        response = await async_client.get(f"/api/v1/obico/cached-frame/{nonce}")
-        assert response.status_code == 200
-        assert "no-store" in response.headers.get("cache-control", "")

backend/tests/unit/test_obico_detection.py

@@ -5,13 +5,7 @@ from unittest.mock import AsyncMock, MagicMock, patch
 import pytest
 
 from backend.app.schemas.settings import AppSettingsUpdate
-from backend.app.services.obico_detection import (
-    FRAME_CACHE_TTL,
-    ObicoDetectionService,
-    _frame_cache,
-    pop_frame,
-    stash_frame,
-)
+from backend.app.services.obico_detection import ObicoDetectionService
 from backend.app.services.obico_smoothing import WARMUP_FRAMES
 
 FAKE_JPEG = b"\xff\xd8\xff\xe0\x00\x10JFIF\x00\x01\x01\x00\x00\x01\x00\x01\x00\x00\xff\xd9"
@@ -139,7 +133,6 @@ class TestPollOneStateLifecycle:
             "action": "notify",
             "poll_interval": 10,
             "enabled_printers": None,
-            "external_url": "http://bambuddy:8000",
         }
         status = MagicMock(state="RUNNING", task_name="new_task", subtask_name="")
 
@@ -147,7 +140,7 @@ class TestPollOneStateLifecycle:
         mock_response.json.return_value = {"detections": []}
         mock_response.raise_for_status = MagicMock()
         mock_client = MagicMock()
-        mock_client.get = AsyncMock(return_value=mock_response)
+        mock_client.post = AsyncMock(return_value=mock_response)
         mock_client.__aenter__ = AsyncMock(return_value=mock_client)
         mock_client.__aexit__ = AsyncMock(return_value=False)
 
@@ -172,12 +165,11 @@ class TestPollOneStateLifecycle:
             "action": "notify",
             "poll_interval": 10,
             "enabled_printers": None,
-            "external_url": "http://bambuddy:8000",
         }
         status = MagicMock(state="RUNNING", task_name="job", subtask_name="")
 
         mock_client = MagicMock()
-        mock_client.get = AsyncMock(side_effect=RuntimeError("connection refused"))
+        mock_client.post = AsyncMock(side_effect=RuntimeError("connection refused"))
         mock_client.__aenter__ = AsyncMock(return_value=mock_client)
         mock_client.__aexit__ = AsyncMock(return_value=False)
 
@@ -201,7 +193,6 @@ class TestPollOneStateLifecycle:
             "action": "notify",
             "poll_interval": 10,
             "enabled_printers": None,
-            "external_url": "http://bambuddy:8000",
         }
         status = MagicMock(state="RUNNING", task_name="job", subtask_name="")
 
@@ -219,7 +210,7 @@ class TestPollOneStateLifecycle:
         mock_response.json.return_value = {"detections": [["failure", 0.9, [0, 0, 1, 1]]]}
         mock_response.raise_for_status = MagicMock()
         mock_client = MagicMock()
-        mock_client.get = AsyncMock(return_value=mock_response)
+        mock_client.post = AsyncMock(return_value=mock_response)
         mock_client.__aenter__ = AsyncMock(return_value=mock_client)
         mock_client.__aexit__ = AsyncMock(return_value=False)
 
@@ -235,70 +226,11 @@ class TestPollOneStateLifecycle:
             assert mock_action.call_count == 1
 
 
-class TestFrameCache:
-    """One-shot JPEG cache that lets us sidestep Obico's 5s read timeout.
-
-    Obico's ML API fetches snapshots via `GET /p/?img=URL` with `timeout=(0.1, 5)`.
-    Our /camera/snapshot can exceed that on cold calls (RTSP keyframe wait). So the
-    detection loop captures locally, stashes the JPEG bytes under a nonce, then hands
-    Obico a URL that returns those bytes instantly. The cache is single-use + TTLed
-    so a leaked nonce can't be replayed.
-    """
-
-    def setup_method(self):
-        _frame_cache.clear()
+class TestCheckPrinterPostsImageDirectly:
+    """The detection loop must POST JPEG bytes directly to the ML API."""
 
     @pytest.mark.asyncio
-    async def test_stash_and_pop_roundtrip(self):
-        nonce = await stash_frame(FAKE_JPEG)
-        assert nonce  # non-empty URL-safe token
-        data = await pop_frame(nonce)
-        assert data == FAKE_JPEG
-
-    @pytest.mark.asyncio
-    async def test_nonce_is_single_use(self):
-        nonce = await stash_frame(FAKE_JPEG)
-        assert await pop_frame(nonce) == FAKE_JPEG
-        # Second pop returns None — caches replay protection
-        assert await pop_frame(nonce) is None
-
-    @pytest.mark.asyncio
-    async def test_unknown_nonce_returns_none(self):
-        assert await pop_frame("not-a-real-nonce") is None
-
-    @pytest.mark.asyncio
-    async def test_stash_produces_unique_nonces(self):
-        nonces = {await stash_frame(FAKE_JPEG) for _ in range(10)}
-        assert len(nonces) == 10
-
-    @pytest.mark.asyncio
-    async def test_expired_entries_are_pruned_on_stash(self):
-        """New entries trigger pruning of TTL-expired ones — prevents unbounded growth."""
-        # Manually seed an entry with a stale timestamp
-        import time as time_module
-
-        _frame_cache["stale-nonce"] = (FAKE_JPEG, time_module.monotonic() - FRAME_CACHE_TTL - 1)
-        await stash_frame(FAKE_JPEG)
-        # Stale entry was pruned
-        assert "stale-nonce" not in _frame_cache
-
-    @pytest.mark.asyncio
-    async def test_pop_rejects_expired_nonce(self):
-        """Even if the entry is still in the dict, an expired TTL returns None."""
-        import time as time_module
-
-        _frame_cache["aging-nonce"] = (FAKE_JPEG, time_module.monotonic() - FRAME_CACHE_TTL - 1)
-        assert await pop_frame("aging-nonce") is None
-
-
-class TestCheckPrinterUsesCachedFrameUrl:
-    """The URL sent to Obico must point at our nonce endpoint, not /camera/snapshot."""
-
-    def setup_method(self):
-        _frame_cache.clear()
-
-    @pytest.mark.asyncio
-    async def test_ml_api_called_with_cached_frame_url(self):
+    async def test_ml_api_called_with_post_and_image_bytes(self):
         svc = ObicoDetectionService()
         settings = {
             "enabled": True,
@@ -307,7 +239,6 @@ class TestCheckPrinterUsesCachedFrameUrl:
             "action": "notify",
             "poll_interval": 10,
             "enabled_printers": None,
-            "external_url": "http://bambuddy:8000",
         }
         status = MagicMock(state="RUNNING", task_name="job", subtask_name="")
 
@@ -315,7 +246,7 @@ class TestCheckPrinterUsesCachedFrameUrl:
         mock_response.json.return_value = {"detections": []}
         mock_response.raise_for_status = MagicMock()
         mock_client = MagicMock()
-        mock_client.get = AsyncMock(return_value=mock_response)
+        mock_client.post = AsyncMock(return_value=mock_response)
         mock_client.__aenter__ = AsyncMock(return_value=mock_client)
         mock_client.__aexit__ = AsyncMock(return_value=False)
 
@@ -325,14 +256,18 @@ class TestCheckPrinterUsesCachedFrameUrl:
         ):
             await svc._check_printer(1, status, settings)
 
-        # Extract the img URL handed to the ML API
-        _args, kwargs = mock_client.get.call_args
-        img_url = kwargs["params"]["img"]
-        assert img_url.startswith("http://bambuddy:8000/api/v1/obico/cached-frame/")
-        # The path segment after /cached-frame/ is the nonce itself — that nonce must
-        # resolve back to our stashed frame (single-use guarantees freshness).
-        nonce = img_url.rsplit("/", 1)[-1]
-        assert await pop_frame(nonce) == FAKE_JPEG
+        # ML API was called via POST
+        mock_client.post.assert_called_once()
+        _args, kwargs = mock_client.post.call_args
+        # URL is the ML API /p/ endpoint
+        assert _args[0] == "http://obico:3333/p/"
+        # Image bytes sent as multipart file upload
+        files = kwargs["files"]
+        assert "img" in files
+        filename, data, content_type = files["img"]
+        assert filename == "snapshot.jpg"
+        assert data == FAKE_JPEG
+        assert content_type == "image/jpeg"
 
     @pytest.mark.asyncio
     async def test_capture_failure_skips_ml_call(self):
@@ -345,12 +280,11 @@ class TestCheckPrinterUsesCachedFrameUrl:
             "action": "notify",
             "poll_interval": 10,
             "enabled_printers": None,
-            "external_url": "http://bambuddy:8000",
         }
         status = MagicMock(state="RUNNING", task_name="job", subtask_name="")
 
         mock_client = MagicMock()
-        mock_client.get = AsyncMock()
+        mock_client.post = AsyncMock()
         mock_client.__aenter__ = AsyncMock(return_value=mock_client)
         mock_client.__aexit__ = AsyncMock(return_value=False)
 
@@ -360,6 +294,6 @@ class TestCheckPrinterUsesCachedFrameUrl:
         ):
             await svc._check_printer(1, status, settings)
 
-        mock_client.get.assert_not_called()
+        mock_client.post.assert_not_called()
         assert svc._last_error is not None
         assert "Failed to capture snapshot" in svc._last_error