From a64f90372cd93858d969ff2e79d314a4edf96a3d Mon Sep 17 00:00:00 2001 From: Simon van Lierde Date: Tue, 27 Oct 2026 16:32:13 +0100 Subject: [PATCH 1/2] fix(backend): keep the backend image id when only image_url is unusable - a 2xx reply with a valid image_id and a bad or missing image_url returned no id, so the sink fell back to the local capture id - the id is now kept when it passes validation on its own; only the URL is dropped Signed-off-by: Simon van Lierde --- CHANGELOG.md | 2 ++ app/backend/client.py | 8 +++++++- tests/unit/backend/test_client.py | 28 +++++++++++++++++++++++++--- 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bb395f3..09631b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). E - A 2xx upload reply that is not JSON or lacks `image_id` or `image_url` now counts as delivered, with a warning in the log. Before, the Pi retried the upload and the backend stored the capture twice. +- A 2xx upload reply with a valid `image_id` but a bad or missing `image_url` now keeps the + backend's id. Before, the Pi fell back to its own capture id and lost the backend's. ## [0.7.1] - 2026-10-27 diff --git a/app/backend/client.py b/app/backend/client.py index 0d6d3d1..0c60429 100644 --- a/app/backend/client.py +++ b/app/backend/client.py @@ -17,7 +17,7 @@ from urllib.parse import quote import httpx -from pydantic import AnyUrl +from pydantic import AnyUrl, ValidationError from app.backend.plugin_api import PLUGIN_API_PREFIX from app.core.runtime_context import get_active_runtime_state @@ -161,6 +161,12 @@ async def upload_image( # queue retry and the backend store a duplicate, so keep what parses and drop the rest. try: ack = DeviceImageUploadAck.model_validate(payload) + except ValidationError as exc: + logger.warning("Backend stored the image but its reply was unusable: %r", payload) + # Keep the backend's id when it passed on its own: the local capture id is not the backend's. + id_is_bad = any(err["loc"][:1] == ("image_id",) for err in exc.errors()) + image_id = payload.get("image_id") if isinstance(payload, dict) and not id_is_bad else None + return UploadedImageInfo(image_id=image_id, image_url=None) except (TypeError, ValueError): logger.warning("Backend stored the image but its reply was unusable: %r", payload) return UploadedImageInfo(image_id=None, image_url=None) diff --git a/tests/unit/backend/test_client.py b/tests/unit/backend/test_client.py index 188b0a2..1a40642 100644 --- a/tests/unit/backend/test_client.py +++ b/tests/unit/backend/test_client.py @@ -246,18 +246,40 @@ async def test_non_json_2xx_counts_as_delivered(self, caplog: pytest.LogCaptureF patched_client.return_value.post.assert_awaited_once() async def test_missing_fields_2xx_counts_as_delivered(self, caplog: pytest.LogCaptureFixture) -> None: - """A 2xx reply without image_url is delivered once: no exception, so no queue retry.""" - response = _fake_response(200, {"image_id": "abc"}) + """A 2xx reply without image_url is delivered once and keeps the backend's valid image_id.""" + response = _fake_response(200, {"image_id": SAMPLE_SERVER_IMAGE_ID}) with _patch_async_client(response) as patched_client, caplog.at_level(logging.WARNING): result = await upload_image( image_bytes=b"\xff\xd8", filename="t.jpg", capture_metadata={}, upload_metadata={} ) - assert result.image_id is None + assert result.image_id == SAMPLE_SERVER_IMAGE_ID assert result.image_url is None assert "unusable" in caplog.text patched_client.return_value.post.assert_awaited_once() + async def test_invalid_image_url_keeps_valid_image_id(self) -> None: + """A bad image_url drops only the URL: the backend's real id must not be lost.""" + response = _fake_response(200, {"image_id": SAMPLE_SERVER_IMAGE_ID, "image_url": "ftp://bad host"}) + with _patch_async_client(response): + result = await upload_image( + image_bytes=b"\xff\xd8", filename="t.jpg", capture_metadata={}, upload_metadata={} + ) + + assert result.image_id == SAMPLE_SERVER_IMAGE_ID + assert result.image_url is None + + async def test_invalid_image_id_is_dropped(self) -> None: + """An invalid image_id is not passed on, even when the URL is fine.""" + response = _fake_response(200, {"image_id": "bad id!", "image_url": BACKEND_IMAGE_URL}) + with _patch_async_client(response): + result = await upload_image( + image_bytes=b"\xff\xd8", filename="t.jpg", capture_metadata={}, upload_metadata={} + ) + + assert result.image_id is None + assert result.image_url is None + async def test_requires_pairing_backend_url(self, monkeypatch: pytest.MonkeyPatch) -> None: """Uploading without a configured pairing backend must fail cleanly.""" monkeypatch.setattr(settings, "pairing_backend_url", "") From 5228adf8a718abdd5e9725bf535e94fb33ae486f Mon Sep 17 00:00:00 2001 From: Simon van Lierde Date: Tue, 27 Oct 2026 16:32:22 +0100 Subject: [PATCH 2/2] refactor(pairing): rename _API_PORT to API_PORT - the constant is imported by another module, so it is not private Signed-off-by: Simon van Lierde --- app/pairing/routers/local_access.py | 4 ++-- app/pairing/routers/setup.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/pairing/routers/local_access.py b/app/pairing/routers/local_access.py index dcda79c..1ed454a 100644 --- a/app/pairing/routers/local_access.py +++ b/app/pairing/routers/local_access.py @@ -23,12 +23,12 @@ router = APIRouter(prefix="/system", tags=["system"]) -_API_PORT = 8018 +API_PORT = 8018 def get_candidate_urls() -> list[str]: """Return all non-loopback IPv4 base URLs for this host.""" - return [f"http://{ip}:{_API_PORT}" for ip in find_lan_ips()] + return [f"http://{ip}:{API_PORT}" for ip in find_lan_ips()] def _get_mdns_name() -> str | None: diff --git a/app/pairing/routers/setup.py b/app/pairing/routers/setup.py index 34c3ebf..ef6bd8a 100644 --- a/app/pairing/routers/setup.py +++ b/app/pairing/routers/setup.py @@ -19,7 +19,7 @@ from app.core.settings import DEFAULT_PAIRING_BACKEND_URL, settings from app.core.templates_config import templates from app.observability.logging import build_log_extra -from app.pairing.routers.local_access import _API_PORT, get_candidate_urls +from app.pairing.routers.local_access import API_PORT, get_candidate_urls from app.pairing.services.credentials import delete_relay_credentials from app.pairing.services.service import ( PAIRING_CODE_TTL_SECONDS, @@ -87,7 +87,7 @@ async def setup_page(request: Request) -> HTMLResponse: if operator_authenticated: candidate_urls = get_candidate_urls() # Strip scheme and port so the template can compose its own URLs. - lan_ips = [u.removeprefix("http://").removesuffix(f":{_API_PORT}") for u in candidate_urls] or [""] + lan_ips = [u.removeprefix("http://").removesuffix(f":{API_PORT}") for u in candidate_urls] or [""] connection_host = lan_ips[0] pairing_backend_reachable = mode is ConnectionMode.PAIRED or ( mode is ConnectionMode.PAIRING and await _pairing_backend_reachable()