diff --git a/backend/app.py b/backend/app.py index caa3c21..e5d5b62 100644 --- a/backend/app.py +++ b/backend/app.py @@ -26,7 +26,8 @@ from starlette.middleware.base import BaseHTTPMiddleware import asyncio from .config import settings from .data_loader import ( - clear_cache, + build_latest_school_data, + load_school_data_as_dataframe, compute_benchmarks, load_school_data, load_latest_school_data, @@ -272,7 +273,7 @@ def _places_payload(urn: int) -> list[dict]: return payload -def _place_sitemap_rows(kinds: tuple[str, ...]) -> list[str]: +def _place_sitemap_rows(kinds: tuple[str, ...], registry=None) -> list[str]: """A per place, plus a phase variant wherever that phase clears the threshold on its own. @@ -282,7 +283,9 @@ def _place_sitemap_rows(kinds: tuple[str, ...]) -> list[str]: linked from the place page either. """ rows: list[str] = [] - for p in sorted(get_place_registry().values(), key=lambda p: (p.kind, p.slug)): + if registry is None: + registry = get_place_registry() + for p in sorted(registry.values(), key=lambda p: (p.kind, p.slug)): if p.kind not in kinds: continue rows.append(_url_element(BASE_URL + _place_url(p))) @@ -296,9 +299,10 @@ def _place_sitemap_rows(kinds: tuple[str, ...]) -> list[str]: return rows -def build_sitemaps() -> dict[str, str]: +def build_sitemaps(df=None, registry=None) -> dict[str, str]: """Build the sitemap index and every child, keyed by name.""" - df = load_school_data() + if df is None: + df = load_school_data() children: dict[str, str] = { "static.xml": _urlset( @@ -318,7 +322,7 @@ def build_sitemaps() -> dict[str, str]: # measured apart from the school pages'. for label, kinds in (("places", ("town", "locality", "authority")), ("outcodes", ("outcode",))): - rows = _place_sitemap_rows(kinds) + rows = _place_sitemap_rows(kinds, registry) chunks = [rows[i:i + SITEMAP_CHUNK_SIZE] for i in range(0, len(rows), SITEMAP_CHUNK_SIZE)] or [[]] for n, chunk in enumerate(chunks, start=1): @@ -700,6 +704,15 @@ async def get_config(): } +@app.get("/api/release") +async def release_identity(): + import json + from pathlib import Path + path = Path(__file__).with_name("build-info.json") + identity = json.loads(path.read_text()) if path.exists() else {"sha": "development", "build_id": "development"} + return JSONResponse(identity, headers={"Cache-Control": "no-store"}) + + @app.get("/api/schools") @limiter.limit(f"{settings.rate_limit_per_minute}/minute") async def get_schools( @@ -736,7 +749,7 @@ async def get_schools( df_latest = load_latest_school_data() if df_latest.empty: - return {"schools": [], "total": 0, "page": page, "page_size": 0} + raise HTTPException(status_code=503, detail="School data temporarily unavailable") # Use configured default if not specified if page_size is None: @@ -835,8 +848,8 @@ async def get_schools( # Apply filters if search: - ts_urns = search_schools_typesense(search) - if ts_urns: + ts_urns = await asyncio.to_thread(search_schools_typesense, search) + if ts_urns is not None: urn_order = {urn: i for i, urn in enumerate(ts_urns)} schools_df = schools_df[schools_df["urn"].isin(set(ts_urns))].copy() schools_df["_ts_rank"] = schools_df["urn"].map(urn_order) @@ -844,9 +857,9 @@ async def get_schools( else: # Fallback: Typesense unavailable, use substring match search_lower = search.lower() - mask = schools_df["school_name"].str.lower().str.contains(search_lower, na=False) + mask = schools_df["school_name"].str.lower().str.contains(search_lower, na=False, regex=False) if "address" in schools_df.columns: - mask = mask | schools_df["address"].str.lower().str.contains(search_lower, na=False) + mask = mask | schools_df["address"].str.lower().str.contains(search_lower, na=False, regex=False) schools_df = schools_df[mask] if local_authority: @@ -905,7 +918,7 @@ async def get_school_details(request: Request, urn: int): df = load_school_data() if df.empty: - raise HTTPException(status_code=404, detail="No data available") + raise HTTPException(status_code=503, detail="School data temporarily unavailable") school_data = df[df["urn"] == urn] @@ -1542,20 +1555,51 @@ async def get_data_info(request: Request): } +_publication_lock = asyncio.Lock() + + +def _prepare_publication(df): + if df.empty: + raise ValueError("Refusing to publish an empty school dataset") + if not {"urn", "year", "school_name"}.issubset(df.columns): + raise ValueError("School dataset is missing required columns") + if df["urn"].isna().any() or df.duplicated(["urn", "year"]).any(): + raise ValueError("School dataset has missing URNs or duplicate school years") + latest = build_latest_school_data(df) + registry = build_place_registry(df) + index = build_place_index(registry) + sitemaps = build_sitemaps(df, registry) + return df, latest, registry, index, sitemaps + + +def _publish(prepared): + # Called on the event loop with no await: routes cannot observe half a swap. + # The application currently runs one worker; replicas require coordination. + from . import data_loader + global _place_registry, _place_index, _place_index_source, _sitemaps + df, latest, registry, index, sitemaps = prepared + data_loader._df_cache = df + data_loader._df_latest_cache = latest + _place_registry = registry + _place_index = index + _place_index_source = registry + _sitemaps = sitemaps + + @app.post("/api/admin/reload") @limiter.limit("5/minute") -async def reload_data( - request: Request, - _: bool = Depends(verify_admin_api_key) -): - """ - Admin endpoint to force data reload (useful after data updates). - Requires X-API-Key header with valid admin API key. - """ - clear_cache() - await asyncio.to_thread(load_school_data) - await asyncio.to_thread(load_latest_school_data) - return {"status": "reloaded"} +async def reload_data(request: Request, _: bool = Depends(verify_admin_api_key)): + """Validate a complete replacement before publishing it; retain data on failure.""" + async with _publication_lock: + try: + df = await asyncio.to_thread(load_school_data_as_dataframe) + prepared = await asyncio.to_thread(_prepare_publication, df) + except Exception as exc: + import logging + logging.getLogger(__name__).exception("Dataset reload failed") + raise HTTPException(status_code=503, detail="Dataset reload failed; previous data retained") from exc + _publish(prepared) + return {"status": "reloaded", "schools": len(prepared[1])} @@ -1607,15 +1651,16 @@ async def regenerate_sitemap( request: Request, _: bool = Depends(verify_admin_api_key), ): - """Rebuild and cache the sitemap from current school data. Called by Airflow after data updates.""" - global _sitemaps, _place_registry - # Places and sitemap are rebuilt together — they read the same marts, and - # letting them drift apart would submit URLs for places that no longer - # exist. - _place_registry = None - _sitemaps = build_sitemaps() - n = sum(x.count("") for x in _sitemaps.values()) - return {"status": "ok", "urls": n, "sitemaps": len(_sitemaps)} + """Rebuild derived publication data without clearing the live registry.""" + async with _publication_lock: + try: + prepared = await asyncio.to_thread(_prepare_publication, load_school_data()) + except Exception as exc: + raise HTTPException(status_code=503, detail="Sitemap rebuild failed; previous data retained") from exc + _publish(prepared) + n = sum(x.count("") for x in prepared[4].values()) + return {"status": "ok", "urls": n, "sitemaps": len(prepared[4])} + # Mount static files directly (must be after all routes to avoid catching API calls) diff --git a/backend/data_loader.py b/backend/data_loader.py index ebd8d05..1816745 100644 --- a/backend/data_loader.py +++ b/backend/data_loader.py @@ -84,21 +84,37 @@ def _get_typesense_client(): return None -def search_schools_typesense(query: str, limit: int = 250) -> List[int]: - """Search Typesense. Returns URNs in relevance order, or [] if unavailable.""" +def search_schools_typesense(query: str) -> Optional[List[int]]: + """Return all matching URNs in relevance order; None means unavailable. + + Filtering and user pagination happen in the API after this search. Returning + only the first search page would silently discard valid local matches. + Never return a partial candidate set if a later page fails. + """ client = _get_typesense_client() if client is None: - return [] + return None + urns = [] try: - result = client.collections["schools"].documents.search({ - "q": query, - "query_by": "school_name,local_authority,postcode", - "per_page": min(limit, 250), - "typo_tokens_threshold": 1, - }) - return [int(h["document"]["urn"]) for h in result.get("hits", [])] + page = 1 + while True: + result = client.collections["schools"].documents.search({ + "q": query, + "query_by": "school_name,local_authority,postcode", + "per_page": 250, + "page": page, + "typo_tokens_threshold": 1, + }) + hits = result.get("hits", []) + urns.extend(int(h["document"]["urn"]) for h in hits) + if len(urns) >= result.get("found", len(urns)): + return list(dict.fromkeys(urns)) + if not hits: + raise ValueError("Search pagination ended before all matches arrived") + page += 1 except Exception: - return [] + logging.getLogger(__name__).exception("School search unavailable") + return None # The most a public endpoint will return in one response. @@ -502,7 +518,12 @@ def load_latest_school_data() -> pd.DataFrame: if _df_latest_cache is not None: return _df_latest_cache - df = load_school_data() + _df_latest_cache = build_latest_school_data(load_school_data()) + return _df_latest_cache + + +def build_latest_school_data(df: pd.DataFrame) -> pd.DataFrame: + """Build a replacement snapshot without mutating the published caches.""" if df.empty: return df @@ -535,8 +556,7 @@ def load_latest_school_data() -> pd.DataFrame: df_latest = pd.concat([df_latest, df_no_perf], ignore_index=True) print(f"Latest-snapshot cache built: {len(df_latest)} schools") - _df_latest_cache = df_latest - return _df_latest_cache + return df_latest def clear_cache(): diff --git a/backend/tests/test_publication.py b/backend/tests/test_publication.py new file mode 100644 index 0000000..07a20bb --- /dev/null +++ b/backend/tests/test_publication.py @@ -0,0 +1,68 @@ +"""Publication must preserve the current dataset until every replacement is ready.""" +import asyncio +import pandas as pd +import pytest +from fastapi.testclient import TestClient +from backend import app as api, data_loader +from backend.tests.test_sixth_form_flag import _schools_df + + +@pytest.fixture +def client(monkeypatch): + old = _schools_df() + monkeypatch.setattr(data_loader, '_df_cache', old) + monkeypatch.setattr(data_loader, '_df_latest_cache', old) + monkeypatch.setattr(api, '_place_registry', {'old': 'registry'}) + monkeypatch.setattr(api, '_place_index', {'old': 'index'}) + monkeypatch.setattr(api, '_place_index_source', api._place_registry) + monkeypatch.setattr(api, '_sitemaps', {'old.xml': 'old sitemap'}) + monkeypatch.setattr(api, '_publication_lock', asyncio.Lock()) + monkeypatch.setattr(api.limiter, 'enabled', False) + api.app.dependency_overrides[api.verify_admin_api_key] = lambda: True + yield TestClient(api.app, raise_server_exceptions=False) + api.app.dependency_overrides.clear() + + +def state(): + return (data_loader._df_cache, data_loader._df_latest_cache, api._place_registry, + api._place_index, api._place_index_source, api._sitemaps) + + +@pytest.mark.parametrize('failure', ['empty', 'database', 'sitemap', 'duplicate']) +def test_failed_reload_preserves_every_published_object(client, monkeypatch, failure): + before = state() + df = _schools_df() + if failure == 'empty': + df = pd.DataFrame() + if failure == 'duplicate': + df = pd.concat([df, df.iloc[:1]], ignore_index=True) + def load(): + if failure == 'database': + raise RuntimeError('database unavailable') + return df + monkeypatch.setattr(api, 'load_school_data_as_dataframe', load) + if failure == 'sitemap': + monkeypatch.setattr(api, 'build_sitemaps', lambda *args: (_ for _ in ()).throw(RuntimeError('bad XML'))) + response = client.post('/api/admin/reload') + assert response.status_code == 503 + assert all(a is b for a, b in zip(before, state())) + + +def test_success_publishes_school_data_places_and_sitemaps(client, monkeypatch): + df = _schools_df() + df.loc[0, 'school_name'] = 'Replacement School' + monkeypatch.setattr(api, 'load_school_data_as_dataframe', lambda: df) + response = client.post('/api/admin/reload') + assert response.status_code == 200 + assert data_loader.load_school_data() is df + assert data_loader.load_latest_school_data().iloc[0].school_name == 'Replacement School' + assert api._place_index_source is api._place_registry + assert 'old.xml' not in api._sitemaps + assert 'replacement-school' in api._sitemaps['schools-1.xml'] + + +def test_failed_sitemap_regeneration_keeps_existing_publication(client, monkeypatch): + before = state() + monkeypatch.setattr(api, 'build_sitemaps', lambda *args: (_ for _ in ()).throw(RuntimeError('bad XML'))) + assert client.post('/api/admin/regenerate-sitemap').status_code == 503 + assert all(a is b for a, b in zip(before, state())) diff --git a/backend/tests/test_search_completeness.py b/backend/tests/test_search_completeness.py new file mode 100644 index 0000000..e170a8b --- /dev/null +++ b/backend/tests/test_search_completeness.py @@ -0,0 +1,73 @@ +from types import SimpleNamespace +import pytest +from fastapi.testclient import TestClient +from backend import app as api, data_loader +from backend.tests.test_sixth_form_flag import _schools_df + + +def client_for(monkeypatch, search): + client = SimpleNamespace(collections={'schools': SimpleNamespace(documents=SimpleNamespace(search=search))}) + monkeypatch.setattr(data_loader, '_get_typesense_client', lambda: client) + + +def test_search_returns_matches_beyond_first_page(monkeypatch): + pages = [] + def search(params): + pages.append(params['page']) + urns = range(100000, 100250) if params['page'] == 1 else [100999] + return {'found': 251, 'hits': [{'document': {'urn': u}} for u in urns]} + client_for(monkeypatch, search) + result = data_loader.search_schools_typesense('academy') + assert len(result) == 251 + assert result[-1] == 100999 + assert pages == [1, 2] + + +def test_later_page_failure_does_not_return_partial_results(monkeypatch): + def search(params): + if params['page'] == 2: + raise RuntimeError('timeout') + return {'found': 251, 'hits': [{'document': {'urn': u}} for u in range(100000, 100250)]} + client_for(monkeypatch, search) + assert data_loader.search_schools_typesense('academy') is None + + +def test_zero_matches_are_distinct_from_unavailable(monkeypatch): + client_for(monkeypatch, lambda _: {'found': 0, 'hits': []}) + assert data_loader.search_schools_typesense('academy') == [] + monkeypatch.setattr(data_loader, '_get_typesense_client', lambda: None) + assert data_loader.search_schools_typesense('academy') is None + + +@pytest.mark.parametrize('matches, expected', [([], []), (None, [100001])]) +def test_fallback_only_on_dependency_failure(monkeypatch, matches, expected): + monkeypatch.setattr(api.limiter, 'enabled', False) + monkeypatch.setattr(api, 'load_latest_school_data', _schools_df) + monkeypatch.setattr(api, 'search_schools_typesense', lambda _: matches) + response = TestClient(api.app).get('/api/schools?search=Alpha') + assert response.status_code == 200 + assert [s['urn'] for s in response.json()['schools']] == expected + + +def test_filtered_api_keeps_match_from_second_search_page(monkeypatch): + monkeypatch.setattr(api.limiter, 'enabled', False) + df = _schools_df() + monkeypatch.setattr(api, 'load_latest_school_data', lambda: df) + def search(params): + urns = range(200000, 200250) if params['page'] == 1 else [100001] + return {'found': 251, 'hits': [{'document': {'urn': u}} for u in urns]} + client_for(monkeypatch, search) + response = TestClient(api.app).get('/api/schools?search=Alpha&local_authority=Testshire') + assert response.status_code == 200 + assert response.json()['total'] == 1 + assert response.json()['schools'][0]['urn'] == 100001 + + +def test_unavailable_dataset_is_not_a_missing_school_or_empty_search(monkeypatch): + import pandas as pd + monkeypatch.setattr(api.limiter, 'enabled', False) + monkeypatch.setattr(api, 'load_school_data', lambda: pd.DataFrame()) + monkeypatch.setattr(api, 'load_latest_school_data', lambda: pd.DataFrame()) + client = TestClient(api.app) + assert client.get('/api/schools/100001').status_code == 503 + assert client.get('/api/schools?search=school').status_code == 503