refactor: remove legacy submission handlers
This commit is contained in:
@@ -58,7 +58,7 @@ Production release отделяет Alembic от runtime: одноразовый
|
|||||||
|
|
||||||
Тяжёлый production bootstrap вынесен в отдельный ручной/еженедельный CI workflow с 30-минутным timeout и сохраняемыми diagnostics; обычный push по-прежнему использует быстрый Compose E2E.
|
Тяжёлый production bootstrap вынесен в отдельный ручной/еженедельный CI workflow с 30-минутным timeout и сохраняемыми diagnostics; обычный push по-прежнему использует быстрый Compose E2E.
|
||||||
|
|
||||||
Публичный API зафиксирован генерируемым [OpenAPI-контрактом](docs/api-contract.md): CI сравнивает `apps/api/openapi.json` с фактической схемой FastAPI, поэтому рефакторинг routers не может незаметно изменить URL, параметры или response models. Декомпозиция выполняется инкрементально: catalog, activity/spots, public data и submissions принадлежат отдельным `APIRouter`; проверка доверенных proxy и persistent rate limit отправки улова изолированы в `submission_security`.
|
Публичный API зафиксирован генерируемым [OpenAPI-контрактом](docs/api-contract.md): CI сравнивает `apps/api/openapi.json` с фактической схемой FastAPI, поэтому рефакторинг routers не может незаметно изменить URL, параметры или response models. Декомпозиция выполняется инкрементально: catalog, activity/spots, public data и submissions принадлежат отдельным `APIRouter`; submission flow больше не дублируется в `main.py`, а проверка доверенных proxy и persistent rate limit изолированы в `submission_security`.
|
||||||
|
|
||||||
После повторных ошибок scheduler увеличивает паузу экспоненциально до 24 часов и возвращается к 30 минутам после успеха. Публичная страница `/status` показывает свежесть и состояние источников без URL запросов, внутренних ошибок и другой диагностической информации.
|
После повторных ошибок scheduler увеличивает паузу экспоненциально до 24 часов и возвращается к 30 минутам после успеха. Публичная страница `/status` показывает свежесть и состояние источников без URL запросов, внутренних ошибок и другой диагностической информации.
|
||||||
|
|
||||||
|
|||||||
+5
-107
@@ -1,31 +1,26 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
from datetime import datetime, timedelta, timezone
|
from datetime import datetime, timedelta, timezone
|
||||||
import hashlib
|
|
||||||
import hmac
|
|
||||||
import json
|
|
||||||
import logging
|
import logging
|
||||||
import secrets
|
|
||||||
import time as time_module
|
import time as time_module
|
||||||
from typing import Annotated, Literal
|
from typing import Annotated, Literal
|
||||||
from uuid import UUID
|
from uuid import UUID
|
||||||
import uuid
|
import uuid
|
||||||
|
|
||||||
import httpx
|
import httpx
|
||||||
from fastapi import Depends, FastAPI, File, Header, HTTPException, Query, Request, Response, UploadFile
|
from fastapi import Depends, FastAPI, Header, HTTPException, Query, Request, Response
|
||||||
from fastapi.middleware.cors import CORSMiddleware
|
from fastapi.middleware.cors import CORSMiddleware
|
||||||
from fastapi.responses import JSONResponse
|
from fastapi.responses import JSONResponse
|
||||||
from sqlalchemy import case, func, or_, select
|
from sqlalchemy import case, func, or_, select
|
||||||
from sqlalchemy.exc import IntegrityError
|
|
||||||
from sqlalchemy.orm import Session, joinedload
|
from sqlalchemy.orm import Session, joinedload
|
||||||
|
|
||||||
from .admin_security import verify_admin
|
from .admin_security import verify_admin
|
||||||
from .config import settings
|
from .config import settings
|
||||||
from .dependencies import Db
|
from .dependencies import Db
|
||||||
from .community_review import ExternalReviewError, map_observation, publish_observation, reject_observation, suggest_aliases
|
from .community_review import ExternalReviewError, map_observation, publish_observation, reject_observation, suggest_aliases
|
||||||
from .importer import ImportAlreadyRunning, ImportSourceError, import_records, normalize
|
from .importer import ImportAlreadyRunning, ImportSourceError, import_records
|
||||||
from .logging_config import configure_logging
|
from .logging_config import configure_logging
|
||||||
from .models import Bait, BaitKind, CatchReport, CommunityImportRun, DataSource, ExternalObservation, Fish, ModerationEvent, ModerationStatus, OfficialRecordImport, SourceType, Spot, SubmissionAttempt, Waterbody
|
from .models import CatchReport, CommunityImportRun, DataSource, ExternalObservation, Fish, ModerationEvent, ModerationStatus, OfficialRecordImport, SourceType, Waterbody
|
||||||
from .readiness import readiness_report
|
from .readiness import readiness_report
|
||||||
from .routers.activity import router as activity_router
|
from .routers.activity import router as activity_router
|
||||||
from .routers.catalog import router as catalog_router
|
from .routers.catalog import router as catalog_router
|
||||||
@@ -33,8 +28,8 @@ from .routers.public_data import router as public_data_router
|
|||||||
from .routers.submissions import router as submissions_router
|
from .routers.submissions import router as submissions_router
|
||||||
from .time_utils import aware
|
from .time_utils import aware
|
||||||
from .public_cache import public_cache
|
from .public_cache import public_cache
|
||||||
from .schemas import ActivityOut, AdminCatchReportOut, AdminModerationHistoryOut, CatchReportAccepted, CatchReportCreate, CatchReportCreated, ExternalAliasSuggestionOut, ExternalObservationAction, ExternalObservationDecision, ExternalObservationMapping, ExternalObservationOut, ExternalObservationPublished, ImportRunOut, ModerationUpdate
|
from .schemas import ActivityOut, AdminCatchReportOut, AdminModerationHistoryOut, CatchReportCreated, ExternalAliasSuggestionOut, ExternalObservationAction, ExternalObservationDecision, ExternalObservationMapping, ExternalObservationOut, ExternalObservationPublished, ImportRunOut, ModerationUpdate
|
||||||
from .storage import ScreenshotError, client as storage_client, delete_screenshot, signed_screenshot_url, upload_screenshot
|
from .storage import client as storage_client, delete_screenshot, signed_screenshot_url
|
||||||
from .submission_security import check_rate_limit
|
from .submission_security import check_rate_limit
|
||||||
from .submission_security import is_trusted_proxy as _is_trusted_proxy
|
from .submission_security import is_trusted_proxy as _is_trusted_proxy
|
||||||
|
|
||||||
@@ -364,103 +359,6 @@ def admin_reject_external_observation(
|
|||||||
raise HTTPException(status_code=409, detail=str(exc)) from exc
|
raise HTTPException(status_code=409, detail=str(exc)) from exc
|
||||||
|
|
||||||
|
|
||||||
def _legacy_create_catch_report(
|
|
||||||
payload: CatchReportCreate, request: Request, db: Db,
|
|
||||||
idempotency_key: Annotated[str | None, Header()] = None,
|
|
||||||
) -> CatchReportAccepted:
|
|
||||||
if payload.website:
|
|
||||||
raise HTTPException(status_code=400, detail="invalid submission")
|
|
||||||
payload_hash = hashlib.sha256(json.dumps(payload.model_dump(mode="json"), sort_keys=True, separators=(",", ":")).encode()).hexdigest()
|
|
||||||
# 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]})
|
|
||||||
report = existing.catch_report
|
|
||||||
if existing.payload_hash and not hmac.compare_digest(existing.payload_hash, payload_hash):
|
|
||||||
raise HTTPException(status_code=409, detail="Idempotency-Key was already used with different payload")
|
|
||||||
if report is None:
|
|
||||||
raise HTTPException(status_code=409, detail="idempotency record is incomplete; retry with a new key")
|
|
||||||
# Re-derive the one-time upload token from the idempotency key;
|
|
||||||
# only its hash is persisted, so the secret is never stored.
|
|
||||||
replay_token = hmac.new(settings.rate_limit_secret.encode(), (key_hash + ":upload").encode(), hashlib.sha256).hexdigest()
|
|
||||||
if not hmac.compare_digest(hashlib.sha256(replay_token.encode()).hexdigest(), report.screenshot_upload_token_hash or ""):
|
|
||||||
raise HTTPException(status_code=409, detail="idempotency record token mismatch; retry with a new key")
|
|
||||||
return JSONResponse(status_code=200, content={"id": str(report.id), "moderation_status": report.moderation_status.value, "screenshot_upload_token": replay_token, "idempotent": True})
|
|
||||||
logger.info("idempotency check miss", extra={"idempotency_key": idempotency_key[:8]})
|
|
||||||
_check_rate_limit(request, db)
|
|
||||||
fish = db.scalar(select(Fish).where(Fish.slug == payload.fish_slug))
|
|
||||||
waterbody = db.scalar(select(Waterbody).where(Waterbody.slug == payload.waterbody_slug))
|
|
||||||
if fish is None or waterbody is None:
|
|
||||||
raise HTTPException(status_code=422, detail="unknown fish or waterbody")
|
|
||||||
spot = db.scalar(select(Spot).where(Spot.waterbody_id == waterbody.id, Spot.x == payload.x, Spot.y == payload.y))
|
|
||||||
if spot is None:
|
|
||||||
spot = Spot(waterbody=waterbody, x=payload.x, y=payload.y)
|
|
||||||
db.add(spot)
|
|
||||||
bait = None
|
|
||||||
if payload.bait_name and payload.bait_name.strip():
|
|
||||||
key = normalize(payload.bait_name)
|
|
||||||
bait = db.scalar(select(Bait).where(Bait.normalized_name == key))
|
|
||||||
if bait is None:
|
|
||||||
bait = Bait(name=payload.bait_name.strip(), normalized_name=key, kind=BaitKind.unknown)
|
|
||||||
db.add(bait)
|
|
||||||
upload_token = (hmac.new(settings.rate_limit_secret.encode(), (key_hash + ":upload").encode(), hashlib.sha256).hexdigest()
|
|
||||||
if idempotency_key else secrets.token_urlsafe(32))
|
|
||||||
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)
|
|
||||||
# Store idempotency key if provided
|
|
||||||
if idempotency_key:
|
|
||||||
key_hash = hmac.new(settings.rate_limit_secret.encode(), idempotency_key.encode(), hashlib.sha256).hexdigest()
|
|
||||||
# One transaction: a unique-key race must roll back the report too.
|
|
||||||
db.add(SubmissionAttempt(client_hash="", idempotency_key=key_hash, catch_report=report, payload_hash=payload_hash, created_at=datetime.now(timezone.utc)))
|
|
||||||
try:
|
|
||||||
db.commit()
|
|
||||||
except IntegrityError:
|
|
||||||
# Another request won the same idempotency key race.
|
|
||||||
db.rollback()
|
|
||||||
winner = db.scalar(select(SubmissionAttempt).where(SubmissionAttempt.idempotency_key == key_hash))
|
|
||||||
if winner and winner.catch_report:
|
|
||||||
replay_token = hmac.new(settings.rate_limit_secret.encode(), (key_hash + ":upload").encode(), hashlib.sha256).hexdigest()
|
|
||||||
return JSONResponse(status_code=200, content={"id": str(winner.catch_report.id), "moderation_status": winner.catch_report.moderation_status.value, "screenshot_upload_token": replay_token, "idempotent": True})
|
|
||||||
raise
|
|
||||||
logger.info("idempotency key stored", extra={"idempotency_key": idempotency_key[:8]})
|
|
||||||
else:
|
|
||||||
db.commit()
|
|
||||||
return CatchReportAccepted(id=report.id, moderation_status=report.moderation_status.value, screenshot_upload_token=upload_token, idempotent=False)
|
|
||||||
|
|
||||||
|
|
||||||
def _legacy_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)
|
|
||||||
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")
|
|
||||||
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):
|
|
||||||
raise HTTPException(status_code=401, detail="invalid screenshot upload token")
|
|
||||||
if report.screenshot_key:
|
|
||||||
raise HTTPException(status_code=409, detail="screenshot already uploaded")
|
|
||||||
raw = screenshot.file.read(settings.screenshot_max_bytes + 1)
|
|
||||||
try:
|
|
||||||
report.screenshot_key = upload_screenshot(raw, filename=screenshot.filename, content_type=screenshot.content_type)
|
|
||||||
except ScreenshotError as exc:
|
|
||||||
raise HTTPException(status_code=422, detail=str(exc)) from exc
|
|
||||||
report.screenshot_upload_token_hash = None
|
|
||||||
db.commit()
|
|
||||||
return Response(status_code=204)
|
|
||||||
|
|
||||||
|
|
||||||
app.include_router(submissions_router)
|
app.include_router(submissions_router)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
+1
-1
@@ -51,7 +51,7 @@
|
|||||||
|
|
||||||
Аудит выполнен на старой базе `13e04e6`; рекомендации ниже повторно проверены по текущей ветке. Уже реализованные или неприменимые предложения не возвращаются в backlog.
|
Аудит выполнен на старой базе `13e04e6`; рекомендации ниже повторно проверены по текущей ветке. Уже реализованные или неприменимые предложения не возвращаются в backlog.
|
||||||
|
|
||||||
- [ ] **Q11 · Декомпозиция API — в работе.** Публичные catalog, activity/spots и records/community/status/import-history вынесены в отдельные `APIRouter`. Рабочие submission handlers теперь физически находятся в `routers/submissions.py` вместе с idempotency и upload flow; security остаётся в `submission_security`, URL и OpenAPI сохранены. Старые функции в `main.py` сняты с регистрации; следующим локальным пакетом удалить этот мёртвый код и лишние импорты, затем вынести admin.
|
- [ ] **Q11 · Декомпозиция API — в работе.** Публичные catalog, activity/spots и records/community/status/import-history вынесены в отдельные `APIRouter`. Submission API полностью изолирован в `routers/submissions.py` вместе с idempotency и upload flow, security остаётся в `submission_security`; мёртвые обработчики и лишние зависимости удалены из `main.py`, URL и OpenAPI сохранены. Следующий локальный пакет — вынести admin endpoints.
|
||||||
- [ ] **Q12 · Query-plan gate.** В рамках Q07 снять `EXPLAIN (ANALYZE, BUFFERS)` для activity, records, spot detail и public spot pages на реалистичном наборе данных. Существующие индексы миграции `0011_query_indexes` не дублировать; индекс с `fish_id`, SQL-агрегацию или materialized view добавлять только по измеренному плану и p95.
|
- [ ] **Q12 · Query-plan gate.** В рамках Q07 снять `EXPLAIN (ANALYZE, BUFFERS)` для activity, records, spot detail и public spot pages на реалистичном наборе данных. Существующие индексы миграции `0011_query_indexes` не дублировать; индекс с `fish_id`, SQL-агрегацию или materialized view добавлять только по измеренному плану и p95.
|
||||||
- [x] **Q13 · Production bootstrap в CI.** Отдельный workflow запускает `deploy/test-production-bootstrap.sh` вручную или раз в неделю, а не на каждом push. Вывод bootstrap всегда сохраняется 14 дней; при падении добавляются Compose status и Playwright diagnostics.
|
- [x] **Q13 · Production bootstrap в CI.** Отдельный workflow запускает `deploy/test-production-bootstrap.sh` вручную или раз в неделю, а не на каждом push. Вывод bootstrap всегда сохраняется 14 дней; при падении добавляются Compose status и Playwright diagnostics.
|
||||||
- [ ] **Q14 · Полная CSP — origin-policy внедрена.** Production ограничивает default/connect/form/font/media/manifest текущим доменом, изображения — self/data/`FILES_DOMAIN`, запрещает inline event handlers, eval, wildcard и HTTP. Инвентаризация зафиксировала динамический JSON-LD, page scripts, scoped styles и CSS variables; из-за них `unsafe-inline` временно остаётся только для script/style элементов и style attributes. Далее вынести page scripts, решить nonce/hash JSON-LD и убрать исключения поэтапно с bootstrap-проверкой report/admin/OG/screenshots.
|
- [ ] **Q14 · Полная CSP — origin-policy внедрена.** Production ограничивает default/connect/form/font/media/manifest текущим доменом, изображения — self/data/`FILES_DOMAIN`, запрещает inline event handlers, eval, wildcard и HTTP. Инвентаризация зафиксировала динамический JSON-LD, page scripts, scoped styles и CSS variables; из-за них `unsafe-inline` временно остаётся только для script/style элементов и style attributes. Далее вынести page scripts, решить nonce/hash JSON-LD и убрать исключения поэтапно с bootstrap-проверкой report/admin/OG/screenshots.
|
||||||
|
|||||||
Reference in New Issue
Block a user