mirror of
https://github.com/zvx-echo6/central.git
synced 2026-06-10 11:54:37 +02:00
feat(gui-bugs): fix eonet dashboard exception + out-of-range map bbox
Kickoff of the v0.7.x GUI rework arc. Two operator-facing bugs confirmed live; production code, central-gui + central-supervisor restart required. Bug 1 (eonet exception leaking to /dashboard): The supervisor calls adapter.bump_last_seen on every dedup hit, but only 4 of 12 adapters defined it and the base class did not. Adapters that re-emit already-published events (eonet re-lists open natural events each poll) raised AttributeError; the supervisor published it as the adapter's status error, which /dashboard rendered as literal text in the Last Poll cell. Fix: add bump_last_seen to the SourceAdapter base class (guarded on getattr(self, "_db", None)); remove the 4 now-redundant identical overrides. Fixes all 8 affected adapters, not just eonet. Documents the method in PRODUCER-INTEGRATION.md 4.3 (producer-doc API guard). Bug 2 (map bbox out of valid range): applyViewportFilter serialized raw Leaflet getEast()/getWest(), which exceed [-180,180] when panned past the dateline at low zoom (e.g. region_east=411.3281, region_west=-608.2031), and _parse_events_params passed them straight to ST_MakeEnvelope. Fix (JS): normalize longitudes into [-180,180]; when the visible span exceeds ~350 deg, omit the bbox entirely. Fix (backend, defense in depth): _parse_events_params treats an out-of-range or inverted envelope as "no bbox" rather than erroring or querying a bogus envelope. Bugs 3 (FIRMS "duplicates") and 4 (missing expand buttons) from the planning walkthrough were investigated and refuted (FIRMS rows are distinct fire pixels, not satellite dupes -- dropping satellite collapses 0 rows; the expand button is present and functional on main), so they are not part of this PR. Tests: registry-derived guard that every adapter resolves bump_last_seen + base-method behavior test; 3 bbox-guard unit tests on _parse_events_params. Full suite: 634 passed, 1 skipped (central and unprivileged zvx). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
76c5e94b39
commit
47e7b4f267
10 changed files with 157 additions and 44 deletions
|
|
@ -262,6 +262,13 @@ required tables. The default is a no-op.
|
||||||
shutdown. Close the HTTP session, close the sqlite3 cursor DB. The default is
|
shutdown. Close the HTTP session, close the sqlite3 cursor DB. The default is
|
||||||
a no-op.
|
a no-op.
|
||||||
|
|
||||||
|
**`def bump_last_seen(self, event_id: str) -> None`** — Optional. Called by the
|
||||||
|
supervisor on every dedup hit (an already-published event re-seen upstream) so
|
||||||
|
the periodic `sweep_old_ids` purge doesn't expire long-lived events. The base
|
||||||
|
class implements the standard `published_ids` bump; adapters using that table
|
||||||
|
inherit it and need not override. It is a safe no-op when the adapter has no
|
||||||
|
`_db` handle.
|
||||||
|
|
||||||
**`async def preview_for_settings(self, settings: BaseModel) -> list[dict] | None`** —
|
**`async def preview_for_settings(self, settings: BaseModel) -> list[dict] | None`** —
|
||||||
Optional. The settings-page preview hook. The default returns `None` (no
|
Optional. The settings-page preview hook. The default returns `None` (no
|
||||||
preview). See [§11](#11-settings-preview-hook) for the contract.
|
preview). See [§11](#11-settings-preview-hook) for the contract.
|
||||||
|
|
|
||||||
|
|
@ -86,6 +86,27 @@ class SourceAdapter(ABC):
|
||||||
"""Optional lifecycle hook called on graceful shutdown."""
|
"""Optional lifecycle hook called on graceful shutdown."""
|
||||||
pass
|
pass
|
||||||
|
|
||||||
|
def bump_last_seen(self, event_id: str) -> None:
|
||||||
|
"""Refresh the dedup ``last_seen`` for an already-published event.
|
||||||
|
|
||||||
|
The supervisor calls this on every dedup hit so the periodic
|
||||||
|
``sweep_old_ids`` purge doesn't expire long-lived events that upstream
|
||||||
|
keeps re-listing (e.g. EONET open events, NWS active alerts). Adapters
|
||||||
|
that maintain the standard ``published_ids`` table inherit this; it is a
|
||||||
|
safe no-op for any adapter without a ``_db`` handle. Previously only
|
||||||
|
four adapters defined it, so adapters that re-emit (eonet, etc.) raised
|
||||||
|
``AttributeError`` here and the supervisor surfaced it on /dashboard.
|
||||||
|
"""
|
||||||
|
db = getattr(self, "_db", None)
|
||||||
|
if db is None:
|
||||||
|
return
|
||||||
|
db.execute(
|
||||||
|
"UPDATE published_ids SET last_seen = CURRENT_TIMESTAMP "
|
||||||
|
"WHERE adapter = ? AND event_id = ?",
|
||||||
|
(self.name, event_id),
|
||||||
|
)
|
||||||
|
db.commit()
|
||||||
|
|
||||||
async def preview_for_settings(self, settings: BaseModel) -> list[dict] | None:
|
async def preview_for_settings(self, settings: BaseModel) -> list[dict] | None:
|
||||||
"""Optional. Override to surface a settings-driven preview on the edit page.
|
"""Optional. Override to surface a settings-driven preview on the edit page.
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -271,16 +271,6 @@ class InciWebAdapter(SourceAdapter):
|
||||||
)
|
)
|
||||||
self._db.commit()
|
self._db.commit()
|
||||||
|
|
||||||
def bump_last_seen(self, event_id: str) -> None:
|
|
||||||
"""Bump the last_seen timestamp for an event."""
|
|
||||||
if not self._db:
|
|
||||||
return
|
|
||||||
self._db.execute(
|
|
||||||
"UPDATE published_ids SET last_seen = CURRENT_TIMESTAMP WHERE adapter = ? AND event_id = ?",
|
|
||||||
(self.name, event_id),
|
|
||||||
)
|
|
||||||
self._db.commit()
|
|
||||||
|
|
||||||
def sweep_old_ids(self) -> int:
|
def sweep_old_ids(self) -> int:
|
||||||
"""Remove published_ids older than 14 days. Returns count deleted."""
|
"""Remove published_ids older than 14 days. Returns count deleted."""
|
||||||
if not self._db:
|
if not self._db:
|
||||||
|
|
|
||||||
|
|
@ -418,16 +418,6 @@ class NWSAdapter(SourceAdapter):
|
||||||
)
|
)
|
||||||
self._db.commit()
|
self._db.commit()
|
||||||
|
|
||||||
def bump_last_seen(self, event_id: str) -> None:
|
|
||||||
"""Bump the last_seen timestamp for an event."""
|
|
||||||
if not self._db:
|
|
||||||
return
|
|
||||||
self._db.execute(
|
|
||||||
"UPDATE published_ids SET last_seen = CURRENT_TIMESTAMP WHERE adapter = ? AND event_id = ?",
|
|
||||||
(self.name, event_id)
|
|
||||||
)
|
|
||||||
self._db.commit()
|
|
||||||
|
|
||||||
def sweep_old_ids(self) -> int:
|
def sweep_old_ids(self) -> int:
|
||||||
"""Remove published_ids older than 8 days. Returns count deleted."""
|
"""Remove published_ids older than 8 days. Returns count deleted."""
|
||||||
if not self._db:
|
if not self._db:
|
||||||
|
|
|
||||||
|
|
@ -158,16 +158,6 @@ class WFIGSIncidentsAdapter(SourceAdapter):
|
||||||
)
|
)
|
||||||
self._db.commit()
|
self._db.commit()
|
||||||
|
|
||||||
def bump_last_seen(self, event_id: str) -> None:
|
|
||||||
"""Bump the last_seen timestamp for an event."""
|
|
||||||
if not self._db:
|
|
||||||
return
|
|
||||||
self._db.execute(
|
|
||||||
"UPDATE published_ids SET last_seen = CURRENT_TIMESTAMP WHERE adapter = ? AND event_id = ?",
|
|
||||||
(self.name, event_id),
|
|
||||||
)
|
|
||||||
self._db.commit()
|
|
||||||
|
|
||||||
def sweep_old_ids(self) -> int:
|
def sweep_old_ids(self) -> int:
|
||||||
"""Remove published_ids older than 14 days. Returns count deleted."""
|
"""Remove published_ids older than 14 days. Returns count deleted."""
|
||||||
if not self._db:
|
if not self._db:
|
||||||
|
|
|
||||||
|
|
@ -158,16 +158,6 @@ class WFIGSPerimetersAdapter(SourceAdapter):
|
||||||
)
|
)
|
||||||
self._db.commit()
|
self._db.commit()
|
||||||
|
|
||||||
def bump_last_seen(self, event_id: str) -> None:
|
|
||||||
"""Bump the last_seen timestamp for an event."""
|
|
||||||
if not self._db:
|
|
||||||
return
|
|
||||||
self._db.execute(
|
|
||||||
"UPDATE published_ids SET last_seen = CURRENT_TIMESTAMP WHERE adapter = ? AND event_id = ?",
|
|
||||||
(self.name, event_id),
|
|
||||||
)
|
|
||||||
self._db.commit()
|
|
||||||
|
|
||||||
def sweep_old_ids(self) -> int:
|
def sweep_old_ids(self) -> int:
|
||||||
"""Remove published_ids older than 14 days. Returns count deleted."""
|
"""Remove published_ids older than 14 days. Returns count deleted."""
|
||||||
if not self._db:
|
if not self._db:
|
||||||
|
|
|
||||||
|
|
@ -2700,6 +2700,17 @@ def _parse_events_params(params) -> tuple[dict | None, str | None]:
|
||||||
except ValueError:
|
except ValueError:
|
||||||
return None, "Region parameters must be valid numbers"
|
return None, "Region parameters must be valid numbers"
|
||||||
|
|
||||||
|
# Defense in depth: the map JS normalizes coordinates, but a degenerate
|
||||||
|
# or out-of-range bbox (e.g. Leaflet pan-past-dateline artifacts like
|
||||||
|
# east=411.3, west=-608.2) must never reach ST_MakeEnvelope. Treat an
|
||||||
|
# invalid envelope as "no bbox" rather than erroring or querying a bogus
|
||||||
|
# envelope that silently matches the wrong rows.
|
||||||
|
if not (
|
||||||
|
-90 <= bbox["south"] < bbox["north"] <= 90
|
||||||
|
and -180 <= bbox["west"] < bbox["east"] <= 180
|
||||||
|
):
|
||||||
|
bbox = None
|
||||||
|
|
||||||
# Parse cursor
|
# Parse cursor
|
||||||
cursor_time = None
|
cursor_time = None
|
||||||
cursor_id = None
|
cursor_id = None
|
||||||
|
|
|
||||||
|
|
@ -274,12 +274,30 @@
|
||||||
viewportDebounceTimer = setTimeout(applyViewportFilter, 400);
|
viewportDebounceTimer = setTimeout(applyViewportFilter, 400);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Normalize a longitude into [-180, 180]. Leaflet returns out-of-range
|
||||||
|
// values (e.g. east=411, west=-608) when the map is panned past the
|
||||||
|
// dateline / wrapped at low zoom; those must not reach the bbox filter.
|
||||||
|
function wrapLon(lon) {
|
||||||
|
return ((((lon + 180) % 360) + 360) % 360) - 180;
|
||||||
|
}
|
||||||
|
|
||||||
function applyViewportFilter() {
|
function applyViewportFilter() {
|
||||||
var bounds = map.getBounds();
|
var bounds = map.getBounds();
|
||||||
northInput.value = bounds.getNorth().toFixed(4);
|
var rawWest = bounds.getWest();
|
||||||
southInput.value = bounds.getSouth().toFixed(4);
|
var rawEast = bounds.getEast();
|
||||||
eastInput.value = bounds.getEast().toFixed(4);
|
// When the visible map spans more than ~350 deg of longitude (zoomed
|
||||||
westInput.value = bounds.getWest().toFixed(4);
|
// far out / wrapped), a bbox filter is meaningless — treat as no bbox.
|
||||||
|
if (rawEast - rawWest > 350) {
|
||||||
|
northInput.value = "";
|
||||||
|
southInput.value = "";
|
||||||
|
eastInput.value = "";
|
||||||
|
westInput.value = "";
|
||||||
|
} else {
|
||||||
|
northInput.value = Math.min(90, bounds.getNorth()).toFixed(4);
|
||||||
|
southInput.value = Math.max(-90, bounds.getSouth()).toFixed(4);
|
||||||
|
eastInput.value = wrapLon(rawEast).toFixed(4);
|
||||||
|
westInput.value = wrapLon(rawWest).toFixed(4);
|
||||||
|
}
|
||||||
htmx.trigger(document.getElementById("filter-form"), "submit");
|
htmx.trigger(document.getElementById("filter-form"), "submit");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -108,3 +108,52 @@ def test_nwis_event_mirrors_centroid_into_data():
|
||||||
assert event.data["longitude"] == -90.25
|
assert event.data["longitude"] == -90.25
|
||||||
# Geo.centroid retained for existing rendering uses.
|
# Geo.centroid retained for existing rendering uses.
|
||||||
assert event.geo.centroid == (-90.25, 41.78)
|
assert event.geo.centroid == (-90.25, 41.78)
|
||||||
|
|
||||||
|
|
||||||
|
# --- bump_last_seen contract (v0.7.0 Bug 1) ---------------------------------
|
||||||
|
|
||||||
|
def test_every_adapter_resolves_bump_last_seen():
|
||||||
|
"""The supervisor calls adapter.bump_last_seen on every dedup hit. Every
|
||||||
|
registered adapter must resolve a callable (inherited from SourceAdapter or
|
||||||
|
overridden) -- otherwise the AttributeError leaks to /dashboard (the eonet
|
||||||
|
bug). Registry-derived, no hardcoded list."""
|
||||||
|
missing = [
|
||||||
|
name for name, cls in discover_adapters().items()
|
||||||
|
if not callable(getattr(cls, "bump_last_seen", None))
|
||||||
|
]
|
||||||
|
assert not missing, f"adapters missing bump_last_seen: {missing}"
|
||||||
|
|
||||||
|
|
||||||
|
def test_base_bump_last_seen_updates_and_is_noop_without_db():
|
||||||
|
"""SourceAdapter.bump_last_seen updates published_ids.last_seen when a _db
|
||||||
|
handle is present, and is a safe no-op when it is not."""
|
||||||
|
import sqlite3
|
||||||
|
|
||||||
|
from central.adapter import SourceAdapter
|
||||||
|
|
||||||
|
class _StubAdapter(SourceAdapter):
|
||||||
|
name = "stub"
|
||||||
|
|
||||||
|
async def poll(self): ... # never called in this test
|
||||||
|
async def apply_config(self, new_config): ...
|
||||||
|
def subject_for(self, event): return ""
|
||||||
|
|
||||||
|
adapter = _StubAdapter()
|
||||||
|
adapter.bump_last_seen("e1") # no _db attribute -> must not raise
|
||||||
|
|
||||||
|
adapter._db = sqlite3.connect(":memory:")
|
||||||
|
adapter._db.execute(
|
||||||
|
"CREATE TABLE published_ids (adapter TEXT, event_id TEXT, "
|
||||||
|
"first_seen TIMESTAMP, last_seen TIMESTAMP)"
|
||||||
|
)
|
||||||
|
adapter._db.execute(
|
||||||
|
"INSERT INTO published_ids VALUES ('stub', 'e1', "
|
||||||
|
"'2020-01-01 00:00:00', '2020-01-01 00:00:00')"
|
||||||
|
)
|
||||||
|
adapter._db.commit()
|
||||||
|
|
||||||
|
adapter.bump_last_seen("e1")
|
||||||
|
row = adapter._db.execute(
|
||||||
|
"SELECT last_seen FROM published_ids WHERE event_id = 'e1'"
|
||||||
|
).fetchone()
|
||||||
|
assert row[0] != "2020-01-01 00:00:00" # bumped to CURRENT_TIMESTAMP
|
||||||
|
|
|
||||||
47
tests/test_events_bbox_guard.py
Normal file
47
tests/test_events_bbox_guard.py
Normal file
|
|
@ -0,0 +1,47 @@
|
||||||
|
"""Bug 2 (v0.7.0): out-of-range map bbox must not reach the SQL envelope.
|
||||||
|
|
||||||
|
Leaflet returns longitudes outside [-180, 180] when the map is panned past the
|
||||||
|
dateline at low zoom (e.g. east=411.3281, west=-608.2031). The map JS now
|
||||||
|
normalizes/omits those, but _parse_events_params also defends in depth: a
|
||||||
|
degenerate or out-of-range region is treated as "no bbox" rather than erroring
|
||||||
|
or building a bogus ST_MakeEnvelope that silently matches the wrong rows.
|
||||||
|
"""
|
||||||
|
from central.gui.routes import _parse_events_params
|
||||||
|
|
||||||
|
|
||||||
|
def _params(**kw):
|
||||||
|
base = {"limit": "50"}
|
||||||
|
base.update(kw)
|
||||||
|
return base
|
||||||
|
|
||||||
|
|
||||||
|
def test_out_of_range_bbox_is_dropped_not_errored():
|
||||||
|
"""The exact pan-past-dateline artifact from the bug report -> no bbox."""
|
||||||
|
parsed, error = _parse_events_params(_params(
|
||||||
|
region_north="42.0", region_south="31.0",
|
||||||
|
region_east="411.3281", region_west="-608.2031",
|
||||||
|
))
|
||||||
|
assert error is None
|
||||||
|
assert parsed is not None
|
||||||
|
assert parsed["bbox"] is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_valid_bbox_is_preserved():
|
||||||
|
parsed, error = _parse_events_params(_params(
|
||||||
|
region_north="42.0", region_south="31.0",
|
||||||
|
region_east="-102.0", region_west="-124.5",
|
||||||
|
))
|
||||||
|
assert error is None
|
||||||
|
assert parsed["bbox"] == {
|
||||||
|
"north": 42.0, "south": 31.0, "east": -102.0, "west": -124.5,
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def test_inverted_bbox_is_dropped():
|
||||||
|
"""west >= east (a wrapped dateline-straddle) is degenerate -> no bbox."""
|
||||||
|
parsed, error = _parse_events_params(_params(
|
||||||
|
region_north="42.0", region_south="31.0",
|
||||||
|
region_east="-124.5", region_west="-102.0",
|
||||||
|
))
|
||||||
|
assert error is None
|
||||||
|
assert parsed["bbox"] is None
|
||||||
Loading…
Add table
Add a link
Reference in a new issue