From 28c25c32ab6e4cbbc125d60d6e0af61f3eabb1c2 Mon Sep 17 00:00:00 2001 From: Florian Egger Date: Sun, 10 May 2026 03:23:07 +0200 Subject: [PATCH] Refactor API routes and hooks to use library clients - Wire `api/geocode` and `api/bike-route` to use `GeocodingClient` and `BikeRoutingClient` instead of raw fetch calls - Move `parseHafasJourneys` from `useJourneys` to `hafas-client` - Remove dead code `src/lib/live-status-utils.ts` - Update test setup to import `@testing-library/jest-dom/vitest` --- CHECKLIST.md | 8 +- src/app/api/__tests__/bike-route.test.ts | 71 +++++----------- src/app/api/bike-route/route.ts | 53 +++--------- src/app/api/geocode/route.ts | 63 ++------------ src/hooks/useJourneys.ts | 69 ++-------------- src/lib/hafas-client.ts | 100 +++++++++++++---------- src/lib/index.ts | 1 - src/lib/live-status-utils.ts | 23 ------ src/test/setup.ts | 2 +- 9 files changed, 108 insertions(+), 282 deletions(-) delete mode 100644 src/lib/live-status-utils.ts diff --git a/CHECKLIST.md b/CHECKLIST.md index 7ac44e3..5b62df5 100644 --- a/CHECKLIST.md +++ b/CHECKLIST.md @@ -21,19 +21,19 @@ Fix bugs that crash the app or lose user data. Eliminate duplicated logic so each integration has one source of truth. -- [ ] **Step 4: Consolidate HAFAS Journey Parsing** (~40 min) +- [x] **Step 4: Consolidate HAFAS Journey Parsing** (~40 min) - Move `parseHafasJourneys` from `useJourneys.ts` into `hafas-client.ts` and export it; import from `useJourneys.ts` (Option A — minimal risk) - Files: `src/lib/hafas-client.ts`, `src/hooks/useJourneys.ts` -- [ ] **Step 5: Wire API Routes to Use Library Clients** (~30 min) +- [x] **Step 5: Wire API Routes to Use Library Clients** (~30 min) - Replace raw `fetch()` in `api/geocode/route.ts` with `GeocodingClient`, and in `api/bike-route/route.ts` with `BikeRoutingClient` (module-level singleton, proper error handling) - Files: `src/app/api/geocode/route.ts`, `src/app/api/bike-route/route.ts` -- [ ] **Step 6: Remove Dead Code** (~5 min) +- [x] **Step 6: Remove Dead Code** (~5 min) - Delete `src/lib/live-status-utils.ts` entirely; remove its export from `src/lib/index.ts` - File: `src/lib/live-status-utils.ts`, `src/lib/index.ts` -- [ ] **Step 7: Create Missing Test Setup File** (~10 min) +- [x] **Step 7: Create Missing Test Setup File** (~10 min) - Create `src/test/setup.ts` with `import "@testing-library/jest-dom/vitest"` so jsdom matchers are registered globally - File: `src/test/setup.ts` diff --git a/src/app/api/__tests__/bike-route.test.ts b/src/app/api/__tests__/bike-route.test.ts index 36f5196..a89df83 100644 --- a/src/app/api/__tests__/bike-route.test.ts +++ b/src/app/api/__tests__/bike-route.test.ts @@ -1,14 +1,19 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { NextRequest } from "next/server"; -import { GET } from "../bike-route/route"; -// Mock the fetch function to avoid making actual HTTP requests -global.fetch = vi.fn(); +const mockGetBikeRoute = vi.fn(); + +vi.mock("@/lib/bike-routing-client", () => ({ + BikeRoutingClient: class MockBikeRoutingClient { + getBikeRoute = mockGetBikeRoute; + }, +})); + +const { GET } = await import("../bike-route/route"); describe("api/bike-route/route", () => { beforeEach(() => { - vi.resetAllMocks(); - global.fetch = vi.fn(); + mockGetBikeRoute.mockReset(); }); it("should return error when no required parameters are provided", async () => { @@ -21,40 +26,14 @@ describe("api/bike-route/route", () => { }); it("should handle valid bike route request", async () => { - // Mock successful fetch response - const mockResponse = { - ok: true, - json: vi.fn().mockResolvedValue({ - distance: 1500, - duration: 300, - routes: [ - { - distance: 1500, - duration: 300, - legs: [ - { - steps: [ - { - name: "Start", - distance: 100, - duration: 10, - maneuver: { instruction: "Go straight", type: "straight" }, - }, - { - name: "Turn left", - distance: 200, - duration: 20, - maneuver: { instruction: "Turn left at the corner", type: "turn", modifier: "left" }, - }, - ], - }, - ], - }, - ], - }), - }; - - vi.mocked(fetch).mockResolvedValue(mockResponse as unknown as Response); + mockGetBikeRoute.mockResolvedValue({ + distance: 1500, + duration: 300, + steps: [ + { name: "Start", distance: 100, duration: 10, instruction: "Go straight" }, + { name: "Turn left", distance: 200, duration: 20, instruction: "Turn left at the corner" }, + ], + }); const request = new NextRequest( "http://localhost/api/bike-route?fromLat=48.2082&fromLng=16.3738&toLat=48.2100&toLng=16.3800", @@ -68,8 +47,8 @@ describe("api/bike-route/route", () => { expect(data).toHaveProperty("steps"); }); - it("should handle fetch error", async () => { - vi.mocked(fetch).mockRejectedValue(new Error("Network error")); + it("should handle client error", async () => { + mockGetBikeRoute.mockRejectedValue(new Error("Network error")); const request = new NextRequest( "http://localhost/api/bike-route?fromLat=48.2082&fromLng=16.3738&toLat=48.2100&toLng=16.3800", @@ -82,15 +61,7 @@ describe("api/bike-route/route", () => { }); it("should handle no route found", async () => { - // Mock fetch response with empty routes - const mockResponse = { - ok: true, - json: vi.fn().mockResolvedValue({ - routes: [], - }), - }; - - vi.mocked(fetch).mockResolvedValue(mockResponse as unknown as Response); + mockGetBikeRoute.mockResolvedValue(null); const request = new NextRequest( "http://localhost/api/bike-route?fromLat=48.2082&fromLng=16.3738&toLat=48.2100&toLng=16.3800", diff --git a/src/app/api/bike-route/route.ts b/src/app/api/bike-route/route.ts index ddae417..bb87c73 100644 --- a/src/app/api/bike-route/route.ts +++ b/src/app/api/bike-route/route.ts @@ -1,62 +1,31 @@ import { NextRequest, NextResponse } from "next/server"; -import { OSRM_URL } from "@/lib/constants"; -import { BikeRoute } from "@/types"; +import { BikeRoutingClient } from "@/lib/bike-routing-client"; -interface OsrmStep { - name: string; - distance: number; - duration: number; - maneuver: { instruction?: string; type: string; modifier?: string }; -} +// Module-level singleton — cache persists across requests +const client = new BikeRoutingClient(); export async function GET(request: NextRequest) { try { const { searchParams } = new URL(request.url); - const fromLat = searchParams.get("fromLat"); - const fromLng = searchParams.get("fromLng"); - const toLat = searchParams.get("toLat"); - const toLng = searchParams.get("toLng"); + const fromLat = parseFloat(searchParams.get("fromLat") ?? ""); + const fromLng = parseFloat(searchParams.get("fromLng") ?? ""); + const toLat = parseFloat(searchParams.get("toLat") ?? ""); + const toLng = parseFloat(searchParams.get("toLng") ?? ""); - if (!fromLat || !fromLng || !toLat || !toLng) { + if (isNaN(fromLat) || isNaN(fromLng) || isNaN(toLat) || isNaN(toLng)) { return NextResponse.json( { error: "Missing required parameters (fromLat, fromLng, toLat, toLng)" }, { status: 400 }, ); } - // Build OSRM URL - const url = new URL(`${OSRM_URL}/route/v1/bicycle/${fromLng},${fromLat};${toLng},${toLat}`); - url.searchParams.append("overview", "false"); + const route = await client.getBikeRoute(fromLat, fromLng, toLat, toLng); - const response = await fetch(url.toString(), { - signal: AbortSignal.timeout(10_000), - }); - - if (!response.ok) { - return NextResponse.json({ error: "Bike routing request failed" }, { status: response.status }); - } - - const data = await response.json(); - - if (!data.routes || data.routes.length === 0) { + if (!route) { return NextResponse.json({ error: "No route found" }, { status: 404 }); } - const route = data.routes[0]; - - const bikeRoute: BikeRoute = { - distance: route.distance, - duration: route.duration, - steps: - route.legs?.[0]?.steps?.map((step: OsrmStep) => ({ - name: step.name || "", - distance: step.distance, - duration: step.duration, - instruction: step.maneuver.instruction, - })) || [], - }; - - return NextResponse.json(bikeRoute); + return NextResponse.json(route); } catch (error) { console.error("Bike route API error:", error); return NextResponse.json({ error: "Internal server error" }, { status: 500 }); diff --git a/src/app/api/geocode/route.ts b/src/app/api/geocode/route.ts index 6be549e..03ab7ae 100644 --- a/src/app/api/geocode/route.ts +++ b/src/app/api/geocode/route.ts @@ -1,10 +1,8 @@ import { NextRequest, NextResponse } from "next/server"; -import { NOMINATIM_URL, NOMINATIM_USER_AGENT } from "@/lib/constants"; -import { GeocodeResult } from "@/types"; +import { GeocodingClient } from "@/lib/geocoding-client"; -// Simple in-memory cache with TTL -const cache = new Map(); -const CACHE_TTL = 5 * 60 * 1000; // 5 minutes +// Module-level singleton — cache persists across requests +const client = new GeocodingClient(); export async function GET(request: NextRequest) { try { @@ -16,62 +14,13 @@ export async function GET(request: NextRequest) { return NextResponse.json({ error: "Missing 'name' parameter" }, { status: 400 }); } - // Create cache key - const cacheKey = `${name}|${countrycodes || ""}`; + const results = await client.geocode(name, countrycodes || undefined); - // Check cache - const cached = cache.get(cacheKey); - if (cached && Date.now() - cached.timestamp < CACHE_TTL) { - return NextResponse.json(cached.result); - } - - // Build URL with parameters - const url = new URL(`${NOMINATIM_URL}/search`); - url.searchParams.append("q", name); - url.searchParams.append("format", "json"); - url.searchParams.append("limit", "1"); - - if (countrycodes) { - url.searchParams.append("countrycodes", countrycodes); - } - - let response: Response; - try { - response = await fetch(url.toString(), { - headers: { - "User-Agent": NOMINATIM_USER_AGENT, - }, - }); - } catch (error) { - console.error("Geocoding fetch failed:", error); - return NextResponse.json({ error: "Internal server error" }, { status: 500 }); - } - - if (!response.ok) { - // If response is not ok, return error status without caching. - return NextResponse.json({ error: "Geocoding request failed" }, { status: response.status }); - } - - const data = await response.json(); - - if (!data || data.length === 0) { - // If no results, return error status without caching. + if (results.length === 0) { return NextResponse.json({ error: "No results found" }, { status: 404 }); } - const result: GeocodeResult = { - lat: parseFloat(data[0].lat), - lng: parseFloat(data[0].lon), - display_name: data[0].display_name, - }; - - // Cache the result ONLY on success path - cache.set(cacheKey, { - result, - timestamp: Date.now(), - }); - - return NextResponse.json(result); + return NextResponse.json(results[0]); } catch (error) { console.error("Geocode API error:", error); return NextResponse.json({ error: "Internal server error" }, { status: 500 }); diff --git a/src/hooks/useJourneys.ts b/src/hooks/useJourneys.ts index f5c1d6f..7815ef9 100644 --- a/src/hooks/useJourneys.ts +++ b/src/hooks/useJourneys.ts @@ -1,67 +1,14 @@ import { useState, useEffect } from "react"; import type { Journey } from "@/types"; -import { parseHafasTime, hafasDateTime } from "@/lib/hafas-time"; +import { parseHafasJourneys } from "@/lib/hafas-client"; +import { hafasDateTime } from "@/lib/hafas-time"; -interface HafasJourney { - ctxRecon?: string; - secL?: Array<{ - dep?: { dTimeS?: string; dTimeR?: string; dPlatfS?: string }; - arr?: { aTimeS?: string; aTimeR?: string }; - jny?: { - prodX?: number; - stopL?: Array<{ name: string }>; - dlySum?: number; - isCncl?: boolean; - }; - chgDurR?: number; - }>; -} - -function parseHafasJourneys(json: unknown, hafasDate: string, queryDate: Date): Journey[] { - // HAFAS response shape is too complex for a clean TypeScript type; use any for parsing. - // eslint-disable-next-line @typescript-eslint/no-explicit-any -- HAFAS response shape is undocumented and deeply nested - const data = json as any; - const outConL: HafasJourney[] = data?.svcResL?.[0]?.res?.outConL ?? []; - - return outConL.map((con, i): Journey => { - const first = con.secL?.[0]; - const last = con.secL?.[con.secL.length - 1]; - const dep = first?.dep; - const arr = last?.arr; - - const sD = dep?.dTimeS ? parseHafasTime(hafasDate, dep.dTimeS) : queryDate; - const rD = dep?.dTimeR ? parseHafasTime(hafasDate, dep.dTimeR) : sD; - const sA = arr?.aTimeS ? parseHafasTime(hafasDate, arr.aTimeS) : sD; - const rA = arr?.aTimeR ? parseHafasTime(hafasDate, arr.aTimeR) : sA; - - const delayMs = rD.getTime() - sD.getTime(); - const delay = Math.max(0, Math.round(delayMs / 60000)); - - const trains = (con.secL ?? []) - .filter((s) => s.jny) - .map((s) => s.jny?.stopL?.[0]?.name ?? "") - .filter(Boolean); - - const cancelled = (con.secL ?? []).some((s) => s.jny?.isCncl === true); - const changes = Math.max(0, (con.secL ?? []).filter((s) => s.jny).length - 1); - const platform = first?.dep?.dPlatfS ?? ""; - - return { - id: con.ctxRecon ?? `journey-${i}`, - sD, - rD, - sA, - rA, - delay, - platform, - changes, - trains, - cancelled, - }; - }); -} - -export function useJourneys(fromStationExtId: string | null, toStationExtId: string | null, date: Date, refreshKey = 0) { +export function useJourneys( + fromStationExtId: string | null, + toStationExtId: string | null, + date: Date, + refreshKey = 0, +) { const [journeys, setJourneys] = useState([]); const [loading, setLoading] = useState(false); const [error, setError] = useState(null); diff --git a/src/lib/hafas-client.ts b/src/lib/hafas-client.ts index a70a3a8..f9cb001 100644 --- a/src/lib/hafas-client.ts +++ b/src/lib/hafas-client.ts @@ -3,9 +3,9 @@ import { HAFAS_URL, HAFAS_TIMEOUT_MS } from "./constants"; import { parseHafasTime, hafasDateTime } from "./hafas-time"; import { ApiClient } from "./api-service"; -// --------------------------------------------------------------------------- +// ------------------------------------------------------------- // HAFAS response types (internal, not exported) -// --------------------------------------------------------------------------- +// ------------------------------------------------------------- interface HafasLocation { type: "S" | "A" | "P"; @@ -23,7 +23,7 @@ interface HafasLocationResponse { }>; } -interface HafasJourney { +export interface HafasJourney { ctxRecon?: string; secL?: Array<{ dep?: { dTimeS?: string; dTimeR?: string; dPlatfS?: string }; @@ -46,9 +46,60 @@ interface HafasTripResponse { }>; } -// --------------------------------------------------------------------------- +// ------------------------------------------------------------- +// Shared HAFAS journey parser +// ------------------------------------------------------------- + +/** + * Parse a raw HAFAS response into Journey[]. + * Works with both typed and untyped (raw JSON) responses. + */ +export function parseHafasJourneys(json: unknown, hafasDate: string, queryDate: Date): Journey[] { + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- HAFAS response shape is undocumented and deeply nested + const data = json as any; + const outConL: HafasJourney[] = data?.svcResL?.[0]?.res?.outConL ?? []; + + return outConL.map((con, i): Journey => { + const first = con.secL?.[0]; + const last = con.secL?.[con.secL.length - 1]; + const dep = first?.dep; + const arr = last?.arr; + + const sD = dep?.dTimeS ? parseHafasTime(hafasDate, dep.dTimeS) : queryDate; + const rD = dep?.dTimeR ? parseHafasTime(hafasDate, dep.dTimeR) : sD; + const sA = arr?.aTimeS ? parseHafasTime(hafasDate, arr.aTimeS) : sD; + const rA = arr?.aTimeR ? parseHafasTime(hafasDate, arr.aTimeR) : sA; + + const delayMs = rD.getTime() - sD.getTime(); + const delay = Math.max(0, Math.round(delayMs / 60000)); + + const trains = (con.secL ?? []) + .filter((s) => s.jny) + .map((s) => s.jny?.stopL?.[0]?.name ?? "") + .filter(Boolean); + + const cancelled = (con.secL ?? []).some((s) => s.jny?.isCncl === true); + const changes = Math.max(0, (con.secL ?? []).filter((s) => s.jny).length - 1); + const platform = first?.dep?.dPlatfS ?? ""; + + return { + id: con.ctxRecon ?? `journey-${i}`, + sD, + rD, + sA, + rA, + delay, + platform, + changes, + trains, + cancelled, + }; + }); +} + +// ------------------------------------------------------------- // HafasClient — Wraps the ÖBB HAFAS journey-planning API -// --------------------------------------------------------------------------- +// ------------------------------------------------------------- export class HafasClient { private client: ApiClient; @@ -103,44 +154,7 @@ export class HafasClient { const res = await this.client.post("", body); - const outConL: HafasJourney[] = res?.svcResL?.[0]?.res?.outConL ?? []; - - return outConL.map((con, i): Journey => { - const first = con.secL?.[0]; - const last = con.secL?.[con.secL.length - 1]; - const dep = first?.dep; - const arr = last?.arr; - - const sD = dep?.dTimeS ? parseHafasTime(hafasDate, dep.dTimeS) : date; - const rD = dep?.dTimeR ? parseHafasTime(hafasDate, dep.dTimeR) : sD; - const sA = arr?.aTimeS ? parseHafasTime(hafasDate, arr.aTimeS) : sD; - const rA = arr?.aTimeR ? parseHafasTime(hafasDate, arr.aTimeR) : sA; - - const delayMs = rD.getTime() - sD.getTime(); - const delay = Math.max(0, Math.round(delayMs / 60000)); - - const trains = (con.secL ?? []) - .filter((s) => s.jny) - .map((s) => s.jny?.stopL?.[0]?.name ?? "") - .filter(Boolean); - - const cancelled = (con.secL ?? []).some((s) => s.jny?.isCncl === true); - const changes = Math.max(0, (con.secL ?? []).filter((s) => s.jny).length - 1); - const platform = first?.dep?.dPlatfS ?? ""; - - return { - id: con.ctxRecon ?? `journey-${i}`, - sD, - rD, - sA, - rA, - delay, - platform, - changes, - trains, - cancelled, - }; - }); + return parseHafasJourneys(res, hafasDate, date); } /** diff --git a/src/lib/index.ts b/src/lib/index.ts index 061e866..503737b 100644 --- a/src/lib/index.ts +++ b/src/lib/index.ts @@ -5,7 +5,6 @@ export * from "./geocoding-client"; export * from "./bike-routing-client"; export * from "./calendar-utils"; export * from "./status-utils"; -export * from "./live-status-utils"; export * from "./countdown-utils"; export * from "./formatting"; export * from "./constants"; diff --git a/src/lib/live-status-utils.ts b/src/lib/live-status-utils.ts deleted file mode 100644 index 1bd236e..0000000 --- a/src/lib/live-status-utils.ts +++ /dev/null @@ -1,23 +0,0 @@ -// Live status utilities for TimeToLeave -import type { LiveStatus } from "@/types"; - -export class LiveStatusUtils { - static async getLiveStatus(_journeyId: string): Promise { - // This would make an actual API call to get live status - // For now, we'll return mock data - return null; - } - - static getStatusMessage(status: LiveStatus): string { - switch (status) { - case true: - return "On time"; - case false: - return "Delayed or cancelled"; - case null: - return "No live data"; - default: - return "Unknown status"; - } - } -} diff --git a/src/test/setup.ts b/src/test/setup.ts index d0de870..f149f27 100644 --- a/src/test/setup.ts +++ b/src/test/setup.ts @@ -1 +1 @@ -import "@testing-library/jest-dom"; +import "@testing-library/jest-dom/vitest";