From fca3f55ae5735b9f4f84e141d442d9553c56626b Mon Sep 17 00:00:00 2001 From: Russ Allbery Date: Mon, 18 Sep 2023 13:55:26 -0700 Subject: [PATCH] Properly test the Pydantic validators The Pydantic field validators were being tested without models, which wasn't ideal since they wouldn't be tested for compatibility with newer versions of Pydantic. Add model-based tests. normalize_datetime now explicitly rejects input other than seconds since epoch or datetime objects with a validation error rather than attempting to treat the input as a datetime object and potentially throwing more obscure errors. --- changelog.d/20230918_134911_rra_DM_40744.md | 3 ++ src/safir/pydantic.py | 14 ++++--- tests/pydantic_test.py | 43 +++++++++++++++------ 3 files changed, 43 insertions(+), 17 deletions(-) create mode 100644 changelog.d/20230918_134911_rra_DM_40744.md diff --git a/changelog.d/20230918_134911_rra_DM_40744.md b/changelog.d/20230918_134911_rra_DM_40744.md new file mode 100644 index 00000000..a88ab71b --- /dev/null +++ b/changelog.d/20230918_134911_rra_DM_40744.md @@ -0,0 +1,3 @@ +### Bug fixes + +- `safir.pydantic.normalize_datetime` now explicitly rejects input other than seconds since epoch or datetime objects with a validation error rather than attempting to treat the input as a datetime object and potentially throwing more obscure errors. diff --git a/src/safir/pydantic.py b/src/safir/pydantic.py index 7abdfc98..8bed00d7 100644 --- a/src/safir/pydantic.py +++ b/src/safir/pydantic.py @@ -20,18 +20,18 @@ ] -def normalize_datetime(v: int | datetime | None) -> datetime | None: +def normalize_datetime(v: Any) -> datetime | None: """Pydantic field validator for datetime fields. - Supports `~datetime.datetime` fields given in either any format supported - by Pydantic natively, or in seconds since epoch (which Pydantic doesn't - support). This field validator ensures that datetimes are always stored - in the model as timezone-aware UTC datetimes. + Supports `~datetime.datetime` fields given as either datetime objects or + seconds since epoch (not the other types Pydantic natively supports) and + ensures that the resulting datetime object is timezone-aware and in the + UTC timezone. Parameters ---------- v - The field representing a `~datetime.datetime`. + Field representing a `~datetime.datetime`. Returns ------- @@ -61,6 +61,8 @@ class Info(BaseModel): return v elif isinstance(v, int): return datetime.fromtimestamp(v, tz=UTC) + elif not isinstance(v, datetime): + raise ValueError("Must be a datetime or seconds since epoch") elif v.tzinfo and v.tzinfo.utcoffset(v) is not None: return v.astimezone(UTC) else: diff --git a/tests/pydantic_test.py b/tests/pydantic_test.py index 70a7037f..4fad7b46 100644 --- a/tests/pydantic_test.py +++ b/tests/pydantic_test.py @@ -6,7 +6,12 @@ from datetime import UTC, datetime, timedelta, timezone import pytest -from pydantic import BaseModel, ValidationError, model_validator +from pydantic import ( + BaseModel, + ValidationError, + field_validator, + model_validator, +) from safir.pydantic import ( CamelCaseModel, @@ -18,39 +23,55 @@ def test_normalize_datetime() -> None: - assert normalize_datetime(None) is None + class TestModel(BaseModel): + time: datetime | None + + _val = field_validator("time", mode="before")(normalize_datetime) + + assert TestModel(time=None).time is None date = datetime.fromtimestamp(1668814932, tz=UTC) - assert normalize_datetime(1668814932) == date + model = TestModel(time=1668814932) # type: ignore[arg-type] + assert model.time == date mst_zone = timezone(-timedelta(hours=7)) mst_date = datetime.now(tz=mst_zone) utc_date = mst_date.astimezone(UTC) - assert normalize_datetime(mst_date) == utc_date + assert TestModel(time=mst_date).time == utc_date naive_date = datetime.utcnow() # noqa: DTZ003 - aware_date = normalize_datetime(naive_date) + aware_date = TestModel(time=naive_date).time assert aware_date == naive_date.replace(tzinfo=UTC) assert aware_date.tzinfo == UTC + with pytest.raises(ValueError, match=r"Must be a datetime or seconds .*"): + TestModel(time="2023-01-25T15:44:00+00:00") # type: ignore[arg-type] + def test_normalize_isodatetime() -> None: - assert normalize_isodatetime(None) is None + class TestModel(BaseModel): + time: datetime | None + + _val = field_validator("time", mode="before")(normalize_isodatetime) + + assert TestModel(time=None).time is None date = datetime.fromisoformat("2023-01-25T15:44:34+00:00") - assert date == normalize_isodatetime("2023-01-25T15:44:34Z") + model = TestModel(time="2023-01-25T15:44:34Z") # type: ignore[arg-type] + assert model.time == date date = datetime.fromisoformat("2023-01-25T15:44:00+00:00") - assert date == normalize_isodatetime("2023-01-25T15:44Z") + model = TestModel(time="2023-01-25T15:44Z") # type: ignore[arg-type] + assert model.time == date with pytest.raises(ValueError, match=r"Must be a string in .* format"): - normalize_isodatetime("2023-01-25T15:44:00+00:00") + TestModel(time="2023-01-25T15:44:00+00:00") # type: ignore[arg-type] with pytest.raises(ValueError, match=r"Must be a string in .* format"): - normalize_isodatetime(1668814932) # type: ignore[arg-type] + TestModel(time=1668814932) # type: ignore[arg-type] with pytest.raises(ValueError, match=r"Must be a string in .* format"): - normalize_isodatetime("next thursday") + TestModel(time="next thursday") # type: ignore[arg-type] def test_to_camel_case() -> None: