From cb22d19b9a9132912e4f3e8c8de7ce377cf2dd91 Mon Sep 17 00:00:00 2001 From: Eldon Marks Date: Wed, 5 Aug 2026 16:11:05 -0400 Subject: [PATCH] Accept DatabaseConfig values by field name, not only by env alias Its aliased fields carry a validation_alias, and a pydantic v2 model accepts an aliased field only by that alias unless it opts into populate_by_name. AuthConfig does; DatabaseConfig did not. So every value passed by field name was silently discarded: DatabaseConfig(dynamodb_table_name="t").dynamodb_table_name -> None DatabaseConfig(**{"JVSPATIAL_DYNAMODB_TABLE_NAME": "t"}) -> "t" Two paths were affected. server_config_overrides_from_env() keys its database group by field name, so its dynamodb and postgres mappings never reached the model. And embedding hosts that build a DatabaseConfig directly -- jvagent passes dynamodb_table_name / dynamodb_region this way -- were likewise ignored, which means an app.yaml table name or region has never taken effect for DynamoDB. Nothing raised, because the adapters read the same settings from env themselves and carried on. The configuration object simply was not the source of truth it appeared to be. DynamoDB has been affected since those fields were added; postgres inherited it in 0.0.16. Fix is the one line AuthConfig already carries. Tests cover both population paths for all aliased fields and drive server_config_overrides_from_env output onto the model -- the path that was inert, and that my earlier postgres tests missed by asserting the dict rather than the model built from it. --- CHANGELOG.md | 14 +++- jvspatial/api/config_groups.py | 5 ++ tests/api/test_database_config_population.py | 71 ++++++++++++++++++++ 3 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 tests/api/test_database_config_population.py 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")