diff --git a/changelog.d/economy-query-ignore-unknown.fixed.md b/changelog.d/economy-query-ignore-unknown.fixed.md new file mode 100644 index 000000000..c99b8b486 --- /dev/null +++ b/changelog.d/economy-query-ignore-unknown.fixed.md @@ -0,0 +1 @@ +Ignore undeclared query parameters on the legacy economy routes instead of returning HTTP 400; declared parameters and the SPM selection object stay strictly validated. The release gate's live suite appends a staging_probe parameter that the typed parser had started rejecting. diff --git a/docs/canonical-spm.md b/docs/canonical-spm.md index 43a96ceee..524746218 100644 --- a/docs/canonical-spm.md +++ b/docs/canonical-spm.md @@ -244,8 +244,10 @@ query string. For example, before URL encoding: ``` Use the client's query-encoding support to encode that JSON once. All query -parameters are scalar: repeated keys, unknown parameters, malformed JSON and -duplicate fields inside the SPM object return HTTP 400. `region` is required; +parameters are scalar: repeated declared keys, malformed JSON and duplicate or +unknown fields inside the SPM object return HTTP 400. Query parameters the route does +not declare are ignored, as the legacy routes always did; an omitted or +misspelled `spm` therefore inherits the certified default measurement. `region` is required; annual requests require `time_period`, while budget-window requests require `start_year` and `window_size` (1–75, ending no later than 2099). Years use four digits. Optional fields are `dataset` (default `default`), `version` (installed diff --git a/docs/engineering/skills/api-routes.md b/docs/engineering/skills/api-routes.md index 1b6a4b91a..2f34ec6b7 100644 --- a/docs/engineering/skills/api-routes.md +++ b/docs/engineering/skills/api-routes.md @@ -48,7 +48,11 @@ uniformity. ## Parsing Rules -- Reject unknown query parameters rather than ignoring misspellings. +- Reject unknown query parameters rather than ignoring misspellings. The one + deliberate exception is the legacy economy GET family (`EconomyQuery` and + its subclasses), which ignores undeclared parameters because callers append + parameters those routes never read, such as the release gate's + `staging_probe`; its declared parameters and the `spm` object stay strict. - Reject a scalar query parameter supplied more than once rather than selecting an arbitrary value. - Accept repeated keys only when the canonical field is explicitly list-valued. @@ -109,7 +113,7 @@ For each new or changed query contract, cover the applicable cases: - optional parameters use their canonical defaults; - valid values receive canonical normalization; - invalid types and out-of-range values are rejected; -- unknown parameters are rejected; +- unknown parameters are rejected (ignored on the legacy economy GET family); - duplicate scalar parameters are rejected; - explicitly list-valued parameters accept the documented repeated form; - OpenAPI declares the runtime name, type, required status, default, and bounds; diff --git a/policyengine_api/query_parameters.py b/policyengine_api/query_parameters.py index 60af041be..01a45ed11 100644 --- a/policyengine_api/query_parameters.py +++ b/policyengine_api/query_parameters.py @@ -93,7 +93,11 @@ def validate_legacy_user_id(value: str) -> str: class StrictQueryParameters(BaseModel): - """Base for query contracts that reject every undeclared field.""" + """Base for query contracts that reject every undeclared field. + + ``EconomyQuery`` and its subclasses are the one family that overrides this + to ignore undeclared parameters; see its docstring for why. + """ model_config = ConfigDict(extra="forbid", frozen=True) @@ -129,7 +133,19 @@ def parse_integer_query_value(value: Any) -> Any: class EconomyQuery(StrictQueryParameters): - """Common legacy economy query options; identifiers remain in the path.""" + """Common legacy economy query options; identifiers remain in the path. + + These legacy GET routes accepted any query string before the typed parser, + and callers still append parameters the routes never read, such as the + ``staging_probe`` correlation id the release gate's live suite sends. An + undeclared parameter is therefore ignored rather than rejected: an omitted + ``spm`` selection already dispatches the certified default measurement, so + a misspelled one cannot select anything else. Declared parameters keep + their validation, repeated scalars are still rejected, and the ``spm`` + object itself stays strict. + """ + + model_config = ConfigDict(extra="ignore", frozen=True) region: EconomyRegion dataset: str = Field(default="default", description="Dataset selection") diff --git a/tests/unit/routes/test_economy_spm_query.py b/tests/unit/routes/test_economy_spm_query.py index aae3f21c8..792ae41af 100644 --- a/tests/unit/routes/test_economy_spm_query.py +++ b/tests/unit/routes/test_economy_spm_query.py @@ -125,12 +125,30 @@ def test_duplicate_scalar_economy_queries_never_dispatch(economy_http, field): gateway.get_spm_capability.assert_not_called() -def test_unknown_economy_query_never_dispatches(economy_http): - client, path, query, dispatch, _, _ = economy_http - response = client.get(path, query_string=query + [("spmm", "{}")]) - assert response.status_code == 400 - assert "spmm" in response.get_json()["message"] - dispatch.assert_not_called() +@pytest.mark.parametrize( + "extra", + [("spmm", "{}"), ("staging_probe", "cloud-run-stg-1-abcdef-utah")], + ids=["misspelled-selection", "release-gate-probe"], +) +def test_undeclared_economy_query_is_ignored_and_dispatches_defaults( + economy_http, extra +): + """Undeclared parameters never reach the service and never block dispatch. + + The release gate's live suite appends ``staging_probe`` to every economy + request, and legacy callers append parameters these GET routes never read. + A misspelled selection cannot choose a measurement the omitted selection + would not: both inherit the certified defaults. + """ + client, path, query, dispatch, _, setups = economy_http + response = client.get(path, query_string=query + [extra]) + assert response.status_code == 200, response.get_json() + assert response.get_json()["status"] == "computing" + dispatch.assert_called_once() + assert extra[0] not in dispatch.call_args.kwargs + assert dispatch.call_args.kwargs["options"] == {} + assert len(setups) == 1 + assert setups[0].options["spm"]["geography_kind"] == "county" def test_economy_required_query_fields_are_rejected_before_dispatch(economy_http): diff --git a/tests/unit/test_query_parameters.py b/tests/unit/test_query_parameters.py index 75ca8f07e..5947d6eae 100644 --- a/tests/unit/test_query_parameters.py +++ b/tests/unit/test_query_parameters.py @@ -348,3 +348,28 @@ def households( } assert parameters["offset"]["schema"]["default"] == 0 assert parameters["limit"]["schema"]["default"] == 100 + + +def _strict_query_subclasses(base: type) -> set[type]: + found: set[type] = set() + for subclass in base.__subclasses__(): + found.add(subclass) + found |= _strict_query_subclasses(subclass) + return found + + +def test_only_the_economy_query_family_ignores_undeclared_parameters() -> None: + """The economy tolerance must not leak into any other query contract.""" + from policyengine_api import query_parameters + + tolerant = { + model + for model in _strict_query_subclasses(query_parameters.StrictQueryParameters) + if model.model_config.get("extra") != "forbid" + } + economy_family = {query_parameters.EconomyQuery} | _strict_query_subclasses( + query_parameters.EconomyQuery + ) + assert tolerant == economy_family + assert all(model.model_config.get("extra") == "ignore" for model in tolerant) + assert query_parameters.StrictQueryParameters.model_config.get("extra") == "forbid"