diff --git a/app/api/routes/location.py b/app/api/routes/location.py index 33132be..6c03bf4 100644 --- a/app/api/routes/location.py +++ b/app/api/routes/location.py @@ -1,4 +1,5 @@ import json +import logging from fastapi import APIRouter, Depends, Request from fastapi.responses import PlainTextResponse, Response @@ -10,6 +11,8 @@ from app.schemas.location import LocationRecordRequest from app.services.location import record_location router = APIRouter(tags=["location"]) +logger = logging.getLogger(__name__) +BAD_REQUEST_MESSAGE = "bad request" @router.post("/location/record") @@ -18,11 +21,15 @@ async def create_location_record(request: Request, db: Session = Depends(get_db) raw_payload = await request.body() data = json.loads(raw_payload) payload = LocationRecordRequest.model_validate(data) + record_location(db, payload) except json.JSONDecodeError as exc: - return PlainTextResponse(str(exc), status_code=400) + logger.warning("Rejected location request due to invalid JSON: %s", exc) + return PlainTextResponse(BAD_REQUEST_MESSAGE, status_code=400) except ValidationError as exc: - return PlainTextResponse(str(exc), status_code=400) + logger.warning("Rejected location request due to payload validation failure: %s", exc) + return PlainTextResponse(BAD_REQUEST_MESSAGE, status_code=400) + except ValueError as exc: + logger.warning("Rejected location request due to invalid numeric input: %s", exc) + return PlainTextResponse(BAD_REQUEST_MESSAGE, status_code=400) - record_location(db, payload) return Response(status_code=200) - diff --git a/app/schemas/location.py b/app/schemas/location.py index 94c41b1..9a7f670 100644 --- a/app/schemas/location.py +++ b/app/schemas/location.py @@ -5,7 +5,6 @@ class LocationRecordRequest(BaseModel): person: str latitude: str longitude: str - altitude: str = "" + altitude: str | None = None model_config = ConfigDict(extra="forbid") - diff --git a/app/services/location.py b/app/services/location.py index 6e9deae..b9b5618 100644 --- a/app/services/location.py +++ b/app/services/location.py @@ -7,13 +7,20 @@ from app.models.location import Location from app.schemas.location import LocationRecordRequest -def _parse_float_compat(value: str) -> float: +def _parse_optional_float_compat(value: str | None) -> float: try: return float(value) except (TypeError, ValueError): return 0.0 +def _parse_required_float(value: str, field_name: str) -> float: + try: + return float(value) + except (TypeError, ValueError) as exc: + raise ValueError(f"Invalid numeric value for {field_name}") from exc + + def _utc_now_rfc3339() -> str: now = datetime.now(timezone.utc).replace(microsecond=0) return now.isoformat().replace("+00:00", "Z") @@ -26,11 +33,10 @@ def record_location(session: Session, payload: LocationRecordRequest) -> None: .values( person=payload.person, datetime=_utc_now_rfc3339(), - latitude=_parse_float_compat(payload.latitude), - longitude=_parse_float_compat(payload.longitude), - altitude=_parse_float_compat(payload.altitude), + latitude=_parse_required_float(payload.latitude, "latitude"), + longitude=_parse_required_float(payload.longitude, "longitude"), + altitude=_parse_optional_float_compat(payload.altitude), ) ) session.execute(stmt) session.commit() - diff --git a/docs/location-recorder.md b/docs/location-recorder.md index 6ed6d9b..f532ec3 100644 --- a/docs/location-recorder.md +++ b/docs/location-recorder.md @@ -2,6 +2,13 @@ 本文档说明 `location recorder` 在 Python 项目中的当前数据库接管策略，以及 legacy SQLite 接管 runbook。 +当前 Python 版本的 `POST /location/record` 请求校验策略是： + +- `latitude` 和 `longitude` 为必填，缺失或无法解析成合法数值时返回 `400 bad request` +- `altitude` 为可选，缺失或非法时按 `0` 处理 +- unknown field 仍返回 `400 bad request` +- 对 caller 的错误响应保持简洁，不直接暴露底层校验细节；详细原因只写日志 + ## Legacy 事实基线 当前 legacy SQLite 中 `location` 表的真实 schema 为： diff --git a/tests/test_location.py b/tests/test_location.py index 522e0d6..47dee9f 100644 --- a/tests/test_location.py +++ b/tests/test_location.py @@ -88,17 +88,115 @@ def test_location_record_endpoint_rejects_unknown_fields(location_client) -> Non ) assert response.status_code == 400 + assert response.text == "bad request" + assert "extra" not in response.text + assert "ValidationError" not in response.text -def test_location_record_endpoint_keeps_legacy_lenient_number_parsing(location_client) -> None: - client, engine = location_client +def test_location_record_endpoint_rejects_missing_latitude(location_client) -> None: + client, _ = location_client + + response = client.post( + "/location/record", + json={ + "person": "tianyu", + "longitude": "4.56", + }, + ) + + assert response.status_code == 400 + assert response.text == "bad request" + assert "latitude" not in response.text + + +def test_location_record_endpoint_rejects_missing_longitude(location_client) -> None: + client, _ = location_client + + response = client.post( + "/location/record", + json={ + "person": "tianyu", + "latitude": "1.23", + }, + ) + + assert response.status_code == 400 + assert response.text == "bad request" + assert "longitude" not in response.text + + +def test_location_record_endpoint_rejects_invalid_latitude(location_client) -> None: + client, _ = location_client response = client.post( "/location/record", json={ "person": "tianyu", "latitude": "bad-lat", + "longitude": "4.56", + }, + ) + + assert response.status_code == 400 + assert response.text == "bad request" + assert "bad-lat" not in response.text + assert "latitude" not in response.text + + +def test_location_record_endpoint_rejects_invalid_longitude(location_client) -> None: + client, _ = location_client + + response = client.post( + "/location/record", + json={ + "person": "tianyu", + "latitude": "1.23", "longitude": "bad-long", + }, + ) + + assert response.status_code == 400 + assert response.text == "bad request" + assert "bad-long" not in response.text + assert "longitude" not in response.text + + +def test_location_record_endpoint_defaults_missing_altitude_to_zero(location_client) -> None: + client, engine = location_client + + response = client.post( + "/location/record", + json={ + "person": "tianyu", + "latitude": "1.23", + "longitude": "4.56", + }, + ) + + assert response.status_code == 200 + + with engine.connect() as conn: + row = conn.execute( + text( + "SELECT latitude, longitude, altitude " + "FROM location ORDER BY datetime DESC LIMIT 1" + ) + ).one() + + assert row.latitude == pytest.approx(1.23) + assert row.longitude == pytest.approx(4.56) + assert row.altitude == pytest.approx(0.0) + + +def test_location_record_endpoint_defaults_invalid_altitude_to_zero(location_client) -> None: + client, engine = location_client + + response = client.post( + "/location/record", + json={ + "person": "tianyu", + "latitude": "1.23", + "longitude": "4.56", "altitude": "bad-alt", }, ) @@ -113,8 +211,8 @@ def test_location_record_endpoint_keeps_legacy_lenient_number_parsing(location_c ) ).one() - assert row.latitude == pytest.approx(0.0) - assert row.longitude == pytest.approx(0.0) + assert row.latitude == pytest.approx(1.23) + assert row.longitude == pytest.approx(4.56) assert row.altitude == pytest.approx(0.0)