From 9a0dae068bd8de818371d4c86de877f0a644fa15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ullrich=20Sch=C3=A4fer?= Date: Wed, 10 Jun 2026 03:28:13 +0200 Subject: [PATCH] journal: gpx-save owns the validate-and-derive step (processGpx) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit createRoute, updateRoute, createActivity, createRouteFromActivity, and the demo-bot each re-implemented the same choreography after validateGpx: flatten tracks into [lon, lat] coords for writeGeom and derive distance / elevation / dayBreaks / description / start time. The stat derivation lived in a private computeRouteStats in routes.server.ts that activities couldn't reach, so the two sides had drifted (activities re-derived inline, with its own start-time logic). processGpx() in gpx-save.server.ts now owns the whole step: parse + validate (GpxValidationError as before), coords extraction, and stat derivation, returning the parsed GpxData so nothing re-parses. Callers keep their own precedence rules between derived and caller-supplied stats — routes let explicit input win wholesale, the activities importer prefers GPX distance unless it is zero. Extends ADR-0006: the gpx-save module remains the only place that understands GPX-to-database derivation. Co-Authored-By: Claude Fable 5 --- apps/journal/app/lib/activities.server.ts | 27 ++++---- apps/journal/app/lib/demo-bot.server.ts | 8 +-- apps/journal/app/lib/gpx-save.server.test.ts | 68 +++++++++++++++++++- apps/journal/app/lib/gpx-save.server.ts | 45 +++++++++++++ apps/journal/app/lib/routes.server.ts | 47 ++++---------- 5 files changed, 141 insertions(+), 54 deletions(-) diff --git a/apps/journal/app/lib/activities.server.ts b/apps/journal/app/lib/activities.server.ts index 6e99d15..cfc77b0 100644 --- a/apps/journal/app/lib/activities.server.ts +++ b/apps/journal/app/lib/activities.server.ts @@ -4,8 +4,8 @@ import { unionAll } from "drizzle-orm/pg-core"; import { getDb } from "./db.ts"; import { activities, routes, syncImports, users, follows, remoteActors } from "@trails-cool/db/schema/journal"; import type { Visibility } from "@trails-cool/db/schema/journal"; -import { validateGpx, writeGeom } from "./gpx-save.server.ts"; -import type { GpxData } from "./gpx-save.server.ts"; +import { processGpx, writeGeom } from "./gpx-save.server.ts"; +import type { ProcessedGpx } from "./gpx-save.server.ts"; import { enqueueOptional } from "./boss.server.ts"; import type { OwnedRef } from "./ownership.server.ts"; import { @@ -70,7 +70,7 @@ export async function createActivity(ownerId: string, input: ActivityInput) { const db = getDb(); const id = randomUUID(); - let parsed: GpxData | null = null; + let processed: ProcessedGpx | null = null; let distance: number | null = input.distance ?? null; let elevationGain: number | null = null; let elevationLoss: number | null = null; @@ -78,13 +78,12 @@ export async function createActivity(ownerId: string, input: ActivityInput) { const duration: number | null = input.duration ?? null; if (input.gpx) { - parsed = await validateGpx(input.gpx); - distance = parsed.distance || distance; - elevationGain = parsed.elevation.gain; - elevationLoss = parsed.elevation.loss; - if (!startedAt && parsed.tracks[0]?.[0]?.time) { - startedAt = new Date(parsed.tracks[0][0].time); - } + processed = await processGpx(input.gpx); + // GPX-derived distance wins unless it is zero; caller input is the fallback + distance = processed.stats.distance || distance; + elevationGain = processed.stats.elevationGain; + elevationLoss = processed.stats.elevationLoss; + startedAt = startedAt ?? processed.stats.startTime; } await db.transaction(async (tx) => { @@ -104,9 +103,8 @@ export async function createActivity(ownerId: string, input: ActivityInput) { ...(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, "activities", coords); + if (input.gpx && processed) { + await writeGeom(tx, id, "activities", processed.coords); } }); @@ -332,8 +330,7 @@ export async function createRouteFromActivity(ownedActivity: OwnedRef): Promise< .where(and(eq(activities.id, activityId), eq(activities.ownerId, ownerId))); 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 { coords } = await processGpx(activity.gpx); const routeId = randomUUID(); await db.transaction(async (tx) => { diff --git a/apps/journal/app/lib/demo-bot.server.ts b/apps/journal/app/lib/demo-bot.server.ts index 8ee5fd5..7917524 100644 --- a/apps/journal/app/lib/demo-bot.server.ts +++ b/apps/journal/app/lib/demo-bot.server.ts @@ -6,7 +6,7 @@ import { getDb } from "./db.ts"; import { activities, routes, users } from "@trails-cool/db/schema/journal"; import { createRoute } from "./routes.server.ts"; import { createActivity } from "./activities.server.ts"; -import { validateGpx } from "./gpx-save.server.ts"; +import { processGpx } from "./gpx-save.server.ts"; import { TERMS_VERSION } from "./legal.ts"; import { logger } from "./logger.server.ts"; import { @@ -649,10 +649,8 @@ export async function generateOneWalk( const name = templateName(now, locale, persona); const description = templateDescription(now, locale, persona); - const parsed = await validateGpx(result.gpx); - const distance = parsed.distance; - const elevationGain = parsed.elevation.gain; - const elevationLoss = parsed.elevation.loss; + const { stats } = await processGpx(result.gpx); + const { distance, elevationGain, elevationLoss } = stats; if (!distance || distance < 500) return null; diff --git a/apps/journal/app/lib/gpx-save.server.test.ts b/apps/journal/app/lib/gpx-save.server.test.ts index b1455c2..3113406 100644 --- a/apps/journal/app/lib/gpx-save.server.test.ts +++ b/apps/journal/app/lib/gpx-save.server.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect } from "vitest"; -import { validateGpx, GpxValidationError } from "./gpx-save.server.ts"; +import { generateGpx } from "@trails-cool/gpx"; +import { processGpx, validateGpx, GpxValidationError } from "./gpx-save.server.ts"; const VALID_GPX = ` @@ -84,3 +85,68 @@ describe("validateGpx", () => { expect(err.name).toBe("GpxValidationError"); }); }); + +const TIMED_GPX = ` + + An alpine loop + + + 500 + 520 + + +`; + +describe("processGpx", () => { + it("extracts coords in [lon, lat] PostGIS axis order", async () => { + const { coords } = await processGpx(VALID_GPX); + expect(coords).toEqual([ + [8.0, 47.0], + [8.1, 47.1], + [8.2, 47.2], + ]); + }); + + it("derives distance, elevation, description, and start time", async () => { + const { stats } = await processGpx(TIMED_GPX); + expect(stats.distance).toBeGreaterThan(0); + expect(stats.elevationGain).not.toBeNull(); + expect(stats.elevationLoss).not.toBeNull(); + expect(stats.description).toBe("An alpine loop"); + expect(stats.startTime).toEqual(new Date("2026-06-01T08:00:00Z")); + }); + + it("returns null startTime and empty dayBreaks when absent", async () => { + const { stats } = await processGpx(VALID_GPX); + expect(stats.startTime).toBeNull(); + expect(stats.dayBreaks).toEqual([]); + }); + + it("derives dayBreaks from day-break waypoints (round-trip via generateGpx)", async () => { + const gpx = generateGpx({ + name: "Multi-day", + waypoints: [ + { lat: 47.0, lon: 8.0, name: "Start" }, + { lat: 47.1, lon: 8.1, name: "Hut", isDayBreak: true }, + { lat: 47.2, lon: 8.2, name: "End" }, + ], + tracks: [[ + { lat: 47.0, lon: 8.0 }, + { lat: 47.1, lon: 8.1 }, + { lat: 47.2, lon: 8.2 }, + ]], + }); + + const { stats } = await processGpx(gpx); + expect(stats.dayBreaks).toEqual([1]); + }); + + it("propagates GpxValidationError from validation", async () => { + await expect(processGpx(ONE_POINT_GPX)).rejects.toThrow(GpxValidationError); + }); + + it("returns the parsed GpxData so callers never re-parse", async () => { + const { parsed } = await processGpx(VALID_GPX); + expect(parsed.tracks.flat()).toHaveLength(3); + }); +}); diff --git a/apps/journal/app/lib/gpx-save.server.ts b/apps/journal/app/lib/gpx-save.server.ts index b1411e9..7431a39 100644 --- a/apps/journal/app/lib/gpx-save.server.ts +++ b/apps/journal/app/lib/gpx-save.server.ts @@ -45,6 +45,51 @@ export async function validateGpx(gpx: string): Promise { return parsed; } +export interface GpxStats { + distance: number | null; + elevationGain: number | null; + elevationLoss: number | null; + /** Indices of waypoints flagged as day breaks. */ + dayBreaks: number[]; + /** GPX-level description, when present. */ + description?: string; + /** Timestamp of the first track point, when present. */ + startTime: Date | null; +} + +export interface ProcessedGpx { + parsed: GpxData; + /** [lon, lat] pairs in PostGIS axis order, ready for writeGeom. */ + coords: Array<[number, number]>; + stats: GpxStats; +} + +/** + * The validate-and-derive step every GPX save starts with: parse + + * validate (throws GpxValidationError), extract the geometry + * coordinates, and derive the stats rows store. Callers own the + * precedence between these derived stats and caller-supplied ones — + * routes let explicit input win wholesale, activities prefer the GPX + * distance unless it is zero. + */ +export async function processGpx(gpx: string): Promise { + const parsed = await validateGpx(gpx); + return { + parsed, + coords: parsed.tracks.flat().map((p) => [p.lon, p.lat] as [number, number]), + stats: { + distance: parsed.distance ?? null, + elevationGain: parsed.elevation.gain ?? null, + elevationLoss: parsed.elevation.loss ?? null, + dayBreaks: parsed.waypoints + .map((w, i) => (w.isDayBreak ? i : -1)) + .filter((i) => i >= 0), + description: parsed.description, + startTime: parsed.tracks[0]?.[0]?.time ? new Date(parsed.tracks[0][0].time) : null, + }, + }; +} + type Tx = Parameters[0]>[0]; /** diff --git a/apps/journal/app/lib/routes.server.ts b/apps/journal/app/lib/routes.server.ts index 664fe9a..91ed3f3 100644 --- a/apps/journal/app/lib/routes.server.ts +++ b/apps/journal/app/lib/routes.server.ts @@ -4,8 +4,8 @@ import { getDb } from "./db.ts"; import { routes, routeVersions } from "@trails-cool/db/schema/journal"; import type { Visibility } from "@trails-cool/db/schema/journal"; import { sql } from "drizzle-orm"; -import { validateGpx, writeGeom } from "./gpx-save.server.ts"; -import type { GpxData } from "./gpx-save.server.ts"; +import { processGpx, writeGeom } from "./gpx-save.server.ts"; +import type { ProcessedGpx } from "./gpx-save.server.ts"; import type { OwnedRef } from "./ownership.server.ts"; export interface RouteInput { @@ -26,21 +26,17 @@ export async function createRoute(ownerId: string, input: RouteInput) { const db = getDb(); const id = randomUUID(); - let parsed: GpxData | null = null; + let processed: ProcessedGpx | null = null; let distance: number | null = input.distance ?? null; let elevationGain: number | null = input.elevationGain ?? null; let elevationLoss: number | null = input.elevationLoss ?? null; let dayBreaks: number[] = input.dayBreaks ?? []; if (input.gpx) { - parsed = await validateGpx(input.gpx); - // Only compute stats from GPX if not pre-supplied by caller + processed = await processGpx(input.gpx); + // Only use GPX-derived stats if not pre-supplied by caller if (input.distance === undefined) { - const stats = computeRouteStats(parsed); - distance = stats.distance; - elevationGain = stats.elevationGain; - elevationLoss = stats.elevationLoss; - dayBreaks = stats.dayBreaks; + ({ distance, elevationGain, elevationLoss, dayBreaks } = processed.stats); } } @@ -60,9 +56,8 @@ export async function createRoute(ownerId: string, input: RouteInput) { ...(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); + if (input.gpx && processed) { + await writeGeom(tx, id, "routes", processed.coords); await tx.insert(routeVersions).values({ id: randomUUID(), @@ -135,9 +130,9 @@ export async function updateRoute(route: OwnedRef, input: Partial) { const { id, ownerId } = route; const db = getDb(); - let parsed: GpxData | null = null; + let processed: ProcessedGpx | null = null; if (input.gpx) { - parsed = await validateGpx(input.gpx); + processed = await processGpx(input.gpx); } const updateData: Record = { updatedAt: new Date() }; @@ -145,8 +140,8 @@ export async function updateRoute(route: OwnedRef, input: Partial) { if (input.description !== undefined) updateData.description = input.description; if (input.visibility !== undefined) updateData.visibility = input.visibility; - if (input.gpx && parsed) { - const stats = computeRouteStats(parsed); + if (input.gpx && processed) { + const { stats } = processed; updateData.gpx = input.gpx; updateData.distance = stats.distance; updateData.elevationGain = stats.elevationGain; @@ -163,9 +158,8 @@ export async function updateRoute(route: OwnedRef, input: Partial) { .set(updateData) .where(and(eq(routes.id, id), eq(routes.ownerId, ownerId))); - if (input.gpx && parsed) { - const coords = parsed.tracks.flat().map((p) => [p.lon, p.lat] as [number, number]); - await writeGeom(tx, id, "routes", coords); + if (input.gpx && processed) { + await writeGeom(tx, id, "routes", processed.coords); const existingVersions = await tx .select() @@ -197,19 +191,6 @@ export async function deleteRoute(route: OwnedRef) { return result.length > 0; } -function computeRouteStats(gpxData: GpxData) { - const dayBreaks = gpxData.waypoints - .map((w, i) => (w.isDayBreak ? i : -1)) - .filter((i) => i >= 0); - return { - distance: gpxData.distance, - elevationGain: gpxData.elevation.gain, - elevationLoss: gpxData.elevation.loss, - dayBreaks, - description: gpxData.description, - }; -} - async function getGeojson(table: "routes" | "activities", id: string): Promise { try { const db = getDb();