stop paging when the provider stops making progress
An ArcGIS layer without supportsPagination accepts resultOffset and ignores it, answering every page with the first one. The VHA profile reader advanced its offset by the page length and stopped at the announced count, so for a count that is a multiple of the page size it collected N copies of page one — and its completeness check, len(features) == candidate_count, passed. Four announced records became four stored records, two of them duplicates, filed under an official provenance. That is the substitution bounded acquisition exists to prevent, arriving through the front door. The reader now refuses a record it already collected. It fails rather than silently dropping the duplicate: a provider that cannot page is a provider whose count proves nothing, so a smaller-but-clean result would still be unverifiable. Its watercourse-name loop was worse — a bare `while True` that ended only when the provider stopped setting exceededTransferLimit, with names deduplicated by code so a stuck provider produced no visible change while the requests continued. It now refuses a repeated page body, and both loops have the page budget the sibling readers already had. Those siblings turned out to be fine. GRB and official vector already refuse a repeated page URL, bound the page count, and deduplicate on feature identity — but none of it had a test, so none of it was known to work. Exercised now, including the case where distinct URLs defeat the loop check and the budget is the only backstop. A duplicate across two genuinely different pages is kept once rather than failing, because a cursor over a changing table produces that legitimately. Also: _bash_path fell back to the raw path whenever wslpath failed, except on timeout, which propagated and reddened the suite when starting WSL took more than ten seconds under load. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,128 @@
|
||||
"""A provider that ignores ``resultOffset`` must not produce duplicated data.
|
||||
|
||||
ArcGIS layers without ``supportsPagination`` accept ``resultOffset`` and ignore
|
||||
it, answering every page with the first one. The profile reader advanced its
|
||||
offset by the page length and stopped when it reached the announced count, so
|
||||
for a count that is a multiple of the page size it collected N copies of page
|
||||
one, matched the expected total exactly, and stored that as an official
|
||||
dataset. The watercourse-name reader had no bound at all: it looped for as long
|
||||
as the provider kept setting ``exceededTransferLimit``.
|
||||
|
||||
The sibling reader for official vector products already refuses a repeated page
|
||||
and deduplicates on feature identity. These tests hold this reader to the same
|
||||
rule.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from typing import Any
|
||||
from urllib.parse import parse_qs, urlparse
|
||||
|
||||
import pytest
|
||||
|
||||
from app.core.config import Settings, get_settings
|
||||
from app.core.errors import AppError
|
||||
from app.services.bathymetry_profile_acquisition_service import (
|
||||
BathymetryProfileAcquisitionService,
|
||||
)
|
||||
|
||||
BBOX = (4.30, 51.20, 4.32, 51.22)
|
||||
|
||||
|
||||
def _settings(**overrides: Any) -> Settings:
|
||||
base = get_settings()
|
||||
return base.model_copy(update={"bathymetry_profiles_page_size": 2, **overrides})
|
||||
|
||||
|
||||
class _Response:
|
||||
def __init__(self, payload: dict[str, Any]) -> None:
|
||||
self._body = json.dumps(payload).encode("utf-8")
|
||||
|
||||
def read(self, _limit: int | None = None) -> bytes:
|
||||
return self._body
|
||||
|
||||
def __enter__(self) -> "_Response":
|
||||
return self
|
||||
|
||||
def __exit__(self, *_exc: object) -> bool:
|
||||
return False
|
||||
|
||||
|
||||
def _profile(object_id: int) -> dict[str, Any]:
|
||||
return {
|
||||
"attributes": {"OBJECTID": object_id, "vhag": 7, "opg_diepte": 1.5},
|
||||
"geometry": {"x": 4.31, "y": 51.21},
|
||||
}
|
||||
|
||||
|
||||
class StuckProvider:
|
||||
"""Answers every page with the same records, as an unpaged layer does."""
|
||||
|
||||
def __init__(self, *, count: int, page: list[dict[str, Any]]) -> None:
|
||||
self.count = count
|
||||
self.page = page
|
||||
self.requests: list[str] = []
|
||||
|
||||
def __call__(self, request: Any, **_kwargs: Any) -> _Response:
|
||||
url = request.full_url if hasattr(request, "full_url") else str(request)
|
||||
self.requests.append(url)
|
||||
query = parse_qs(urlparse(url).query)
|
||||
if query.get("returnCountOnly") == ["true"]:
|
||||
return _Response({"count": self.count})
|
||||
return _Response({"features": list(self.page), "exceededTransferLimit": True})
|
||||
|
||||
|
||||
def test_a_provider_that_ignores_the_offset_is_refused_not_duplicated() -> None:
|
||||
"""Four announced records, two per page, the same two every time.
|
||||
|
||||
Advancing by page length reaches the announced total after two pages, so the
|
||||
completeness check passed while every record was stored twice.
|
||||
"""
|
||||
|
||||
provider = StuckProvider(count=4, page=[_profile(1), _profile(2)])
|
||||
|
||||
with pytest.raises(AppError) as exc_info:
|
||||
BathymetryProfileAcquisitionService._fetch_profiles(BBOX, _settings(), provider)
|
||||
|
||||
assert exc_info.value.code == "BATHYMETRY_PROVIDER_UNSTABLE_PAGINATION"
|
||||
assert exc_info.value.status_code == 502
|
||||
|
||||
|
||||
def test_honest_pagination_still_returns_every_record() -> None:
|
||||
"""The guard must not reject a provider that pages correctly."""
|
||||
|
||||
pages = [[_profile(1), _profile(2)], [_profile(3), _profile(4)]]
|
||||
|
||||
def opener(request: Any, **_kwargs: Any) -> _Response:
|
||||
url = request.full_url
|
||||
query = parse_qs(urlparse(url).query)
|
||||
if query.get("returnCountOnly") == ["true"]:
|
||||
return _Response({"count": 4})
|
||||
offset = int(query.get("resultOffset", ["0"])[0])
|
||||
index = offset // 2
|
||||
page = pages[index] if index < len(pages) else []
|
||||
return _Response({"features": page})
|
||||
|
||||
features, provenance = BathymetryProfileAcquisitionService._fetch_profiles(
|
||||
BBOX, _settings(), opener
|
||||
)
|
||||
|
||||
assert [item["attributes"]["OBJECTID"] for item in features] == [1, 2, 3, 4]
|
||||
assert provenance["candidate_count"] == 4
|
||||
|
||||
|
||||
def test_watercourse_names_stop_instead_of_looping_forever() -> None:
|
||||
"""``exceededTransferLimit`` forever is not a reason to request forever."""
|
||||
|
||||
provider = StuckProvider(
|
||||
count=0,
|
||||
page=[{"attributes": {"wlasvl.vhag": 7, "VHAG_TABEL.naam": "Schelde"}}],
|
||||
)
|
||||
|
||||
with pytest.raises(AppError) as exc_info:
|
||||
BathymetryProfileAcquisitionService._fetch_watercourse_names({7}, _settings(), provider)
|
||||
|
||||
assert exc_info.value.code == "BATHYMETRY_PROVIDER_UNSTABLE_PAGINATION"
|
||||
# Bounded, and bounded early: it must not have hammered the provider first.
|
||||
assert len(provider.requests) <= 3
|
||||
Reference in New Issue
Block a user