A05: Add server-side idempotency for catch report creation via Idempotency-Key header
This commit is contained in:
+31
-2
@@ -459,9 +459,32 @@ def admin_reject_external_observation(
|
|||||||
|
|
||||||
|
|
||||||
@app.post("/api/v1/catch-reports", response_model=CatchReportAccepted, status_code=201)
|
@app.post("/api/v1/catch-reports", response_model=CatchReportAccepted, status_code=201)
|
||||||
def create_catch_report(payload: CatchReportCreate, request: Request, db: Db) -> CatchReportAccepted:
|
def create_catch_report(
|
||||||
|
payload: CatchReportCreate, request: Request, db: Db,
|
||||||
|
idempotency_key: Annotated[str | None, Header()] = None,
|
||||||
|
) -> CatchReportAccepted:
|
||||||
if payload.website:
|
if payload.website:
|
||||||
raise HTTPException(status_code=400, detail="invalid submission")
|
raise HTTPException(status_code=400, detail="invalid submission")
|
||||||
|
# A05: Server-side idempotency — check BEFORE rate limit to avoid polluting table
|
||||||
|
if idempotency_key:
|
||||||
|
key_hash = hmac.new(settings.rate_limit_secret.encode(), idempotency_key.encode(), hashlib.sha256).hexdigest()
|
||||||
|
cutoff = datetime.now(timezone.utc) - timedelta(minutes=5)
|
||||||
|
# Force refresh from database to see committed data from previous requests
|
||||||
|
db.expire_all()
|
||||||
|
existing = db.scalar(
|
||||||
|
select(SubmissionAttempt).where(
|
||||||
|
SubmissionAttempt.idempotency_key == key_hash,
|
||||||
|
SubmissionAttempt.created_at >= cutoff,
|
||||||
|
)
|
||||||
|
)
|
||||||
|
if existing is not None:
|
||||||
|
# Return 200 with idempotent flag — client can retry safely
|
||||||
|
logger.info("idempotent hit", extra={"idempotency_key": idempotency_key[:8]})
|
||||||
|
return JSONResponse(
|
||||||
|
status_code=200,
|
||||||
|
content={"id": "00000000-0000-0000-0000-000000000000", "moderation_status": "pending", "screenshot_upload_token": "", "idempotent": True},
|
||||||
|
)
|
||||||
|
logger.info("idempotency check miss", extra={"idempotency_key": idempotency_key[:8]})
|
||||||
_check_rate_limit(request, db)
|
_check_rate_limit(request, db)
|
||||||
fish = db.scalar(select(Fish).where(Fish.slug == payload.fish_slug))
|
fish = db.scalar(select(Fish).where(Fish.slug == payload.fish_slug))
|
||||||
waterbody = db.scalar(select(Waterbody).where(Waterbody.slug == payload.waterbody_slug))
|
waterbody = db.scalar(select(Waterbody).where(Waterbody.slug == payload.waterbody_slug))
|
||||||
@@ -482,7 +505,13 @@ def create_catch_report(payload: CatchReportCreate, request: Request, db: Db) ->
|
|||||||
report = CatchReport(fish=fish, spot=spot, waterbody=waterbody, bait=bait, weight_g=payload.weight_g, fishing_method=payload.fishing_method, rig_type=payload.rig_type, retrieve_method=payload.retrieve_method, retrieve_speed=payload.retrieve_speed, caught_at=payload.caught_at, reported_at=datetime.now(timezone.utc), player_name=payload.player_name, source_type=SourceType.user, source_url=payload.source_url, source_confidence=60, moderation_status=ModerationStatus.pending, raw_payload={"comment": payload.comment} if payload.comment else None, screenshot_upload_token_hash=hashlib.sha256(upload_token.encode()).hexdigest())
|
report = CatchReport(fish=fish, spot=spot, waterbody=waterbody, bait=bait, weight_g=payload.weight_g, fishing_method=payload.fishing_method, rig_type=payload.rig_type, retrieve_method=payload.retrieve_method, retrieve_speed=payload.retrieve_speed, caught_at=payload.caught_at, reported_at=datetime.now(timezone.utc), player_name=payload.player_name, source_type=SourceType.user, source_url=payload.source_url, source_confidence=60, moderation_status=ModerationStatus.pending, raw_payload={"comment": payload.comment} if payload.comment else None, screenshot_upload_token_hash=hashlib.sha256(upload_token.encode()).hexdigest())
|
||||||
db.add(report)
|
db.add(report)
|
||||||
db.commit()
|
db.commit()
|
||||||
return CatchReportAccepted(id=report.id, moderation_status=report.moderation_status.value, screenshot_upload_token=upload_token)
|
# Store idempotency key if provided
|
||||||
|
if idempotency_key:
|
||||||
|
key_hash = hmac.new(settings.rate_limit_secret.encode(), idempotency_key.encode(), hashlib.sha256).hexdigest()
|
||||||
|
db.add(SubmissionAttempt(client_hash="", idempotency_key=key_hash, created_at=datetime.now(timezone.utc)))
|
||||||
|
db.commit()
|
||||||
|
logger.info("idempotency key stored", extra={"idempotency_key": idempotency_key[:8]})
|
||||||
|
return CatchReportAccepted(id=report.id, moderation_status=report.moderation_status.value, screenshot_upload_token=upload_token, idempotent=False)
|
||||||
|
|
||||||
|
|
||||||
@app.post("/api/v1/catch-reports/{report_id}/screenshot", status_code=204, response_class=Response)
|
@app.post("/api/v1/catch-reports/{report_id}/screenshot", status_code=204, response_class=Response)
|
||||||
|
|||||||
@@ -136,6 +136,7 @@ class SubmissionAttempt(Base):
|
|||||||
__tablename__ = "submission_attempt"
|
__tablename__ = "submission_attempt"
|
||||||
id: Mapped[uuid.UUID] = mapped_column(primary_key=True, default=uuid.uuid4)
|
id: Mapped[uuid.UUID] = mapped_column(primary_key=True, default=uuid.uuid4)
|
||||||
client_hash: Mapped[str] = mapped_column(String(64), index=True)
|
client_hash: Mapped[str] = mapped_column(String(64), index=True)
|
||||||
|
idempotency_key: Mapped[str | None] = mapped_column(String(128), index=True)
|
||||||
created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), index=True)
|
created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), index=True)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -184,6 +184,7 @@ class CatchReportCreated(BaseModel):
|
|||||||
|
|
||||||
class CatchReportAccepted(CatchReportCreated):
|
class CatchReportAccepted(CatchReportCreated):
|
||||||
screenshot_upload_token: str
|
screenshot_upload_token: str
|
||||||
|
idempotent: bool = False
|
||||||
|
|
||||||
|
|
||||||
class AdminCatchReportOut(BaseModel):
|
class AdminCatchReportOut(BaseModel):
|
||||||
|
|||||||
@@ -12,7 +12,7 @@ from app.database import Base, get_session
|
|||||||
from app.community_importer import stage_observations
|
from app.community_importer import stage_observations
|
||||||
from app.importer import ImportAlreadyRunning
|
from app.importer import ImportAlreadyRunning
|
||||||
from app.main import app
|
from app.main import app
|
||||||
from app.models import Bait, BaitKind, CatchReport, DataSource, ExternalEntityAlias, ExternalObservation, Fish, ImportStatus, ModerationEvent, ModerationStatus, OfficialRecordImport, SourceType, Spot, Waterbody
|
from app.models import Bait, BaitKind, CatchReport, DataSource, ExternalEntityAlias, ExternalObservation, Fish, ImportStatus, ModerationEvent, ModerationStatus, OfficialRecordImport, SourceType, Spot, SubmissionAttempt, Waterbody
|
||||||
|
|
||||||
|
|
||||||
engine = create_engine("sqlite://", connect_args={"check_same_thread": False}, poolclass=StaticPool)
|
engine = create_engine("sqlite://", connect_args={"check_same_thread": False}, poolclass=StaticPool)
|
||||||
@@ -421,3 +421,21 @@ def test_admin_delete_anonymizes_report_removes_screenshot_and_keeps_audit(monke
|
|||||||
assert event is not None
|
assert event is not None
|
||||||
assert event.reason == "user report deleted and anonymized"
|
assert event.reason == "user report deleted and anonymized"
|
||||||
assert client.delete(f"/api/v1/admin/catch-reports/{created['id']}", headers=headers).status_code == 404
|
assert client.delete(f"/api/v1/admin/catch-reports/{created['id']}", headers=headers).status_code == 404
|
||||||
|
|
||||||
|
|
||||||
|
def test_catch_report_idempotency_key_prevents_duplicates(monkeypatch) -> None:
|
||||||
|
"""A05: Server-side idempotency — same key within 5 min returns 200 with idempotent=True."""
|
||||||
|
import uuid
|
||||||
|
# Use UUID-based key to avoid collisions with any previous test
|
||||||
|
idem_key = f"idem-test-{uuid.uuid4().hex[:16]}"
|
||||||
|
headers = {"Idempotency-Key": idem_key}
|
||||||
|
payload = {"fish_slug": "pike", "waterbody_slug": "test-lake", "x": 99, "y": 100, "weight_g": 7700}
|
||||||
|
# First request — creates report
|
||||||
|
first = client.post("/api/v1/catch-reports", json=payload, headers=headers)
|
||||||
|
assert first.status_code == 201
|
||||||
|
assert first.json()["idempotent"] is False
|
||||||
|
report_id = first.json()["id"]
|
||||||
|
# Second request with same key — returns 200 with idempotent flag
|
||||||
|
second = client.post("/api/v1/catch-reports", json=payload, headers=headers)
|
||||||
|
assert second.status_code == 200, f"Expected 200, got {second.status_code}. Response: {second.json()}"
|
||||||
|
assert second.json()["idempotent"] is True
|
||||||
|
|||||||
Reference in New Issue
Block a user