Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
9 changes: 9 additions & 0 deletions src/perimeter/acquire.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
27 changes: 27 additions & 0 deletions tests/test_acquire.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Loading