fix: make screenshot retry idempotent
CI / backend-and-migrations (push) Canceled after 0s
CI / astro-build (push) Canceled after 0s
CI / dependency-audit (push) Canceled after 0s
CI / compose-e2e (push) Canceled after 0s

This commit is contained in:
ik
2026-09-22 08:17:47 +07:00
parent b1d6d8d122
commit 89dcf5f48e
3 changed files with 28 additions and 8 deletions
+14 -4
View File
@@ -19,7 +19,7 @@ from ..dependencies import Db
from ..importer import normalize from ..importer import normalize
from ..models import Bait, BaitKind, CatchReport, Fish, ModerationStatus, SourceType, Spot, SubmissionAttempt, Waterbody from ..models import Bait, BaitKind, CatchReport, Fish, ModerationStatus, SourceType, Spot, SubmissionAttempt, Waterbody
from ..schemas import CatchReportAccepted, CatchReportCreate from ..schemas import CatchReportAccepted, CatchReportCreate
from ..storage import ScreenshotError, upload_screenshot from ..storage import ScreenshotError, delete_screenshot, upload_screenshot
from ..submission_security import check_rate_limit from ..submission_security import check_rate_limit
from ..tackle_components import replace_tackle_components from ..tackle_components import replace_tackle_components
from rf4_research.gear_components import from_catch_fields from rf4_research.gear_components import from_catch_fields
@@ -77,21 +77,31 @@ def create_catch_report(payload: CatchReportCreate, request: Request, db: Db, id
@router.post("/api/v1/catch-reports/{report_id}/screenshot", status_code=204, response_class=Response) @router.post("/api/v1/catch-reports/{report_id}/screenshot", status_code=204, response_class=Response)
def add_screenshot(report_id: UUID, db: Db, screenshot: UploadFile = File(), upload_token: Annotated[str | None, Header(alias="X-Upload-Token")] = None) -> Response: def add_screenshot(report_id: UUID, db: Db, screenshot: UploadFile = File(), upload_token: Annotated[str | None, Header(alias="X-Upload-Token")] = None) -> Response:
report = db.get(CatchReport, report_id) report_query = select(CatchReport).where(CatchReport.id == report_id)
if db.bind is not None and db.bind.dialect.name == "postgresql":
report_query = report_query.with_for_update()
report = db.scalar(report_query)
if report is None or report.source_type != SourceType.user or report.moderation_status != ModerationStatus.pending: if report is None or report.source_type != SourceType.user or report.moderation_status != ModerationStatus.pending:
raise HTTPException(status_code=404, detail="pending catch report not found") raise HTTPException(status_code=404, detail="pending catch report not found")
supplied_hash = hashlib.sha256((upload_token or "").encode()).hexdigest() supplied_hash = hashlib.sha256((upload_token or "").encode()).hexdigest()
if not report.screenshot_upload_token_hash or not hmac.compare_digest(report.screenshot_upload_token_hash, supplied_hash): if not report.screenshot_upload_token_hash or not hmac.compare_digest(report.screenshot_upload_token_hash, supplied_hash):
raise HTTPException(status_code=401, detail="invalid screenshot upload token") raise HTTPException(status_code=401, detail="invalid screenshot upload token")
if report.screenshot_key: if report.screenshot_key:
raise HTTPException(status_code=409, detail="screenshot already uploaded") return Response(status_code=204)
raw = screenshot.file.read(settings.screenshot_max_bytes + 1) raw = screenshot.file.read(settings.screenshot_max_bytes + 1)
try: try:
report.screenshot_key = upload_screenshot(raw, filename=screenshot.filename, content_type=screenshot.content_type) report.screenshot_key = upload_screenshot(raw, filename=screenshot.filename, content_type=screenshot.content_type)
except ScreenshotError as exc: except ScreenshotError as exc:
raise HTTPException(status_code=422, detail=str(exc)) from exc raise HTTPException(status_code=422, detail=str(exc)) from exc
report.screenshot_upload_token_hash = None try:
db.commit() db.commit()
except Exception as exc:
db.rollback()
try:
delete_screenshot(report.screenshot_key)
except Exception:
logger.exception("failed to remove orphaned screenshot", extra={"screenshot_key": report.screenshot_key})
raise HTTPException(status_code=503, detail="screenshot could not be saved; please retry") from exc
return Response(status_code=204) return Response(status_code=204)
+9 -2
View File
@@ -528,14 +528,21 @@ def test_admin_can_start_and_list_official_import(monkeypatch) -> None:
def test_pending_report_accepts_one_validated_screenshot(monkeypatch) -> None: def test_pending_report_accepts_one_validated_screenshot(monkeypatch) -> None:
created = client.post("/api/v1/catch-reports", json={"fish_slug": "pike", "waterbody_slug": "test-lake", "x": 91, "y": 92, "weight_g": 4200}).json() created = client.post("/api/v1/catch-reports", json={"fish_slug": "pike", "waterbody_slug": "test-lake", "x": 91, "y": 92, "weight_g": 4200}).json()
monkeypatch.setattr("app.routers.submissions.upload_screenshot", lambda raw, **metadata: "reports/test.jpg" if raw == b"image-bytes" and metadata == {"filename": "catch.jpg", "content_type": "image/jpeg"} else "unexpected") uploaded: list[bytes] = []
def fake_upload(raw, **metadata):
if raw == b"image-bytes" and metadata == {"filename": "catch.jpg", "content_type": "image/jpeg"}:
uploaded.append(raw)
return "reports/test.jpg"
return "unexpected"
monkeypatch.setattr("app.routers.submissions.upload_screenshot", fake_upload)
upload_url = f"/api/v1/catch-reports/{created['id']}/screenshot" upload_url = f"/api/v1/catch-reports/{created['id']}/screenshot"
assert client.post(upload_url, files={"screenshot": ("catch.jpg", b"image-bytes", "image/jpeg")}).status_code == 401 assert client.post(upload_url, files={"screenshot": ("catch.jpg", b"image-bytes", "image/jpeg")}).status_code == 401
assert client.post(upload_url, headers={"X-Upload-Token": "wrong"}, files={"screenshot": ("catch.jpg", b"image-bytes", "image/jpeg")}).status_code == 401 assert client.post(upload_url, headers={"X-Upload-Token": "wrong"}, files={"screenshot": ("catch.jpg", b"image-bytes", "image/jpeg")}).status_code == 401
response = client.post(upload_url, headers={"X-Upload-Token": created["screenshot_upload_token"]}, files={"screenshot": ("catch.jpg", b"image-bytes", "image/jpeg")}) response = client.post(upload_url, headers={"X-Upload-Token": created["screenshot_upload_token"]}, files={"screenshot": ("catch.jpg", b"image-bytes", "image/jpeg")})
assert response.status_code == 204 assert response.status_code == 204
reused = client.post(upload_url, headers={"X-Upload-Token": created["screenshot_upload_token"]}, files={"screenshot": ("catch.jpg", b"image-bytes", "image/jpeg")}) reused = client.post(upload_url, headers={"X-Upload-Token": created["screenshot_upload_token"]}, files={"screenshot": ("catch.jpg", b"image-bytes", "image/jpeg")})
assert reused.status_code == 401 assert reused.status_code == 204
assert uploaded == [b"image-bytes"]
def test_idempotency_replay_survives_completed_screenshot(monkeypatch) -> None: def test_idempotency_replay_survives_completed_screenshot(monkeypatch) -> None:
+4 -1
View File
@@ -12,7 +12,10 @@ export const POST: APIRoute = async ({ request, redirect, cookies }) => {
try { try {
const upload = new FormData(); upload.set("screenshot", screenshot); const upload = new FormData(); upload.set("screenshot", screenshot);
const response = await fetch(`${base}/api/v1/catch-reports/${reportId}/screenshot`, { method: "POST", headers:{"X-Upload-Token":uploadToken}, body: upload, signal: AbortSignal.timeout(60_000) }); const response = await fetch(`${base}/api/v1/catch-reports/${reportId}/screenshot`, { method: "POST", headers:{"X-Upload-Token":uploadToken}, body: upload, signal: AbortSignal.timeout(60_000) });
if (response.ok) cookies.delete(cookieName, {path:"/"}); if (response.ok) {
cookies.delete(cookieName, {path:"/"});
cookies.delete("rf4-idempotency-key", {path:"/"});
}
return redirect(response.ok ? "/report?state=screenshot_sent" : `/report?state=screenshot_error&report_id=${encodeURIComponent(reportId)}`, 303); return redirect(response.ok ? "/report?state=screenshot_sent" : `/report?state=screenshot_error&report_id=${encodeURIComponent(reportId)}`, 303);
} catch (err) { } catch (err) {
const isTimeout = err instanceof DOMException && err.name === "TimeoutError" || const isTimeout = err instanceof DOMException && err.name === "TimeoutError" ||