diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e0ccbb..978fc5d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,12 @@ Format: [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this project a reason to expose the walk at all is that the offset rule stops being copied, and a non-default format reaching the same records by an untested path would leave the consumer with a second implementation of the subtle part after all. +- **A `features` value that is not a list is refused.** `yield from` over a mapping + yields its keys and `len()` of it counts them, so a page shaped that way made the walk + emit strings and step its offset by a number with nothing to do with any record, and + nothing raised. Found by the consuming project's own copy of this walk, which checked + the type: a compensation cannot be retired until the thing it compensates for is gone, + so the check belongs here. ### Added, a survey command that inventories a retrieval's candidate markers diff --git a/src/perimeter/acquire.py b/src/perimeter/acquire.py index 08180ea..730f576 100644 --- a/src/perimeter/acquire.py +++ b/src/perimeter/acquire.py @@ -287,6 +287,15 @@ def iter_features( "early and write a short file with a clean hash." ) features = payload["features"] + if not isinstance(features, list): + raise AcquisitionFailed( + f"{endpoint} answered a page whose 'features' is a " + f"{type(features).__name__} rather than a list, at offset {offset}. " + "The walk yields from it and steps its offset by its length, and both " + "of those do something plausible to a mapping: it would yield the " + "field names and step by the number of them. A page shaped like that " + "is not a page of features." + ) if not features: break yield from features diff --git a/tests/test_acquire.py b/tests/test_acquire.py index f0eec15..266eff9 100644 --- a/tests/test_acquire.py +++ b/tests/test_acquire.py @@ -774,6 +774,33 @@ def fake_get(url: str, **_: object) -> dict[str, Any]: assert "features" in str(raised.value) +def test_a_features_value_that_is_not_a_list_is_refused( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A mapping under `features` walks and produces nonsense, silently. + + `yield from` over a dict yields its keys, and `len()` of it is the number of + them, so a page shaped that way makes the walk emit strings and step its + offset by a number that has nothing to do with any record. Nothing raises. + + Found by a consuming project's own copy of this walk, which checked the type + and would have had to keep the check to move onto this one. That is the sort + of thing the audit in that repository exists to surface: a compensation + cannot be retired until the thing it compensates for is gone. + """ + + def fake_get(url: str, **_: object) -> dict[str, Any]: + return { + "features": {"OBJECTID": 1, "YEAR_": 2020}, + "exceededTransferLimit": False, + } + + monkeypatch.setattr(acquire_mod, "_get", fake_get) + with pytest.raises(AcquisitionFailed) as raised: + list(iter_features("https://example.invalid/query", ("OBJECTID",))) + assert "rather than a list" in str(raised.value) + + def test_a_layer_that_really_is_empty_is_still_walked_to_a_clean_stop( monkeypatch: pytest.MonkeyPatch, ) -> None: