Merge pull request #375 from trails-cool/stigi/atomic-gpx-save
Atomic GPX save: validate + persist row + geometry in one transaction
This commit is contained in:
commit
4189187dd4
14 changed files with 707 additions and 182 deletions
18
CONTEXT.md
18
CONTEXT.md
|
|
@ -9,6 +9,24 @@ first. If the term you need isn't here, propose it (don't invent a synonym).
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
## GPX Save
|
||||||
|
|
||||||
|
The atomic unit of persisting spatial data in the Journal. Any write of a GPX track — whether a new route, an updated route, a new activity, or a route derived from an activity — goes through a single path that validates, writes the row, and writes the PostGIS geometry in one transaction.
|
||||||
|
|
||||||
|
### gpx-save module
|
||||||
|
`apps/journal/app/lib/gpx-save.server.ts`. The sole owner of GPX validation and geometry persistence. Imported by `routes.server.ts`, `activities.server.ts`, and `demo-bot.server.ts`. Nothing else calls `setGeomFromGpx` directly.
|
||||||
|
|
||||||
|
### GpxValidationError
|
||||||
|
Typed error thrown by `validateGpx` when the GPX string cannot produce a valid LineString. Conditions: fewer than 2 track points, or coordinates outside valid ranges (lat −90..90, lon −180..180). Callers catch this to return a user-facing 400.
|
||||||
|
|
||||||
|
### validateGpx
|
||||||
|
`(gpx: string) → Promise<ParsedGpx>`. Entry point of the gpx-save module. Parses the GPX string once and validates the result. Returns the `ParsedGpx` so callers can extract stats without re-parsing. Throws `GpxValidationError` on invalid input. Called at the start of `createRoute`, `updateRoute`, `createActivity`, and `createRouteFromActivity` — before any DB write.
|
||||||
|
|
||||||
|
### atomic GPX save
|
||||||
|
The invariant: a route or activity row with a `gpx` column set **always** has a corresponding `geom` column set. Enforced by wrapping the row insert/update, the PostGIS geometry write, and the version snapshot in a single `db.transaction()`. A PostGIS failure rolls back the row write; partial state (row exists, geom NULL) is not possible through the normal save path.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
## Connected Services
|
## Connected Services
|
||||||
|
|
||||||
The user-facing surface for linking external accounts and devices to a Journal
|
The user-facing surface for linking external accounts and devices to a Journal
|
||||||
|
|
|
||||||
|
|
@ -3,8 +3,8 @@ import { eq, desc, and, sql } from "drizzle-orm";
|
||||||
import { getDb } from "./db.ts";
|
import { getDb } from "./db.ts";
|
||||||
import { activities, routes, syncImports, users, follows } from "@trails-cool/db/schema/journal";
|
import { activities, routes, syncImports, users, follows } from "@trails-cool/db/schema/journal";
|
||||||
import type { Visibility } from "@trails-cool/db/schema/journal";
|
import type { Visibility } from "@trails-cool/db/schema/journal";
|
||||||
import { parseGpxAsync } from "@trails-cool/gpx";
|
import { validateGpx, writeGeom } from "./gpx-save.server.ts";
|
||||||
import { setGeomFromGpx } from "./routes.server.ts";
|
import type { GpxData } from "./gpx-save.server.ts";
|
||||||
|
|
||||||
export interface ActivityInput {
|
export interface ActivityInput {
|
||||||
name: string;
|
name: string;
|
||||||
|
|
@ -15,6 +15,7 @@ export interface ActivityInput {
|
||||||
duration?: number | null;
|
duration?: number | null;
|
||||||
startedAt?: Date | null;
|
startedAt?: Date | null;
|
||||||
visibility?: Visibility;
|
visibility?: Visibility;
|
||||||
|
synthetic?: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
export async function updateActivityVisibility(
|
export async function updateActivityVisibility(
|
||||||
|
|
@ -46,44 +47,45 @@ export async function createActivity(ownerId: string, input: ActivityInput) {
|
||||||
const db = getDb();
|
const db = getDb();
|
||||||
const id = randomUUID();
|
const id = randomUUID();
|
||||||
|
|
||||||
|
let parsed: GpxData | null = null;
|
||||||
let distance: number | null = input.distance ?? null;
|
let distance: number | null = input.distance ?? null;
|
||||||
let elevationGain: number | null = null;
|
let elevationGain: number | null = null;
|
||||||
let elevationLoss: number | null = null;
|
let elevationLoss: number | null = null;
|
||||||
let startedAt: Date | null = input.startedAt ?? null;
|
let startedAt: Date | null = input.startedAt ?? null;
|
||||||
const duration: number | null = input.duration ?? null;
|
const duration: number | null = input.duration ?? null;
|
||||||
if (input.gpx) {
|
|
||||||
try {
|
|
||||||
const gpxData = await parseGpxAsync(input.gpx);
|
|
||||||
distance = gpxData.distance || distance;
|
|
||||||
elevationGain = gpxData.elevation.gain;
|
|
||||||
elevationLoss = gpxData.elevation.loss;
|
|
||||||
|
|
||||||
if (!startedAt && gpxData.tracks[0]?.[0]?.time) {
|
if (input.gpx) {
|
||||||
startedAt = new Date(gpxData.tracks[0][0].time);
|
parsed = await validateGpx(input.gpx);
|
||||||
}
|
distance = parsed.distance || distance;
|
||||||
} catch {
|
elevationGain = parsed.elevation.gain;
|
||||||
// Continue without stats if GPX parsing fails
|
elevationLoss = parsed.elevation.loss;
|
||||||
|
if (!startedAt && parsed.tracks[0]?.[0]?.time) {
|
||||||
|
startedAt = new Date(parsed.tracks[0][0].time);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
await db.insert(activities).values({
|
await db.transaction(async (tx) => {
|
||||||
id,
|
await tx.insert(activities).values({
|
||||||
ownerId,
|
id,
|
||||||
routeId: input.routeId ?? null,
|
ownerId,
|
||||||
name: input.name,
|
routeId: input.routeId ?? null,
|
||||||
description: input.description ?? "",
|
name: input.name,
|
||||||
gpx: input.gpx,
|
description: input.description ?? "",
|
||||||
distance,
|
gpx: input.gpx,
|
||||||
duration,
|
distance,
|
||||||
elevationGain,
|
duration,
|
||||||
elevationLoss,
|
elevationGain,
|
||||||
startedAt,
|
elevationLoss,
|
||||||
...(input.visibility ? { visibility: input.visibility } : {}),
|
startedAt,
|
||||||
});
|
...(input.visibility ? { visibility: input.visibility } : {}),
|
||||||
|
...(input.synthetic ? { synthetic: true } : {}),
|
||||||
|
});
|
||||||
|
|
||||||
if (input.gpx) {
|
if (input.gpx && parsed) {
|
||||||
await setGeomFromGpx(id, "activities", input.gpx);
|
const coords = parsed.tracks.flat().map((p) => [p.lon, p.lat] as [number, number]);
|
||||||
}
|
await writeGeom(tx, id, "activities", coords);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
// Public activities at creation also fan out (matches the
|
// Public activities at creation also fan out (matches the
|
||||||
// updateActivityVisibility path for the case where visibility is set
|
// updateActivityVisibility path for the case where visibility is set
|
||||||
|
|
@ -236,26 +238,27 @@ export async function createRouteFromActivity(activityId: string, ownerId: strin
|
||||||
const [activity] = await db.select().from(activities).where(eq(activities.id, activityId));
|
const [activity] = await db.select().from(activities).where(eq(activities.id, activityId));
|
||||||
if (!activity?.gpx) return null;
|
if (!activity?.gpx) return null;
|
||||||
|
|
||||||
|
const parsed = await validateGpx(activity.gpx);
|
||||||
|
const coords = parsed.tracks.flat().map((p) => [p.lon, p.lat] as [number, number]);
|
||||||
const routeId = randomUUID();
|
const routeId = randomUUID();
|
||||||
await db.insert(routes).values({
|
|
||||||
id: routeId,
|
await db.transaction(async (tx) => {
|
||||||
ownerId,
|
await tx.insert(routes).values({
|
||||||
name: `Route from: ${activity.name}`,
|
id: routeId,
|
||||||
description: `Created from activity "${activity.name}"`,
|
ownerId,
|
||||||
gpx: activity.gpx,
|
name: `Route from: ${activity.name}`,
|
||||||
distance: activity.distance,
|
description: `Created from activity "${activity.name}"`,
|
||||||
elevationGain: activity.elevationGain,
|
gpx: activity.gpx,
|
||||||
elevationLoss: activity.elevationLoss,
|
distance: activity.distance,
|
||||||
|
elevationGain: activity.elevationGain,
|
||||||
|
elevationLoss: activity.elevationLoss,
|
||||||
|
});
|
||||||
|
|
||||||
|
await writeGeom(tx, routeId, "routes", coords);
|
||||||
|
|
||||||
|
await tx.update(activities).set({ routeId }).where(eq(activities.id, activityId));
|
||||||
});
|
});
|
||||||
|
|
||||||
await setGeomFromGpx(routeId, "routes", activity.gpx);
|
|
||||||
|
|
||||||
// Link the activity to the new route
|
|
||||||
await db
|
|
||||||
.update(activities)
|
|
||||||
.set({ routeId })
|
|
||||||
.where(eq(activities.id, activityId));
|
|
||||||
|
|
||||||
return routeId;
|
return routeId;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -4,9 +4,10 @@ import { z } from "zod";
|
||||||
import { and, eq, lt, sql } from "drizzle-orm";
|
import { and, eq, lt, sql } from "drizzle-orm";
|
||||||
import { getDb } from "./db.ts";
|
import { getDb } from "./db.ts";
|
||||||
import { activities, routes, users } from "@trails-cool/db/schema/journal";
|
import { activities, routes, users } from "@trails-cool/db/schema/journal";
|
||||||
import { setGeomFromGpx } from "./routes.server.ts";
|
import { createRoute } from "./routes.server.ts";
|
||||||
|
import { createActivity } from "./activities.server.ts";
|
||||||
|
import { validateGpx } from "./gpx-save.server.ts";
|
||||||
import { TERMS_VERSION } from "./legal.ts";
|
import { TERMS_VERSION } from "./legal.ts";
|
||||||
import { parseGpxAsync } from "@trails-cool/gpx";
|
|
||||||
import { logger } from "./logger.server.ts";
|
import { logger } from "./logger.server.ts";
|
||||||
import {
|
import {
|
||||||
demoBotSyntheticActivitiesTotal,
|
demoBotSyntheticActivitiesTotal,
|
||||||
|
|
@ -648,59 +649,40 @@ export async function generateOneWalk(
|
||||||
const name = templateName(now, locale, persona);
|
const name = templateName(now, locale, persona);
|
||||||
const description = templateDescription(now, locale, persona);
|
const description = templateDescription(now, locale, persona);
|
||||||
|
|
||||||
let distance: number;
|
const parsed = await validateGpx(result.gpx);
|
||||||
let elevationGain: number;
|
const distance = parsed.distance;
|
||||||
let elevationLoss: number;
|
const elevationGain = parsed.elevation.gain;
|
||||||
try {
|
const elevationLoss = parsed.elevation.loss;
|
||||||
const parsed = await parseGpxAsync(result.gpx);
|
|
||||||
distance = parsed.distance;
|
|
||||||
elevationGain = parsed.elevation.gain;
|
|
||||||
elevationLoss = parsed.elevation.loss;
|
|
||||||
} catch {
|
|
||||||
return null;
|
|
||||||
}
|
|
||||||
|
|
||||||
if (!distance || distance < 500) return null;
|
if (!distance || distance < 500) return null;
|
||||||
|
|
||||||
const db = getDb();
|
|
||||||
const routeId = randomUUID();
|
|
||||||
const activityId = randomUUID();
|
|
||||||
|
|
||||||
await db.insert(routes).values({
|
|
||||||
id: routeId,
|
|
||||||
ownerId,
|
|
||||||
name,
|
|
||||||
description,
|
|
||||||
gpx: result.gpx,
|
|
||||||
routingProfile: "trekking",
|
|
||||||
distance,
|
|
||||||
elevationGain,
|
|
||||||
elevationLoss,
|
|
||||||
visibility: "public",
|
|
||||||
synthetic: true,
|
|
||||||
});
|
|
||||||
await setGeomFromGpx(routeId, "routes", result.gpx);
|
|
||||||
|
|
||||||
const walkingMetersPerSecond = 4.5 * 1000 / 3600; // ~4.5 km/h
|
const walkingMetersPerSecond = 4.5 * 1000 / 3600; // ~4.5 km/h
|
||||||
const jitter = 0.85 + Math.random() * 0.3; // ±15 %
|
const jitter = 0.85 + Math.random() * 0.3; // ±15 %
|
||||||
const durationSeconds = Math.round((distance / walkingMetersPerSecond) * jitter);
|
const durationSeconds = Math.round((distance / walkingMetersPerSecond) * jitter);
|
||||||
|
|
||||||
await db.insert(activities).values({
|
const routeId = await createRoute(ownerId, {
|
||||||
id: activityId,
|
|
||||||
ownerId,
|
|
||||||
routeId,
|
|
||||||
name,
|
name,
|
||||||
description,
|
description,
|
||||||
gpx: result.gpx,
|
gpx: result.gpx,
|
||||||
startedAt: now,
|
routingProfile: "trekking",
|
||||||
duration: durationSeconds,
|
visibility: "public",
|
||||||
|
synthetic: true,
|
||||||
distance,
|
distance,
|
||||||
elevationGain,
|
elevationGain,
|
||||||
elevationLoss,
|
elevationLoss,
|
||||||
|
});
|
||||||
|
|
||||||
|
await createActivity(ownerId, {
|
||||||
|
name,
|
||||||
|
description,
|
||||||
|
gpx: result.gpx,
|
||||||
|
routeId,
|
||||||
|
startedAt: now,
|
||||||
|
duration: durationSeconds,
|
||||||
|
distance,
|
||||||
visibility: "public",
|
visibility: "public",
|
||||||
synthetic: true,
|
synthetic: true,
|
||||||
});
|
});
|
||||||
await setGeomFromGpx(activityId, "activities", result.gpx);
|
|
||||||
|
|
||||||
return routeId;
|
return routeId;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
86
apps/journal/app/lib/gpx-save.server.test.ts
Normal file
86
apps/journal/app/lib/gpx-save.server.test.ts
Normal file
|
|
@ -0,0 +1,86 @@
|
||||||
|
import { describe, it, expect } from "vitest";
|
||||||
|
import { validateGpx, GpxValidationError } from "./gpx-save.server.ts";
|
||||||
|
|
||||||
|
const VALID_GPX = `<?xml version="1.0" encoding="UTF-8"?>
|
||||||
|
<gpx version="1.1" xmlns="http://www.topografix.com/GPX/1/1">
|
||||||
|
<trk>
|
||||||
|
<trkseg>
|
||||||
|
<trkpt lat="47.0" lon="8.0"><ele>500</ele></trkpt>
|
||||||
|
<trkpt lat="47.1" lon="8.1"><ele>520</ele></trkpt>
|
||||||
|
<trkpt lat="47.2" lon="8.2"><ele>510</ele></trkpt>
|
||||||
|
</trkseg>
|
||||||
|
</trk>
|
||||||
|
</gpx>`;
|
||||||
|
|
||||||
|
const ONE_POINT_GPX = `<?xml version="1.0" encoding="UTF-8"?>
|
||||||
|
<gpx version="1.1" xmlns="http://www.topografix.com/GPX/1/1">
|
||||||
|
<trk>
|
||||||
|
<trkseg>
|
||||||
|
<trkpt lat="47.0" lon="8.0"><ele>500</ele></trkpt>
|
||||||
|
</trkseg>
|
||||||
|
</trk>
|
||||||
|
</gpx>`;
|
||||||
|
|
||||||
|
const EMPTY_GPX = `<?xml version="1.0" encoding="UTF-8"?>
|
||||||
|
<gpx version="1.1" xmlns="http://www.topografix.com/GPX/1/1">
|
||||||
|
<trk><trkseg></trkseg></trk>
|
||||||
|
</gpx>`;
|
||||||
|
|
||||||
|
const OUT_OF_RANGE_LAT_GPX = `<?xml version="1.0" encoding="UTF-8"?>
|
||||||
|
<gpx version="1.1" xmlns="http://www.topografix.com/GPX/1/1">
|
||||||
|
<trk>
|
||||||
|
<trkseg>
|
||||||
|
<trkpt lat="91.0" lon="8.0"></trkpt>
|
||||||
|
<trkpt lat="47.0" lon="8.0"></trkpt>
|
||||||
|
</trkseg>
|
||||||
|
</trk>
|
||||||
|
</gpx>`;
|
||||||
|
|
||||||
|
const OUT_OF_RANGE_LON_GPX = `<?xml version="1.0" encoding="UTF-8"?>
|
||||||
|
<gpx version="1.1" xmlns="http://www.topografix.com/GPX/1/1">
|
||||||
|
<trk>
|
||||||
|
<trkseg>
|
||||||
|
<trkpt lat="47.0" lon="181.0"></trkpt>
|
||||||
|
<trkpt lat="47.1" lon="8.0"></trkpt>
|
||||||
|
</trkseg>
|
||||||
|
</trk>
|
||||||
|
</gpx>`;
|
||||||
|
|
||||||
|
describe("validateGpx", () => {
|
||||||
|
it("returns parsed GpxData for valid GPX", async () => {
|
||||||
|
const result = await validateGpx(VALID_GPX);
|
||||||
|
expect(result.tracks.flat().length).toBe(3);
|
||||||
|
expect(result.tracks[0]![0]!.lat).toBeCloseTo(47.0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("throws GpxValidationError for GPX with fewer than 2 track points", async () => {
|
||||||
|
await expect(validateGpx(ONE_POINT_GPX)).rejects.toThrow(GpxValidationError);
|
||||||
|
await expect(validateGpx(ONE_POINT_GPX)).rejects.toThrow("at least 2 track points");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("throws GpxValidationError for GPX with zero track points", async () => {
|
||||||
|
await expect(validateGpx(EMPTY_GPX)).rejects.toThrow(GpxValidationError);
|
||||||
|
await expect(validateGpx(EMPTY_GPX)).rejects.toThrow("at least 2 track points");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("throws GpxValidationError for out-of-range latitude", async () => {
|
||||||
|
await expect(validateGpx(OUT_OF_RANGE_LAT_GPX)).rejects.toThrow(GpxValidationError);
|
||||||
|
await expect(validateGpx(OUT_OF_RANGE_LAT_GPX)).rejects.toThrow("out-of-range coordinates");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("throws GpxValidationError for out-of-range longitude", async () => {
|
||||||
|
await expect(validateGpx(OUT_OF_RANGE_LON_GPX)).rejects.toThrow(GpxValidationError);
|
||||||
|
await expect(validateGpx(OUT_OF_RANGE_LON_GPX)).rejects.toThrow("out-of-range coordinates");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("throws GpxValidationError for unparseable XML", async () => {
|
||||||
|
await expect(validateGpx("not xml at all")).rejects.toThrow(GpxValidationError);
|
||||||
|
await expect(validateGpx("<broken<xml")).rejects.toThrow(GpxValidationError);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("GpxValidationError has the correct name", async () => {
|
||||||
|
const err = await validateGpx(EMPTY_GPX).catch((e) => e);
|
||||||
|
expect(err).toBeInstanceOf(GpxValidationError);
|
||||||
|
expect(err.name).toBe("GpxValidationError");
|
||||||
|
});
|
||||||
|
});
|
||||||
66
apps/journal/app/lib/gpx-save.server.ts
Normal file
66
apps/journal/app/lib/gpx-save.server.ts
Normal file
|
|
@ -0,0 +1,66 @@
|
||||||
|
import { sql } from "drizzle-orm";
|
||||||
|
import type { Database } from "@trails-cool/db";
|
||||||
|
import { parseGpxAsync } from "@trails-cool/gpx";
|
||||||
|
import type { GpxData } from "@trails-cool/gpx";
|
||||||
|
|
||||||
|
// Re-export so callers only need one import.
|
||||||
|
export type { GpxData };
|
||||||
|
|
||||||
|
export class GpxValidationError extends Error {
|
||||||
|
constructor(message: string) {
|
||||||
|
super(message);
|
||||||
|
this.name = "GpxValidationError";
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Parse and validate a GPX string. Returns the parsed data so callers
|
||||||
|
* don't need to parse again for stat extraction. Throws GpxValidationError
|
||||||
|
* when the GPX is malformed, has fewer than 2 track points, or contains
|
||||||
|
* out-of-range coordinates.
|
||||||
|
*/
|
||||||
|
export async function validateGpx(gpx: string): Promise<GpxData> {
|
||||||
|
let parsed: GpxData;
|
||||||
|
try {
|
||||||
|
parsed = await parseGpxAsync(gpx);
|
||||||
|
} catch (e) {
|
||||||
|
throw new GpxValidationError(`GPX parse failed: ${(e as Error).message}`);
|
||||||
|
}
|
||||||
|
|
||||||
|
const points = parsed.tracks.flat();
|
||||||
|
if (points.length < 2) {
|
||||||
|
throw new GpxValidationError(
|
||||||
|
`GPX must contain at least 2 track points (got ${points.length})`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
for (const p of points) {
|
||||||
|
if (p.lat < -90 || p.lat > 90 || p.lon < -180 || p.lon > 180) {
|
||||||
|
throw new GpxValidationError(
|
||||||
|
`GPX contains out-of-range coordinates: lat=${p.lat}, lon=${p.lon}`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return parsed;
|
||||||
|
}
|
||||||
|
|
||||||
|
type Tx = Parameters<Parameters<Database["transaction"]>[0]>[0];
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Write a PostGIS LineString geometry from already-validated track points.
|
||||||
|
* Must be called inside a db.transaction() — the tx client is required so
|
||||||
|
* the geometry write participates in the enclosing transaction.
|
||||||
|
* Throws on failure; callers must NOT swallow the error.
|
||||||
|
*/
|
||||||
|
export async function writeGeom(
|
||||||
|
tx: Tx,
|
||||||
|
id: string,
|
||||||
|
table: "routes" | "activities",
|
||||||
|
coords: Array<[number, number]>,
|
||||||
|
): Promise<void> {
|
||||||
|
const geojson = JSON.stringify({ type: "LineString", coordinates: coords });
|
||||||
|
await tx.execute(
|
||||||
|
sql`UPDATE ${sql.identifier("journal")}.${sql.identifier(table)} SET geom = ST_GeomFromGeoJSON(${geojson}) WHERE id = ${id}`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
@ -3,8 +3,9 @@ import { eq, desc, and } from "drizzle-orm";
|
||||||
import { getDb } from "./db.ts";
|
import { getDb } from "./db.ts";
|
||||||
import { routes, routeVersions } from "@trails-cool/db/schema/journal";
|
import { routes, routeVersions } from "@trails-cool/db/schema/journal";
|
||||||
import type { Visibility } from "@trails-cool/db/schema/journal";
|
import type { Visibility } from "@trails-cool/db/schema/journal";
|
||||||
import { parseGpxAsync } from "@trails-cool/gpx";
|
|
||||||
import { sql } from "drizzle-orm";
|
import { sql } from "drizzle-orm";
|
||||||
|
import { validateGpx, writeGeom } from "./gpx-save.server.ts";
|
||||||
|
import type { GpxData } from "./gpx-save.server.ts";
|
||||||
|
|
||||||
export interface RouteInput {
|
export interface RouteInput {
|
||||||
name: string;
|
name: string;
|
||||||
|
|
@ -12,52 +13,65 @@ export interface RouteInput {
|
||||||
gpx?: string;
|
gpx?: string;
|
||||||
routingProfile?: string;
|
routingProfile?: string;
|
||||||
visibility?: Visibility;
|
visibility?: Visibility;
|
||||||
|
// Pre-computed stats — when provided, skip re-parsing (used by demo-bot)
|
||||||
|
distance?: number | null;
|
||||||
|
elevationGain?: number | null;
|
||||||
|
elevationLoss?: number | null;
|
||||||
|
dayBreaks?: number[];
|
||||||
|
synthetic?: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
export async function createRoute(ownerId: string, input: RouteInput) {
|
export async function createRoute(ownerId: string, input: RouteInput) {
|
||||||
const db = getDb();
|
const db = getDb();
|
||||||
const id = randomUUID();
|
const id = randomUUID();
|
||||||
|
|
||||||
let distance: number | null = null;
|
let parsed: GpxData | null = null;
|
||||||
let elevationGain: number | null = null;
|
let distance: number | null = input.distance ?? null;
|
||||||
let elevationLoss: number | null = null;
|
let elevationGain: number | null = input.elevationGain ?? null;
|
||||||
let dayBreaks: number[] = [];
|
let elevationLoss: number | null = input.elevationLoss ?? null;
|
||||||
|
let dayBreaks: number[] = input.dayBreaks ?? [];
|
||||||
|
|
||||||
if (input.gpx) {
|
if (input.gpx) {
|
||||||
const stats = await computeRouteStats(input.gpx);
|
parsed = await validateGpx(input.gpx);
|
||||||
distance = stats.distance;
|
// Only compute stats from GPX if not pre-supplied by caller
|
||||||
elevationGain = stats.elevationGain;
|
if (input.distance === undefined) {
|
||||||
elevationLoss = stats.elevationLoss;
|
const stats = computeRouteStats(parsed);
|
||||||
dayBreaks = stats.dayBreaks;
|
distance = stats.distance;
|
||||||
|
elevationGain = stats.elevationGain;
|
||||||
|
elevationLoss = stats.elevationLoss;
|
||||||
|
dayBreaks = stats.dayBreaks;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
await db.insert(routes).values({
|
await db.transaction(async (tx) => {
|
||||||
id,
|
await tx.insert(routes).values({
|
||||||
ownerId,
|
id,
|
||||||
name: input.name,
|
ownerId,
|
||||||
description: input.description ?? "",
|
name: input.name,
|
||||||
gpx: input.gpx,
|
description: input.description ?? "",
|
||||||
routingProfile: input.routingProfile,
|
|
||||||
distance,
|
|
||||||
elevationGain,
|
|
||||||
elevationLoss,
|
|
||||||
dayBreaks,
|
|
||||||
});
|
|
||||||
|
|
||||||
if (input.gpx) {
|
|
||||||
await setGeomFromGpx(id, "routes", input.gpx);
|
|
||||||
}
|
|
||||||
|
|
||||||
// Create initial version if GPX provided
|
|
||||||
if (input.gpx) {
|
|
||||||
await db.insert(routeVersions).values({
|
|
||||||
id: randomUUID(),
|
|
||||||
routeId: id,
|
|
||||||
version: 1,
|
|
||||||
gpx: input.gpx,
|
gpx: input.gpx,
|
||||||
createdBy: ownerId,
|
routingProfile: input.routingProfile,
|
||||||
changeDescription: "Initial version",
|
distance,
|
||||||
|
elevationGain,
|
||||||
|
elevationLoss,
|
||||||
|
dayBreaks,
|
||||||
|
...(input.synthetic ? { synthetic: true } : {}),
|
||||||
});
|
});
|
||||||
}
|
|
||||||
|
if (input.gpx && parsed) {
|
||||||
|
const coords = parsed.tracks.flat().map((p) => [p.lon, p.lat] as [number, number]);
|
||||||
|
await writeGeom(tx, id, "routes", coords);
|
||||||
|
|
||||||
|
await tx.insert(routeVersions).values({
|
||||||
|
id: randomUUID(),
|
||||||
|
routeId: id,
|
||||||
|
version: 1,
|
||||||
|
gpx: input.gpx,
|
||||||
|
createdBy: ownerId,
|
||||||
|
changeDescription: "Initial version",
|
||||||
|
});
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
return id;
|
return id;
|
||||||
}
|
}
|
||||||
|
|
@ -92,7 +106,6 @@ export async function listRoutes(ownerId: string) {
|
||||||
.where(eq(routes.ownerId, ownerId))
|
.where(eq(routes.ownerId, ownerId))
|
||||||
.orderBy(desc(routes.updatedAt));
|
.orderBy(desc(routes.updatedAt));
|
||||||
|
|
||||||
// Batch-fetch simplified GeoJSON for list thumbnails
|
|
||||||
const ids = rows.map((r) => r.id);
|
const ids = rows.map((r) => r.id);
|
||||||
const geojsonMap = ids.length > 0 ? await getSimplifiedGeojsonBatch(ids) : new Map();
|
const geojsonMap = ids.length > 0 ? await getSimplifiedGeojsonBatch(ids) : new Map();
|
||||||
return rows.map((r) => ({ ...r, geojson: geojsonMap.get(r.id) ?? null }));
|
return rows.map((r) => ({ ...r, geojson: geojsonMap.get(r.id) ?? null }));
|
||||||
|
|
@ -122,14 +135,19 @@ export async function updateRoute(
|
||||||
) {
|
) {
|
||||||
const db = getDb();
|
const db = getDb();
|
||||||
|
|
||||||
|
let parsed: GpxData | null = null;
|
||||||
|
if (input.gpx) {
|
||||||
|
parsed = await validateGpx(input.gpx);
|
||||||
|
}
|
||||||
|
|
||||||
const updateData: Record<string, unknown> = { updatedAt: new Date() };
|
const updateData: Record<string, unknown> = { updatedAt: new Date() };
|
||||||
if (input.name !== undefined) updateData.name = input.name;
|
if (input.name !== undefined) updateData.name = input.name;
|
||||||
if (input.description !== undefined) updateData.description = input.description;
|
if (input.description !== undefined) updateData.description = input.description;
|
||||||
if (input.visibility !== undefined) updateData.visibility = input.visibility;
|
if (input.visibility !== undefined) updateData.visibility = input.visibility;
|
||||||
|
|
||||||
if (input.gpx) {
|
if (input.gpx && parsed) {
|
||||||
|
const stats = computeRouteStats(parsed);
|
||||||
updateData.gpx = input.gpx;
|
updateData.gpx = input.gpx;
|
||||||
const stats = await computeRouteStats(input.gpx);
|
|
||||||
updateData.distance = stats.distance;
|
updateData.distance = stats.distance;
|
||||||
updateData.elevationGain = stats.elevationGain;
|
updateData.elevationGain = stats.elevationGain;
|
||||||
updateData.elevationLoss = stats.elevationLoss;
|
updateData.elevationLoss = stats.elevationLoss;
|
||||||
|
|
@ -137,33 +155,35 @@ export async function updateRoute(
|
||||||
if (stats.description && input.description === undefined) {
|
if (stats.description && input.description === undefined) {
|
||||||
updateData.description = stats.description;
|
updateData.description = stats.description;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Get next version number
|
|
||||||
const existingVersions = await db
|
|
||||||
.select()
|
|
||||||
.from(routeVersions)
|
|
||||||
.where(eq(routeVersions.routeId, id))
|
|
||||||
.orderBy(desc(routeVersions.version));
|
|
||||||
|
|
||||||
const nextVersion = (existingVersions[0]?.version ?? 0) + 1;
|
|
||||||
|
|
||||||
await db.insert(routeVersions).values({
|
|
||||||
id: randomUUID(),
|
|
||||||
routeId: id,
|
|
||||||
version: nextVersion,
|
|
||||||
gpx: input.gpx,
|
|
||||||
createdBy: ownerId,
|
|
||||||
});
|
|
||||||
}
|
}
|
||||||
|
|
||||||
await db
|
await db.transaction(async (tx) => {
|
||||||
.update(routes)
|
await tx
|
||||||
.set(updateData)
|
.update(routes)
|
||||||
.where(and(eq(routes.id, id), eq(routes.ownerId, ownerId)));
|
.set(updateData)
|
||||||
|
.where(and(eq(routes.id, id), eq(routes.ownerId, ownerId)));
|
||||||
|
|
||||||
if (input.gpx) {
|
if (input.gpx && parsed) {
|
||||||
await setGeomFromGpx(id, "routes", input.gpx);
|
const coords = parsed.tracks.flat().map((p) => [p.lon, p.lat] as [number, number]);
|
||||||
}
|
await writeGeom(tx, id, "routes", coords);
|
||||||
|
|
||||||
|
const existingVersions = await tx
|
||||||
|
.select()
|
||||||
|
.from(routeVersions)
|
||||||
|
.where(eq(routeVersions.routeId, id))
|
||||||
|
.orderBy(desc(routeVersions.version));
|
||||||
|
|
||||||
|
const nextVersion = (existingVersions[0]?.version ?? 0) + 1;
|
||||||
|
|
||||||
|
await tx.insert(routeVersions).values({
|
||||||
|
id: randomUUID(),
|
||||||
|
routeId: id,
|
||||||
|
version: nextVersion,
|
||||||
|
gpx: input.gpx,
|
||||||
|
createdBy: ownerId,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
export async function deleteRoute(id: string, ownerId: string) {
|
export async function deleteRoute(id: string, ownerId: string) {
|
||||||
|
|
@ -175,41 +195,19 @@ export async function deleteRoute(id: string, ownerId: string) {
|
||||||
return result.length > 0;
|
return result.length > 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
async function computeRouteStats(gpxString: string) {
|
function computeRouteStats(gpxData: GpxData) {
|
||||||
try {
|
const dayBreaks = gpxData.waypoints
|
||||||
const gpxData = await parseGpxAsync(gpxString);
|
.map((w, i) => (w.isDayBreak ? i : -1))
|
||||||
const dayBreaks = gpxData.waypoints
|
.filter((i) => i >= 0);
|
||||||
.map((w, i) => (w.isDayBreak ? i : -1))
|
return {
|
||||||
.filter((i) => i >= 0);
|
distance: gpxData.distance,
|
||||||
return {
|
elevationGain: gpxData.elevation.gain,
|
||||||
distance: gpxData.distance,
|
elevationLoss: gpxData.elevation.loss,
|
||||||
elevationGain: gpxData.elevation.gain,
|
dayBreaks,
|
||||||
elevationLoss: gpxData.elevation.loss,
|
description: gpxData.description,
|
||||||
dayBreaks,
|
};
|
||||||
description: gpxData.description,
|
|
||||||
};
|
|
||||||
} catch {
|
|
||||||
return { distance: null, elevationGain: null, elevationLoss: null, dayBreaks: [] as number[], description: undefined };
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
async function setGeomFromGpx(id: string, table: "routes" | "activities", gpxString: string) {
|
|
||||||
try {
|
|
||||||
const gpxData = await parseGpxAsync(gpxString);
|
|
||||||
const coords = gpxData.tracks.flat().map((p) => [p.lon, p.lat] as [number, number]);
|
|
||||||
if (coords.length < 2) return;
|
|
||||||
const geojson = JSON.stringify({ type: "LineString", coordinates: coords });
|
|
||||||
const db = getDb();
|
|
||||||
await db.execute(
|
|
||||||
sql`UPDATE ${sql.identifier("journal")}.${sql.identifier(table)} SET geom = ST_GeomFromGeoJSON(${geojson}) WHERE id = ${id}`,
|
|
||||||
);
|
|
||||||
} catch (e) {
|
|
||||||
console.error(`Failed to set geom for ${table}/${id}:`, e);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
export { setGeomFromGpx };
|
|
||||||
|
|
||||||
async function getGeojson(table: "routes" | "activities", id: string): Promise<string | null> {
|
async function getGeojson(table: "routes" | "activities", id: string): Promise<string | null> {
|
||||||
try {
|
try {
|
||||||
const db = getDb();
|
const db = getDb();
|
||||||
|
|
|
||||||
|
|
@ -2,6 +2,7 @@ import { data } from "react-router";
|
||||||
import type { Route } from "./+types/api.routes.$id.callback";
|
import type { Route } from "./+types/api.routes.$id.callback";
|
||||||
import { verifyRouteToken } from "~/lib/jwt.server";
|
import { verifyRouteToken } from "~/lib/jwt.server";
|
||||||
import { updateRoute, getRoute } from "~/lib/routes.server";
|
import { updateRoute, getRoute } from "~/lib/routes.server";
|
||||||
|
import { GpxValidationError } from "~/lib/gpx-save.server";
|
||||||
|
|
||||||
const PLANNER_ORIGIN = process.env.PLANNER_URL ?? "http://localhost:3001";
|
const PLANNER_ORIGIN = process.env.PLANNER_URL ?? "http://localhost:3001";
|
||||||
|
|
||||||
|
|
@ -65,6 +66,9 @@ export async function action({ params, request }: Route.ActionArgs) {
|
||||||
|
|
||||||
return data({ success: true, routeId: params.id }, { headers: corsHeaders() });
|
return data({ success: true, routeId: params.id }, { headers: corsHeaders() });
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
|
if (e instanceof GpxValidationError) {
|
||||||
|
return data({ error: e.message }, { status: 400, headers: corsHeaders() });
|
||||||
|
}
|
||||||
return data({ error: (e as Error).message }, { status: 401, headers: corsHeaders() });
|
return data({ error: (e as Error).message }, { status: 401, headers: corsHeaders() });
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
28
docs/adr/0006-atomic-gpx-save.md
Normal file
28
docs/adr/0006-atomic-gpx-save.md
Normal file
|
|
@ -0,0 +1,28 @@
|
||||||
|
# ADR-0006: Atomic GPX Save via gpx-save Module
|
||||||
|
|
||||||
|
**Status:** Accepted
|
||||||
|
**Date:** 2026-05-10
|
||||||
|
|
||||||
|
## Context
|
||||||
|
|
||||||
|
Routes and activities store GPX text in a `gpx` column and a derived PostGIS LineString in a `geom` column. Originally, the row insert and the `ST_GeomFromGeoJSON` update were separate statements issued outside any transaction. If the geometry write failed (e.g. PostGIS unavailable, malformed coordinates), the row would be committed with `geom IS NULL` and no error surfaced. This silent failure made the geom column unreliable and was invisible to callers.
|
||||||
|
|
||||||
|
Additionally, GPX validation (parse, minimum track points, coordinate range check) was either absent or scattered across callers, with some paths swallowing parse errors and continuing without geometry.
|
||||||
|
|
||||||
|
## Decision
|
||||||
|
|
||||||
|
1. **Single module owns geometry persistence.** `apps/journal/app/lib/gpx-save.server.ts` is the sole place that validates GPX and writes PostGIS geometry. No other module issues raw `UPDATE … SET geom = …` statements.
|
||||||
|
|
||||||
|
2. **`validateGpx(gpx) → Promise<GpxData>`** parses the GPX string, asserts ≥ 2 track points, and checks all coordinates are within valid ranges (lat −90..90, lon −180..180). It throws `GpxValidationError` on any failure. It returns the parsed result so callers don't re-parse for stat extraction.
|
||||||
|
|
||||||
|
3. **`writeGeom(tx, id, table, coords)`** accepts a Drizzle transaction client and writes the PostGIS geometry inside that transaction. It throws on failure — no try/catch anywhere in the call chain.
|
||||||
|
|
||||||
|
4. **Every save that includes GPX is wrapped in `db.transaction()`** covering: the row insert/update, the `writeGeom` call, and any version snapshot insert. A PostGIS failure rolls back the entire transaction — a row with `gpx IS NOT NULL` and `geom IS NULL` cannot be produced through the normal save path.
|
||||||
|
|
||||||
|
5. **Fail loudly everywhere.** The demo-bot and all callers surface errors rather than swallowing them. The demo-bot uses `createRoute` / `createActivity` rather than raw inserts.
|
||||||
|
|
||||||
|
## Consequences
|
||||||
|
|
||||||
|
- `GpxValidationError` surfaces to API callers. The Planner callback endpoint (`api.routes.$id.callback.ts`) catches it and returns 400 so the Planner receives a typed error.
|
||||||
|
- The `synthetic` flag is threaded through `RouteInput` and `ActivityInput` so the demo-bot can mark rows without bypassing the shared save functions.
|
||||||
|
- Any future module that needs to persist geometry **must** go through `gpx-save.server.ts`. Do not re-introduce inline `UPDATE … SET geom = …` statements elsewhere.
|
||||||
|
|
@ -0,0 +1,2 @@
|
||||||
|
schema: spec-driven
|
||||||
|
created: 2026-05-10
|
||||||
101
openspec/changes/archive/2026-05-10-atomic-gpx-save/design.md
Normal file
101
openspec/changes/archive/2026-05-10-atomic-gpx-save/design.md
Normal file
|
|
@ -0,0 +1,101 @@
|
||||||
|
## Context
|
||||||
|
|
||||||
|
Every write of a GPX track in the Journal follows a two-step pattern: insert/update the row, then call `setGeomFromGpx` to write the PostGIS `geom` column via a raw `UPDATE`. The two steps are not in a transaction. `setGeomFromGpx` wraps its error in a `console.error` and returns, leaving the row with `geom IS NULL`. This affects `createRoute`, `updateRoute`, `createActivity`, `createRouteFromActivity`, and `demo-bot.server.ts` (which bypasses the save functions entirely with raw inserts).
|
||||||
|
|
||||||
|
`setGeomFromGpx` is currently exported from `routes.server.ts` and imported by `activities.server.ts` and `demo-bot.server.ts`, making it a de-facto shared utility that lives in the wrong module.
|
||||||
|
|
||||||
|
## Goals / Non-Goals
|
||||||
|
|
||||||
|
**Goals:**
|
||||||
|
- `geom IS NULL` on a row that has `gpx` set is impossible through the normal save path.
|
||||||
|
- GPX is validated (≥2 track points, valid coordinate ranges) before any DB write.
|
||||||
|
- All callers fail loudly on invalid or unpersistable GPX — no silent degradation.
|
||||||
|
- PostGIS write participates in the same `db.transaction()` as the row write and version snapshot.
|
||||||
|
- `setGeomFromGpx` is internal to the new `gpx-save.server.ts` module; no other file calls it.
|
||||||
|
- `demo-bot.server.ts` uses `createRoute` / `createActivity` and gets the invariant for free.
|
||||||
|
|
||||||
|
**Non-Goals:**
|
||||||
|
- No API surface changes.
|
||||||
|
- No DB schema changes or migrations.
|
||||||
|
- No changes to the `@trails-cool/gpx` parser.
|
||||||
|
- No changes to how the Planner callback endpoint validates its authorization (JWT stays as-is); validation of the GPX payload is now handled inside `updateRoute`.
|
||||||
|
- No changes to `getGeojson` / `getSimplifiedGeojsonBatch` — read paths are out of scope.
|
||||||
|
- No fix for the O(N) `getSimplifiedGeojsonBatch` query pattern (separate concern).
|
||||||
|
|
||||||
|
## Decisions
|
||||||
|
|
||||||
|
### 1. New module: `gpx-save.server.ts`
|
||||||
|
|
||||||
|
`setGeomFromGpx` belongs in neither `routes.server.ts` nor `activities.server.ts` — it is shared infrastructure. A dedicated `apps/journal/app/lib/gpx-save.server.ts` owns validation and geometry persistence. `routes.server.ts` and `activities.server.ts` both import from it.
|
||||||
|
|
||||||
|
Exports:
|
||||||
|
- `validateGpx(gpx: string): Promise<ParsedGpx>` — public
|
||||||
|
- `writeGeom(tx, id, table, coords): Promise<void>` — internal (not exported)
|
||||||
|
- `GpxValidationError` — public (callers catch it to return 400)
|
||||||
|
|
||||||
|
The existing `setGeomFromGpx` export from `routes.server.ts` is removed. Any file that currently imports it must migrate.
|
||||||
|
|
||||||
|
### 2. Validation inside save functions, not at call sites
|
||||||
|
|
||||||
|
`validateGpx` is called at the start of `createRoute`, `updateRoute`, `createActivity`, and `createRouteFromActivity` — before any transaction is opened. This means:
|
||||||
|
|
||||||
|
- Every caller gets validation for free, including the mobile API (`api.v1.routes`) and the Planner callback.
|
||||||
|
- The callback endpoint does not need its own validation logic; it catches `GpxValidationError` and returns 400.
|
||||||
|
- Future callers can't bypass validation by accident.
|
||||||
|
|
||||||
|
Alternative considered: validate only at the callback boundary. Rejected because it leaves mobile API and direct DB writes unprotected.
|
||||||
|
|
||||||
|
### 3. `db.transaction()` wrapping row write + geom write + version snapshot
|
||||||
|
|
||||||
|
Drizzle supports `db.transaction(async (tx) => { ... })`. The raw PostGIS `UPDATE` inside `writeGeom` receives `tx` as its first argument and executes within the same transaction. If `writeGeom` throws, Drizzle rolls back the entire transaction — the row insert is reversed.
|
||||||
|
|
||||||
|
```
|
||||||
|
db.transaction(async (tx) => {
|
||||||
|
// 1. validateGpx already ran (before tx opened)
|
||||||
|
await tx.insert(routes).values(...) // or update
|
||||||
|
await writeGeom(tx, id, "routes", coords) // throws → rollback
|
||||||
|
await tx.insert(routeVersions).values(...)
|
||||||
|
})
|
||||||
|
```
|
||||||
|
|
||||||
|
The transaction is opened *after* `validateGpx` succeeds. This keeps the transaction as short as possible (no async GPX parsing inside a held transaction).
|
||||||
|
|
||||||
|
### 4. `activities.server.ts` stat extraction uses `validateGpx` result
|
||||||
|
|
||||||
|
Currently `createActivity` calls `parseGpxAsync` inline to extract `distance`, `elevationGain`, `elevationLoss`, `startedAt`. After this change, `validateGpx` returns the `ParsedGpx` object. `createActivity` uses that result directly — one parse, not two.
|
||||||
|
|
||||||
|
The activity-specific fields (`startedAt` from `gpxData.tracks[0][0].time`) are still extracted by `createActivity`; `validateGpx` returns the full parsed object so callers have access to everything.
|
||||||
|
|
||||||
|
### 5. `demo-bot.server.ts` migrates to `createRoute` / `createActivity`
|
||||||
|
|
||||||
|
The demo-bot currently does:
|
||||||
|
```ts
|
||||||
|
await db.insert(routes).values({ ... distance, elevationGain, ... })
|
||||||
|
await setGeomFromGpx(routeId, "routes", result.gpx)
|
||||||
|
await db.insert(activities).values({ ... })
|
||||||
|
await setGeomFromGpx(activityId, "activities", result.gpx)
|
||||||
|
```
|
||||||
|
|
||||||
|
After migration it calls `createRoute` and `createActivity`, which own all of this. The demo-bot passes pre-computed stats via `RouteInput` / `ActivityInput` (already supported: the input types accept optional `distance`, `elevationGain`, etc.). `createRoute` will prefer input stats over re-parsed stats when provided, avoiding redundant parsing for the demo-bot's case where BRouter already computed them.
|
||||||
|
|
||||||
|
Alternative considered: leave demo-bot as-is and just make `setGeomFromGpx` throw. Rejected because it keeps a second bypass path alive and requires demo-bot to handle its own transaction.
|
||||||
|
|
||||||
|
### 6. ADR recorded
|
||||||
|
|
||||||
|
ADR-0006 documents that PostGIS geometry writes are always transactional with row writes, so future code doesn't re-introduce the split pattern.
|
||||||
|
|
||||||
|
## Risks / Trade-offs
|
||||||
|
|
||||||
|
- **Transaction duration**: Opening a DB transaction and then doing a raw SQL UPDATE adds a short hold on the row. For typical GPX sizes this is negligible — `validateGpx` runs before the transaction opens, so the held time is just the two DB statements.
|
||||||
|
- **demo-bot stat re-computation**: `createRoute` will call `validateGpx` on GPX that the demo-bot already parsed. Minor redundancy; acceptable given demo-bot is not a hot path.
|
||||||
|
- **Existing NULL geom rows**: Rows already in production with `geom IS NULL` are not backfilled by this change. A separate data-repair script would be needed. Out of scope.
|
||||||
|
- **`GpxValidationError` surfaces to users**: Routes/activities that previously saved silently will now return errors. This is the intended behaviour (fail loudly), but any caller that didn't expect an exception from `updateRoute` must now handle `GpxValidationError`. All current call sites are audited in the tasks.
|
||||||
|
|
||||||
|
## Migration Plan
|
||||||
|
|
||||||
|
No DB migration required. The change is purely in application code. Deploy is a standard app rollout — no staged migration, no feature flag.
|
||||||
|
|
||||||
|
Existing rows with `geom IS NULL` remain as-is (pre-existing data debt). A follow-up query can identify and optionally repair them:
|
||||||
|
```sql
|
||||||
|
SELECT id FROM journal.routes WHERE gpx IS NOT NULL AND geom IS NULL;
|
||||||
|
```
|
||||||
|
|
@ -0,0 +1,32 @@
|
||||||
|
## Why
|
||||||
|
|
||||||
|
Route and activity rows can currently be saved with `geom IS NULL` when the PostGIS geometry write fails after the row insert succeeds — the error is swallowed silently with a `console.error`. This means missing map thumbnails, broken route-push deduplication, and corrupted spatial queries with no signal to the user or operator. The Planner callback endpoint also accepts GPX from an external source with no validation before writing.
|
||||||
|
|
||||||
|
## What Changes
|
||||||
|
|
||||||
|
- **New `gpx-save.server.ts` module** owns GPX validation and atomic geometry persistence; `setGeomFromGpx` moves here and becomes internal.
|
||||||
|
- **`validateGpx(gpx)`** parses the GPX string, checks ≥2 track points and valid coordinate ranges, throws `GpxValidationError` on failure, and returns the parsed result so callers don't re-parse.
|
||||||
|
- **`createRoute`, `updateRoute`, `createActivity`, `createRouteFromActivity`** all wrap their DB writes (row insert/update + geom write + version snapshot) in `db.transaction()` — partial state (row exists, geom NULL) is no longer possible.
|
||||||
|
- **`setGeomFromGpx` stops swallowing errors** — it throws, causing the transaction to roll back.
|
||||||
|
- **`demo-bot.server.ts`** migrated from raw inserts + `setGeomFromGpx` to `createRoute` / `createActivity`, eliminating a bypass of the new invariant.
|
||||||
|
- **`activities.server.ts`** removes inline `parseGpxAsync` call; uses the `ParsedGpx` returned by `validateGpx` for stat extraction instead.
|
||||||
|
- **ADR recorded**: PostGIS geometry writes are always transactional with row writes.
|
||||||
|
|
||||||
|
## Capabilities
|
||||||
|
|
||||||
|
### New Capabilities
|
||||||
|
|
||||||
|
- `gpx-save`: Atomic GPX validation and PostGIS geometry persistence for routes and activities.
|
||||||
|
|
||||||
|
### Modified Capabilities
|
||||||
|
|
||||||
|
*(none — no user-visible requirement changes; this is an implementation-layer correctness fix)*
|
||||||
|
|
||||||
|
## Impact
|
||||||
|
|
||||||
|
- `apps/journal/app/lib/routes.server.ts` — `setGeomFromGpx` export removed; callers updated.
|
||||||
|
- `apps/journal/app/lib/activities.server.ts` — inline `parseGpxAsync` removed; imports from `gpx-save`.
|
||||||
|
- `apps/journal/app/lib/demo-bot.server.ts` — raw insert pattern replaced with `createRoute` / `createActivity`.
|
||||||
|
- `apps/journal/app/routes/api.routes.$id.callback.ts` — no validation changes needed (validation now inside `updateRoute`); GPX errors surface as thrown exceptions → callers return 400.
|
||||||
|
- `@trails-cool/gpx` — no changes; `parseGpxAsync` is still the underlying parser.
|
||||||
|
- No API surface changes, no schema changes, no migration required.
|
||||||
|
|
@ -0,0 +1,83 @@
|
||||||
|
# gpx-save Specification
|
||||||
|
|
||||||
|
## Purpose
|
||||||
|
|
||||||
|
Atomic GPX validation and PostGIS geometry persistence for routes and activities. Every write of a GPX track in the Journal SHALL validate the track and persist the row + geometry in a single transaction. Silent geometry failures are not permitted.
|
||||||
|
|
||||||
|
## Requirements
|
||||||
|
|
||||||
|
### Requirement: GPX validation before any DB write
|
||||||
|
|
||||||
|
The Journal SHALL validate any GPX string before writing a route or activity row. Validation SHALL confirm the GPX parses successfully, contains at least 2 track points, and all track points have coordinates within valid ranges (latitude −90..90, longitude −180..180). Validation SHALL throw `GpxValidationError` on failure.
|
||||||
|
|
||||||
|
#### Scenario: Valid GPX proceeds to save
|
||||||
|
|
||||||
|
- **WHEN** a caller passes a GPX string with ≥2 track points and valid coordinates to `createRoute`, `updateRoute`, `createActivity`, or `createRouteFromActivity`
|
||||||
|
- **THEN** the save proceeds and no `GpxValidationError` is thrown
|
||||||
|
|
||||||
|
#### Scenario: GPX with fewer than 2 track points is rejected
|
||||||
|
|
||||||
|
- **WHEN** a caller passes a GPX string that produces fewer than 2 track points
|
||||||
|
- **THEN** `GpxValidationError` is thrown before any DB write occurs
|
||||||
|
|
||||||
|
#### Scenario: GPX with out-of-range coordinates is rejected
|
||||||
|
|
||||||
|
- **WHEN** a caller passes a GPX string containing coordinates outside valid ranges
|
||||||
|
- **THEN** `GpxValidationError` is thrown before any DB write occurs
|
||||||
|
|
||||||
|
#### Scenario: Unparseable GPX is rejected
|
||||||
|
|
||||||
|
- **WHEN** a caller passes a malformed GPX string that cannot be parsed
|
||||||
|
- **THEN** `GpxValidationError` is thrown before any DB write occurs
|
||||||
|
|
||||||
|
### Requirement: Atomic row + geometry persistence
|
||||||
|
|
||||||
|
Every route or activity save that includes a GPX string SHALL wrap the row write, PostGIS geometry write, and version snapshot in a single database transaction. If any step fails, the entire transaction SHALL be rolled back — a route or activity row with a non-null `gpx` column and a null `geom` column SHALL NOT be possible through the normal save path.
|
||||||
|
|
||||||
|
#### Scenario: Successful save stores row and geometry atomically
|
||||||
|
|
||||||
|
- **WHEN** `createRoute` is called with valid GPX
|
||||||
|
- **THEN** the `routes` row, the `geom` column update, and the `routeVersions` row are all committed in a single transaction
|
||||||
|
|
||||||
|
#### Scenario: PostGIS failure rolls back the row insert
|
||||||
|
|
||||||
|
- **WHEN** the PostGIS geometry write fails during a `createRoute` call
|
||||||
|
- **THEN** the transaction is rolled back and no `routes` row is persisted
|
||||||
|
|
||||||
|
#### Scenario: Route update is atomic
|
||||||
|
|
||||||
|
- **WHEN** `updateRoute` is called with valid GPX
|
||||||
|
- **THEN** the row update, geometry update, and new version snapshot are committed atomically
|
||||||
|
|
||||||
|
#### Scenario: Activity save is atomic
|
||||||
|
|
||||||
|
- **WHEN** `createActivity` is called with valid GPX
|
||||||
|
- **THEN** the `activities` row and `geom` column update are committed atomically
|
||||||
|
|
||||||
|
### Requirement: Loud failure on geometry errors
|
||||||
|
|
||||||
|
The Journal SHALL NOT swallow PostGIS geometry write errors. Any failure during the geometry write SHALL propagate as a thrown exception, causing the enclosing transaction to roll back and the error to surface to the caller.
|
||||||
|
|
||||||
|
#### Scenario: Geometry write error surfaces to caller
|
||||||
|
|
||||||
|
- **WHEN** the PostGIS `UPDATE geom` statement fails
|
||||||
|
- **THEN** an exception is thrown, the transaction is rolled back, and the caller receives the error
|
||||||
|
|
||||||
|
#### Scenario: demo-bot route creation fails loudly
|
||||||
|
|
||||||
|
- **WHEN** `createRoute` is called from the demo-bot and the PostGIS write fails
|
||||||
|
- **THEN** the exception propagates — no partial route row is committed
|
||||||
|
|
||||||
|
### Requirement: Single gpx-save module owns geometry persistence
|
||||||
|
|
||||||
|
The Journal SHALL have exactly one module responsible for GPX validation and PostGIS geometry writes: `apps/journal/app/lib/gpx-save.server.ts`. No other module SHALL call PostGIS geometry update statements directly or implement GPX validation independently.
|
||||||
|
|
||||||
|
#### Scenario: activities.server.ts uses gpx-save for geometry
|
||||||
|
|
||||||
|
- **WHEN** `createActivity` or `createRouteFromActivity` writes geometry
|
||||||
|
- **THEN** the write goes through the `gpx-save` module, not an inline raw SQL statement
|
||||||
|
|
||||||
|
#### Scenario: demo-bot uses createRoute and createActivity
|
||||||
|
|
||||||
|
- **WHEN** the demo-bot creates synthetic routes and activities
|
||||||
|
- **THEN** it calls `createRoute` and `createActivity` rather than issuing raw inserts and calling `setGeomFromGpx` directly
|
||||||
39
openspec/changes/archive/2026-05-10-atomic-gpx-save/tasks.md
Normal file
39
openspec/changes/archive/2026-05-10-atomic-gpx-save/tasks.md
Normal file
|
|
@ -0,0 +1,39 @@
|
||||||
|
## 1. New gpx-save module
|
||||||
|
|
||||||
|
- [x] 1.1 Create `apps/journal/app/lib/gpx-save.server.ts` with `GpxValidationError` class and `validateGpx(gpx: string): Promise<ParsedGpx>` — parses, checks ≥2 track points, checks coordinate ranges, throws on failure, returns parsed result
|
||||||
|
- [x] 1.2 Move `setGeomFromGpx` logic into `gpx-save.server.ts` as an internal `writeGeom(tx, id, table, coords)` function that accepts a Drizzle transaction client and throws on failure (no try/catch)
|
||||||
|
- [x] 1.3 Write unit tests in `gpx-save.server.test.ts` covering: valid GPX passes, <2 track points throws, out-of-range coordinates throw, unparseable GPX throws
|
||||||
|
|
||||||
|
## 2. Migrate routes.server.ts
|
||||||
|
|
||||||
|
- [x] 2.1 Import `validateGpx` and `writeGeom` from `gpx-save.server.ts`; remove local `setGeomFromGpx` implementation and its export
|
||||||
|
- [x] 2.2 Wrap `createRoute` in `db.transaction()`: validate GPX → insert row → `writeGeom` → insert `routeVersions`, all within the transaction client
|
||||||
|
- [x] 2.3 Wrap `updateRoute` in `db.transaction()`: validate GPX → update row → `writeGeom` → insert new `routeVersions`, all within the transaction client
|
||||||
|
- [x] 2.4 Verify `computeRouteStats` is called with the `ParsedGpx` returned by `validateGpx` (avoid re-parsing)
|
||||||
|
|
||||||
|
## 3. Migrate activities.server.ts
|
||||||
|
|
||||||
|
- [x] 3.1 Replace inline `parseGpxAsync` call in `createActivity` with `validateGpx`; extract stats from the returned `ParsedGpx`
|
||||||
|
- [x] 3.2 Wrap `createActivity` in `db.transaction()`: validate GPX → insert row → `writeGeom`, all within the transaction client
|
||||||
|
- [x] 3.3 Wrap `createRouteFromActivity` in `db.transaction()`: insert route row → `writeGeom` → update activity's `routeId`, all within the transaction client
|
||||||
|
- [x] 3.4 Remove `import { setGeomFromGpx } from "./routes.server.ts"` — no longer needed
|
||||||
|
|
||||||
|
## 4. Migrate demo-bot.server.ts
|
||||||
|
|
||||||
|
- [x] 4.1 Replace the raw `db.insert(routes).values(...)` + `setGeomFromGpx(routeId, ...)` block with a `createRoute(ownerId, input)` call; pass pre-computed stats via `RouteInput` fields so `createRoute` doesn't re-parse BRouter's output
|
||||||
|
- [x] 4.2 Replace the raw `db.insert(activities).values(...)` + `setGeomFromGpx(activityId, ...)` block with a `createActivity(ownerId, input)` call
|
||||||
|
- [x] 4.3 Remove `import { setGeomFromGpx } from "./routes.server.ts"` from `demo-bot.server.ts`
|
||||||
|
|
||||||
|
## 5. Callback endpoint error handling
|
||||||
|
|
||||||
|
- [x] 5.1 In `api.routes.$id.callback.ts`, catch `GpxValidationError` from `updateRoute` and return a 400 response with the validation message (currently the catch block returns 401 for all errors — narrow it)
|
||||||
|
|
||||||
|
## 6. ADR
|
||||||
|
|
||||||
|
- [x] 6.1 Write `docs/adr/0006-atomic-gpx-save.md` documenting that PostGIS geometry writes are always transactional with row writes and that `gpx-save.server.ts` is the sole owner of geometry persistence
|
||||||
|
|
||||||
|
## 7. Verification
|
||||||
|
|
||||||
|
- [x] 7.1 Run `pnpm typecheck` — no new type errors
|
||||||
|
- [x] 7.2 Run `pnpm test` — all existing tests pass; new `gpx-save.server.test.ts` tests pass
|
||||||
|
- [x] 7.3 Confirm `setGeomFromGpx` is no longer exported from any file (`grep -r "setGeomFromGpx" apps/journal/app` returns no results outside `gpx-save.server.ts`)
|
||||||
83
openspec/specs/gpx-save/spec.md
Normal file
83
openspec/specs/gpx-save/spec.md
Normal file
|
|
@ -0,0 +1,83 @@
|
||||||
|
# gpx-save Specification
|
||||||
|
|
||||||
|
## Purpose
|
||||||
|
|
||||||
|
Atomic GPX validation and PostGIS geometry persistence for routes and activities. Every write of a GPX track in the Journal SHALL validate the track and persist the row + geometry in a single transaction. Silent geometry failures are not permitted.
|
||||||
|
|
||||||
|
## Requirements
|
||||||
|
|
||||||
|
### Requirement: GPX validation before any DB write
|
||||||
|
|
||||||
|
The Journal SHALL validate any GPX string before writing a route or activity row. Validation SHALL confirm the GPX parses successfully, contains at least 2 track points, and all track points have coordinates within valid ranges (latitude −90..90, longitude −180..180). Validation SHALL throw `GpxValidationError` on failure.
|
||||||
|
|
||||||
|
#### Scenario: Valid GPX proceeds to save
|
||||||
|
|
||||||
|
- **WHEN** a caller passes a GPX string with ≥2 track points and valid coordinates to `createRoute`, `updateRoute`, `createActivity`, or `createRouteFromActivity`
|
||||||
|
- **THEN** the save proceeds and no `GpxValidationError` is thrown
|
||||||
|
|
||||||
|
#### Scenario: GPX with fewer than 2 track points is rejected
|
||||||
|
|
||||||
|
- **WHEN** a caller passes a GPX string that produces fewer than 2 track points
|
||||||
|
- **THEN** `GpxValidationError` is thrown before any DB write occurs
|
||||||
|
|
||||||
|
#### Scenario: GPX with out-of-range coordinates is rejected
|
||||||
|
|
||||||
|
- **WHEN** a caller passes a GPX string containing coordinates outside valid ranges
|
||||||
|
- **THEN** `GpxValidationError` is thrown before any DB write occurs
|
||||||
|
|
||||||
|
#### Scenario: Unparseable GPX is rejected
|
||||||
|
|
||||||
|
- **WHEN** a caller passes a malformed GPX string that cannot be parsed
|
||||||
|
- **THEN** `GpxValidationError` is thrown before any DB write occurs
|
||||||
|
|
||||||
|
### Requirement: Atomic row + geometry persistence
|
||||||
|
|
||||||
|
Every route or activity save that includes a GPX string SHALL wrap the row write, PostGIS geometry write, and version snapshot in a single database transaction. If any step fails, the entire transaction SHALL be rolled back — a route or activity row with a non-null `gpx` column and a null `geom` column SHALL NOT be possible through the normal save path.
|
||||||
|
|
||||||
|
#### Scenario: Successful save stores row and geometry atomically
|
||||||
|
|
||||||
|
- **WHEN** `createRoute` is called with valid GPX
|
||||||
|
- **THEN** the `routes` row, the `geom` column update, and the `routeVersions` row are all committed in a single transaction
|
||||||
|
|
||||||
|
#### Scenario: PostGIS failure rolls back the row insert
|
||||||
|
|
||||||
|
- **WHEN** the PostGIS geometry write fails during a `createRoute` call
|
||||||
|
- **THEN** the transaction is rolled back and no `routes` row is persisted
|
||||||
|
|
||||||
|
#### Scenario: Route update is atomic
|
||||||
|
|
||||||
|
- **WHEN** `updateRoute` is called with valid GPX
|
||||||
|
- **THEN** the row update, geometry update, and new version snapshot are committed atomically
|
||||||
|
|
||||||
|
#### Scenario: Activity save is atomic
|
||||||
|
|
||||||
|
- **WHEN** `createActivity` is called with valid GPX
|
||||||
|
- **THEN** the `activities` row and `geom` column update are committed atomically
|
||||||
|
|
||||||
|
### Requirement: Loud failure on geometry errors
|
||||||
|
|
||||||
|
The Journal SHALL NOT swallow PostGIS geometry write errors. Any failure during the geometry write SHALL propagate as a thrown exception, causing the enclosing transaction to roll back and the error to surface to the caller.
|
||||||
|
|
||||||
|
#### Scenario: Geometry write error surfaces to caller
|
||||||
|
|
||||||
|
- **WHEN** the PostGIS `UPDATE geom` statement fails
|
||||||
|
- **THEN** an exception is thrown, the transaction is rolled back, and the caller receives the error
|
||||||
|
|
||||||
|
#### Scenario: demo-bot route creation fails loudly
|
||||||
|
|
||||||
|
- **WHEN** `createRoute` is called from the demo-bot and the PostGIS write fails
|
||||||
|
- **THEN** the exception propagates — no partial route row is committed
|
||||||
|
|
||||||
|
### Requirement: Single gpx-save module owns geometry persistence
|
||||||
|
|
||||||
|
The Journal SHALL have exactly one module responsible for GPX validation and PostGIS geometry writes: `apps/journal/app/lib/gpx-save.server.ts`. No other module SHALL call PostGIS geometry update statements directly or implement GPX validation independently.
|
||||||
|
|
||||||
|
#### Scenario: activities.server.ts uses gpx-save for geometry
|
||||||
|
|
||||||
|
- **WHEN** `createActivity` or `createRouteFromActivity` writes geometry
|
||||||
|
- **THEN** the write goes through the `gpx-save` module, not an inline raw SQL statement
|
||||||
|
|
||||||
|
#### Scenario: demo-bot uses createRoute and createActivity
|
||||||
|
|
||||||
|
- **WHEN** the demo-bot creates synthetic routes and activities
|
||||||
|
- **THEN** it calls `createRoute` and `createActivity` rather than issuing raw inserts and calling `setGeomFromGpx` directly
|
||||||
Loading…
Add table
Add a link
Reference in a new issue