Check derived store before loading history on content routes
All checks were successful
PR build (required check) / changes (pull_request) Successful in 8s
secrets-guard / encrypted (pull_request) Successful in 6s
shell-lint / shellcheck (pull_request) Successful in 10s
PR build (required check) / build-frontend (pull_request) Has been skipped
PR build (required check) / validate-observability (pull_request) Has been skipped
PR build (required check) / build-backend (pull_request) Successful in 52s
PR build (required check) / gate (pull_request) Successful in 2s
All checks were successful
PR build (required check) / changes (pull_request) Successful in 8s
secrets-guard / encrypted (pull_request) Successful in 6s
shell-lint / shellcheck (pull_request) Successful in 10s
PR build (required check) / build-frontend (pull_request) Has been skipped
PR build (required check) / validate-observability (pull_request) Has been skipped
PR build (required check) / build-backend (pull_request) Successful in 52s
PR build (required check) / gate (pull_request) Successful in 2s
The three SEO content handlers (city / month / records) resolved the city by loading its full ~45-year archive up front, only then checking the derived-store cache, so even a cache HIT paid the full-history load (2-6s on prod). Restructure so the cheap path runs first: city lookup + grid snap + the cheap content_token (indexed MAX(date), no history load) produce the cache key/ETag, and the expensive load (history, recent forecast) moves into the build() closure that _cached only calls on a miss. A store hit, or a 304, now returns without ever touching the archive. Cache keys/kinds/tokens are unchanged. Add route tests asserting a second identical request loads history zero more times, that 304 revalidation loads nothing, plus payload/404 coverage.
This commit is contained in:
parent
85810c21c5
commit
9d41236ec5
2 changed files with 158 additions and 17 deletions
|
|
@ -11,7 +11,7 @@ from fastapi import APIRouter, HTTPException, Request, Response
|
||||||
|
|
||||||
from api import content_payloads as payloads
|
from api import content_payloads as payloads
|
||||||
from api import sitemap as sitemap_mod
|
from api import sitemap as sitemap_mod
|
||||||
from api.payloads import history_token
|
from api.payloads import content_token
|
||||||
from data import climate
|
from data import climate
|
||||||
from data import cities
|
from data import cities
|
||||||
from data import grid
|
from data import grid
|
||||||
|
|
@ -58,13 +58,21 @@ def _cached(request: Request, kind: str, cell_id: str, key: str, token: str, bui
|
||||||
return _json_response(store.put_payload(kind, cell_id, key, token, payload), etag)
|
return _json_response(store.put_payload(kind, cell_id, key, token, payload), etag)
|
||||||
|
|
||||||
|
|
||||||
def _resolve_city(slug: str):
|
def _city_cell(slug: str):
|
||||||
"""(city, cell, history) for a slug, or raise 404 (unknown) / 503 (warming) --
|
"""(city, cell) for a slug, or raise 404 — both steps are cheap (a dict
|
||||||
mirrors web/content.py's _resolve_city."""
|
lookup and a pure grid snap) and, crucially, load no history, so the
|
||||||
|
derived-store token can be computed and a cache hit served without ever
|
||||||
|
touching the ~45-year archive."""
|
||||||
city = cities.get(slug)
|
city = cities.get(slug)
|
||||||
if city is None:
|
if city is None:
|
||||||
raise HTTPException(status_code=404, detail="Unknown city.")
|
raise HTTPException(status_code=404, detail="Unknown city.")
|
||||||
cell = grid.snap(city["lat"], city["lon"])
|
return city, grid.snap(city["lat"], city["lon"])
|
||||||
|
|
||||||
|
|
||||||
|
def _load_history(cell):
|
||||||
|
"""The cell's ~45-year archive, or raise 503 while it's still warming --
|
||||||
|
the expensive load that used to run on every request. Now called only from
|
||||||
|
the content handlers' build() closures, i.e. only on a derived-store miss."""
|
||||||
history = climate.load_cached_history(cell)
|
history = climate.load_cached_history(cell)
|
||||||
if history is None or history.is_empty():
|
if history is None or history.is_empty():
|
||||||
try:
|
try:
|
||||||
|
|
@ -73,7 +81,7 @@ def _resolve_city(slug: str):
|
||||||
history = None
|
history = None
|
||||||
if history is None or history.is_empty():
|
if history is None or history.is_empty():
|
||||||
raise HTTPException(status_code=503, detail="Climate data is warming up; please retry shortly.")
|
raise HTTPException(status_code=503, detail="Climate data is warming up; please retry shortly.")
|
||||||
return city, cell, history
|
return history
|
||||||
|
|
||||||
|
|
||||||
def _origin(request: Request) -> str:
|
def _origin(request: Request) -> str:
|
||||||
|
|
@ -105,16 +113,17 @@ def content_home(request: Request):
|
||||||
|
|
||||||
@router.get("/content/city/{slug}")
|
@router.get("/content/city/{slug}")
|
||||||
def content_city(request: Request, slug: str):
|
def content_city(request: Request, slug: str):
|
||||||
city, cell, history = _resolve_city(slug)
|
city, cell = _city_cell(slug)
|
||||||
recent = None
|
token = content_token(cell["id"])
|
||||||
try:
|
|
||||||
recent = climate.get_recent_forecast(cell)
|
|
||||||
except Exception: # noqa: BLE001 - today_vs_normal degrades to None
|
|
||||||
pass
|
|
||||||
token = history_token(history)
|
|
||||||
origin = _origin(request)
|
origin = _origin(request)
|
||||||
|
|
||||||
def build():
|
def build():
|
||||||
|
history = _load_history(cell)
|
||||||
|
recent = None
|
||||||
|
try:
|
||||||
|
recent = climate.get_recent_forecast(cell)
|
||||||
|
except Exception: # noqa: BLE001 - today_vs_normal degrades to None
|
||||||
|
pass
|
||||||
return payloads.city_payload(origin, BASE, city, history, recent)
|
return payloads.city_payload(origin, BASE, city, history, recent)
|
||||||
|
|
||||||
# origin is folded into the cache key (not just slug) because the payload's
|
# origin is folded into the cache key (not just slug) because the payload's
|
||||||
|
|
@ -131,10 +140,11 @@ def content_city(request: Request, slug: str):
|
||||||
def content_city_month(request: Request, slug: str, month: str):
|
def content_city_month(request: Request, slug: str, month: str):
|
||||||
if month not in payloads.MONTH_INDEX:
|
if month not in payloads.MONTH_INDEX:
|
||||||
raise HTTPException(status_code=404, detail="Unknown month.")
|
raise HTTPException(status_code=404, detail="Unknown month.")
|
||||||
city, cell, history = _resolve_city(slug)
|
city, cell = _city_cell(slug)
|
||||||
token = history_token(history)
|
token = content_token(cell["id"])
|
||||||
|
|
||||||
def build():
|
def build():
|
||||||
|
history = _load_history(cell)
|
||||||
return payloads.month_payload(BASE, city, history, payloads.MONTH_INDEX[month])
|
return payloads.month_payload(BASE, city, history, payloads.MONTH_INDEX[month])
|
||||||
|
|
||||||
return _cached(request, "content-month", cell["id"], f"{slug}:{month}", token, build)
|
return _cached(request, "content-month", cell["id"], f"{slug}:{month}", token, build)
|
||||||
|
|
@ -142,11 +152,12 @@ def content_city_month(request: Request, slug: str, month: str):
|
||||||
|
|
||||||
@router.get("/content/city/{slug}/records")
|
@router.get("/content/city/{slug}/records")
|
||||||
def content_city_records(request: Request, slug: str):
|
def content_city_records(request: Request, slug: str):
|
||||||
city, cell, history = _resolve_city(slug)
|
city, cell = _city_cell(slug)
|
||||||
token = history_token(history)
|
token = content_token(cell["id"])
|
||||||
origin = _origin(request)
|
origin = _origin(request)
|
||||||
|
|
||||||
def build():
|
def build():
|
||||||
|
history = _load_history(cell)
|
||||||
return payloads.records_payload(origin, BASE, city, history)
|
return payloads.records_payload(origin, BASE, city, history)
|
||||||
|
|
||||||
# See content_city's comment: origin folded into the cache key for the
|
# See content_city's comment: origin folded into the cache key for the
|
||||||
|
|
|
||||||
130
backend/tests/api/test_content_routes.py
Normal file
130
backend/tests/api/test_content_routes.py
Normal file
|
|
@ -0,0 +1,130 @@
|
||||||
|
"""Route tests for the SSR content JSON API (api/content_routes.py).
|
||||||
|
|
||||||
|
The point of these is the latency fix: a derived-store cache HIT must be O(1) —
|
||||||
|
it must NOT load the ~45-year archive. So the weather layer is faked with call
|
||||||
|
counters, and the core assertion is that a second identical request loads the
|
||||||
|
history zero more times than the first. ETag/304 revalidation and the 404/payload
|
||||||
|
shapes are covered alongside, against a fresh per-test store.
|
||||||
|
"""
|
||||||
|
import pytest
|
||||||
|
from fastapi.testclient import TestClient
|
||||||
|
|
||||||
|
from web import app as appmod
|
||||||
|
from data import climate
|
||||||
|
from data import cities
|
||||||
|
|
||||||
|
BASE = "/thermograph/api/v2/content"
|
||||||
|
|
||||||
|
CITY = {
|
||||||
|
"slug": "testville",
|
||||||
|
"name": "Testville",
|
||||||
|
"admin1": "Washington",
|
||||||
|
"country": "United States",
|
||||||
|
"country_code": "US",
|
||||||
|
"lat": 47.6062,
|
||||||
|
"lon": -122.3321,
|
||||||
|
"population": 100000,
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def counts():
|
||||||
|
return {"load_cached_history": 0, "get_history": 0}
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def client(monkeypatch, history, recent, counts, tmp_store):
|
||||||
|
"""TestClient with the weather + city layer faked and the history loaders
|
||||||
|
wrapped in counters. tmp_store gives each test a fresh derived store so the
|
||||||
|
miss->hit sequence is deterministic."""
|
||||||
|
def load_cached_history(cell):
|
||||||
|
counts["load_cached_history"] += 1
|
||||||
|
return history.clone()
|
||||||
|
|
||||||
|
def get_history(cell):
|
||||||
|
counts["get_history"] += 1
|
||||||
|
return history.clone(), {"cached": True, "cache_age_days": 3}
|
||||||
|
|
||||||
|
monkeypatch.setattr(climate, "load_cached_history", load_cached_history)
|
||||||
|
monkeypatch.setattr(climate, "get_history", get_history)
|
||||||
|
monkeypatch.setattr(climate, "get_recent_forecast", lambda cell: recent.clone())
|
||||||
|
# Stable token: content_token() -> payloads -> climate.history_max_date.
|
||||||
|
monkeypatch.setattr(climate, "history_max_date", lambda cell_id: "2026-07-01")
|
||||||
|
monkeypatch.setattr(climate, "reverse_geocode", lambda lat, lon: "Testville, Washington")
|
||||||
|
# Only "testville" is a known city; anything else -> 404.
|
||||||
|
monkeypatch.setattr(cities, "get", lambda slug: CITY if slug == "testville" else None)
|
||||||
|
return TestClient(appmod.app)
|
||||||
|
|
||||||
|
|
||||||
|
# --- the regression guard: a cache hit loads no history ----------------------
|
||||||
|
|
||||||
|
def test_cache_hit_does_not_load_history(client, counts):
|
||||||
|
"""First request misses -> builds -> loads history exactly once. A second
|
||||||
|
identical request is served from the derived store and loads history ZERO
|
||||||
|
more times. This is the whole point of the fix."""
|
||||||
|
r1 = client.get(f"{BASE}/city/testville")
|
||||||
|
assert r1.status_code == 200
|
||||||
|
assert counts["load_cached_history"] == 1
|
||||||
|
assert counts["get_history"] == 0 # cached load was non-empty, no live fetch
|
||||||
|
|
||||||
|
r2 = client.get(f"{BASE}/city/testville")
|
||||||
|
assert r2.status_code == 200
|
||||||
|
assert r2.content == r1.content # replayed from the store
|
||||||
|
assert counts["load_cached_history"] == 1 # unchanged: no history load on the hit
|
||||||
|
assert counts["get_history"] == 0
|
||||||
|
|
||||||
|
|
||||||
|
def test_304_revalidation_loads_no_history(client, counts):
|
||||||
|
"""If-None-Match on a stored payload -> 304 with no history load."""
|
||||||
|
r1 = client.get(f"{BASE}/city/testville")
|
||||||
|
assert r1.status_code == 200
|
||||||
|
etag = r1.headers["etag"]
|
||||||
|
before = counts["load_cached_history"]
|
||||||
|
|
||||||
|
r304 = client.get(f"{BASE}/city/testville", headers={"If-None-Match": etag})
|
||||||
|
assert r304.status_code == 304
|
||||||
|
assert r304.headers["etag"] == etag
|
||||||
|
assert counts["load_cached_history"] == before # 304 loaded nothing
|
||||||
|
|
||||||
|
|
||||||
|
def test_month_and_records_hits_skip_history_load(client, counts):
|
||||||
|
"""Same O(1)-hit guarantee for the month and records handlers."""
|
||||||
|
for path in ("month/july", "records"):
|
||||||
|
counts["load_cached_history"] = 0
|
||||||
|
assert client.get(f"{BASE}/city/testville/{path}").status_code == 200
|
||||||
|
assert counts["load_cached_history"] == 1
|
||||||
|
assert client.get(f"{BASE}/city/testville/{path}").status_code == 200
|
||||||
|
assert counts["load_cached_history"] == 1 # second call is a store hit
|
||||||
|
|
||||||
|
|
||||||
|
# --- payload shapes ----------------------------------------------------------
|
||||||
|
|
||||||
|
def test_city_payload_shape(client):
|
||||||
|
body = client.get(f"{BASE}/city/testville").json()
|
||||||
|
assert body["city"]["slug"] == "testville"
|
||||||
|
assert body["canonical_path"] == "/climate/testville"
|
||||||
|
assert len(body["months"]) == 12
|
||||||
|
assert body["today_vs_normal"] is not None # recent forecast folded in
|
||||||
|
|
||||||
|
|
||||||
|
def test_month_payload_shape(client):
|
||||||
|
body = client.get(f"{BASE}/city/testville/month/july").json()
|
||||||
|
assert body["month_slug"] == "july"
|
||||||
|
assert body["canonical_path"] == "/climate/testville/july"
|
||||||
|
|
||||||
|
|
||||||
|
def test_records_payload_shape(client):
|
||||||
|
body = client.get(f"{BASE}/city/testville/records").json()
|
||||||
|
assert body["canonical_path"] == "/climate/testville/records"
|
||||||
|
|
||||||
|
|
||||||
|
# --- 404s --------------------------------------------------------------------
|
||||||
|
|
||||||
|
def test_unknown_slug_is_404(client, counts):
|
||||||
|
for path in ("city/nope", "city/nope/month/july", "city/nope/records"):
|
||||||
|
assert client.get(f"{BASE}/{path}").status_code == 404
|
||||||
|
assert counts["load_cached_history"] == 0 # never reached the archive
|
||||||
|
|
||||||
|
|
||||||
|
def test_unknown_month_is_404(client):
|
||||||
|
assert client.get(f"{BASE}/city/testville/month/smarch").status_code == 404
|
||||||
Loading…
Reference in a new issue