diff --git a/README.md b/README.md index e9a5634d..dbcee9a7 100644 --- a/README.md +++ b/README.md @@ -18,6 +18,21 @@ 3. Open the dev server at https://localhost:3000 4. Log in with username `hello@commonknowledge.coop` and password `1234`. +### Troubleshooting: every page and API route returns 500 in dev + +If `npm run dev` fails with `Can't resolve '@vercel/turbopack-next/internal/font/google/font'` +and `next/font/google queries have exactly one entry`, Turbopack has cached a Google +Fonts response that uses dynamic-subset URLs (`fonts.gstatic.com/l/font?kit=…&skey=…`). +It can't parse the `&` in those URLs, and in dev that one compile error turns every +route into a 500, the REST API included. Stop the dev server, delete the cache and +start again: + +```bash +rm -rf .next/dev +``` + +`next dev --webpack` isn't affected, so it also works as a stopgap. + ### Migrations - Create with `npm run kysely migrate:make [name]` diff --git a/migrations/1790100000000_data_record_postcode_index.ts b/migrations/1790100000000_data_record_postcode_index.ts new file mode 100644 index 00000000..40f819ad --- /dev/null +++ b/migrations/1790100000000_data_record_postcode_index.ts @@ -0,0 +1,48 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import { type Kysely, sql } from "kysely"; + +/** + * Index postcode look-ups on data_record. + * + * The expression matches what FilterType.EXACT generates in + * src/server/repositories/DataRecord.ts — lower(json->>column) = search — so + * `?filter={"type":"EXACT","column":"postcode","search":"LS1 1AA"}` becomes an + * index scan instead of a sequential scan over every record in the source. + * + * Deliberately NOT a partial index (WHERE json ? 'postcode'): the EXACT + * predicate doesn't imply that condition, so the planner can't use a partial + * index for it and silently falls back to the sequential scan. + * + * Not CONCURRENTLY, because migrations run inside a transaction. Building it + * blocks writes to data_record (not reads) for the duration of the build: + * 3.3 s on a 7 GB, 1.95M-row data_record locally. To avoid even that, build it + * by hand before deploying and this migration becomes a no-op: + * + * CREATE INDEX CONCURRENTLY IF NOT EXISTS data_record_postcode_idx + * ON data_record (data_source_id, (lower(json->>'postcode'))); + * + * A failed CONCURRENTLY build leaves an INVALID index behind under the same + * name, which IF NOT EXISTS would silently accept, so drop that first. + */ +export async function up(db: Kysely): Promise { + await sql` + DO $$ + BEGIN + IF EXISTS ( + SELECT 1 FROM pg_index i + JOIN pg_class c ON c.oid = i.indexrelid + WHERE c.relname = 'data_record_postcode_idx' AND NOT i.indisvalid + ) THEN + DROP INDEX data_record_postcode_idx; + END IF; + END $$; + `.execute(db); + await sql` + CREATE INDEX IF NOT EXISTS data_record_postcode_idx + ON data_record (data_source_id, (lower(json->>'postcode'))); + `.execute(db); +} + +export async function down(db: Kysely): Promise { + await sql`DROP INDEX IF EXISTS data_record_postcode_idx;`.execute(db); +} diff --git a/migrations/1790200000000_drop_duplicate_data_record_covering_index.ts b/migrations/1790200000000_drop_duplicate_data_record_covering_index.ts new file mode 100644 index 00000000..6c676f2b --- /dev/null +++ b/migrations/1790200000000_drop_duplicate_data_record_covering_index.ts @@ -0,0 +1,35 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import { type Kysely, sql } from "kysely"; + +/** + * Drop a duplicate of data_record_source_id_covering_id. + * + * Production has two identical indexes, both + * btree (data_source_id) INCLUDE (id), 350 MB each: + * + * data_record_source_id_covering_id - created by migration + * 1770310811295_data_record_id_covering_index + * idx_data_record_source_id_covering - created by hand; it appears nowhere + * in this repo's history + * + * Identical definitions give the planner nothing to choose between. The + * hand-made one had 7 scans against 3,123 for the migration one (stats since + * 2026-04-08), and it backs no constraint. Dropping it frees ~350 MB and + * removes one index to maintain on every data_record write. + * + * IF EXISTS: other environments never had it. + * + * DROP INDEX takes a brief ACCESS EXCLUSIVE lock on data_record. The drop + * itself is instant, but it has to wait for running queries on the table to + * finish, and the migration CLI's 10 s lock_timeout bounds that wait. + */ +export async function up(db: Kysely): Promise { + await sql`DROP INDEX IF EXISTS idx_data_record_source_id_covering;`.execute( + db, + ); +} + +export async function down(): Promise { + // Nothing to restore: the migration-managed data_record_source_id_covering_id + // is the same index, and recreating the duplicate would reintroduce the waste. +} diff --git a/src/server/mapping/geocode.ts b/src/server/mapping/geocode.ts index 605a10cd..5c6dcc03 100644 --- a/src/server/mapping/geocode.ts +++ b/src/server/mapping/geocode.ts @@ -244,8 +244,20 @@ const geocodeRecordByCoordinates = async ( throw new Error(`Missing longitude column "${longitudeColumn}" in row`); } - const lat = Number(dataRecordJson[latitudeColumn]); - const lng = Number(dataRecordJson[longitudeColumn]); + const rawLat = dataRecordJson[latitudeColumn]; + const rawLng = dataRecordJson[longitudeColumn]; + // Number("") and Number(null) are 0, which would silently place a record + // with no coordinates at 0,0 in the Gulf of Guinea. + const isBlank = (v: unknown) => + v === null || v === undefined || String(v).trim() === ""; + if (isBlank(rawLat) || isBlank(rawLng)) { + throw new Error( + `Missing coordinates: latitude=${rawLat}, longitude=${rawLng}`, + ); + } + + const lat = Number(rawLat); + const lng = Number(rawLng); if (isNaN(lat) || isNaN(lng)) { throw new Error( diff --git a/tests/unit/server/mapping/geocode.test.ts b/tests/unit/server/mapping/geocode.test.ts index b38484fb..6f24e87f 100644 --- a/tests/unit/server/mapping/geocode.test.ts +++ b/tests/unit/server/mapping/geocode.test.ts @@ -27,4 +27,24 @@ describe("geocode", () => { expect(result?.centralPoint?.lng).toBeCloseTo(-0.8, 0); expect(result?.areas[AreaSetCode.PC]).toBe("HP20 2QB"); }); + + test("geocodeRecord by coordinates returns null for blank coordinates rather than 0,0", async () => { + const geocodingConfig = { + type: GeocodingType.Coordinates as const, + latitudeColumn: "latitude", + longitudeColumn: "longitude", + }; + + for (const [latitude, longitude] of [ + ["", ""], + ["", "-1.5"], + [null, null], + ]) { + const result = await geocodeRecord( + { externalId: "test-blank", json: { latitude, longitude } }, + geocodingConfig, + ); + expect(result).toBeNull(); + } + }); });