From 3b9672e0ff784676d205f382cc518c1454342ddb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ullrich=20Sch=C3=A4fer?= Date: Wed, 10 Jun 2026 01:35:04 +0200 Subject: [PATCH 1/4] planner: give the Yjs document a typed schema seam The routeData Y.Map's ~15 string keys (geojson, coordinates, segmentBoundaries, road metadata, profile, colorMode, baseLayer, overlays, poiCategories) were read and written raw at ~30 call sites, each with its own JSON parsing and casts; parseJsonArray existed twice and waypoint extraction four times. GPX assembly was duplicated between SaveToJournalButton and ExportButton, so the saved plan and the exported file could silently diverge. - new lib/route-data.ts owns the routeData (+ noGoAreas) schema: typed read/write, JSON encoding internal, ColorMode moves here (re-exported from ColoredRoute for existing importers) - new lib/gpx-export.ts owns GPX assembly: buildRouteGpx / buildPlanGpx / buildDayGpxFiles / hasDayBreaks; multi-day splitting becomes a pure, tested function - waypoint-ymap.ts gains extractWaypoints / extractWaypointData; the four hand-rolled copies (use-routing, use-waypoint-manager, WaypointSidebar, use-days) now share it, and WaypointSidebar's moveWaypoint reuses the round-trip helpers instead of re-listing every waypoint field - all hooks/components consume the seam; no raw routeData key strings remain outside route-data.ts Co-Authored-By: Claude Fable 5 --- apps/planner/app/components/ColoredRoute.tsx | 6 +- .../planner/app/components/ElevationChart.tsx | 3 +- apps/planner/app/components/ExportButton.tsx | 88 +------ apps/planner/app/components/MapHelpers.tsx | 22 +- .../app/components/ProfileSelector.tsx | 7 +- .../app/components/SaveToJournalButton.tsx | 30 +-- apps/planner/app/components/SessionView.tsx | 13 +- .../app/components/WaypointSidebar.tsx | 47 +--- apps/planner/app/components/YjsDebugPanel.tsx | 9 +- apps/planner/app/lib/gpx-export.test.ts | 144 +++++++++++ apps/planner/app/lib/gpx-export.ts | 101 ++++++++ apps/planner/app/lib/route-data.test.ts | 243 ++++++++++++++++++ apps/planner/app/lib/route-data.ts | 221 ++++++++++++++++ apps/planner/app/lib/use-days.ts | 37 +-- apps/planner/app/lib/use-elevation-data.ts | 34 +-- apps/planner/app/lib/use-profile-defaults.ts | 3 +- apps/planner/app/lib/use-routing.ts | 48 +--- apps/planner/app/lib/use-waypoint-manager.ts | 66 +---- apps/planner/app/lib/use-yjs-poi-sync.ts | 20 +- apps/planner/app/lib/waypoint-ymap.test.ts | 77 ++++++ apps/planner/app/lib/waypoint-ymap.ts | 24 ++ 21 files changed, 921 insertions(+), 322 deletions(-) create mode 100644 apps/planner/app/lib/gpx-export.test.ts create mode 100644 apps/planner/app/lib/gpx-export.ts create mode 100644 apps/planner/app/lib/route-data.test.ts create mode 100644 apps/planner/app/lib/route-data.ts create mode 100644 apps/planner/app/lib/waypoint-ymap.test.ts diff --git a/apps/planner/app/components/ColoredRoute.tsx b/apps/planner/app/components/ColoredRoute.tsx index f0379d7..b78c5c3 100644 --- a/apps/planner/app/components/ColoredRoute.tsx +++ b/apps/planner/app/components/ColoredRoute.tsx @@ -11,7 +11,11 @@ import { elevationColor, routeGradeColor, maxspeedColor, } from "@trails-cool/map-core"; -export type ColorMode = "plain" | "elevation" | "surface" | "grade" | "highway" | "maxspeed" | "smoothness" | "tracktype" | "cycleway" | "bikeroute"; +import type { ColorMode } from "~/lib/route-data"; + +// ColorMode lives in the routeData schema module; re-exported here for +// existing importers. +export type { ColorMode }; interface ColoredRouteProps { coordinates: [number, number, number][]; // [lon, lat, ele] diff --git a/apps/planner/app/components/ElevationChart.tsx b/apps/planner/app/components/ElevationChart.tsx index 392f8cb..a4b5ada 100644 --- a/apps/planner/app/components/ElevationChart.tsx +++ b/apps/planner/app/components/ElevationChart.tsx @@ -14,6 +14,7 @@ import { } from "@trails-cool/map-core"; import { drawElevationChart, PADDING } from "~/lib/elevation-chart-draw"; import { useElevationData } from "~/lib/use-elevation-data"; +import { setColorMode, type ColorMode } from "~/lib/route-data"; interface ElevationChartProps { yjs: YjsState; @@ -371,7 +372,7 @@ export function ElevationChart({ yjs, onHover, highlightDistance, onClickPositio ); const setMode = useCallback((mode: string) => { - yjs.routeData.set("colorMode", mode); + setColorMode(yjs.routeData, mode as ColorMode); }, [yjs.routeData]); if (points.length < 2) return null; diff --git a/apps/planner/app/components/ExportButton.tsx b/apps/planner/app/components/ExportButton.tsx index 31b99c2..87ceabc 100644 --- a/apps/planner/app/components/ExportButton.tsx +++ b/apps/planner/app/components/ExportButton.tsx @@ -1,33 +1,12 @@ import { useCallback, useState, useRef, useEffect } from "react"; import { useTranslation } from "react-i18next"; -import * as Y from "yjs"; import type { YjsState } from "~/lib/use-yjs"; -import { generateGpx, computeDays } from "@trails-cool/gpx"; -import type { TrackPoint, NoGoArea } from "@trails-cool/gpx"; -import { waypointFromYMap } from "~/lib/waypoint-ymap"; - -function getTracks(yjs: YjsState): TrackPoint[][] { - const geojsonStr = yjs.routeData.get("geojson") as string | undefined; - if (!geojsonStr) return []; - try { - const geojson = JSON.parse(geojsonStr); - const coords: number[][] = geojson.features?.[0]?.geometry?.coordinates ?? []; - if (coords.length > 0) { - return [coords.map((c) => ({ lat: c[1]!, lon: c[0]!, ele: c[2] }))]; - } - } catch { /* invalid geojson */ } - return []; -} - -function getWaypoints(yjs: YjsState) { - return yjs.waypoints.toArray().map(waypointFromYMap); -} - -function getNoGoAreas(yjs: YjsState): NoGoArea[] { - return yjs.noGoAreas.toArray().map((yMap: Y.Map) => ({ - points: (yMap.get("points") as Array<{ lat: number; lon: number }>) ?? [], - })).filter((a) => a.points.length >= 3); -} +import { + buildDayGpxFiles, + buildPlanGpx, + buildRouteGpx, + hasDayBreaks, +} from "~/lib/gpx-export"; function download(gpx: string, filename: string) { const blob = new Blob([gpx], { type: "application/gpx+xml" }); @@ -55,68 +34,23 @@ export function ExportButton({ yjs }: { yjs: YjsState }) { }, [open]); const handleExportRoute = useCallback(() => { - const tracks = getTracks(yjs); - const gpx = generateGpx({ name: "trails.cool route", waypoints: [], tracks }); - download(gpx, "route.gpx"); + download(buildRouteGpx(yjs), "route.gpx"); setOpen(false); }, [yjs]); const handleExportPlan = useCallback(() => { - const tracks = getTracks(yjs); - const waypoints = getWaypoints(yjs); - const noGoAreas = getNoGoAreas(yjs); - const notes = yjs.notes.toString() || undefined; - const gpx = generateGpx({ name: "trails.cool route", description: notes, waypoints, tracks, noGoAreas }); - download(gpx, "route-plan.gpx"); + download(buildPlanGpx(yjs), "route-plan.gpx"); setOpen(false); }, [yjs]); const handleExportDays = useCallback(() => { - const tracks = getTracks(yjs); - const waypoints = getWaypoints(yjs); - const allPoints = tracks.flat(); - if (allPoints.length === 0 || waypoints.length === 0) return; - - const days = computeDays(waypoints, tracks); - if (days.length <= 1) { - // Single day — just export the full route - const gpx = generateGpx({ name: "trails.cool route", tracks }); - download(gpx, "route.gpx"); - setOpen(false); - return; - } - - // Find closest track index for each waypoint - const wpTrackIndices = waypoints.map((wp) => { - let bestIdx = 0; - let bestDist = Infinity; - for (let i = 0; i < allPoints.length; i++) { - const dx = allPoints[i]!.lat - wp.lat; - const dy = allPoints[i]!.lon - wp.lon; - const d = dx * dx + dy * dy; - if (d < bestDist) { bestDist = d; bestIdx = i; } - } - return bestIdx; - }); - - for (const day of days) { - const startIdx = wpTrackIndices[day.startWaypointIndex]!; - const endIdx = wpTrackIndices[day.endWaypointIndex]!; - const dayPoints = allPoints.slice(startIdx, endIdx + 1); - const dayName = day.startName && day.endName - ? `Day ${day.dayNumber}: ${day.startName} - ${day.endName}` - : `Day ${day.dayNumber}`; - const gpx = generateGpx({ name: dayName, tracks: [dayPoints] }); - const filename = `day-${day.dayNumber}${day.startName ? `-${day.startName.toLowerCase().replace(/\s+/g, "-")}` : ""}.gpx`; - download(gpx, filename); + for (const file of buildDayGpxFiles(yjs)) { + download(file.gpx, file.filename); } setOpen(false); }, [yjs]); - const hasMultipleDays = (() => { - const waypoints = getWaypoints(yjs); - return waypoints.some((w) => w.isDayBreak); - })(); + const hasMultipleDays = hasDayBreaks(yjs); return (
diff --git a/apps/planner/app/components/MapHelpers.tsx b/apps/planner/app/components/MapHelpers.tsx index e75ba34..2e4b184 100644 --- a/apps/planner/app/components/MapHelpers.tsx +++ b/apps/planner/app/components/MapHelpers.tsx @@ -5,6 +5,7 @@ import type { YjsState } from "~/lib/use-yjs"; import { overlayLayers } from "@trails-cool/map"; import { usePois } from "~/lib/use-pois"; import { Z_CURSOR } from "@trails-cool/map-core"; +import { getBaseLayer, getOverlays, setBaseLayer, setOverlays } from "~/lib/route-data"; // Exposes the Leaflet map instance on window.__leafletMap for E2E testing and external integrations. export function MapExposer() { @@ -188,11 +189,10 @@ export function OverlaySync({ if (suppressRef.current) return; const layer = overlayLayers.find((l) => l.name === e.name); if (!layer) return; - const raw = yjs.routeData.get("overlays") as string | undefined; - const current: string[] = raw ? JSON.parse(raw) : []; + const current = getOverlays(yjs.routeData) ?? []; if (!current.includes(layer.id)) { const updated = [...current, layer.id]; - yjs.routeData.set("overlays", JSON.stringify(updated)); + setOverlays(yjs.routeData, updated); onOverlayChange(updated); } }; @@ -201,16 +201,14 @@ export function OverlaySync({ if (suppressRef.current) return; const layer = overlayLayers.find((l) => l.name === e.name); if (!layer) return; - const raw = yjs.routeData.get("overlays") as string | undefined; - const current: string[] = raw ? JSON.parse(raw) : []; - const updated = current.filter((id) => id !== layer.id); - yjs.routeData.set("overlays", JSON.stringify(updated)); + const updated = (getOverlays(yjs.routeData) ?? []).filter((id) => id !== layer.id); + setOverlays(yjs.routeData, updated); onOverlayChange(updated); }; const handleBaseChange = (e: L.LayersControlEvent) => { if (suppressRef.current) return; - yjs.routeData.set("baseLayer", e.name); + setBaseLayer(yjs.routeData, e.name); }; map.on("overlayadd", handleAdd as L.LeafletEventHandlerFn); @@ -225,11 +223,9 @@ export function OverlaySync({ useEffect(() => { const handleChange = () => { - const raw = yjs.routeData.get("overlays") as string | undefined; - if (raw) { - try { onOverlayChange(JSON.parse(raw)); } catch { /* ignore */ } - } - const base = yjs.routeData.get("baseLayer") as string | undefined; + const overlays = getOverlays(yjs.routeData); + if (overlays) onOverlayChange(overlays); + const base = getBaseLayer(yjs.routeData); if (base) onBaseLayerChange(base); }; yjs.routeData.observe(handleChange); diff --git a/apps/planner/app/components/ProfileSelector.tsx b/apps/planner/app/components/ProfileSelector.tsx index 1c6c5b2..3ee079a 100644 --- a/apps/planner/app/components/ProfileSelector.tsx +++ b/apps/planner/app/components/ProfileSelector.tsx @@ -1,6 +1,7 @@ import { useEffect, useState, useCallback } from "react"; import { useTranslation } from "react-i18next"; import type { YjsState } from "~/lib/use-yjs"; +import { DEFAULT_PROFILE, getProfile, setProfile as setYjsProfile } from "~/lib/route-data"; const PROFILE_IDS = ["fastbike", "safety", "shortest", "car", "trekking"] as const; @@ -10,11 +11,11 @@ interface ProfileSelectorProps { export function ProfileSelector({ yjs }: ProfileSelectorProps) { const { t } = useTranslation("planner"); - const [profile, setProfile] = useState("fastbike"); + const [profile, setProfile] = useState(DEFAULT_PROFILE); useEffect(() => { const update = () => { - const p = yjs.routeData.get("profile") as string | undefined; + const p = getProfile(yjs.routeData); if (p) setProfile(p); }; yjs.routeData.observe(update); @@ -26,7 +27,7 @@ export function ProfileSelector({ yjs }: ProfileSelectorProps) { (e: React.ChangeEvent) => { const value = e.target.value; setProfile(value); - yjs.routeData.set("profile", value); + setYjsProfile(yjs.routeData, value); }, [yjs.routeData], ); diff --git a/apps/planner/app/components/SaveToJournalButton.tsx b/apps/planner/app/components/SaveToJournalButton.tsx index 63ad849..056d4bc 100644 --- a/apps/planner/app/components/SaveToJournalButton.tsx +++ b/apps/planner/app/components/SaveToJournalButton.tsx @@ -1,10 +1,7 @@ import { useState, useCallback } from "react"; import { useTranslation } from "react-i18next"; -import * as Y from "yjs"; import type { YjsState } from "~/lib/use-yjs"; -import { generateGpx } from "@trails-cool/gpx"; -import type { TrackPoint, NoGoArea } from "@trails-cool/gpx"; -import { waypointFromYMap } from "~/lib/waypoint-ymap"; +import { buildPlanGpx } from "~/lib/gpx-export"; interface SaveToJournalButtonProps { yjs: YjsState; @@ -23,28 +20,9 @@ export function SaveToJournalButton({ yjs, sessionId, returnUrl }: SaveToJournal setError(null); try { - // Build GPX from computed track with planning data (no-go areas) - // so the route round-trips correctly through the journal. - let tracks: TrackPoint[][] = []; - const geojsonStr = yjs.routeData.get("geojson") as string | undefined; - if (geojsonStr) { - try { - const geojson = JSON.parse(geojsonStr); - const coords: number[][] = geojson.features?.[0]?.geometry?.coordinates ?? []; - if (coords.length > 0) { - tracks = [coords.map((c) => ({ lat: c[1]!, lon: c[0]!, ele: c[2] }))]; - } - } catch { /* invalid geojson */ } - } - - const noGoAreas: NoGoArea[] = yjs.noGoAreas.toArray().map((yMap: Y.Map) => ({ - points: (yMap.get("points") as Array<{ lat: number; lon: number }>) ?? [], - })).filter((a) => a.points.length >= 3); - - const waypoints = yjs.waypoints.toArray().map(waypointFromYMap); - - const notes = yjs.notes.toString() || undefined; - const gpx = generateGpx({ name: "trails.cool route", description: notes, waypoints, tracks, noGoAreas }); + // Full plan GPX (track + waypoints + no-go areas + notes) so the + // route round-trips correctly through the journal. + const gpx = buildPlanGpx(yjs); // POST to the planner's server-side proxy. The proxy attaches the // journal Bearer token (stored on the session row) and forwards diff --git a/apps/planner/app/components/SessionView.tsx b/apps/planner/app/components/SessionView.tsx index 7a9e170..38650d4 100644 --- a/apps/planner/app/components/SessionView.tsx +++ b/apps/planner/app/components/SessionView.tsx @@ -6,6 +6,7 @@ import type { TFunction } from "i18next"; import * as Sentry from "@sentry/react"; import { useYjs, type YjsState } from "~/lib/use-yjs"; import { useRouting, type RouteError } from "~/lib/use-routing"; +import { getCoordinates } from "~/lib/route-data"; import { useDays } from "~/lib/use-days"; import { useUndo, useUndoShortcuts } from "~/lib/use-undo"; import { ProfileSelector } from "~/components/ProfileSelector"; @@ -208,14 +209,10 @@ export function SessionView({ sessionId, hasJournalCallback, returnUrl, initialW const handleResetZoom = useCallback(() => { const map = window.__leafletMap; if (!map || !yjs) return; - const coordsJson = yjs.routeData.get("coordinates") as string | undefined; - if (!coordsJson) return; - try { - const coords: [number, number, number][] = JSON.parse(coordsJson); - if (coords.length < 2) return; - const latLngs = coords.map((c) => [c[1]!, c[0]!] as [number, number]); - map.fitBounds(L.latLngBounds(latLngs), { padding: [50, 50] }); - } catch { /* ignore */ } + const coords = getCoordinates(yjs.routeData); + if (!coords || coords.length < 2) return; + const latLngs = coords.map((c) => [c[1], c[0]] as [number, number]); + map.fitBounds(L.latLngBounds(latLngs), { padding: [50, 50] }); setIsZoomedByChart(false); }, [yjs]); diff --git a/apps/planner/app/components/WaypointSidebar.tsx b/apps/planner/app/components/WaypointSidebar.tsx index baaf992..0c9c862 100644 --- a/apps/planner/app/components/WaypointSidebar.tsx +++ b/apps/planner/app/components/WaypointSidebar.tsx @@ -1,32 +1,21 @@ import { useEffect, useState, useCallback, useRef } from "react"; -import * as Y from "yjs"; import { useTranslation } from "react-i18next"; import type { YjsState } from "~/lib/use-yjs"; import type { DayStage } from "@trails-cool/gpx"; -import { setOvernight, isOvernight } from "~/lib/overnight"; +import { setOvernight } from "~/lib/overnight"; import { DayBreakdown } from "./DayBreakdown"; import { useNearbyPois } from "~/lib/use-nearby-pois"; import { poiCategories } from "@trails-cool/map-core"; import type { Poi } from "~/lib/overpass"; -import { waypointFromYMap } from "~/lib/waypoint-ymap"; +import { + extractWaypointData, + waypointFromYMap, + waypointToYMap, + type WaypointData, +} from "~/lib/waypoint-ymap"; const NOTE_MAX = 500; -interface WaypointData { - lat: number; - lon: number; - name?: string; - note?: string; - overnight: boolean; -} - -function getWaypointsFromYjs(waypoints: Y.Array>): WaypointData[] { - return waypoints.toArray().map((yMap) => { - const wp = waypointFromYMap(yMap); - return { lat: wp.lat, lon: wp.lon, name: wp.name, note: wp.note, overnight: isOvernight(yMap) }; - }); -} - interface WaypointSidebarProps { yjs: YjsState; routeStats?: { @@ -48,7 +37,7 @@ export function WaypointSidebar({ yjs, routeStats, days, onWaypointHover, onWayp const textareaRef = useRef(null); useEffect(() => { - const update = () => setWaypoints(getWaypointsFromYjs(yjs.waypoints)); + const update = () => setWaypoints(extractWaypointData(yjs.waypoints)); yjs.waypoints.observeDeep(update); update(); return () => yjs.waypoints.unobserveDeep(update); @@ -66,26 +55,10 @@ export function WaypointSidebar({ yjs, routeStats, days, onWaypointHover, onWayp if (from === to || from < 0 || to < 0) return; const item = yjs.waypoints.get(from); if (!item) return; - const data = { - lat: item.get("lat") as number, - lon: item.get("lon") as number, - name: item.get("name") as string | undefined, - note: item.get("note") as string | undefined, - overnight: isOvernight(item), - osmId: item.get("osmId") as number | undefined, - poiTags: item.get("poiTags") as Record | undefined, - }; + const wp = waypointFromYMap(item); yjs.doc.transact(() => { yjs.waypoints.delete(from, 1); - const yMap = new Y.Map(); - yMap.set("lat", data.lat); - yMap.set("lon", data.lon); - if (data.name) yMap.set("name", data.name); - if (data.note) yMap.set("note", data.note); - if (data.overnight) yMap.set("overnight", true); - if (data.osmId !== undefined) yMap.set("osmId", data.osmId); - if (data.poiTags) yMap.set("poiTags", data.poiTags); - yjs.waypoints.insert(to, [yMap]); + yjs.waypoints.insert(to, [waypointToYMap(wp)]); }, "local"); }, [yjs.waypoints, yjs.doc], diff --git a/apps/planner/app/components/YjsDebugPanel.tsx b/apps/planner/app/components/YjsDebugPanel.tsx index d788b95..7139988 100644 --- a/apps/planner/app/components/YjsDebugPanel.tsx +++ b/apps/planner/app/components/YjsDebugPanel.tsx @@ -2,6 +2,7 @@ import { useEffect, useState, useCallback } from "react"; import * as Y from "yjs"; import * as awarenessProtocol from "y-protocols/awareness"; import type { YjsState } from "~/lib/use-yjs"; +import { DEFAULT_PROFILE, clearRouteData, getProfile, setGeojson } from "~/lib/route-data"; // --- localStorage helpers --- @@ -125,8 +126,8 @@ export function YjsDebugPanel({ yjs, sessionId }: { yjs: YjsState; sessionId: st while (yjs.waypoints.length > 0) { yjs.waypoints.delete(0, 1); } - yjs.routeData.delete("geojson"); - yjs.routeData.delete("profile"); + // Nested transact joins this transaction + clearRouteData(yjs.doc, yjs.routeData); }); }, [yjs]); @@ -138,7 +139,7 @@ export function YjsDebugPanel({ yjs, sessionId }: { yjs: YjsState; sessionId: st if (waypoints.length < 2) return; - const profile = (yjs.routeData.get("profile") as string) ?? "trekking"; + const profile = getProfile(yjs.routeData) ?? DEFAULT_PROFILE; const response = await fetch("/api/route", { method: "POST", headers: { "Content-Type": "application/json" }, @@ -147,7 +148,7 @@ export function YjsDebugPanel({ yjs, sessionId }: { yjs: YjsState; sessionId: st if (response.ok) { const geojson = await response.json(); - yjs.routeData.set("geojson", JSON.stringify(geojson)); + setGeojson(yjs.routeData, geojson); } }, [yjs, sessionId]); diff --git a/apps/planner/app/lib/gpx-export.test.ts b/apps/planner/app/lib/gpx-export.test.ts new file mode 100644 index 0000000..110d023 --- /dev/null +++ b/apps/planner/app/lib/gpx-export.test.ts @@ -0,0 +1,144 @@ +import { describe, it, expect } from "vitest"; +import * as Y from "yjs"; +import type { Waypoint } from "@trails-cool/types"; +import { waypointToYMap } from "./waypoint-ymap.ts"; +import { + buildDayGpxFiles, + buildPlanGpx, + buildRouteGpx, + hasDayBreaks, + type PlanDoc, +} from "./gpx-export.ts"; + +function createDoc(): PlanDoc & { doc: Y.Doc } { + const doc = new Y.Doc(); + return { + doc, + routeData: doc.getMap("routeData"), + waypoints: doc.getArray>("waypoints"), + noGoAreas: doc.getArray>("noGoAreas"), + notes: doc.getText("notes"), + }; +} + +function addWaypoints(doc: PlanDoc, waypoints: Waypoint[]): void { + doc.waypoints.push(waypoints.map(waypointToYMap)); +} + +function setTrack(doc: PlanDoc, coords: [number, number, number][]): void { + doc.routeData.set("coordinates", JSON.stringify(coords)); +} + +const TRACK: [number, number, number][] = [ + [13.4, 52.5, 35], + [13.41, 52.51, 40], + [13.42, 52.52, 38], + [13.43, 52.53, 42], +]; + +describe("buildRouteGpx", () => { + it("contains the track but no waypoints", () => { + const doc = createDoc(); + setTrack(doc, TRACK); + addWaypoints(doc, [ + { lat: 52.5, lon: 13.4, name: "Start" }, + { lat: 52.53, lon: 13.43, name: "End" }, + ]); + + const gpx = buildRouteGpx(doc); + expect(gpx).toContain(""); + expect(gpx).toContain('lat="52.5"'); + expect(gpx).not.toContain(" { + const gpx = buildRouteGpx(createDoc()); + expect(gpx).toContain(" { + it("includes waypoints, no-go areas, and notes", () => { + const doc = createDoc(); + setTrack(doc, TRACK); + addWaypoints(doc, [ + { lat: 52.5, lon: 13.4, name: "Start" }, + { lat: 52.53, lon: 13.43, name: "End" }, + ]); + const area = new Y.Map(); + area.set("points", [ + { lat: 52.5, lon: 13.4 }, + { lat: 52.51, lon: 13.41 }, + { lat: 52.52, lon: 13.4 }, + ]); + doc.noGoAreas.push([area]); + doc.notes.insert(0, "Pack sunscreen"); + + const gpx = buildPlanGpx(doc); + expect(gpx).toContain(" { + const doc = createDoc(); + setTrack(doc, TRACK); + const routeTrkpts = (buildRouteGpx(doc).match(/ { + it("hasDayBreaks reflects overnight waypoints", () => { + const doc = createDoc(); + addWaypoints(doc, [{ lat: 52.5, lon: 13.4 }, { lat: 52.53, lon: 13.43 }]); + expect(hasDayBreaks(doc)).toBe(false); + + const withBreak = createDoc(); + addWaypoints(withBreak, [ + { lat: 52.5, lon: 13.4 }, + { lat: 52.51, lon: 13.41, isDayBreak: true }, + { lat: 52.53, lon: 13.43 }, + ]); + expect(hasDayBreaks(withBreak)).toBe(true); + }); + + it("returns [] without a track or waypoints", () => { + expect(buildDayGpxFiles(createDoc())).toEqual([]); + }); + + it("exports a single route file when there are no day breaks", () => { + const doc = createDoc(); + setTrack(doc, TRACK); + addWaypoints(doc, [{ lat: 52.5, lon: 13.4 }, { lat: 52.53, lon: 13.43 }]); + + const files = buildDayGpxFiles(doc); + expect(files).toHaveLength(1); + expect(files[0]!.filename).toBe("route.gpx"); + }); + + it("splits at day-break waypoints into per-day files", () => { + const doc = createDoc(); + setTrack(doc, TRACK); + addWaypoints(doc, [ + { lat: 52.5, lon: 13.4, name: "Alpha" }, + { lat: 52.51, lon: 13.41, name: "Hut", isDayBreak: true }, + { lat: 52.53, lon: 13.43, name: "Omega" }, + ]); + + const files = buildDayGpxFiles(doc); + expect(files).toHaveLength(2); + expect(files[0]!.filename).toBe("day-1-alpha.gpx"); + expect(files[0]!.gpx).toContain("Day 1: Alpha - Hut"); + expect(files[1]!.filename).toBe("day-2-hut.gpx"); + expect(files[1]!.gpx).toContain("Day 2: Hut - Omega"); + + // Day 1 covers track points up to the hut; day 2 the rest. + expect((files[0]!.gpx.match(/; + waypoints: Y.Array>; + noGoAreas: Y.Array>; + notes: Y.Text; +} + +export const ROUTE_NAME = "trails.cool route"; + +function getTracks(doc: PlanDoc): TrackPoint[][] { + const coords = getCoordinates(doc.routeData); + if (!coords || coords.length === 0) return []; + return [coords.map((c) => ({ lat: c[1], lon: c[0], ele: c[2] }))]; +} + +/** The computed track only — no waypoints, no planning data. */ +export function buildRouteGpx(doc: PlanDoc): string { + return generateGpx({ name: ROUTE_NAME, waypoints: [], tracks: getTracks(doc) }); +} + +/** + * The full plan: track, waypoints, no-go areas, and session notes — + * everything needed to round-trip the route through the Journal. + */ +export function buildPlanGpx(doc: PlanDoc): string { + return generateGpx({ + name: ROUTE_NAME, + description: doc.notes.toString() || undefined, + waypoints: extractWaypoints(doc.waypoints), + tracks: getTracks(doc), + noGoAreas: extractNoGoAreas(doc.noGoAreas), + }); +} + +export function hasDayBreaks(doc: PlanDoc): boolean { + return extractWaypoints(doc.waypoints).some((w) => w.isDayBreak); +} + +export interface GpxFile { + filename: string; + gpx: string; +} + +/** + * One GPX file per day stage, split at day-break waypoints. A route + * without day breaks (or without a computed track) yields a single + * full-route file. + */ +export function buildDayGpxFiles(doc: PlanDoc): GpxFile[] { + const tracks = getTracks(doc); + const waypoints = extractWaypoints(doc.waypoints); + const allPoints = tracks.flat(); + if (allPoints.length === 0 || waypoints.length === 0) return []; + + const days = computeDays(waypoints, tracks); + if (days.length <= 1) { + return [{ filename: "route.gpx", gpx: generateGpx({ name: ROUTE_NAME, tracks }) }]; + } + + // Find closest track index for each waypoint + const wpTrackIndices = waypoints.map((wp: Waypoint) => { + let bestIdx = 0; + let bestDist = Infinity; + for (let i = 0; i < allPoints.length; i++) { + const dx = allPoints[i]!.lat - wp.lat; + const dy = allPoints[i]!.lon - wp.lon; + const d = dx * dx + dy * dy; + if (d < bestDist) { + bestDist = d; + bestIdx = i; + } + } + return bestIdx; + }); + + return days.map((day) => { + const startIdx = wpTrackIndices[day.startWaypointIndex]!; + const endIdx = wpTrackIndices[day.endWaypointIndex]!; + const dayPoints = allPoints.slice(startIdx, endIdx + 1); + const dayName = + day.startName && day.endName + ? `Day ${day.dayNumber}: ${day.startName} - ${day.endName}` + : `Day ${day.dayNumber}`; + const filename = `day-${day.dayNumber}${ + day.startName ? `-${day.startName.toLowerCase().replace(/\s+/g, "-")}` : "" + }.gpx`; + return { filename, gpx: generateGpx({ name: dayName, tracks: [dayPoints] }) }; + }); +} diff --git a/apps/planner/app/lib/route-data.test.ts b/apps/planner/app/lib/route-data.test.ts new file mode 100644 index 0000000..44b7474 --- /dev/null +++ b/apps/planner/app/lib/route-data.test.ts @@ -0,0 +1,243 @@ +import { describe, it, expect } from "vitest"; +import * as Y from "yjs"; +import type { EnrichedRoute } from "./route-merge.ts"; +import { + DEFAULT_PROFILE, + PROFILE_KEY, + ROAD_METADATA_KEYS, + clearRouteData, + extractNoGoAreas, + getBaseLayer, + getColorMode, + getCoordinates, + getGeojson, + getOverlays, + getPoiCategories, + getProfile, + parseJsonArray, + readComputedRoute, + readRoadMetadata, + setBaseLayer, + setColorMode, + setGeojson, + setOverlays, + setPoiCategories, + setProfile, + writeComputedRoute, +} from "./route-data.ts"; + +function createDoc(): { doc: Y.Doc; routeData: Y.Map } { + const doc = new Y.Doc(); + return { doc, routeData: doc.getMap("routeData") }; +} + +function enrichedFixture(overrides: Partial = {}): EnrichedRoute { + return { + coordinates: [ + [13.4, 52.5, 35], + [13.41, 52.51, 40], + [13.42, 52.52, 38], + ], + segmentBoundaries: [0], + surfaces: ["asphalt", "asphalt", "gravel"], + highways: ["cycleway", "cycleway", "track"], + maxspeeds: ["30", "30", ""], + smoothnesses: ["good", "good", "bad"], + tracktypes: ["", "", "grade2"], + cycleways: ["lane", "lane", ""], + bikeroutes: ["", "rcn", "rcn"], + totalLength: 2500, + totalAscend: 12, + totalTime: 600, + geojson: { + type: "FeatureCollection", + features: [ + { + type: "Feature", + properties: {}, + geometry: { + type: "LineString", + coordinates: [ + [13.4, 52.5, 35], + [13.41, 52.51, 40], + [13.42, 52.52, 38], + ], + }, + }, + ], + }, + ...overrides, + }; +} + +describe("parseJsonArray", () => { + it("returns [] for undefined and corrupt JSON", () => { + expect(parseJsonArray(undefined)).toEqual([]); + expect(parseJsonArray("{not json")).toEqual([]); + }); + + it("parses a JSON array", () => { + expect(parseJsonArray("[1,2,3]")).toEqual([1, 2, 3]); + }); +}); + +describe("computed route round-trip", () => { + it("writes an enriched route and reads it back", () => { + const { doc, routeData } = createDoc(); + const enriched = enrichedFixture(); + + writeComputedRoute(doc, routeData, enriched); + const computed = readComputedRoute(routeData); + + expect(computed.coordinates).toEqual(enriched.coordinates); + expect(computed.segmentBoundaries).toEqual(enriched.segmentBoundaries); + for (const key of ROAD_METADATA_KEYS) { + expect(computed[key]).toEqual(enriched[key]); + } + expect(getGeojson(routeData)).toBe(JSON.stringify(enriched.geojson)); + }); + + it("keeps previous metadata when a recompute yields empty arrays", () => { + const { doc, routeData } = createDoc(); + writeComputedRoute(doc, routeData, enrichedFixture()); + writeComputedRoute(doc, routeData, enrichedFixture({ surfaces: [] })); + + // empty surfaces are not written; the previous value remains + expect(readComputedRoute(routeData).surfaces).toEqual(["asphalt", "asphalt", "gravel"]); + }); + + it("writes in a single transaction", () => { + const { doc, routeData } = createDoc(); + let events = 0; + routeData.observe(() => events++); + writeComputedRoute(doc, routeData, enrichedFixture()); + expect(events).toBe(1); + }); + + it("returns null coordinates and empty arrays for an empty document", () => { + const { routeData } = createDoc(); + const computed = readComputedRoute(routeData); + expect(computed.coordinates).toBeNull(); + expect(computed.segmentBoundaries).toEqual([]); + expect(computed.surfaces).toEqual([]); + }); +}); + +describe("getCoordinates", () => { + it("falls back to geojson for documents without a coordinates key", () => { + const { routeData } = createDoc(); + setGeojson(routeData, enrichedFixture().geojson); + + expect(getCoordinates(routeData)).toEqual([ + [13.4, 52.5, 35], + [13.41, 52.51, 40], + [13.42, 52.52, 38], + ]); + }); + + it("defaults missing elevation to 0 in the geojson fallback", () => { + const { routeData } = createDoc(); + setGeojson(routeData, { + type: "FeatureCollection", + features: [ + { + type: "Feature", + properties: {}, + geometry: { type: "LineString", coordinates: [[13.4, 52.5], [13.41, 52.51]] }, + }, + ], + }); + + expect(getCoordinates(routeData)).toEqual([ + [13.4, 52.5, 0], + [13.41, 52.51, 0], + ]); + }); + + it("returns null when neither key is set", () => { + const { routeData } = createDoc(); + expect(getCoordinates(routeData)).toBeNull(); + }); +}); + +describe("profile and view preferences", () => { + it("round-trips the profile", () => { + const { routeData } = createDoc(); + expect(getProfile(routeData)).toBeUndefined(); + setProfile(routeData, DEFAULT_PROFILE); + expect(getProfile(routeData)).toBe(DEFAULT_PROFILE); + expect(routeData.has(PROFILE_KEY)).toBe(true); + }); + + it("defaults colorMode to plain and round-trips it", () => { + const { routeData } = createDoc(); + expect(getColorMode(routeData)).toBe("plain"); + setColorMode(routeData, "surface"); + expect(getColorMode(routeData)).toBe("surface"); + }); + + it("round-trips baseLayer and overlays", () => { + const { routeData } = createDoc(); + expect(getBaseLayer(routeData)).toBeUndefined(); + expect(getOverlays(routeData)).toBeUndefined(); + setBaseLayer(routeData, "OpenTopoMap"); + setOverlays(routeData, ["hiking", "cycling"]); + expect(getBaseLayer(routeData)).toBe("OpenTopoMap"); + expect(getOverlays(routeData)).toEqual(["hiking", "cycling"]); + }); + + it("treats corrupt overlays/poiCategories as unset", () => { + const { routeData } = createDoc(); + routeData.set("overlays", "{corrupt"); + routeData.set("poiCategories", "{corrupt"); + expect(getOverlays(routeData)).toBeUndefined(); + expect(getPoiCategories(routeData)).toBeUndefined(); + }); + + it("round-trips poiCategories", () => { + const { routeData } = createDoc(); + setPoiCategories(routeData, ["water", "camping"]); + expect(getPoiCategories(routeData)).toEqual(["water", "camping"]); + }); +}); + +describe("clearRouteData", () => { + it("removes the computed route and profile but keeps view preferences", () => { + const { doc, routeData } = createDoc(); + writeComputedRoute(doc, routeData, enrichedFixture()); + setProfile(routeData, "trekking"); + setColorMode(routeData, "surface"); + setOverlays(routeData, ["hiking"]); + + clearRouteData(doc, routeData); + + expect(getCoordinates(routeData)).toBeNull(); + expect(getGeojson(routeData)).toBeUndefined(); + expect(getProfile(routeData)).toBeUndefined(); + expect(readRoadMetadata(routeData).surfaces).toEqual([]); + expect(getColorMode(routeData)).toBe("surface"); + expect(getOverlays(routeData)).toEqual(["hiking"]); + }); +}); + +describe("extractNoGoAreas", () => { + it("reads areas and drops degenerate ones", () => { + const doc = new Y.Doc(); + const noGoAreas = doc.getArray>("noGoAreas"); + + const valid = new Y.Map(); + valid.set("points", [ + { lat: 52.5, lon: 13.4 }, + { lat: 52.51, lon: 13.41 }, + { lat: 52.52, lon: 13.4 }, + ]); + const degenerate = new Y.Map(); + degenerate.set("points", [{ lat: 52.5, lon: 13.4 }]); + const empty = new Y.Map(); + noGoAreas.push([valid, degenerate, empty]); + + const areas = extractNoGoAreas(noGoAreas); + expect(areas).toHaveLength(1); + expect(areas[0]!.points).toHaveLength(3); + }); +}); diff --git a/apps/planner/app/lib/route-data.ts b/apps/planner/app/lib/route-data.ts new file mode 100644 index 0000000..45b8ce1 --- /dev/null +++ b/apps/planner/app/lib/route-data.ts @@ -0,0 +1,221 @@ +import * as Y from "yjs"; +import type { EnrichedRoute } from "./route-merge.ts"; + +// The schema of the shared `routeData` Y.Map (and the `noGoAreas` Y.Array). +// This module is the only place that knows the key strings and JSON +// encoding of the collaborative document's route state — consumers read +// and write through the typed functions below. Waypoints have their own +// schema module: waypoint-ymap.ts. + +export type ColorMode = + | "plain" + | "elevation" + | "surface" + | "grade" + | "highway" + | "maxspeed" + | "smoothness" + | "tracktype" + | "cycleway" + | "bikeroute"; + +export const DEFAULT_PROFILE = "fastbike"; + +/** routeData key observed externally (use-routing recomputes on change). */ +export const PROFILE_KEY = "profile"; + +/** Per-coordinate road attributes BRouter enriches the route with. */ +export const ROAD_METADATA_KEYS = [ + "surfaces", + "highways", + "maxspeeds", + "smoothnesses", + "tracktypes", + "cycleways", + "bikeroutes", +] as const; + +export type RoadMetadataKey = (typeof ROAD_METADATA_KEYS)[number]; +export type RoadMetadata = Record; + +/** The computed-route portion of routeData, decoded. */ +export interface ComputedRoute extends RoadMetadata { + /** [lon, lat, ele] triples, GeoJSON axis order. */ + coordinates: [number, number, number][] | null; + segmentBoundaries: number[]; +} + +export interface NoGoAreaData { + points: Array<{ lat: number; lon: number }>; +} + +export function parseJsonArray(json: string | undefined): T[] { + if (!json) return []; + try { + return JSON.parse(json); + } catch { + return []; + } +} + +/** Like parseJsonArray, but distinguishes "absent or unparseable" from "empty". */ +function parseJsonArrayOrUndefined(json: string | undefined): T[] | undefined { + if (!json) return undefined; + try { + return JSON.parse(json); + } catch { + return undefined; + } +} + +// --- Computed route (written by the routing host, read by everyone) --- + +export function getGeojson(routeData: Y.Map): string | undefined { + return routeData.get("geojson") as string | undefined; +} + +/** + * Route coordinates as [lon, lat, ele] triples. Reads the "coordinates" + * key; falls back to extracting them from "geojson" for documents written + * before the coordinates key existed. + */ +export function getCoordinates(routeData: Y.Map): [number, number, number][] | null { + const coordsJson = routeData.get("coordinates") as string | undefined; + if (coordsJson) { + try { + return JSON.parse(coordsJson); + } catch { + /* fall through to geojson */ + } + } + const geojson = getGeojson(routeData); + if (geojson) { + try { + const parsed = JSON.parse(geojson); + const coords: number[][] | undefined = parsed.features?.[0]?.geometry?.coordinates; + if (coords) { + return coords.map((c) => [c[0]!, c[1]!, c[2] ?? 0] as [number, number, number]); + } + } catch { + /* invalid geojson */ + } + } + return null; +} + +export function readRoadMetadata(routeData: Y.Map): RoadMetadata { + const metadata = {} as RoadMetadata; + for (const key of ROAD_METADATA_KEYS) { + metadata[key] = parseJsonArray(routeData.get(key) as string | undefined); + } + return metadata; +} + +export function readComputedRoute(routeData: Y.Map): ComputedRoute { + return { + coordinates: getCoordinates(routeData), + segmentBoundaries: parseJsonArray( + routeData.get("segmentBoundaries") as string | undefined, + ), + ...readRoadMetadata(routeData), + }; +} + +/** + * Stores an enriched route for all participants in one transaction. + * Metadata keys are only written when non-empty, mirroring what the + * routing host has always done — a recompute that yields no metadata + * leaves the previous arrays in place. + */ +export function writeComputedRoute( + doc: Y.Doc, + routeData: Y.Map, + enriched: EnrichedRoute, +): void { + doc.transact(() => { + routeData.set("geojson", JSON.stringify(enriched.geojson)); + routeData.set("coordinates", JSON.stringify(enriched.coordinates)); + routeData.set("segmentBoundaries", JSON.stringify(enriched.segmentBoundaries)); + for (const key of ROAD_METADATA_KEYS) { + if (enriched[key]?.length) { + routeData.set(key, JSON.stringify(enriched[key])); + } + } + }); +} + +/** Debug/recovery escape hatch: store a raw GeoJSON route without enrichment. */ +export function setGeojson(routeData: Y.Map, geojson: unknown): void { + routeData.set("geojson", JSON.stringify(geojson)); +} + +/** Removes the computed route and profile (view preferences are kept). */ +export function clearRouteData(doc: Y.Doc, routeData: Y.Map): void { + doc.transact(() => { + routeData.delete("geojson"); + routeData.delete("coordinates"); + routeData.delete("segmentBoundaries"); + for (const key of ROAD_METADATA_KEYS) { + routeData.delete(key); + } + routeData.delete(PROFILE_KEY); + }); +} + +// --- Routing profile --- + +export function getProfile(routeData: Y.Map): string | undefined { + return routeData.get(PROFILE_KEY) as string | undefined; +} + +export function setProfile(routeData: Y.Map, profile: string): void { + routeData.set(PROFILE_KEY, profile); +} + +// --- View preferences (shared across participants) --- + +export function getColorMode(routeData: Y.Map): ColorMode { + return (routeData.get("colorMode") as ColorMode | undefined) ?? "plain"; +} + +export function setColorMode(routeData: Y.Map, mode: ColorMode): void { + routeData.set("colorMode", mode); +} + +export function getBaseLayer(routeData: Y.Map): string | undefined { + return routeData.get("baseLayer") as string | undefined; +} + +export function setBaseLayer(routeData: Y.Map, name: string): void { + routeData.set("baseLayer", name); +} + +export function getOverlays(routeData: Y.Map): string[] | undefined { + return parseJsonArrayOrUndefined(routeData.get("overlays") as string | undefined); +} + +export function setOverlays(routeData: Y.Map, ids: string[]): void { + routeData.set("overlays", JSON.stringify(ids)); +} + +export function getPoiCategories(routeData: Y.Map): string[] | undefined { + return parseJsonArrayOrUndefined( + routeData.get("poiCategories") as string | undefined, + ); +} + +export function setPoiCategories(routeData: Y.Map, categories: string[]): void { + routeData.set("poiCategories", JSON.stringify(categories)); +} + +// --- No-go areas (their own Y.Array on the document) --- + +/** Reads all no-go areas, dropping degenerate ones (fewer than 3 points). */ +export function extractNoGoAreas(noGoAreas: Y.Array>): NoGoAreaData[] { + return noGoAreas + .toArray() + .map((yMap) => ({ + points: (yMap.get("points") as Array<{ lat: number; lon: number }>) ?? [], + })) + .filter((a) => a.points.length >= 3); +} diff --git a/apps/planner/app/lib/use-days.ts b/apps/planner/app/lib/use-days.ts index f56326a..d67f93e 100644 --- a/apps/planner/app/lib/use-days.ts +++ b/apps/planner/app/lib/use-days.ts @@ -1,10 +1,9 @@ import { useEffect, useState } from "react"; -import * as Y from "yjs"; import { computeDays, type DayStage } from "@trails-cool/gpx"; -import type { Waypoint } from "@trails-cool/types"; import type { TrackPoint } from "@trails-cool/gpx"; import type { YjsState } from "./use-yjs.ts"; -import { isOvernight } from "./overnight.ts"; +import { extractWaypoints } from "./waypoint-ymap.ts"; +import { getCoordinates } from "./route-data.ts"; /** * Reactive hook that computes day stages from Yjs waypoints and route data. @@ -17,13 +16,7 @@ export function useDays(yjs: YjsState | null): DayStage[] { if (!yjs) return; const recompute = () => { - // Extract waypoints with isDayBreak from Yjs - const waypoints: Waypoint[] = yjs.waypoints.toArray().map((yMap: Y.Map) => ({ - lat: yMap.get("lat") as number, - lon: yMap.get("lon") as number, - name: yMap.get("name") as string | undefined, - isDayBreak: isOvernight(yMap) || undefined, - })); + const waypoints = extractWaypoints(yjs.waypoints); // Check if any waypoint has isDayBreak const hasBreaks = waypoints.some((w) => w.isDayBreak); @@ -32,26 +25,18 @@ export function useDays(yjs: YjsState | null): DayStage[] { return; } - // Extract track points from route coordinates stored in Yjs - const coordsStr = yjs.routeData.get("coordinates") as string | undefined; - if (!coordsStr) { + const coords = getCoordinates(yjs.routeData); + if (!coords) { setDays([]); return; } - try { - // coordinates are stored as [[lon, lat, ele], ...] (GeoJSON format) - const coords: number[][] = JSON.parse(coordsStr); - const trackPoints: TrackPoint[] = coords.map((c) => ({ - lat: c[1]!, - lon: c[0]!, - ele: c[2], - })); - - setDays(computeDays(waypoints, [trackPoints])); - } catch { - setDays([]); - } + const trackPoints: TrackPoint[] = coords.map((c) => ({ + lat: c[1], + lon: c[0], + ele: c[2], + })); + setDays(computeDays(waypoints, [trackPoints])); }; yjs.waypoints.observeDeep(recompute); diff --git a/apps/planner/app/lib/use-elevation-data.ts b/apps/planner/app/lib/use-elevation-data.ts index 9ad3e01..f29e71c 100644 --- a/apps/planner/app/lib/use-elevation-data.ts +++ b/apps/planner/app/lib/use-elevation-data.ts @@ -1,7 +1,13 @@ import { useEffect, useState } from "react"; import * as Y from "yjs"; -import type { ColorMode } from "~/components/ColoredRoute"; import type { ElevationPoint } from "~/lib/elevation-chart-draw"; +import { + getColorMode, + getGeojson, + readRoadMetadata, + type ColorMode, + type RoadMetadata, +} from "~/lib/route-data"; function haversine(lat1: number, lon1: number, lat2: number, lon2: number): number { const R = 6371000; @@ -44,21 +50,9 @@ function extractElevation(geojsonStr: string): ElevationPoint[] { } } -function parseJsonArray(json: string | undefined): string[] { - if (!json) return []; - try { return JSON.parse(json); } catch { return []; } -} - -export interface ElevationData { +export interface ElevationData extends RoadMetadata { points: ElevationPoint[]; colorMode: ColorMode; - surfaces: string[]; - highways: string[]; - maxspeeds: string[]; - smoothnesses: string[]; - tracktypes: string[]; - cycleways: string[]; - bikeroutes: string[]; } export function useElevationData(routeData: Y.Map): ElevationData { @@ -76,17 +70,11 @@ export function useElevationData(routeData: Y.Map): ElevationData { useEffect(() => { const update = () => { - const geojson = routeData.get("geojson") as string | undefined; + const geojson = getGeojson(routeData); setData({ points: geojson ? extractElevation(geojson) : [], - colorMode: (routeData.get("colorMode") as ColorMode | undefined) ?? "plain", - surfaces: parseJsonArray(routeData.get("surfaces") as string | undefined), - highways: parseJsonArray(routeData.get("highways") as string | undefined), - maxspeeds: parseJsonArray(routeData.get("maxspeeds") as string | undefined), - smoothnesses: parseJsonArray(routeData.get("smoothnesses") as string | undefined), - tracktypes: parseJsonArray(routeData.get("tracktypes") as string | undefined), - cycleways: parseJsonArray(routeData.get("cycleways") as string | undefined), - bikeroutes: parseJsonArray(routeData.get("bikeroutes") as string | undefined), + colorMode: getColorMode(routeData), + ...readRoadMetadata(routeData), }); }; routeData.observe(update); diff --git a/apps/planner/app/lib/use-profile-defaults.ts b/apps/planner/app/lib/use-profile-defaults.ts index 66cff60..43f7be3 100644 --- a/apps/planner/app/lib/use-profile-defaults.ts +++ b/apps/planner/app/lib/use-profile-defaults.ts @@ -2,6 +2,7 @@ import { useEffect, useRef } from "react"; import type { YjsState } from "./use-yjs.ts"; import type { PoiState } from "./use-pois.ts"; import { getCategoriesForProfile } from "@trails-cool/map-core"; +import { getProfile } from "./route-data.ts"; /** * Auto-enable relevant POI categories when the routing profile changes. @@ -15,7 +16,7 @@ export function useProfileDefaults(yjs: YjsState | null, poiState: PoiState): vo if (!yjs) return; const handleChange = () => { - const profile = yjs.routeData.get("profile") as string | undefined; + const profile = getProfile(yjs.routeData); if (!profile) return; // Skip initial load — respect existing state diff --git a/apps/planner/app/lib/use-routing.ts b/apps/planner/app/lib/use-routing.ts index a9d897a..9ffabe1 100644 --- a/apps/planner/app/lib/use-routing.ts +++ b/apps/planner/app/lib/use-routing.ts @@ -9,6 +9,14 @@ import { type NoGoArea, } from "./route-merge.ts"; import { SegmentCache } from "./segment-cache.ts"; +import { + DEFAULT_PROFILE, + PROFILE_KEY, + extractNoGoAreas, + getProfile, + writeComputedRoute, +} from "./route-data.ts"; +import { extractWaypoints } from "./waypoint-ymap.ts"; interface RouteStats { distance?: number; @@ -22,10 +30,7 @@ interface WaypointData { } function getWaypointsFromYjs(waypoints: Y.Array>): WaypointData[] { - return waypoints.toArray().map((yMap) => ({ - lat: yMap.get("lat") as number, - lon: yMap.get("lon") as number, - })); + return extractWaypoints(waypoints).map((wp) => ({ lat: wp.lat, lon: wp.lon })); } function restoreWaypoints(yjs: YjsState, snapshot: WaypointData[], restoringRef: React.RefObject) { @@ -72,9 +77,7 @@ export function useRouting(yjs: YjsState | null, sessionId: string) { if (!yjs || !isHost || waypoints.length < 2) return; // Collect no-go areas from Yjs - const noGoAreas: NoGoArea[] = yjs.noGoAreas.toArray().map((yMap) => ({ - points: (yMap.get("points") as Array<{ lat: number; lon: number }>) ?? [], - })).filter((a) => a.points.length >= 3); + const noGoAreas: NoGoArea[] = extractNoGoAreas(yjs.noGoAreas); // Save current waypoints so we can restore on failure const snapshotBeforeCompute = getWaypointsFromYjs(yjs.waypoints); @@ -84,7 +87,7 @@ export function useRouting(yjs: YjsState | null, sessionId: string) { const controller = new AbortController(); inflightAbortRef.current = controller; - const profile = (yjs.routeData.get("profile") as string) ?? "fastbike"; + const profile = getProfile(yjs.routeData) ?? DEFAULT_PROFILE; const noGoHash = hashNoGoAreas(noGoAreas); const cache = segmentCacheRef.current; @@ -158,32 +161,7 @@ export function useRouting(yjs: YjsState | null, sessionId: string) { }); // Store enriched route data in Yjs for all participants - yjs.doc.transact(() => { - yjs.routeData.set("geojson", JSON.stringify(enriched.geojson)); - yjs.routeData.set("coordinates", JSON.stringify(enriched.coordinates)); - yjs.routeData.set("segmentBoundaries", JSON.stringify(enriched.segmentBoundaries)); - if (enriched.surfaces?.length) { - yjs.routeData.set("surfaces", JSON.stringify(enriched.surfaces)); - } - if (enriched.highways?.length) { - yjs.routeData.set("highways", JSON.stringify(enriched.highways)); - } - if (enriched.maxspeeds?.length) { - yjs.routeData.set("maxspeeds", JSON.stringify(enriched.maxspeeds)); - } - if (enriched.smoothnesses?.length) { - yjs.routeData.set("smoothnesses", JSON.stringify(enriched.smoothnesses)); - } - if (enriched.tracktypes?.length) { - yjs.routeData.set("tracktypes", JSON.stringify(enriched.tracktypes)); - } - if (enriched.cycleways?.length) { - yjs.routeData.set("cycleways", JSON.stringify(enriched.cycleways)); - } - if (enriched.bikeroutes?.length) { - yjs.routeData.set("bikeroutes", JSON.stringify(enriched.bikeroutes)); - } - }); + writeComputedRoute(yjs.doc, yjs.routeData, enriched); } catch (err) { // A superseding request aborted this one — leave state alone so // the newer call's result becomes authoritative. @@ -222,7 +200,7 @@ export function useRouting(yjs: YjsState | null, sessionId: string) { // Observe routeData for profile changes const profileObserver = (event: Y.YMapEvent) => { - if (event.keysChanged.has("profile")) { + if (event.keysChanged.has(PROFILE_KEY)) { triggerRecompute(); } }; diff --git a/apps/planner/app/lib/use-waypoint-manager.ts b/apps/planner/app/lib/use-waypoint-manager.ts index 866e27d..b832d2b 100644 --- a/apps/planner/app/lib/use-waypoint-manager.ts +++ b/apps/planner/app/lib/use-waypoint-manager.ts @@ -1,27 +1,17 @@ import { useEffect, useState, useCallback } from "react"; import * as Y from "yjs"; import type { YjsState } from "~/lib/use-yjs"; -import type { ColorMode } from "~/components/ColoredRoute"; import { usePois } from "~/lib/use-pois"; import { snapToPoi } from "~/lib/poi-snap"; -import { isOvernight } from "~/lib/overnight"; import { findSegmentForPoint } from "~/components/ColoredRoute"; -import { waypointFromYMap } from "~/lib/waypoint-ymap"; +import { extractWaypointData, type WaypointData } from "~/lib/waypoint-ymap"; +import { + getColorMode, + readComputedRoute, + type ColorMode, +} from "~/lib/route-data"; -export interface WaypointData { - lat: number; - lon: number; - name?: string; - note?: string; - overnight: boolean; -} - -function getWaypointsFromYjs(waypoints: Y.Array>): WaypointData[] { - return waypoints.toArray().map((yMap) => { - const wp = waypointFromYMap(yMap); - return { lat: wp.lat, lon: wp.lon, name: wp.name, note: wp.note, overnight: isOvernight(yMap) }; - }); -} +export type { WaypointData }; function pointToSegmentDist( pLat: number, pLon: number, @@ -38,11 +28,6 @@ function pointToSegmentDist( return Math.sqrt((pLon - projLon) ** 2 + (pLat - projLat) ** 2); } -function parseJsonArray(json: string | undefined): T[] { - if (!json) return []; - try { return JSON.parse(json); } catch { return []; } -} - export interface RouteState { waypoints: WaypointData[]; routeCoordinates: [number, number, number][] | null; @@ -80,7 +65,7 @@ export function useWaypointManager( // Sync waypoints from Yjs useEffect(() => { const update = () => { - const wps = getWaypointsFromYjs(yjs.waypoints); + const wps = extractWaypointData(yjs.waypoints); setState((prev) => ({ ...prev, waypoints: wps })); if (wps.length >= 2 && onRouteRequest) { onRouteRequest(wps); @@ -94,39 +79,12 @@ export function useWaypointManager( // Sync route data from Yjs useEffect(() => { const update = () => { - const coordsJson = yjs.routeData.get("coordinates") as string | undefined; - const boundsJson = yjs.routeData.get("segmentBoundaries") as string | undefined; - const modeVal = yjs.routeData.get("colorMode") as ColorMode | undefined; - - let routeCoordinates: [number, number, number][] | null = null; - if (coordsJson) { - try { routeCoordinates = JSON.parse(coordsJson); } catch { /* ignore */ } - } else { - // Fallback: parse from geojson for backwards compat - const geojson = yjs.routeData.get("geojson") as string | undefined; - if (geojson) { - try { - const parsed = JSON.parse(geojson); - const coords = parsed.features?.[0]?.geometry?.coordinates; - if (coords) { - routeCoordinates = coords.map((c: number[]) => [c[0]!, c[1]!, c[2] ?? 0] as [number, number, number]); - } - } catch { /* ignore */ } - } - } - + const { coordinates, ...computed } = readComputedRoute(yjs.routeData); setState((prev) => ({ ...prev, - routeCoordinates, - segmentBoundaries: parseJsonArray(boundsJson), - surfaces: parseJsonArray(yjs.routeData.get("surfaces") as string | undefined), - highways: parseJsonArray(yjs.routeData.get("highways") as string | undefined), - maxspeeds: parseJsonArray(yjs.routeData.get("maxspeeds") as string | undefined), - smoothnesses: parseJsonArray(yjs.routeData.get("smoothnesses") as string | undefined), - tracktypes: parseJsonArray(yjs.routeData.get("tracktypes") as string | undefined), - cycleways: parseJsonArray(yjs.routeData.get("cycleways") as string | undefined), - bikeroutes: parseJsonArray(yjs.routeData.get("bikeroutes") as string | undefined), - colorMode: modeVal ?? "plain", + ...computed, + routeCoordinates: coordinates, + colorMode: getColorMode(yjs.routeData), })); }; yjs.routeData.observe(update); diff --git a/apps/planner/app/lib/use-yjs-poi-sync.ts b/apps/planner/app/lib/use-yjs-poi-sync.ts index fc4ee3c..e7cfcc0 100644 --- a/apps/planner/app/lib/use-yjs-poi-sync.ts +++ b/apps/planner/app/lib/use-yjs-poi-sync.ts @@ -1,8 +1,7 @@ import { useEffect, useRef } from "react"; import type { YjsState } from "./use-yjs.ts"; import type { PoiState } from "./use-pois.ts"; - -const YJS_KEY_POI_CATEGORIES = "poiCategories"; +import { getPoiCategories, setPoiCategories } from "./route-data.ts"; /** * Bidirectional sync between POI/overlay state and Yjs routeData. @@ -21,7 +20,7 @@ export function useYjsPoiSync(yjs: YjsState | null, poiState: PoiState): void { prevCategories.current = current; suppressYjsUpdate.current = true; - yjs.routeData.set(YJS_KEY_POI_CATEGORIES, JSON.stringify(current)); + setPoiCategories(yjs.routeData, current); // Allow Yjs observer to fire but suppress our handler queueMicrotask(() => { suppressYjsUpdate.current = false; }); }, [yjs, poiState.enabledCategories]); @@ -33,17 +32,12 @@ export function useYjsPoiSync(yjs: YjsState | null, poiState: PoiState): void { const handleChange = () => { if (suppressYjsUpdate.current) return; - const raw = yjs.routeData.get(YJS_KEY_POI_CATEGORIES) as string | undefined; - if (!raw) return; + const categories = getPoiCategories(yjs.routeData); + if (!categories) return; - try { - const categories = JSON.parse(raw) as string[]; - if (!arraysEqual(categories, prevCategories.current)) { - prevCategories.current = categories; - poiState.setEnabledCategories(categories); - } - } catch { - // Invalid JSON in Yjs — ignore + if (!arraysEqual(categories, prevCategories.current)) { + prevCategories.current = categories; + poiState.setEnabledCategories(categories); } }; diff --git a/apps/planner/app/lib/waypoint-ymap.test.ts b/apps/planner/app/lib/waypoint-ymap.test.ts new file mode 100644 index 0000000..2421bda --- /dev/null +++ b/apps/planner/app/lib/waypoint-ymap.test.ts @@ -0,0 +1,77 @@ +import { describe, it, expect } from "vitest"; +import * as Y from "yjs"; +import type { Waypoint } from "@trails-cool/types"; +import { + extractWaypointData, + extractWaypoints, + waypointFromYMap, + waypointToYMap, +} from "./waypoint-ymap.ts"; + +function createArray(): Y.Array> { + return new Y.Doc().getArray>("waypoints"); +} + +/** Y.Maps only expose values once integrated into a document. */ +function roundTrip(wp: Waypoint): Waypoint { + const arr = createArray(); + arr.push([waypointToYMap(wp)]); + return waypointFromYMap(arr.get(0)!); +} + +describe("waypoint round-trip", () => { + it("preserves all fields through toYMap → fromYMap", () => { + const wp: Waypoint = { + lat: 52.5, + lon: 13.4, + name: "Hut", + note: "Book ahead", + isDayBreak: true, + osmId: 12345, + poiTags: { tourism: "alpine_hut" }, + }; + expect(roundTrip(wp)).toEqual(wp); + }); + + it("leaves optional fields undefined", () => { + const wp = roundTrip({ lat: 1, lon: 2 }); + expect(wp).toEqual({ + lat: 1, + lon: 2, + name: undefined, + note: undefined, + isDayBreak: undefined, + osmId: undefined, + poiTags: undefined, + }); + }); +}); + +describe("extractWaypoints", () => { + it("reads the whole list in order", () => { + const arr = createArray(); + arr.push([ + waypointToYMap({ lat: 1, lon: 2, name: "A" }), + waypointToYMap({ lat: 3, lon: 4, isDayBreak: true }), + ]); + + const wps = extractWaypoints(arr); + expect(wps).toHaveLength(2); + expect(wps[0]!.name).toBe("A"); + expect(wps[1]!.isDayBreak).toBe(true); + }); +}); + +describe("extractWaypointData", () => { + it("flattens isDayBreak to a plain overnight boolean", () => { + const arr = createArray(); + arr.push([ + waypointToYMap({ lat: 1, lon: 2 }), + waypointToYMap({ lat: 3, lon: 4, isDayBreak: true, note: "n" }), + ]); + + const data = extractWaypointData(arr); + expect(data[0]!.overnight).toBe(false); + expect(data[1]!).toEqual({ lat: 3, lon: 4, name: undefined, note: "n", overnight: true }); + }); +}); diff --git a/apps/planner/app/lib/waypoint-ymap.ts b/apps/planner/app/lib/waypoint-ymap.ts index d2ac67c..9f27c25 100644 --- a/apps/planner/app/lib/waypoint-ymap.ts +++ b/apps/planner/app/lib/waypoint-ymap.ts @@ -39,3 +39,27 @@ export function waypointToYMap(wp: Waypoint): Y.Map { applyWaypointToYMap(yMap, wp); return yMap; } + +/** Reads the whole shared waypoint list as plain Waypoints. */ +export function extractWaypoints(waypoints: Y.Array>): Waypoint[] { + return waypoints.toArray().map(waypointFromYMap); +} + +/** Waypoint flattened for UI state (isDayBreak as a plain boolean). */ +export interface WaypointData { + lat: number; + lon: number; + name?: string; + note?: string; + overnight: boolean; +} + +export function extractWaypointData(waypoints: Y.Array>): WaypointData[] { + return extractWaypoints(waypoints).map((wp) => ({ + lat: wp.lat, + lon: wp.lon, + name: wp.name, + note: wp.note, + overnight: wp.isDayBreak === true, + })); +} From 154dccf31298d076de9e67059b0dcf4b603742e3 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 9 Jun 2026 23:36:01 +0000 Subject: [PATCH 2/4] chore(deps): Bump node from 25-slim to 26-slim in /apps/planner Bumps node from 25-slim to 26-slim. --- updated-dependencies: - dependency-name: node dependency-version: 26-slim dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- apps/planner/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/planner/Dockerfile b/apps/planner/Dockerfile index e4fd73e..2130008 100644 --- a/apps/planner/Dockerfile +++ b/apps/planner/Dockerfile @@ -1,4 +1,4 @@ -FROM node:25-slim AS base +FROM node:26-slim AS base # curl is used by the docker-compose healthcheck (node:25-slim is Debian # trixie-slim and ships neither curl nor wget by default). RUN apt-get update && apt-get install -y --no-install-recommends ca-certificates curl && rm -rf /var/lib/apt/lists/* From ce5fd6da0f96c1b93aebb0c1767978f15acfa515 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 9 Jun 2026 23:41:11 +0000 Subject: [PATCH 3/4] chore(deps): Bump node from 25-slim to 26-slim in /apps/journal Bumps node from 25-slim to 26-slim. --- updated-dependencies: - dependency-name: node dependency-version: 26-slim dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- apps/journal/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/journal/Dockerfile b/apps/journal/Dockerfile index 86a0a89..caada24 100644 --- a/apps/journal/Dockerfile +++ b/apps/journal/Dockerfile @@ -1,4 +1,4 @@ -FROM node:25-slim AS base +FROM node:26-slim AS base # curl is used by the docker-compose healthcheck (node:25-slim is Debian # trixie-slim and ships neither curl nor wget by default). RUN apt-get update && apt-get install -y --no-install-recommends ca-certificates curl && rm -rf /var/lib/apt/lists/* From 855244747c7118dbce4c2727e94cda22ff40d557 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ullrich=20Sch=C3=A4fer?= Date: Wed, 10 Jun 2026 02:05:24 +0200 Subject: [PATCH 4/4] journal: typed job seam + Komoot credentials through the manager Two related fixes from the architecture review: Typed job seam. Job payloads were `unknown` end-to-end: every handler opened with `job.data as SomePayload`, every enqueue site passed a bare string queue name and an unchecked object, and a typo meant a runtime failure in a background worker. packages/jobs gains defineJob(), which keeps the payload typed inside the handler and performs the contravariance cast once inside the package. The journal declares all 14 queues and their payload shapes in app/jobs/payloads.ts; enqueue()/ enqueueOptional() and defineJournalJob() key off that map, so enqueue sites and handlers cannot drift and queue names are compile-checked. Komoot credential bypass. The bulk-import route enqueued the raw credentials JSONB in the pg-boss payload, skipping withFreshCredentials entirely: credentials sat at rest in the job table, were never refreshed if stale, and markNeedsRelink never fired. The payload now carries only the serviceId; the handler resolves fresh credentials through the ConnectedServiceManager at execution time, and marks the import batch failed (instead of leaving it pending forever) when credential resolution itself fails. Co-Authored-By: Claude Fable 5 --- .../app/jobs/backfill-user-keypairs.ts | 6 +- apps/journal/app/jobs/consumed-jti-sweep.ts | 6 +- apps/journal/app/jobs/deliver-activity.ts | 8 +-- apps/journal/app/jobs/demo-bot-generate.ts | 6 +- apps/journal/app/jobs/demo-bot-prune.ts | 6 +- apps/journal/app/jobs/federation-kv-sweep.ts | 6 +- .../app/jobs/garmin-import-activity.ts | 13 ++-- apps/journal/app/jobs/import-batches-sweep.ts | 6 +- .../app/jobs/komoot-bulk-import.test.ts | 62 +++++++++++++++++++ apps/journal/app/jobs/komoot-bulk-import.ts | 43 +++++++------ apps/journal/app/jobs/notifications-fanout.ts | 13 ++-- apps/journal/app/jobs/notifications-purge.ts | 6 +- apps/journal/app/jobs/payloads.test.ts | 47 ++++++++++++++ apps/journal/app/jobs/payloads.ts | 41 ++++++++++++ apps/journal/app/jobs/poll-remote-actor.ts | 13 ++-- apps/journal/app/jobs/poll-remote-outboxes.ts | 6 +- apps/journal/app/jobs/send-welcome-email.ts | 13 ++-- apps/journal/app/lib/boss.server.ts | 24 +++++-- .../app/lib/komoot-bulk-import.server.ts | 23 ++++++- .../app/routes/api.sync.komoot.import.ts | 9 +-- apps/journal/server.ts | 4 +- packages/jobs/src/index.ts | 3 +- packages/jobs/src/types.test.ts | 28 +++++++++ packages/jobs/src/types.ts | 35 +++++++++-- 24 files changed, 327 insertions(+), 100 deletions(-) create mode 100644 apps/journal/app/jobs/komoot-bulk-import.test.ts create mode 100644 apps/journal/app/jobs/payloads.test.ts create mode 100644 apps/journal/app/jobs/payloads.ts create mode 100644 packages/jobs/src/types.test.ts diff --git a/apps/journal/app/jobs/backfill-user-keypairs.ts b/apps/journal/app/jobs/backfill-user-keypairs.ts index cd267cf..1f79e4b 100644 --- a/apps/journal/app/jobs/backfill-user-keypairs.ts +++ b/apps/journal/app/jobs/backfill-user-keypairs.ts @@ -1,4 +1,4 @@ -import type { JobDefinition } from "@trails-cool/jobs"; +import { defineJournalJob } from "./payloads.ts"; import { ensureUserKeypair, listUsersWithoutKeypair } from "../lib/federation-keys.server.ts"; import { logger } from "../lib/logger.server.ts"; @@ -10,7 +10,7 @@ import { logger } from "../lib/logger.server.ts"; * only touches users whose public_key IS NULL, so re-runs are no-ops. * New users get keys at registration and never appear in this workload. */ -export const backfillUserKeypairsJob: JobDefinition = { +export const backfillUserKeypairsJob = defineJournalJob({ name: "backfill-user-keypairs", retryLimit: 3, expireInSeconds: 300, @@ -23,4 +23,4 @@ export const backfillUserKeypairsJob: JobDefinition = { logger.info({ candidates: ids.length, generated }, "backfill-user-keypairs"); return { candidates: ids.length, generated }; }, -}; +}); diff --git a/apps/journal/app/jobs/consumed-jti-sweep.ts b/apps/journal/app/jobs/consumed-jti-sweep.ts index 1b8a7c0..5be848c 100644 --- a/apps/journal/app/jobs/consumed-jti-sweep.ts +++ b/apps/journal/app/jobs/consumed-jti-sweep.ts @@ -1,4 +1,4 @@ -import type { JobDefinition } from "@trails-cool/jobs"; +import { defineJournalJob } from "./payloads.ts"; import { lt } from "drizzle-orm"; import { consumedJwtJti } from "@trails-cool/db/schema/journal"; import { getDb } from "../lib/db.ts"; @@ -13,7 +13,7 @@ import { logger } from "../lib/logger.server.ts"; * * See planner-audit #2 Phase B. */ -export const consumedJtiSweepJob: JobDefinition = { +export const consumedJtiSweepJob = defineJournalJob({ name: "consumed-jti-sweep", cron: "45 3 * * *", // daily at 03:45 UTC (offset from notifications-purge) retryLimit: 1, @@ -28,4 +28,4 @@ export const consumedJtiSweepJob: JobDefinition = { logger.info({ purged }, "consumed-jti-sweep"); return { purged }; }, -}; +}); diff --git a/apps/journal/app/jobs/deliver-activity.ts b/apps/journal/app/jobs/deliver-activity.ts index c40ab44..b75631c 100644 --- a/apps/journal/app/jobs/deliver-activity.ts +++ b/apps/journal/app/jobs/deliver-activity.ts @@ -1,4 +1,4 @@ -import type { JobDefinition } from "@trails-cool/jobs"; +import { defineJournalJob } from "./payloads.ts"; import { and, eq } from "drizzle-orm"; import { activities, users } from "@trails-cool/db/schema/journal"; import { getDb } from "../lib/db.ts"; @@ -39,12 +39,12 @@ async function paceHost(host: string): Promise { * failed and pg-boss retries; exhausting the budget is the permanent * failure, logged by the final catch. */ -export const deliverActivityJob: JobDefinition = { +export const deliverActivityJob = defineJournalJob({ name: "deliver-activity", expireInSeconds: 60, async handler(jobs) { for (const job of jobs) { - const p = job.data as DeliveryPayload; + const p = job.data; try { await deliverOne(p); } catch (err) { @@ -56,7 +56,7 @@ export const deliverActivityJob: JobDefinition = { } } }, -}; +}); async function deliverOne(p: DeliveryPayload): Promise { const federation = getFederation(); diff --git a/apps/journal/app/jobs/demo-bot-generate.ts b/apps/journal/app/jobs/demo-bot-generate.ts index a25020a..7257a3d 100644 --- a/apps/journal/app/jobs/demo-bot-generate.ts +++ b/apps/journal/app/jobs/demo-bot-generate.ts @@ -1,4 +1,4 @@ -import type { JobDefinition } from "@trails-cool/jobs"; +import { defineJournalJob } from "./payloads.ts"; import { DEMO_BACKFILL_TARGET, DEMO_DAILY_CAP, @@ -21,7 +21,7 @@ import { logger } from "../lib/logger.server.ts"; * 3. Otherwise apply the decide-to-walk gate (local hour + p=0.09) and * daily cap; on pass, insert one route+activity via `generateOneWalk`. */ -export const demoBotGenerateJob: JobDefinition = { +export const demoBotGenerateJob = defineJournalJob({ name: "demo-bot-generate", cron: "0,30 * * * *", retryLimit: 1, @@ -57,4 +57,4 @@ export const demoBotGenerateJob: JobDefinition = { await refreshDemoBotGauges(); return { mode: "single", routeId: id }; }, -}; +}); diff --git a/apps/journal/app/jobs/demo-bot-prune.ts b/apps/journal/app/jobs/demo-bot-prune.ts index 6ac2855..b065bf8 100644 --- a/apps/journal/app/jobs/demo-bot-prune.ts +++ b/apps/journal/app/jobs/demo-bot-prune.ts @@ -1,4 +1,4 @@ -import type { JobDefinition } from "@trails-cool/jobs"; +import { defineJournalJob } from "./payloads.ts"; import { demoRetentionDays, isDemoBotEnabled, @@ -11,7 +11,7 @@ import { logger } from "../lib/logger.server.ts"; * Daily prune. Deletes synthetic rows older than * `DEMO_BOT_RETENTION_DAYS` (default 14). Never touches real users. */ -export const demoBotPruneJob: JobDefinition = { +export const demoBotPruneJob = defineJournalJob({ name: "demo-bot-prune", cron: "15 3 * * *", retryLimit: 1, @@ -24,4 +24,4 @@ export const demoBotPruneJob: JobDefinition = { await refreshDemoBotGauges(); return { days, ...counts }; }, -}; +}); diff --git a/apps/journal/app/jobs/federation-kv-sweep.ts b/apps/journal/app/jobs/federation-kv-sweep.ts index b21a7bb..6bec691 100644 --- a/apps/journal/app/jobs/federation-kv-sweep.ts +++ b/apps/journal/app/jobs/federation-kv-sweep.ts @@ -1,4 +1,4 @@ -import type { JobDefinition } from "@trails-cool/jobs"; +import { defineJournalJob } from "./payloads.ts"; import { PostgresKvStore } from "../lib/federation-kv.server.ts"; import { logger } from "../lib/logger.server.ts"; @@ -7,7 +7,7 @@ import { logger } from "../lib/logger.server.ts"; * nonces and caches carry TTLs; reads already filter expired rows, this * keeps the table from growing unbounded). */ -export const federationKvSweepJob: JobDefinition = { +export const federationKvSweepJob = defineJournalJob({ name: "federation-kv-sweep", cron: "15 4 * * *", // daily at 04:15 UTC (offset from the other sweeps) retryLimit: 1, @@ -17,4 +17,4 @@ export const federationKvSweepJob: JobDefinition = { logger.info({ purged }, "federation-kv-sweep"); return { purged }; }, -}; +}); diff --git a/apps/journal/app/jobs/garmin-import-activity.ts b/apps/journal/app/jobs/garmin-import-activity.ts index edff614..b396150 100644 --- a/apps/journal/app/jobs/garmin-import-activity.ts +++ b/apps/journal/app/jobs/garmin-import-activity.ts @@ -1,9 +1,6 @@ -import type { JobDefinition } from "@trails-cool/jobs"; +import { defineJournalJob } from "./payloads.ts"; import { logger } from "../lib/logger.server.ts"; -import { - runGarminActivityImport, - type GarminImportData, -} from "../lib/connected-services/providers/garmin/import.server.ts"; +import { runGarminActivityImport } from "../lib/connected-services/providers/garmin/import.server.ts"; // Garmin webhook notifications enqueue here (spec: garmin-import, // "Push-notification activity import"): the webhook answers 200 @@ -11,14 +8,14 @@ import { // download, FIT→GPX, activity creation. Backfill bursts deliver many // notifications at once; the queue absorbs them and pg-boss retries // transient download failures. -export const garminImportActivityJob: JobDefinition = { +export const garminImportActivityJob = defineJournalJob({ name: "garmin-import-activity", retryLimit: 3, expireInSeconds: 300, async handler(jobs) { const batch = Array.isArray(jobs) ? jobs : [jobs]; for (const job of batch) { - const data = job.data as GarminImportData; + const data = job.data; try { await runGarminActivityImport(data); } catch (err) { @@ -30,4 +27,4 @@ export const garminImportActivityJob: JobDefinition = { } } }, -}; +}); diff --git a/apps/journal/app/jobs/import-batches-sweep.ts b/apps/journal/app/jobs/import-batches-sweep.ts index 2cb405d..5bee88e 100644 --- a/apps/journal/app/jobs/import-batches-sweep.ts +++ b/apps/journal/app/jobs/import-batches-sweep.ts @@ -1,4 +1,4 @@ -import type { JobDefinition } from "@trails-cool/jobs"; +import { defineJournalJob } from "./payloads.ts"; import { and, lt, inArray, count } from "drizzle-orm"; import { getDb } from "../lib/db.ts"; import { importBatches, type ImportBatchStatus } from "@trails-cool/db/schema/journal"; @@ -7,7 +7,7 @@ import { logger } from "../lib/logger.server.ts"; const STALE_MS = 10 * 60 * 1000; const STALE_STATUSES: ImportBatchStatus[] = ["pending", "running"]; -export const importBatchesSweepJob: JobDefinition = { +export const importBatchesSweepJob = defineJournalJob({ name: "import-batches-sweep", cron: "* * * * *", retryLimit: 0, @@ -39,4 +39,4 @@ export const importBatchesSweepJob: JobDefinition = { logger.info({ count: result.length }, "import-batches-sweep: marked stale batches as failed"); }, -}; +}); diff --git a/apps/journal/app/jobs/komoot-bulk-import.test.ts b/apps/journal/app/jobs/komoot-bulk-import.test.ts new file mode 100644 index 0000000..bcf3d42 --- /dev/null +++ b/apps/journal/app/jobs/komoot-bulk-import.test.ts @@ -0,0 +1,62 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("../lib/logger.server.ts", () => ({ + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, +})); + +const withFreshCredentials = vi.fn(); +vi.mock("../lib/connected-services/manager.ts", () => ({ + withFreshCredentials: (...args: unknown[]) => withFreshCredentials(...args), +})); + +const runKomootBulkImport = vi.fn(); +const markBatchFailed = vi.fn(); +vi.mock("../lib/komoot-bulk-import.server.ts", () => ({ + runKomootBulkImport: (...args: unknown[]) => runKomootBulkImport(...args), + markBatchFailed: (...args: unknown[]) => markBatchFailed(...args), +})); + +import { komootBulkImportJob } from "./komoot-bulk-import.ts"; + +type HandlerJobs = Parameters[0]; + +function jobWith(data: unknown): HandlerJobs { + return [{ id: "j1", data }] as unknown as HandlerJobs; +} + +const PAYLOAD = { batchId: "batch-1", userId: "user-1", serviceId: "svc-1" }; + +describe("komoot-bulk-import job", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("resolves credentials through the manager — only the serviceId crosses the queue", async () => { + const creds = { mode: "public", komootUserId: "k1" }; + withFreshCredentials.mockImplementation(async (_serviceId, fn) => fn(creds)); + runKomootBulkImport.mockResolvedValue(undefined); + + await komootBulkImportJob.handler(jobWith(PAYLOAD)); + + expect(withFreshCredentials).toHaveBeenCalledWith("svc-1", expect.any(Function)); + expect(runKomootBulkImport).toHaveBeenCalledWith("batch-1", "user-1", creds); + expect(markBatchFailed).not.toHaveBeenCalled(); + }); + + it("marks the batch failed and rethrows when credential resolution fails", async () => { + withFreshCredentials.mockRejectedValue(new Error("needs relink")); + + await expect(komootBulkImportJob.handler(jobWith(PAYLOAD))).rejects.toThrow("needs relink"); + + expect(runKomootBulkImport).not.toHaveBeenCalled(); + expect(markBatchFailed).toHaveBeenCalledWith("batch-1", "needs relink"); + }); + + it("also marks failed when the import itself rejects (markBatchFailed is a no-op on terminal batches)", async () => { + withFreshCredentials.mockImplementation(async (_serviceId, fn) => fn({ mode: "public" })); + runKomootBulkImport.mockRejectedValue(new Error("komoot 500")); + + await expect(komootBulkImportJob.handler(jobWith(PAYLOAD))).rejects.toThrow("komoot 500"); + expect(markBatchFailed).toHaveBeenCalledWith("batch-1", "komoot 500"); + }); +}); diff --git a/apps/journal/app/jobs/komoot-bulk-import.ts b/apps/journal/app/jobs/komoot-bulk-import.ts index 87d788d..fa0c9a2 100644 --- a/apps/journal/app/jobs/komoot-bulk-import.ts +++ b/apps/journal/app/jobs/komoot-bulk-import.ts @@ -1,29 +1,36 @@ -import type { JobDefinition } from "@trails-cool/jobs"; +import { defineJournalJob } from "./payloads.ts"; import { logger } from "../lib/logger.server.ts"; -import { runKomootBulkImport } from "../lib/komoot-bulk-import.server.ts"; +import { withFreshCredentials } from "../lib/connected-services/manager.ts"; +import { + markBatchFailed, + runKomootBulkImport, + type KomootCreds, +} from "../lib/komoot-bulk-import.server.ts"; -type KomootCreds = - | { mode: "public"; komootUserId: string } - | { mode: "authenticated"; email: string; encryptedPassword: string; komootUserId: string }; - -interface KomootBulkImportData { - batchId: string; - userId: string; - creds: KomootCreds; -} - -export const komootBulkImportJob: JobDefinition = { +export const komootBulkImportJob = defineJournalJob({ name: "komoot-bulk-import", retryLimit: 1, expireInSeconds: 1800, async handler(jobs) { const batch = Array.isArray(jobs) ? jobs : [jobs]; for (const job of batch) { - // pg-boss serialized payload — caller (enqueueOptional) wrote it - // as KomootBulkImportData. Narrow at the boundary. - const { batchId, userId, creds } = job.data as KomootBulkImportData; + const { batchId, userId, serviceId } = job.data; logger.info({ batchId, userId }, "komoot bulk import job started"); - await runKomootBulkImport(batchId, userId, creds); + try { + // Credentials are resolved through the ConnectedServiceManager at + // execution time — the payload carries only the serviceId, so + // nothing credential-shaped sits in the job table and a relink + // between enqueue and execution is picked up here. + await withFreshCredentials(serviceId, (creds) => + runKomootBulkImport(batchId, userId, creds as KomootCreds), + ); + } catch (err) { + // runKomootBulkImport marks its own failures; this covers errors + // before it ran (service missing/not active/needs relink), where + // the batch would otherwise stay "pending" forever. + await markBatchFailed(batchId, err instanceof Error ? err.message : String(err)); + throw err; + } } }, -}; +}); diff --git a/apps/journal/app/jobs/notifications-fanout.ts b/apps/journal/app/jobs/notifications-fanout.ts index ef96f78..70caae9 100644 --- a/apps/journal/app/jobs/notifications-fanout.ts +++ b/apps/journal/app/jobs/notifications-fanout.ts @@ -1,21 +1,17 @@ -import type { JobDefinition } from "@trails-cool/jobs"; +import { defineJournalJob } from "./payloads.ts"; import { and, eq, isNotNull } from "drizzle-orm"; import { getDb } from "../lib/db.ts"; import { activities, follows, users } from "@trails-cool/db/schema/journal"; import { createNotification } from "../lib/notifications.server.ts"; import { logger } from "../lib/logger.server.ts"; -interface FanoutData { - activityId: string; -} - /** * Fan out an `activity_published` notification to every accepted * follower of the activity's owner. Idempotent at the DB level via the * `(recipient_user_id, type, subject_id)` unique partial index — a * retry after partial failure won't double-insert. */ -export const notificationsFanoutJob: JobDefinition = { +export const notificationsFanoutJob = defineJournalJob({ name: "notifications-fanout", retryLimit: 3, expireInSeconds: 300, @@ -24,11 +20,10 @@ export const notificationsFanoutJob: JobDefinition = { // process whichever shape we get. const batch = Array.isArray(job) ? job : [job]; for (const item of batch) { - const data = item.data as FanoutData; - await fanout(data.activityId); + await fanout(item.data.activityId); } }, -}; +}); export async function fanout(activityId: string): Promise { const db = getDb(); diff --git a/apps/journal/app/jobs/notifications-purge.ts b/apps/journal/app/jobs/notifications-purge.ts index cf87fa9..625689c 100644 --- a/apps/journal/app/jobs/notifications-purge.ts +++ b/apps/journal/app/jobs/notifications-purge.ts @@ -1,4 +1,4 @@ -import type { JobDefinition } from "@trails-cool/jobs"; +import { defineJournalJob } from "./payloads.ts"; import { purgeReadOlderThan } from "../lib/notifications.server.ts"; import { logger } from "../lib/logger.server.ts"; @@ -7,7 +7,7 @@ import { logger } from "../lib/logger.server.ts"; * than 90 days; unread rows are kept indefinitely so users never miss * an event. */ -export const notificationsPurgeJob: JobDefinition = { +export const notificationsPurgeJob = defineJournalJob({ name: "notifications-purge", cron: "30 3 * * *", // daily at 03:30 UTC (offset from demo-bot-prune to spread load) retryLimit: 1, @@ -18,4 +18,4 @@ export const notificationsPurgeJob: JobDefinition = { logger.info({ days, purged }, "notifications-purge"); return { days, purged }; }, -}; +}); diff --git a/apps/journal/app/jobs/payloads.test.ts b/apps/journal/app/jobs/payloads.test.ts new file mode 100644 index 0000000..00e4844 --- /dev/null +++ b/apps/journal/app/jobs/payloads.test.ts @@ -0,0 +1,47 @@ +import { describe, it, expect } from "vitest"; + +// Importing every job module pulls in their (transitively heavy) +// server deps; mock the leaf modules with side effects so this stays a +// unit test of the registry wiring. +import { vi } from "vitest"; +vi.mock("../lib/db.ts", () => ({ getDb: vi.fn() })); +vi.mock("../lib/logger.server.ts", () => ({ + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, +})); + +describe("job registry", () => { + it("every job's queue name is unique and a key of JobPayloads", async () => { + const modules = await Promise.all([ + import("./backfill-user-keypairs.ts"), + import("./consumed-jti-sweep.ts"), + import("./deliver-activity.ts"), + import("./demo-bot-generate.ts"), + import("./demo-bot-prune.ts"), + import("./federation-kv-sweep.ts"), + import("./garmin-import-activity.ts"), + import("./import-batches-sweep.ts"), + import("./komoot-bulk-import.ts"), + import("./notifications-fanout.ts"), + import("./notifications-purge.ts"), + import("./poll-remote-actor.ts"), + import("./poll-remote-outboxes.ts"), + import("./send-welcome-email.ts"), + ]); + + const names = modules.flatMap((m) => + Object.values(m as Record) + .filter( + (v): v is { name: string; handler: unknown } => + typeof v === "object" && v !== null && "handler" in v && "name" in v, + ) + .map((def) => def.name), + ); + + expect(names).toHaveLength(14); + expect(new Set(names).size).toBe(names.length); + // pg-boss v11+ queue-name constraint, mirrored from packages/jobs + for (const name of names) { + expect(name).toMatch(/^[A-Za-z0-9_.-]+$/); + } + }); +}); diff --git a/apps/journal/app/jobs/payloads.ts b/apps/journal/app/jobs/payloads.ts new file mode 100644 index 0000000..6178f5c --- /dev/null +++ b/apps/journal/app/jobs/payloads.ts @@ -0,0 +1,41 @@ +import { defineJob, type JobDefinition, type TypedJobDefinition } from "@trails-cool/jobs"; +import type { DeliveryPayload } from "../lib/federation-delivery.server.ts"; +import type { GarminImportData } from "../lib/connected-services/providers/garmin/import.server.ts"; + +/** + * Every journal job queue and its payload shape, in one place. The + * typed `enqueue` / `enqueueOptional` in boss.server.ts and the + * `defineJournalJob` helper below both key off this map, so an enqueue + * site and its handler cannot drift apart, and a queue-name typo is a + * compile error instead of an orphaned queue. + * + * `void` marks cron-only jobs that are never enqueued with data. + */ +export interface JobPayloads { + "backfill-user-keypairs": Record; + "consumed-jti-sweep": void; + "deliver-activity": DeliveryPayload; + "demo-bot-generate": void; + "demo-bot-prune": void; + "federation-kv-sweep": void; + "garmin-import-activity": GarminImportData; + "import-batches-sweep": void; + "komoot-bulk-import": { batchId: string; userId: string; serviceId: string }; + "notifications-fanout": { activityId: string }; + "notifications-purge": void; + "poll-remote-actor": { actorIri: string }; + "poll-remote-outboxes": void; + "send-welcome-email": { email: string; username: string }; +} + +export type JobName = keyof JobPayloads; + +/** + * defineJob, constrained to the journal's queue map: the name must be + * a known queue and the handler's payload type follows from it. + */ +export function defineJournalJob( + definition: TypedJobDefinition & { name: K }, +): JobDefinition { + return defineJob(definition); +} diff --git a/apps/journal/app/jobs/poll-remote-actor.ts b/apps/journal/app/jobs/poll-remote-actor.ts index 31582c4..18dc1d7 100644 --- a/apps/journal/app/jobs/poll-remote-actor.ts +++ b/apps/journal/app/jobs/poll-remote-actor.ts @@ -1,26 +1,23 @@ -import type { JobDefinition } from "@trails-cool/jobs"; +import { defineJournalJob } from "./payloads.ts"; import { pollRemoteActor } from "../lib/federation-ingest.server.ts"; import { logger } from "../lib/logger.server.ts"; -interface PollPayload { - actorIri?: string; -} - /** * Poll one remote trails actor's outbox (spec §7). Enqueued by the * inbox Accept(Follow) listener (first poll, 7.5) and fanned out by * the poll-remote-outboxes cron sweep (7.1). */ -export const pollRemoteActorJob: JobDefinition = { +export const pollRemoteActorJob = defineJournalJob({ name: "poll-remote-actor", retryLimit: 2, expireInSeconds: 120, async handler(jobs) { for (const job of jobs) { - const { actorIri } = (job.data ?? {}) as PollPayload; + // Defensive: jobs enqueued before the typed seam may carry no data. + const actorIri = job.data?.actorIri; if (!actorIri) continue; const result = await pollRemoteActor(actorIri); logger.info({ actorIri, result }, "poll-remote-actor"); } }, -}; +}); diff --git a/apps/journal/app/jobs/poll-remote-outboxes.ts b/apps/journal/app/jobs/poll-remote-outboxes.ts index c6cf085..1847ad2 100644 --- a/apps/journal/app/jobs/poll-remote-outboxes.ts +++ b/apps/journal/app/jobs/poll-remote-outboxes.ts @@ -1,4 +1,4 @@ -import type { JobDefinition } from "@trails-cool/jobs"; +import { defineJournalJob } from "./payloads.ts"; import { listActorsDuePolling } from "../lib/federation-ingest.server.ts"; import { enqueueOptional } from "../lib/boss.server.ts"; import { logger } from "../lib/logger.server.ts"; @@ -9,7 +9,7 @@ import { logger } from "../lib/logger.server.ts"; * been polled within the last hour, and fan out one poll-remote-actor * job each. Per-host pacing lives in the poll itself. */ -export const pollRemoteOutboxesJob: JobDefinition = { +export const pollRemoteOutboxesJob = defineJournalJob({ name: "poll-remote-outboxes", cron: "*/5 * * * *", retryLimit: 1, @@ -22,4 +22,4 @@ export const pollRemoteOutboxesJob: JobDefinition = { if (due.length > 0) logger.info({ due: due.length }, "poll-remote-outboxes sweep"); return { due: due.length }; }, -}; +}); diff --git a/apps/journal/app/jobs/send-welcome-email.ts b/apps/journal/app/jobs/send-welcome-email.ts index b8a426d..69551d3 100644 --- a/apps/journal/app/jobs/send-welcome-email.ts +++ b/apps/journal/app/jobs/send-welcome-email.ts @@ -1,26 +1,21 @@ -import type { JobDefinition } from "@trails-cool/jobs"; +import { defineJournalJob } from "./payloads.ts"; import { sendWelcome } from "../lib/email.server.ts"; import { logger } from "../lib/logger.server.ts"; -interface WelcomeEmailData { - email: string; - username: string; -} - /** * Queue-backed welcome email send. The old code did * `sendWelcome(...).catch(log)` inline, which silently dropped failures. * pg-boss retries on transient failure (3 attempts) and surfaces persistent * failures via the dead-letter queue / logs. */ -export const sendWelcomeEmailJob: JobDefinition = { +export const sendWelcomeEmailJob = defineJournalJob({ name: "send-welcome-email", retryLimit: 3, expireInSeconds: 120, async handler(job) { const batch = Array.isArray(job) ? job : [job]; for (const item of batch) { - const { email, username } = item.data as WelcomeEmailData; + const { email, username } = item.data; try { await sendWelcome(email, username); } catch (err) { @@ -29,4 +24,4 @@ export const sendWelcomeEmailJob: JobDefinition = { } } }, -}; +}); diff --git a/apps/journal/app/lib/boss.server.ts b/apps/journal/app/lib/boss.server.ts index a224d38..94970a4 100644 --- a/apps/journal/app/lib/boss.server.ts +++ b/apps/journal/app/lib/boss.server.ts @@ -16,6 +16,7 @@ // global registry key. import { logger } from "./logger.server.ts"; +import type { JobName, JobPayloads } from "../jobs/payloads.ts"; // Structurally typed (we only need `send`) so we don't have to pull // pg-boss into the journal app's dep graph just for the typedef. @@ -51,20 +52,33 @@ export function getBoss(): BossLike { return boss; } +/** + * Enqueue a job. The queue name must be a key of JobPayloads and the + * payload must match its declared shape. Throws if the queue is down — + * use this when the caller's correctness depends on the job existing + * (e.g. a batch row that would otherwise wait forever). + */ +export async function enqueue( + queue: K, + data: JobPayloads[K], + options?: BossSendOptions, +): Promise { + await getBoss().send(queue, data, options); +} + /** * Best-effort enqueue: log + swallow errors so a downstream queue * outage doesn't fail the user-visible request that triggered the * fan-out. Use this for "fire and forget" notifications work. */ -export async function enqueueOptional( - queue: string, - data: unknown, +export async function enqueueOptional( + queue: K, + data: JobPayloads[K], ctx: Record = {}, options?: BossSendOptions, ): Promise { try { - const boss = getBoss(); - await boss.send(queue, data, options); + await enqueue(queue, data, options); } catch (err) { logger.warn({ err, queue, ...ctx }, "boss.send failed; continuing"); } diff --git a/apps/journal/app/lib/komoot-bulk-import.server.ts b/apps/journal/app/lib/komoot-bulk-import.server.ts index 7468fd7..474be09 100644 --- a/apps/journal/app/lib/komoot-bulk-import.server.ts +++ b/apps/journal/app/lib/komoot-bulk-import.server.ts @@ -1,4 +1,4 @@ -import { eq } from "drizzle-orm"; +import { and, eq, notInArray } from "drizzle-orm"; import { getDb } from "./db.ts"; import { importBatches } from "@trails-cool/db/schema/journal"; import { fetchKomootTours, fetchKomootTourGpx } from "./komoot.server.ts"; @@ -7,10 +7,29 @@ import { createActivity } from "./activities.server.ts"; import { decrypt } from "./crypto.server.ts"; import { logger } from "./logger.server.ts"; -type KomootCreds = +export type KomootCreds = | { mode: "public"; komootUserId: string } | { mode: "authenticated"; email: string; encryptedPassword: string; komootUserId: string }; +/** + * Marks a batch failed unless it already reached a terminal state. + * Used by the job handler when credential resolution fails before + * runKomootBulkImport (which owns failure-marking for its own errors) + * ever runs — otherwise the batch would sit "pending" forever. + */ +export async function markBatchFailed(batchId: string, message: string): Promise { + const db = getDb(); + await db + .update(importBatches) + .set({ status: "failed", errorMessage: message, completedAt: new Date() }) + .where( + and( + eq(importBatches.id, batchId), + notInArray(importBatches.status, ["completed", "failed"]), + ), + ); +} + function getBasicAuthToken(creds: KomootCreds): string | undefined { if (creds.mode !== "authenticated") return undefined; const password = decrypt(creds.encryptedPassword); diff --git a/apps/journal/app/routes/api.sync.komoot.import.ts b/apps/journal/app/routes/api.sync.komoot.import.ts index 4c84a95..2394eeb 100644 --- a/apps/journal/app/routes/api.sync.komoot.import.ts +++ b/apps/journal/app/routes/api.sync.komoot.import.ts @@ -7,7 +7,7 @@ import { data } from "react-router"; import type { Route } from "./+types/api.sync.komoot.import"; import { requireSessionUser } from "~/lib/auth/session.server"; import { getService } from "~/lib/connected-services/manager"; -import { getBoss } from "~/lib/boss.server"; +import { enqueue } from "~/lib/boss.server"; import { getDb } from "~/lib/db"; import { importBatches } from "@trails-cool/db/schema/journal"; @@ -27,11 +27,12 @@ export async function action({ request }: Route.ActionArgs) { status: "pending", }); - const boss = getBoss(); - await boss.send("komoot-bulk-import", { + // Only the serviceId crosses the queue — the job resolves fresh + // credentials through the ConnectedServiceManager at execution time. + await enqueue("komoot-bulk-import", { batchId, userId: user.id, - creds: service.credentials, + serviceId: service.id, }); return data({ batchId }); diff --git a/apps/journal/server.ts b/apps/journal/server.ts index 682d93f..02ca41d 100644 --- a/apps/journal/server.ts +++ b/apps/journal/server.ts @@ -193,7 +193,7 @@ server.listen(port, async () => { await startWorker(boss, jobs); // Register the started boss so feature code can enqueue jobs against // the same instance via getBoss() / enqueueOptional(). - const { setBoss } = await import("./app/lib/boss.server.ts"); + const { setBoss, enqueue } = await import("./app/lib/boss.server.ts"); setBoss(boss); logger.info("Background job worker started"); @@ -201,7 +201,7 @@ server.listen(port, async () => { // users get keys before any federation traffic). Each run only // touches users whose public_key IS NULL, so repeats are no-ops. if (process.env.FEDERATION_ENABLED === "true") { - await boss.send("backfill-user-keypairs", {}); + await enqueue("backfill-user-keypairs", {}); logger.info("federation keypair backfill enqueued"); } }); diff --git a/packages/jobs/src/index.ts b/packages/jobs/src/index.ts index 4d47f98..8aa2f13 100644 --- a/packages/jobs/src/index.ts +++ b/packages/jobs/src/index.ts @@ -1,3 +1,4 @@ export { createBoss } from "./boss.ts"; export { startWorker } from "./worker.ts"; -export type { JobDefinition } from "./types.ts"; +export { defineJob } from "./types.ts"; +export type { JobDefinition, TypedJobDefinition } from "./types.ts"; diff --git a/packages/jobs/src/types.test.ts b/packages/jobs/src/types.test.ts new file mode 100644 index 0000000..4310424 --- /dev/null +++ b/packages/jobs/src/types.test.ts @@ -0,0 +1,28 @@ +import { describe, it, expect } from "vitest"; +import type { Job } from "pg-boss"; +import { defineJob } from "./types.ts"; + +describe("defineJob", () => { + it("returns the definition unchanged (the cast is type-level only)", async () => { + const seen: string[] = []; + const def = defineJob<{ widgetId: string }>({ + name: "widget-job", + retryLimit: 2, + async handler(jobs) { + for (const job of jobs) { + // job.data is typed { widgetId: string } — no cast needed + seen.push(job.data.widgetId); + } + return null; + }, + }); + + expect(def.name).toBe("widget-job"); + expect(def.retryLimit).toBe(2); + + // The returned JobDefinition's handler is the same function and + // receives whatever pg-boss hands it. + await def.handler([{ data: { widgetId: "w1" } } as Job]); + expect(seen).toEqual(["w1"]); + }); +}); diff --git a/packages/jobs/src/types.ts b/packages/jobs/src/types.ts index 3f75205..ae071a2 100644 --- a/packages/jobs/src/types.ts +++ b/packages/jobs/src/types.ts @@ -1,12 +1,15 @@ import type { Job } from "pg-boss"; /** - * A pg-boss job definition. The payload type is intentionally - * `unknown` at this boundary: a heterogeneous `JobDefinition[]` array - * (e.g. the one in `apps/journal/server.ts`) was previously forced to - * cast each typed job to `any` because of contravariance — a handler - * taking `Job` is not assignable to one taking - * `Job`. Handlers narrow internally instead. + * A pg-boss job definition. The payload type is `unknown` at this + * boundary: a heterogeneous `JobDefinition[]` array (e.g. the one in + * `apps/journal/server.ts`) would otherwise be impossible because of + * contravariance — a handler taking `Job` is not + * assignable to one taking `Job`. + * + * Don't write handlers against this type directly; author them with + * `defineJob`, which keeps the payload typed inside the handler and + * performs the contravariance cast once, here in the package. */ export interface JobDefinition { name: string; @@ -15,3 +18,23 @@ export interface JobDefinition { retryLimit?: number; expireInSeconds?: number; } + +/** A JobDefinition whose handler sees the payload type it was enqueued with. */ +export interface TypedJobDefinition { + name: string; + handler: (jobs: Job[]) => Promise; + cron?: string; + retryLimit?: number; + expireInSeconds?: number; +} + +/** + * Author a job with a typed payload. The handler receives + * `Job[]` — no `job.data as ...` casts at the call sites. + * This is the single place the contravariance cast happens. + */ +export function defineJob( + definition: TypedJobDefinition, +): JobDefinition { + return definition as unknown as JobDefinition; +}