diff --git a/CHANGELOG.md b/CHANGELOG.md index 772bd9f..11d9787 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -## [0.0.16] - 2026-08-04 +### Fixed + +- **`DatabaseConfig` ignored values passed by field name** (`jvspatial/api/config_groups.py`). + Its aliased fields (`dynamodb_*`, `postgres_*`) carry a `validation_alias`, and a + pydantic v2 model accepts an aliased field *only* by its alias unless it opts into + `populate_by_name` — which `AuthConfig` does and `DatabaseConfig` did not. So every + such value passed by field name was silently discarded: both + `server_config_overrides_from_env()`, which keys its database group by field name, + and embedding hosts constructing a `DatabaseConfig` directly. Nothing raised, + because the adapters then read the same settings from env themselves — the + configuration object was simply never the source of truth it appeared to be. + DynamoDB was affected the whole time; Postgres inherited it in 0.0.16. Coverage: + `tests/api/test_database_config_population.py`. ### Added diff --git a/jvspatial/api/config_groups.py b/jvspatial/api/config_groups.py index 3105cf7..42474b4 100644 --- a/jvspatial/api/config_groups.py +++ b/jvspatial/api/config_groups.py @@ -17,6 +17,11 @@ class DatabaseConfig(BaseModel): """Database configuration group.""" + # Aliased fields below are settable only by their alias unless the model + # opts in here, so callers passing them by field name -- including + # ``server_config_overrides_from_env`` -- were silently ignored. + model_config = ConfigDict(populate_by_name=True) + db_type: Optional[str] = None db_path: Optional[str] = None db_path_resolve: Optional[str] = Field( diff --git a/tests/api/test_database_config_population.py b/tests/api/test_database_config_population.py new file mode 100644 index 0000000..42ff309 --- /dev/null +++ b/tests/api/test_database_config_population.py @@ -0,0 +1,71 @@ +"""DatabaseConfig accepts its fields by name, not only by env alias. + +Aliased fields on a pydantic v2 model are settable *only* by their alias +unless the model opts into ``populate_by_name``. ``DatabaseConfig`` did not, +so every ``dynamodb_*`` (and later ``postgres_*``) value passed by field name +-- by ``server_config_overrides_from_env`` and by embedding hosts building a +``DatabaseConfig`` directly -- was silently dropped. Nothing raised; the +backend simply fell back to reading env itself, which masked it. + +``AuthConfig`` already sets ``populate_by_name``; these tests pin the same +contract for ``DatabaseConfig``. +""" + +from __future__ import annotations + +import pytest + +from jvspatial.api.config_groups import DatabaseConfig + +_ALIASED_FIELDS = [ + ("dynamodb_table_name", "JVSPATIAL_DYNAMODB_TABLE_NAME", "my-table"), + ("dynamodb_region", "JVSPATIAL_DYNAMODB_REGION", "eu-west-1"), + ("dynamodb_endpoint_url", "JVSPATIAL_DYNAMODB_ENDPOINT_URL", "http://localhost"), + ("dynamodb_access_key_id", "AWS_ACCESS_KEY_ID", "AKIAEXAMPLE"), + ("dynamodb_secret_access_key", "AWS_SECRET_ACCESS_KEY", "secret"), + ("postgres_dsn", "JVSPATIAL_POSTGRES_DSN", "postgresql://u:p@h:5432/db"), + ("postgres_pooler_mode", "JVSPATIAL_POSTGRES_POOLER_MODE", "transaction"), +] + + +@pytest.mark.parametrize("field,alias,value", _ALIASED_FIELDS) +def test_field_name_population(field: str, alias: str, value: str) -> None: + assert getattr(DatabaseConfig(**{field: value}), field) == value + + +@pytest.mark.parametrize("field,alias,value", _ALIASED_FIELDS) +def test_alias_population_still_works(field: str, alias: str, value: str) -> None: + """The env alias path must keep working — it is how env overrides land.""" + assert getattr(DatabaseConfig(**{alias: value}), field) == value + + +@pytest.mark.parametrize( + "field,value", + [("postgres_min_pool_size", 2), ("postgres_max_pool_size", 20)], +) +def test_integer_fields_populate_by_name(field: str, value: int) -> None: + assert getattr(DatabaseConfig(**{field: value}), field) == value + + +def test_env_adapter_output_lands_on_the_model(monkeypatch: pytest.MonkeyPatch) -> None: + """The dict built from env must actually populate the model. + + ``server_config_overrides_from_env`` keys its database group by field + name, so this is the path that was quietly inert. + """ + from jvspatial.env_adapter import server_config_overrides_from_env + + monkeypatch.setenv("JVSPATIAL_DB_TYPE", "postgres") + monkeypatch.setenv("JVSPATIAL_POSTGRES_DSN", "postgresql://u:p@h:5432/db") + monkeypatch.setenv("JVSPATIAL_POSTGRES_MAX_POOL_SIZE", "7") + + db = DatabaseConfig(**server_config_overrides_from_env()["database"]) + + assert db.db_type == "postgres" + assert db.postgres_dsn == "postgresql://u:p@h:5432/db" + assert db.postgres_max_pool_size == 7 + + +def test_unaliased_fields_unaffected() -> None: + db = DatabaseConfig(db_type="json", db_path="./jvdb") + assert (db.db_type, db.db_path) == ("json", "./jvdb")