diff --git a/commoner_probe/aspnet.py b/commoner_probe/aspnet.py index 31741f5..1741e47 100644 --- a/commoner_probe/aspnet.py +++ b/commoner_probe/aspnet.py @@ -202,8 +202,11 @@ def write_buttons(page: str) -> list[tuple[str, str]]: before submitting anything is the difference between reading a government database and inserting a false record into one. """ + # The label arrives as numeric HTML entities on Hindi-labelled forms, so a + # literal match against the Devanagari hints never fires and a live write + # control reads as harmless. Decode first. return [(n, v) for n, v in submit_buttons(page) - if any(h in (n + " " + v).lower() for h in WRITE_HINTS)] + if any(h in _html.unescape(n + " " + v).lower() for h in WRITE_HINTS)] def export_controls(page: str) -> list[tuple[str, str]]: diff --git a/commoner_probe/aspnet_cascade.py b/commoner_probe/aspnet_cascade.py index ca985d9..6160dc6 100644 --- a/commoner_probe/aspnet_cascade.py +++ b/commoner_probe/aspnet_cascade.py @@ -35,7 +35,7 @@ import html import re -from typing import Any, Iterable, Iterator +from typing import Any, Callable, Iterable, Iterator from .http_client import make_session @@ -49,10 +49,42 @@ class CascadeCrawler: """ def __init__(self, report_url: str, controls: dict[str, str], *, - session: Any | None = None, rate_limit_sec: float = 1.0) -> None: + session: Any | None = None, rate_limit_sec: float = 1.0, + user_agent: str | None = None, + session_factory: Callable[[], Any] | None = None) -> None: + """`user_agent` is exposed because some deployments answer HTTP 500 to + this package's own identifier and 200 to a browser string. See failure + mode 10 in `aspnet`. Overriding it is a deliberate act, so there is no + default: a caller decides, and records the decision where it is made. + + `session_factory` builds a REPLACEMENT session on reseat, and passing + one is the caller SAYING that a rebuild is theirs to define. So it wins + whenever it is given, including alongside an injected `session`: a + caller whose session carries a login supplies a factory that can + re-establish it. + + With a `session` and NO factory, the session is never replaced. This + class did not build that session, and discarding a caller's + authentication is not its call. A crawl in that state cannot recover + from an expired session, and that is the caller's trade to make. + """ self.report_url = report_url self.controls = controls - self.session = session or make_session(rate_limit_sec=rate_limit_sec) + # `is not None` on BOTH, for the same reason. A factory is a callable + # OBJECT as often as a lambda, and one that defines __len__ or __bool__ + # can be falsey. `or` would then drop the caller's factory and rebuild + # with the default client, or with nothing. + if session_factory is not None: + self._factory = session_factory + elif session is not None: + self._factory = None + else: + self._factory = lambda: make_session(rate_limit_sec=rate_limit_sec, + user_agent=user_agent) + # `session is not None`, never truthiness. A session object that + # defines __len__ or __bool__ can be falsey, and `or` would discard the + # caller's object and build another one in silence. + self.session = session if session is not None else self._factory() self._selected: dict[str, str] = {} self.page = self._get() @@ -110,8 +142,23 @@ def select(self, level: str, value: str) -> None: self.page = resp.content.decode("utf8", "replace") def reset(self) -> None: - """Drop every selection and refetch, recovering an expired session.""" + """Replace the session, drop every selection, and refetch. + + The stale state lives in the session cookie, so refetching with the + SAME session returns the same HTTP 500 and the crawl never recovers. + + WHICH session survives, in full: + + * a `session_factory` was given — the factory builds a new session, + including when a `session` was injected beside it. Supplying a + factory is the instruction for how to rebuild. + * a `session` alone — it is kept. Replacing it would discard an + authentication this class did not create. + * neither — the default client is rebuilt. + """ self._selected.clear() + if self._factory is not None: + self.session = self._factory() self.page = self._get() def reseat(self, path: Iterable[tuple[str, str]]) -> None: diff --git a/commoner_probe/cdn_dashboard.py b/commoner_probe/cdn_dashboard.py index 2a7e48c..e7573ee 100644 --- a/commoner_probe/cdn_dashboard.py +++ b/commoner_probe/cdn_dashboard.py @@ -89,6 +89,19 @@ class Place: district_code: str +class GeoFenced(RuntimeError): + """The edge refused the client's country. + + Separate from an absent period ON PURPOSE. Both answer 403, and conflating + them turns a run that was blocked outright into a clean empty dataset — + the failure this package exists to refuse. + """ + + +#: The edge names the country block in the body. Matched case-insensitively. +GEO_FENCE_MARKERS = ("block access from your country", "not available in your region") + + def payload_url(year: int, month: int, state_id: int, district_id: int, endpoint: str) -> str: """The CDN object for one place, period and endpoint. @@ -309,10 +322,15 @@ def parse_beneficiaries(payload: dict) -> dict: def fetch(session: Any, year: int, month: int, place: Place, kind: str) -> dict | None: """One endpoint for one district-month. None when the period is absent. - A 403 here means "no object at this path", not "forbidden": the bucket + **Most 403s mean "no object at this path", not "forbidden."** The bucket denies listing, so a period that was never published and a path that is - wrong are the same response. Callers cannot distinguish them and should - not try. + wrong produce the same response. Callers cannot separate those two, and + should not try. + + **One 403 is different, and it raises.** The edge refuses a client outside + the publisher's country, and names the country block in the body. Every + object answers that way from such a client, so returning None would turn a + blocked run into a clean empty dataset. That case raises `GeoFenced`. `respect_robots=False` is deliberate and narrow. The CDN host publishes NO robots.txt — the request returns S3 AccessDenied, and RobotFileParser turns @@ -325,7 +343,16 @@ def fetch(session: Any, year: int, month: int, place: Place, kind: str) -> dict """ url = payload_url(year, month, place.state_id, place.district_id, ENDPOINTS[kind]) resp = session.get(url, respect_robots=False) - if resp.status_code in (403, 404): + if resp.status_code == 403: + body = (getattr(resp, "text", "") or "").lower() + if any(m in body for m in GEO_FENCE_MARKERS): + raise GeoFenced( + f"the edge refused this client's country for {url}. " + "Every object will 403 from here, so an empty harvest would be " + "a vantage-point artefact. Fetch from within the publisher's " + "country.") + return None + if resp.status_code == 404: return None resp.raise_for_status() return resp.json() diff --git a/commoner_probe/geoserver.py b/commoner_probe/geoserver.py index 1363ebc..46596e0 100644 --- a/commoner_probe/geoserver.py +++ b/commoner_probe/geoserver.py @@ -118,7 +118,7 @@ def wfs_status(base: str, *, session: Any = None, timeout: int = 60) -> dict[str module: it returns real geometry for every feature type, including the lines and polygons that WMS extraction cannot honestly recover. """ - sess = session or make_session() + sess = session if session is not None else make_session() out: dict[str, Any] = {"enabled": False, "versions": {}, "message": None} for version in ("2.0.0", "1.1.0", "1.0.0"): url = f"{base.rstrip('/')}/wfs?" + urlencode( @@ -178,6 +178,47 @@ def offset(self, fraction: float = 0.5) -> "Tile": self.east + dx, self.north + dy, self.depth) +def _point_in(feature: dict, bbox: Sequence[float]) -> bool | None: + """Whether a feature's point lies in `bbox`. None when it cannot be judged. + + The verification grid starts half a cell BEFORE the region, so it queries + ground outside it. A feature there was never this extraction's to find, and + counting it as a first-pass miss makes a complete sweep report + `saturated=False`. + + A feature with no point geometry returns None. It is kept and counted, + because dropping it would hide a real miss and keeping it silently would + hide the doubt. + """ + geom = (feature or {}).get("geometry") or {} + coords = geom.get("coordinates") + kind = geom.get("type") + # A point layer serves MultiPoint as readily as Point, and treating that as + # unplaceable let an out-of-region feature count as a first-pass miss. Any + # other geometry stays unplaceable: a polygon has no single answer here, and + # guessing one would be the invented geometry this module refuses. + if kind == "Point": + points = [coords] + elif kind == "MultiPoint" and isinstance(coords, (list, tuple)): + points = list(coords) + else: + return None + + placed = False + for point in points: + if not isinstance(point, (list, tuple)) or len(point) < 2: + continue + try: + lon, lat = float(point[0]), float(point[1]) + except (TypeError, ValueError): + continue + placed = True + # One point inside makes the feature this region's. + if bbox[0] <= lon <= bbox[2] and bbox[1] <= lat <= bbox[3]: + return True + return False if placed else None + + @dataclass class GeoServer: """A WMS-only GeoServer, swept for point features. @@ -195,7 +236,7 @@ class GeoServer: _stats: dict[str, int] = field(default_factory=lambda: {"requests": 0, "capped": 0}) def __post_init__(self) -> None: - self.session = self.session or make_session() + self.session = self.session if self.session is not None else make_session() self.base = self.base.rstrip("/") # ---------------------------------------------------------------- discovery @@ -248,7 +289,8 @@ def features_at(self, layer: str, tile: Tile, *, timeout: int = 120) -> list[dic def sweep(self, layer: str, bbox: Sequence[float], *, start_span: float = 2.0, min_span: float = 1 / 32, key: str | None = None, on_batch: Callable[[dict[str, dict]], None] | None = None, - tolerate_tile_errors: bool = True) -> dict[str, dict]: + tolerate_tile_errors: bool = True, + status: dict | None = None) -> dict[str, dict]: """Recursively subdivide ``bbox`` until no tile is capped, collecting points. ``key`` names the attribute that identifies a feature (a school code, an @@ -276,6 +318,7 @@ def sweep(self, layer: str, bbox: Sequence[float], *, start_span: float = 2.0, found: dict[str, dict] = {} failures: list[tuple[Tile, str]] = [] + capped: list[Tile] = [] while queue: tile = queue.pop() try: @@ -288,19 +331,39 @@ def sweep(self, layer: str, bbox: Sequence[float], *, start_span: float = 2.0, if len(feats) >= self.feature_count and tile.span > min_span: queue.extend(tile.quarter()) continue + if len(feats) >= self.feature_count: + # The tile is at `min_span` and still capped, so the walk cannot + # subdivide further. A cap means "there may be more", so this + # leaf is INCOMPLETE. Ingesting it as a complete result truncates + # the densest clusters, which are the ones a reader most wants. + capped.append(tile) for f in feats: props = f.get("properties", {}) or {} ident = str(props.get(key)) if key else str(f.get("id")) if ident and ident not in ("None", ""): - found[ident] = props + # Keep the WHOLE feature. Storing only `properties` dropped + # the coordinates, and a point extractor that returns no + # points answers a different question than the one asked. + found[ident] = f if on_batch and len(found) % 500 < len(feats): on_batch(found) if failures: self.log(f" {layer}: {len(failures)} tile(s) failed and were SKIPPED — " f"this sweep is PARTIAL, not complete") - for t, msg in failures[:5]: - self.log(f" {t.west},{t.south},{t.east},{t.north}: {msg}") + for tl, msg in failures[:5]: + self.log(f" {tl.west},{tl.south},{tl.east},{tl.north}: {msg}") + if capped: + self.log(f" {layer}: {len(capped)} tile(s) hit the feature cap at the " + f"minimum span — this sweep is PARTIAL, and those tiles are " + f"a LOWER BOUND") + if status is not None: + # The caller cannot see a log line. `verify` must not certify a + # sweep it cannot see the holes in. + box = lambda t: (t.west, t.south, t.east, t.north) # noqa: E731 + status.update(failed=[box(t) for t, _ in failures], + capped=[box(t) for t in capped], + partial=bool(failures or capped)) return found def verify(self, layer: str, bbox: Sequence[float], known: Iterable[str], *, @@ -314,16 +377,56 @@ def verify(self, layer: str, bbox: Sequence[float], known: Iterable[str], *, """ known = set(known) west, south, east, north = bbox - shifted = Tile(west, south, east, north).offset(0.5) - got = self.sweep(layer, (shifted.west, shifted.south, shifted.east, shifted.north), - start_span=start_span, key=key) + # Shift by half a CELL, not half the region: a 4-degree box with + # 2-degree cells must move 1 degree. Moving half the region queries + # ground outside it and leaves the leading edge untested. + # + # And never by more than half the region itself. A box smaller than one + # cell would otherwise move clean off its own ground: (76,12,77,13) with + # a 2-degree cell became (77,13,78,14), where finding nothing new + # certifies saturation over a region this layer never claimed. Bounded + # per axis, because a box can be wide and short. + step_x = min(start_span, east - west) / 2 + step_y = min(start_span, north - south) / 2 + # Shift the grid ORIGIN backwards, not the whole box forwards. Moving + # the box east and north offsets the cell boundaries, but it also + # leaves the western and southern strips of the region with no second + # pass at all — and saturation is claimed for the WHOLE region. Starting + # half a cell before the region keeps the offset and covers every part + # of it. + shifted = (west - step_x, south - step_y, east, north) + status: dict = {} + swept = self.sweep(layer, shifted, start_span=start_span, key=key, status=status) + got, unlocatable = {}, 0 + for ident, feature in swept.items(): + verdict = _point_in(feature, bbox) + if verdict is False: + continue + if verdict is None: + unlocatable += 1 + got[ident] = feature new = set(got) - known + partial = bool(status.get("partial")) + if partial: + self.log(f" {layer}: the verification pass is itself PARTIAL — " + f"{len(status.get('failed', []))} failed and " + f"{len(status.get('capped', []))} capped tile(s). " + "Saturation is NOT claimed.") return { "pass1": len(known), "pass2": len(got), "new": len(new), "recall": (len(known & set(got)) / len(known)) if known else 0.0, - "saturated": not new, + # An empty `new` proves saturation only when the second pass + # actually asked every question. A pass with holes produces the + # same empty set for the opposite reason. + "saturated": not new and not partial, + "partial": partial, + # Features the second pass could not place. They are counted as + # in-region, so this number is the doubt in `new`. + "unlocatable": unlocatable, + "failed_tiles": status.get("failed", []), + "capped_tiles": status.get("capped", []), "new_ids": sorted(new)[:50], } diff --git a/commoner_probe/http_client.py b/commoner_probe/http_client.py index 8b58854..f3fe9b6 100644 --- a/commoner_probe/http_client.py +++ b/commoner_probe/http_client.py @@ -354,6 +354,18 @@ def _request( url = url + sep + urlencode(params) timeout = kwargs.get("timeout") or 60 body = kwargs.get("data") + if body is None and kwargs.get("json") is not None: + # The requests path reads `json=`; this one read only `data=` and + # dropped it in silence, so a default install posted an EMPTY body + # and the server answered as if the caller had sent nothing. Encode + # it here rather than in each caller: every adapter shares this + # client, and the next one would repeat the bug. + body = json.dumps(kwargs["json"]).encode("utf-8") + # HTTP header names are case-insensitive. `setdefault` is not, so a + # caller who sent `content-type` in another case got a SECOND, + # conflicting header instead of keeping their own. + if not any(k.lower() == "content-type" for k in headers): + headers["Content-Type"] = "application/json" if isinstance(body, str): body = body.encode("utf-8") req = urllib.request.Request(url, data=body, headers=headers, method=method) diff --git a/commoner_probe/members.py b/commoner_probe/members.py index 307180d..46615f8 100644 --- a/commoner_probe/members.py +++ b/commoner_probe/members.py @@ -24,7 +24,7 @@ class MPRoster: """Fetches and matches members from Sansad rosters (LS/RS).""" def __init__(self, session: StdlibSession | None = None): - self.session = session or make_session() + self.session = session if session is not None else make_session() self._roster: dict[str, MemberInfo] = {} self._normalized_map: dict[str, str] = {} diff --git a/commoner_probe/nada.py b/commoner_probe/nada.py index c7f34e7..d8c1c6b 100644 --- a/commoner_probe/nada.py +++ b/commoner_probe/nada.py @@ -106,7 +106,7 @@ def __init__( self.base_url = base_url.rstrip("/") self.api = f"{self.base_url}/index.php/api/catalog" self.pages = f"{self.base_url}/index.php/catalog" - self.session = session or make_session() + self.session = session if session is not None else make_session() self.sleep = sleep def _get_json(self, url: str, params: dict | None = None) -> dict: diff --git a/commoner_probe/ogd_resource_api.py b/commoner_probe/ogd_resource_api.py index 2c825cc..4366825 100644 --- a/commoner_probe/ogd_resource_api.py +++ b/commoner_probe/ogd_resource_api.py @@ -268,7 +268,7 @@ def __init__( ) -> None: self.out_dir = Path(out_dir) self.sleep = sleep - self.session = session or make_session(rate_limit_sec=sleep) + self.session = session if session is not None else make_session(rate_limit_sec=sleep) self.api_key = resolve_api_key(api_key) self.manifest = self.out_dir / "manifest.jsonl" self.rows_dir = self.out_dir / "rows" diff --git a/commoner_probe/rest_dataset_api.py b/commoner_probe/rest_dataset_api.py index 40a3802..f554bfc 100644 --- a/commoner_probe/rest_dataset_api.py +++ b/commoner_probe/rest_dataset_api.py @@ -118,7 +118,7 @@ class MospiClient: """Thin typed wrapper over the eSankhyiki REST routes.""" def __init__(self, *, sleep: float = 0.5, session=None) -> None: - self.session = session or make_session() + self.session = session if session is not None else make_session() self.sleep = sleep def _get_json(self, path: str, params: dict[str, Any]) -> dict: diff --git a/commoner_probe/spa_jwt_api.py b/commoner_probe/spa_jwt_api.py index de7cc0b..3ce606d 100644 --- a/commoner_probe/spa_jwt_api.py +++ b/commoner_probe/spa_jwt_api.py @@ -218,7 +218,7 @@ def request_otp(mobile: str, *, base: str = DSP_BASE, session: Any = None, if solve is None: raise ValueError("request_otp needs solve= str>; " "a human must read the captcha") - sess = session or make_session() + sess = session if session is not None else make_session() key, png = _captcha(base, sess, timeout) value = solve(png) r = sess.post(base + DSP_ENDPOINTS["send_otp"], @@ -238,7 +238,7 @@ def verify_otp(mobile: str, otp: str, *, base: str = DSP_BASE, ``mobile_invalid_strict`` rather than "expired", and the real OTP is burnt by the time the quoting is fixed. """ - sess = session or make_session() + sess = session if session is not None else make_session() r = sess.post(base + DSP_ENDPOINTS["verify_otp"], json={"mobile": mobile, "otp": otp}, timeout=timeout) return r.json() if r.content else {} @@ -247,7 +247,7 @@ def verify_otp(mobile: str, otp: str, *, base: str = DSP_BASE, def probe_public(base: str = DSP_BASE, *, session: Any = None, timeout: int = 45) -> dict[str, Any]: """Report which endpoints answer without credentials. Reconnaissance only.""" - sess = session or make_session() + sess = session if session is not None else make_session() out: dict[str, Any] = {"base": base, "results": {}, "note": None} for path in ("/csv-download/years", "/csv-download", "/api/public/captcha"): try: diff --git a/commoner_probe/wayback.py b/commoner_probe/wayback.py index 65509ed..a6ee298 100644 --- a/commoner_probe/wayback.py +++ b/commoner_probe/wayback.py @@ -87,7 +87,7 @@ def _latest_capture(url: str, *, session: Any, timeout: float) -> tuple[dict | N load — observed live 2026-07-26 on back-to-back CDX calls). A re-check that treats a 503 as "nothing to compare" silently stops detecting change. """ - session = session or make_session() + session = session if session is not None else make_session() params = {"url": url, "output": "json", "limit": -1, "fl": _CDX_FIELDS} try: r = session.get(CDX_API, params=params, timeout=timeout) @@ -129,7 +129,7 @@ def request_save(url: str, *, session: Any = None, timeout: float = SAVE_TIMEOUT SPN2 queues work and anonymous callers are throttled. Confirm with latest_capture() rather than trusting this return value. """ - session = session or make_session() + session = session if session is not None else make_session() try: r = session.get(f"{SAVE_BASE}{quote(url, safe=':/?&=#%')}", timeout=timeout) r.raise_for_status() @@ -165,7 +165,7 @@ def snapshot_fields( Merge the result into the record; never gate acquisition on it. """ - session = session or make_session() + session = session if session is not None else make_session() before, _ = _latest_capture(url, session=session, timeout=timeout) if save else (None, "") saved = request_save(url, session=session) if save else True capture, failure = _latest_capture(url, session=session, timeout=timeout) @@ -390,7 +390,7 @@ def iter_captures( caller that read a 503 as "never archived" would record an outage as a fact about the source, which is the failure ``recheck()`` exists to prevent. """ - session = session or make_session() + session = session if session is not None else make_session() resume: str | None = None emitted = 0 seen_batches = 0 @@ -491,7 +491,7 @@ def __init__( self.out_dir = Path(out_dir) self.sleep = sleep self.manifest = self.out_dir / "manifest.jsonl" - self.session = session or make_session(rate_limit_sec=sleep) + self.session = session if session is not None else make_session(rate_limit_sec=sleep) def load_seen(self) -> set: seen: set = set() diff --git a/pyproject.toml b/pyproject.toml index 0634d1a..88ae64c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -50,9 +50,12 @@ budget = ["lxml>=5.0"] # Sikkim alone publishes Statement V as OLE2 .xls; the other 34 states ship xlsx # that the stdlib reader handles, so this stays optional. xls = ["xlrd>=2.0.1"] +# AES-CBC envelope decryption for dashboards that wrap their reference data. +crypto = ["cryptography>=42"] academia = ["beautifulsoup4>=4.12", "pdfminer.six>=20231228"] -dev = ["jsonschema>=4.20", "pytest>=7", "ruff>=0.4", "lxml>=5.0", "beautifulsoup4>=4.12", "xlrd>=2.0.1"] +dev = ["jsonschema>=4.20", "pytest>=7", "ruff>=0.4", "lxml>=5.0", "beautifulsoup4>=4.12", "xlrd>=2.0.1", "cryptography>=42"] all = [ + "cryptography>=42", "pdfminer.six>=20231228", "requests>=2.31.0", "lxml>=5.0", diff --git a/tests/test_geoserver.py b/tests/test_geoserver.py index 3a21de7..6081225 100644 --- a/tests/test_geoserver.py +++ b/tests/test_geoserver.py @@ -138,3 +138,167 @@ def test_verify_reports_saturation_only_when_nothing_new_appears(): out2 = gs2.verify("ws:layer", (76.0, 12.0, 77.0, 13.0), known={"1000"}, key="code") assert out2["saturated"] is False and out2["new"] == 3 + + +# --- findings from review, 2026-08-17 ------------------------------------------ + + +def _fc_geo(n, start=0): + """A feature collection in the shape the standard actually specifies: + coordinates live in `geometry`, not in `properties`.""" + return json.dumps({"features": [ + {"id": f"f.{i}", "properties": {"code": str(1000 + i)}, + "geometry": {"type": "Point", "coordinates": [76.0 + i, 12.0 + i]}} + for i in range(start, start + n)]}) + + +def test_the_point_location_survives_the_sweep(): + """This module exists to extract POINTS. Returning attributes without + coordinates answers a different question than the one asked.""" + sess = _Session([_fc_geo(2)]) + gs = GeoServer("http://x/geoserver", session=sess, feature_count=400) + got = gs.sweep("ws:layer", (76.0, 12.0, 77.0, 13.0), start_span=2.0, key="code") + assert got["1000"]["geometry"]["coordinates"] == [76.0, 12.0] + assert got["1000"]["properties"]["code"] == "1000" + + +def test_a_tile_capped_at_the_floor_is_reported_not_ingested(): + """A cap means 'there may be more'. At `min_span` the walk can subdivide no + further, so the honest move is to record the leaf as incomplete.""" + sess = _Session([_fc(2)]) + gs = GeoServer("http://x/geoserver", session=sess, feature_count=2) + status: dict = {} + gs.sweep("ws:layer", (76.0, 12.0, 76.5, 12.5), start_span=0.5, + min_span=0.5, key="code", status=status) + assert status["capped"], "a capped leaf must be reported" + assert status["partial"] is True + + +def test_the_offset_grid_shifts_by_one_cell_not_by_the_whole_region(): + """A 4-degree box with 2-degree cells must move 1 degree, not 2. Moving by + half the region tests ground outside it and leaves the leading edge + unexamined, then calls the result saturated.""" + import re + from urllib.parse import unquote + + sess = _Session([]) + gs = GeoServer("http://x/geoserver", session=sess) + gs.verify("ws:layer", (76.0, 12.0, 80.0, 16.0), known=[], start_span=2.0, key="code") + wests = sorted(float(unquote(re.search(r"bbox=([^&]+)", u, re.I).group(1)).split(",")[0]) + for u in sess.urls) + # Half a cell (1.0), not half the region (2.0), and BACKWARDS so the + # western strip of the region still gets a second pass. + assert wests[0] == 75.0, f"expected a half-cell shift to 75.0, got {wests[0]}" + assert 76.0 not in wests, "an unshifted grid re-asks the first pass's questions" + + +def test_saturation_is_refused_when_a_verification_tile_failed(): + """With every offset tile failing, `new` is empty for the wrong reason. + Reporting saturation there certifies completeness from no evidence.""" + class _Failing: + urls: list = [] + + def get(self, url, timeout=None): + raise RuntimeError("boom") + + gs = GeoServer("http://x/geoserver", session=_Failing(), feature_count=400) + out = gs.verify("ws:layer", (76.0, 12.0, 80.0, 16.0), known=["1000"], + start_span=2.0, key="code") + assert out["saturated"] is False + assert out["partial"] is True + + +def test_the_offset_grid_still_overlaps_a_box_smaller_than_one_cell(): + """The one-cell shift must not walk off the region it is verifying. + + A 1-degree box with 2-degree cells would move a full degree and land + entirely outside itself. The pass then finds nothing, finds nothing NEW, + and certifies saturation from ground the layer was never claimed to cover. + """ + import re + from urllib.parse import unquote + + sess = _Session([]) + gs = GeoServer("http://x/geoserver", session=sess) + gs.verify("ws:layer", (76.0, 12.0, 77.0, 13.0), known=[], start_span=2.0, key="code") + boxes = [[float(x) for x in unquote(re.search(r"bbox=([^&]+)", u, re.I).group(1)).split(",")] + for u in sess.urls] + assert boxes, "the verification pass made no request" + overlaps = [b for b in boxes if b[0] < 77.0 and b[2] > 76.0 and b[1] < 13.0 and b[3] > 12.0] + assert overlaps, f"no verification tile overlaps the original box: {boxes}" + + +def test_every_part_of_the_box_gets_a_verification_tile(): + """Saturation is claimed for the WHOLE region, so the second pass must + cover the whole region. Shifting east and north left the western and + southern strips untested, and a first-pass miss there was never examined.""" + import re + from urllib.parse import unquote + + sess = _Session([]) + gs = GeoServer("http://x/geoserver", session=sess) + gs.verify("ws:layer", (76.0, 12.0, 80.0, 16.0), known=[], start_span=2.0, key="code") + boxes = [[float(x) for x in unquote(re.search(r"bbox=([^&]+)", u, re.I).group(1)).split(",")] + for u in sess.urls] + + def covered(lon, lat): + return any(b[0] <= lon <= b[2] and b[1] <= lat <= b[3] for b in boxes) + + for lon, lat in ((76.01, 12.01), (76.01, 15.99), (79.99, 12.01), (79.99, 15.99), + (78.0, 14.0)): + assert covered(lon, lat), f"({lon},{lat}) is in the box and no tile covers it" + + +def test_a_feature_outside_the_region_is_not_a_first_pass_miss(): + """The backward shift queries ground west and south of the region. A + feature there is not something the first pass missed, and counting it as + one makes a complete extraction report `saturated=False`.""" + outside = json.dumps({"features": [ + {"id": "f.out", "properties": {"code": "OUT"}, + "geometry": {"type": "Point", "coordinates": [75.5, 11.5]}}]}) + inside = json.dumps({"features": [ + {"id": "f.in", "properties": {"code": "IN"}, + "geometry": {"type": "Point", "coordinates": [78.0, 14.0]}}]}) + sess = _Session([outside] + [inside] * 8) + gs = GeoServer("http://x/geoserver", session=sess, feature_count=400) + out = gs.verify("ws:layer", (76.0, 12.0, 80.0, 16.0), known=["IN"], + start_span=2.0, key="code") + assert "OUT" not in out["new_ids"], "an out-of-region feature is not a miss" + assert out["saturated"] is True + + +def test_a_feature_with_no_geometry_is_kept_and_counted_as_unlocatable(): + """Some servers omit geometry. Dropping those would hide real misses, and + keeping them silently would hide the doubt. Keep, and report the count.""" + nogeo = json.dumps({"features": [ + {"id": "f.x", "properties": {"code": "X"}}]}) + sess = _Session([nogeo] * 9) + gs = GeoServer("http://x/geoserver", session=sess, feature_count=400) + out = gs.verify("ws:layer", (76.0, 12.0, 80.0, 16.0), known=[], + start_span=2.0, key="code") + assert out["unlocatable"] >= 1 + assert "X" in out["new_ids"] + + +def test_a_multipoint_feature_is_placed_like_a_point(): + """A point layer can serve MultiPoint. Treating it as unplaceable let an + out-of-region feature count as a first-pass miss.""" + from commoner_probe.geoserver import _point_in + + bbox = (76.0, 12.0, 80.0, 16.0) + inside = {"geometry": {"type": "MultiPoint", "coordinates": [[78.0, 14.0]]}} + outside = {"geometry": {"type": "MultiPoint", "coordinates": [[75.5, 11.5]]}} + straddling = {"geometry": {"type": "MultiPoint", + "coordinates": [[75.5, 11.5], [78.0, 14.0]]}} + assert _point_in(inside, bbox) is True + assert _point_in(outside, bbox) is False + assert _point_in(straddling, bbox) is True, "one point inside makes it ours" + + +def test_an_unknown_geometry_is_still_unplaceable(): + """A polygon cannot be hit-tested into this answer. It stays unlocatable, + which is counted and reported rather than guessed.""" + from commoner_probe.geoserver import _point_in + + poly = {"geometry": {"type": "Polygon", "coordinates": [[[76.0, 12.0]]]}} + assert _point_in(poly, (76.0, 12.0, 80.0, 16.0)) is None diff --git a/tests/test_review_findings_2026_08_17.py b/tests/test_review_findings_2026_08_17.py new file mode 100644 index 0000000..a57343c --- /dev/null +++ b/tests/test_review_findings_2026_08_17.py @@ -0,0 +1,262 @@ +"""Regression tests for the review findings of 2026-08-17.""" +from __future__ import annotations + +import json + +import pytest + +from commoner_probe import aspnet, aspnet_cascade, cdn_dashboard, spa_jwt_api +from commoner_probe.http_client import StdlibSession + + +class TestTheStdlibClientSendsJsonBodies: + """`json=` was read by the requests path and ignored by the stdlib one, so + a default install posted an empty body and the OTP flow could not work.""" + + def test_a_json_body_reaches_the_wire(self, monkeypatch): + monkeypatch.setattr("commoner_probe.http_client.is_safe_url", lambda u: True) + sent = {} + + def fake_open(self, req, timeout=None): + sent["body"] = req.data + sent["type"] = req.headers.get("Content-type") + raise SystemExit # stop before the network + + monkeypatch.setattr("urllib.request.OpenerDirector.open", fake_open) + s = StdlibSession() + with pytest.raises(SystemExit): + s.post("https://example.gov.in/api", json={"mobile": "9"}, + respect_robots=False) + assert json.loads(sent["body"]) == {"mobile": "9"} + assert sent["type"] == "application/json" + + def test_an_explicit_data_body_still_wins(self, monkeypatch): + monkeypatch.setattr("commoner_probe.http_client.is_safe_url", lambda u: True) + sent = {} + + def fake_open(self, req, timeout=None): + sent["body"] = req.data + raise SystemExit + + monkeypatch.setattr("urllib.request.OpenerDirector.open", fake_open) + s = StdlibSession() + with pytest.raises(SystemExit): + s.post("https://example.gov.in/api", data="a=1", respect_robots=False) + assert sent["body"] == b"a=1" + + +class TestWriteButtonDetection: + """A missed write button means a crawler posts a form that inserts a record + into a live government system.""" + + def test_an_entity_encoded_label_is_decoded_before_matching(self): + page = ('') + assert aspnet.write_buttons(page), "the Hindi label decodes to a save button" + + def test_a_plain_read_button_is_still_harmless(self): + page = '' + assert aspnet.write_buttons(page) == [] + + +class TestTheGeoFenceIsNotAnEmptyPeriod: + """Outside the publisher's country every object 403s. Reading that as + 'unpublished' turns a blocked run into a clean empty dataset.""" + + class _Resp: + def __init__(self, status, text=""): + self.status_code, self.text = status, text + + def json(self): + return {} + + def raise_for_status(self): + pass + + class _Session: + def __init__(self, resp): + self.resp = resp + + def get(self, url, **kw): + return self.resp + + def test_a_geo_fence_403_raises(self): + body = "configured to block access from your country" + sess = self._Session(self._Resp(403, body)) + place = cdn_dashboard.Place(17, "S", 112, "D", "24", "24445") + with pytest.raises(cdn_dashboard.GeoFenced): + cdn_dashboard.fetch(sess, 2026, 6, place, "growth") + + def test_an_ordinary_403_is_still_an_absent_period(self): + sess = self._Session(self._Resp(403, "AccessDenied")) + place = cdn_dashboard.Place(17, "S", 112, "D", "24", "24445") + assert cdn_dashboard.fetch(sess, 2026, 6, place, "growth") is None + + +class TestTheCascadeRecovery: + def test_reseating_builds_a_new_session(self): + """The stale state lives in the session cookie. Refetching with the same + session returns the same 500.""" + built = [] + + class _Sess: + def __init__(self): + built.append(self) + + def get(self, url, **kw): + class R: + content = b'' + return R() + + def post(self, url, **kw): + raise RuntimeError("HTTP 500") + + crawler = aspnet_cascade.CascadeCrawler( + "https://x.gov.in/r.aspx", {"a": "ctl00$a"}, + session_factory=lambda: _Sess()) + first = crawler.session + crawler.reset() + assert crawler.session is not first, "a poisoned session must be replaced" + + def test_an_injected_session_is_never_replaced(self): + """A test double, or a caller's authenticated session, must survive.""" + class _Sess: + def get(self, url, **kw): + class R: + content = b"" + return R() + + mine = _Sess() + crawler = aspnet_cascade.CascadeCrawler("https://x.gov.in/r.aspx", + {"a": "ctl00$a"}, session=mine) + crawler.reset() + assert crawler.session is mine + + +def test_the_cryptography_extra_is_declared(): + """`decrypt_envelope` imports cryptography. A clean install must be able to + get it from a named extra rather than by guessing.""" + import pathlib + import re + + # No tomllib. It arrived in 3.11, and CI runs this suite on 3.10 too. + root = pathlib.Path(spa_jwt_api.__file__).resolve().parent.parent + text = (root / "pyproject.toml").read_text(encoding="utf-8") + for extra in ("crypto", "all"): + block = re.search(rf"^{extra} = (\[.*?\])", text, re.S | re.M) + assert block, f"no {extra} extra is declared" + assert "cryptography" in block.group(1), f"{extra} does not carry cryptography" + + +def test_a_lower_cased_content_type_is_not_duplicated(monkeypatch): + """Header names are case-insensitive. A caller's own `content-type` must + survive, rather than gain a second header beside it.""" + sent = {} + + def fake_open(self, req, timeout=None): + sent["headers"] = dict(req.headers) + raise SystemExit + + monkeypatch.setattr("commoner_probe.http_client.is_safe_url", lambda u: True) + monkeypatch.setattr("urllib.request.OpenerDirector.open", fake_open) + s = StdlibSession() + with pytest.raises(SystemExit): + s.post("https://example.gov.in/api", json={"a": 1}, + headers={"content-type": "application/vnd.api+json"}, + respect_robots=False) + types = [v for k, v in sent["headers"].items() if k.lower() == "content-type"] + assert types == ["application/vnd.api+json"], types + + +class TestTheSessionContract: + """Three combinations, three documented behaviours. The middle one was + contradicted by the docstring until review caught it.""" + + class _Sess: + def __init__(self, tag="x"): + self.tag = tag + + def get(self, url, **kw): + class R: + content = b"" + return R() + + def _crawler(self, **kw): + return aspnet_cascade.CascadeCrawler("https://x.gov.in/r.aspx", + {"a": "ctl00$a"}, **kw) + + def test_a_session_alone_is_never_replaced(self): + mine = self._Sess("mine") + c = self._crawler(session=mine) + c.reset() + assert c.session is mine + + def test_a_factory_beside_a_session_wins_on_reseat(self): + """Passing a factory IS the instruction to rebuild. A caller whose + session carries a login supplies a factory that re-establishes it.""" + built = [] + + def factory(): + s = TestTheSessionContract._Sess("rebuilt") + built.append(s) + return s + + mine = self._Sess("mine") + c = self._crawler(session=mine, session_factory=factory) + assert c.session is mine, "the injected session is used until a reseat" + c.reset() + assert c.session is built[-1] + + def test_the_default_client_is_rebuilt(self): + c = self._crawler(session_factory=lambda: TestTheSessionContract._Sess()) + first = c.session + c.reset() + assert c.session is not first + + +def test_a_falsey_injected_session_is_still_used(): + """`session or factory()` discarded any session object whose truthiness is + false. A session with `__len__` is ordinary; silently replacing it hands + the caller a client they did not build.""" + class _Falsey: + def __len__(self): + return 0 + + def get(self, url, **kw): + class R: + content = b"" + return R() + + mine = _Falsey() + c = aspnet_cascade.CascadeCrawler("https://x.gov.in/r.aspx", + {"a": "ctl00$a"}, session=mine) + assert c.session is mine + + +def test_a_falsey_session_factory_is_still_used(): + """A factory is often a callable object, not a lambda, and an object with + `__len__` can be falsey. `or` dropped it and built the default client.""" + class _Sess: + def get(self, url, **kw): + class R: + content = b"" + return R() + + class _Factory: + def __init__(self): + self.built = [] + + def __len__(self): + return 0 # falsey, and still a perfectly good factory + + def __call__(self): + s = _Sess() + self.built.append(s) + return s + + factory = _Factory() + c = aspnet_cascade.CascadeCrawler("https://x.gov.in/r.aspx", + {"a": "ctl00$a"}, session_factory=factory) + c.reset() + assert len(factory.built) == 2, "the caller's factory built both sessions" + assert c.session is factory.built[-1]