feat(mileage): suggest driving distance from the from/to addresses (#1778)
* feat(mileage): suggest driving distance from the from/to addresses When both endpoints are typed in the trip form (create mode), a debounced lookup geocodes them via Nominatim and fetches the driving distance via OSRM, both proxied through /api/mileage/distance so addresses leave only our server, without user identifiers. The suggestion renders as a click-to-apply hint under the distance field, never auto-fills, and stays fully editable. Tooltip shows what the geocoder matched. In-instance caching (24h hits, 10min misses) plus 1.1s politeness spacing keep usage inside the OSM public-endpoint policies. OSMF is disclosed as a data recipient on the privacy page. Requested by a beta user: first-time routes had to be measured by hand; route memory (PR #1657) only helps from the second trip onward. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(mileage): resolve skeptic findings on the distance suggestion Compliance: routing switched from router.project-osrm.org (demo server, non-commercial use only) to FOSSGIS's routing.openstreetmap.de; the lookup is now click-triggered ("Foresla stracka") instead of as-you-type, per Nominatim's no-autocomplete policy; visible OpenStreetMap attribution next to the applied suggestion; privacy page reworked to name OSMF and FOSSGIS e.V. as independent recipients outside the sub-processor table, with an honest note that typed addresses can themselves be personal data. Correctness: suggestion-cache key separator changed from '|' (collidable by address text) to newline; routes rounding to 0.0 km are no longer suggested (the form rejects 0); the Nominatim politeness queue is bounded at 3s wait and bails to null instead of holding request handlers open. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(mileage): resolve CodeRabbit findings on the distance suggestion A generation counter invalidates in-flight lookups when the route or km field changes or the dialog closes, so a slow response can never write an old route's distance into a changed form. Privacy page now states each recipient's actual payload (Nominatim gets address texts, FOSSGIS only coordinates), discloses the 24h in-memory server cache, and carries today's revision date. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Mattsson
co-authored by
Claude Fable 5
Claude Fable 5
parent
60920ec794
commit
8249fcab5e
@@ -3581,6 +3581,11 @@ export const MileageSalaryPushSchema = z
|
||||
message: 'Milersättning bokförs per kalenderår: dela upp perioden per år',
|
||||
})
|
||||
|
||||
export const MileageDistanceQuerySchema = z.object({
|
||||
from: z.string().trim().min(2).max(200),
|
||||
to: z.string().trim().min(2).max(200),
|
||||
})
|
||||
|
||||
// ============================================================
|
||||
// Bank file import
|
||||
// ============================================================
|
||||
|
||||
@@ -0,0 +1,148 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { clearDistanceCachesForTests, fetchDistanceSuggestion } from '../distance'
|
||||
|
||||
function jsonResponse(body: unknown, ok = true, status = 200): Response {
|
||||
return { ok, status, json: async () => body } as unknown as Response
|
||||
}
|
||||
|
||||
function geocodeHit(lat: string, lon: string, name: string) {
|
||||
return [{ lat, lon, display_name: name }]
|
||||
}
|
||||
|
||||
const OSRM_OK = { code: 'Ok', routes: [{ distance: 47_250 }] }
|
||||
|
||||
// Every call passes minIntervalMs: 0 so tests never sleep for the
|
||||
// Nominatim politeness spacing.
|
||||
const OPTS = (fetchImpl: typeof fetch) => ({ fetchImpl, minIntervalMs: 0 })
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
clearDistanceCachesForTests()
|
||||
})
|
||||
|
||||
describe('fetchDistanceSuggestion', () => {
|
||||
it('geocodes both endpoints and returns the OSRM distance rounded to 1 decimal', async () => {
|
||||
const fetchImpl = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce(jsonResponse(geocodeHit('56.87', '14.80', 'Växjö, Sverige')))
|
||||
.mockResolvedValueOnce(jsonResponse(geocodeHit('56.89', '14.55', 'Alvesta, Sverige')))
|
||||
.mockResolvedValueOnce(jsonResponse(OSRM_OK))
|
||||
|
||||
const result = await fetchDistanceSuggestion('Växjö', 'Alvesta', OPTS(fetchImpl))
|
||||
|
||||
expect(result).toEqual({
|
||||
distance_km: 47.3,
|
||||
from_label: 'Växjö, Sverige',
|
||||
to_label: 'Alvesta, Sverige',
|
||||
})
|
||||
expect(fetchImpl).toHaveBeenCalledTimes(3)
|
||||
expect(String(fetchImpl.mock.calls[0][0])).toContain('nominatim.openstreetmap.org')
|
||||
// FOSSGIS instance on purpose: the project-osrm.org demo server is
|
||||
// restricted to non-commercial use.
|
||||
expect(String(fetchImpl.mock.calls[2][0])).toContain('routing.openstreetmap.de')
|
||||
// OSRM wants lon,lat ordering.
|
||||
expect(String(fetchImpl.mock.calls[2][0])).toContain('14.80,56.87;14.55,56.89')
|
||||
})
|
||||
|
||||
it('returns null and skips the second geocode when the first endpoint has no match', async () => {
|
||||
const fetchImpl = vi.fn().mockResolvedValueOnce(jsonResponse([]))
|
||||
|
||||
const result = await fetchDistanceSuggestion('hemma', 'Alvesta', OPTS(fetchImpl))
|
||||
|
||||
expect(result).toBeNull()
|
||||
expect(fetchImpl).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('returns null when OSRM finds no route', async () => {
|
||||
const fetchImpl = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce(jsonResponse(geocodeHit('56.87', '14.80', 'A')))
|
||||
.mockResolvedValueOnce(jsonResponse(geocodeHit('56.89', '14.55', 'B')))
|
||||
.mockResolvedValueOnce(jsonResponse({ code: 'NoRoute', routes: [] }))
|
||||
|
||||
expect(await fetchDistanceSuggestion('A', 'B', OPTS(fetchImpl))).toBeNull()
|
||||
})
|
||||
|
||||
it('returns null on an upstream failure instead of throwing', async () => {
|
||||
const fetchImpl = vi.fn().mockResolvedValue(jsonResponse({}, false, 503))
|
||||
|
||||
expect(await fetchDistanceSuggestion('Växjö', 'Alvesta', OPTS(fetchImpl))).toBeNull()
|
||||
})
|
||||
|
||||
it('serves repeat lookups from cache without new upstream calls', async () => {
|
||||
const fetchImpl = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce(jsonResponse(geocodeHit('56.87', '14.80', 'A')))
|
||||
.mockResolvedValueOnce(jsonResponse(geocodeHit('56.89', '14.55', 'B')))
|
||||
.mockResolvedValueOnce(jsonResponse(OSRM_OK))
|
||||
|
||||
const first = await fetchDistanceSuggestion('Växjö', 'Alvesta', OPTS(fetchImpl))
|
||||
// Same route, differently cased/spaced: normalizes to the same cache key.
|
||||
const second = await fetchDistanceSuggestion(' växjö ', 'ALVESTA', OPTS(fetchImpl))
|
||||
|
||||
expect(second).toEqual(first)
|
||||
expect(fetchImpl).toHaveBeenCalledTimes(3)
|
||||
})
|
||||
|
||||
it('caches a miss so half-typed routes are not re-queried immediately', async () => {
|
||||
const fetchImpl = vi.fn().mockResolvedValue(jsonResponse([]))
|
||||
|
||||
await fetchDistanceSuggestion('hemma', 'jobbet', OPTS(fetchImpl))
|
||||
await fetchDistanceSuggestion('hemma', 'jobbet', OPTS(fetchImpl))
|
||||
|
||||
expect(fetchImpl).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('does not suggest a distance that rounds to 0 km', async () => {
|
||||
const fetchImpl = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce(jsonResponse(geocodeHit('56.87', '14.80', 'A')))
|
||||
.mockResolvedValueOnce(jsonResponse(geocodeHit('56.87', '14.80', 'B')))
|
||||
.mockResolvedValueOnce(jsonResponse({ code: 'Ok', routes: [{ distance: 30 }] }))
|
||||
|
||||
expect(await fetchDistanceSuggestion('Storgatan 1', 'Storgatan 3', OPTS(fetchImpl))).toBeNull()
|
||||
})
|
||||
|
||||
it('does not collide cache keys when an address contains the "|" character', async () => {
|
||||
const fetchImpl = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce(jsonResponse(geocodeHit('59.40', '17.94', 'Kista|Stockholm')))
|
||||
.mockResolvedValueOnce(jsonResponse(geocodeHit('59.86', '17.64', 'Uppsala')))
|
||||
.mockResolvedValueOnce(jsonResponse({ code: 'Ok', routes: [{ distance: 60_000 }] }))
|
||||
.mockResolvedValueOnce(jsonResponse(geocodeHit('59.40', '17.94', 'Kista')))
|
||||
.mockResolvedValueOnce(jsonResponse(geocodeHit('59.33', '18.06', 'Stockholm|Uppsala')))
|
||||
.mockResolvedValueOnce(jsonResponse({ code: 'Ok', routes: [{ distance: 12_000 }] }))
|
||||
|
||||
const a = await fetchDistanceSuggestion('Kista|Stockholm', 'Uppsala', OPTS(fetchImpl))
|
||||
// Same '|'-joined text, different route: must NOT be served a's cache.
|
||||
const b = await fetchDistanceSuggestion('Kista', 'Stockholm|Uppsala', OPTS(fetchImpl))
|
||||
|
||||
expect(a?.distance_km).toBe(60)
|
||||
expect(b?.distance_km).toBe(12)
|
||||
expect(fetchImpl).toHaveBeenCalledTimes(6)
|
||||
})
|
||||
|
||||
it('bails out instead of queueing when the politeness wait exceeds the bound', async () => {
|
||||
const fetchImpl = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce(jsonResponse(geocodeHit('56.87', '14.80', 'Växjö, Sverige')))
|
||||
.mockResolvedValueOnce(jsonResponse(OSRM_OK))
|
||||
|
||||
// 'Växjö'/'växjö' share a geocode cache entry, so the first lookup makes
|
||||
// one live Nominatim call and reserves one 10s slot. The next uncached
|
||||
// route would have to wait far past MAX_QUEUE_WAIT_MS and must bail to
|
||||
// null without any upstream call instead of holding the handler open.
|
||||
const first = await fetchDistanceSuggestion('Växjö', 'växjö', {
|
||||
fetchImpl,
|
||||
minIntervalMs: 10_000,
|
||||
})
|
||||
const second = await fetchDistanceSuggestion('Ljungby', 'Markaryd', {
|
||||
fetchImpl,
|
||||
minIntervalMs: 10_000,
|
||||
})
|
||||
|
||||
expect(first?.distance_km).toBe(47.3)
|
||||
expect(second).toBeNull()
|
||||
expect(fetchImpl).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,196 @@
|
||||
import { createLogger } from '@/lib/logger'
|
||||
|
||||
const log = createLogger('mileage-distance')
|
||||
|
||||
/**
|
||||
* Driving-distance suggestion for the körjournal trip form, resolved from
|
||||
* free OpenStreetMap services: Nominatim (geocoding) + OSRM (routing).
|
||||
* Suggestion-only: every failure path resolves to null and the user always
|
||||
* confirms or edits the value, so a wrong match can never book itself.
|
||||
*/
|
||||
export interface DistanceSuggestion {
|
||||
/** One-way driving distance in km, rounded to 1 decimal like stored trips. */
|
||||
distance_km: number
|
||||
/** What the geocoder actually matched, so the UI can show it for trust. */
|
||||
from_label: string
|
||||
to_label: string
|
||||
}
|
||||
|
||||
interface GeocodeHit {
|
||||
lat: string
|
||||
lon: string
|
||||
display_name: string
|
||||
}
|
||||
|
||||
export interface DistanceFetchOptions {
|
||||
fetchImpl?: typeof fetch
|
||||
/** Spacing between live Nominatim calls (usage policy: max 1 req/s). */
|
||||
minIntervalMs?: number
|
||||
}
|
||||
|
||||
const NOMINATIM_URL = 'https://nominatim.openstreetmap.org/search'
|
||||
// FOSSGIS e.V.'s public OSRM instance (the one osm.org itself routes with).
|
||||
// Unlike router.project-osrm.org it carries no non-commercial restriction;
|
||||
// its policy asks for max 1 req/s, no heavy usage, a valid User-Agent and
|
||||
// visible attribution, all of which this module and the UI provide.
|
||||
const OSRM_URL = 'https://routing.openstreetmap.de/routed-car/route/v1/driving'
|
||||
// Nominatim's usage policy requires an identifying User-Agent.
|
||||
const USER_AGENT = 'Accounted korjournal (https://accounted.se)'
|
||||
const REQUEST_TIMEOUT_MS = 5_000
|
||||
const DEFAULT_MIN_INTERVAL_MS = 1_100
|
||||
const HIT_TTL_MS = 24 * 60 * 60 * 1000
|
||||
// Misses retry sooner: the address may simply not be typed out fully yet.
|
||||
const MISS_TTL_MS = 10 * 60 * 1000
|
||||
const CACHE_MAX_ENTRIES = 500
|
||||
|
||||
interface CacheEntry<T> {
|
||||
value: T
|
||||
expires: number
|
||||
}
|
||||
|
||||
const geocodeCache = new Map<string, CacheEntry<GeocodeHit | null>>()
|
||||
const suggestionCache = new Map<string, CacheEntry<DistanceSuggestion | null>>()
|
||||
|
||||
// Newline can never appear in normalizeQuery output (all whitespace collapses
|
||||
// to single spaces), so it is a collision-free separator between the two
|
||||
// endpoints; '|' would let 'Kista|Stockholm'->'Uppsala' share a key with
|
||||
// 'Kista'->'Stockholm|Uppsala'. Same reasoning as route-memory's ROUTE_KEY.
|
||||
const ROUTE_CACHE_SEPARATOR = String.fromCharCode(10)
|
||||
|
||||
export function clearDistanceCachesForTests(): void {
|
||||
geocodeCache.clear()
|
||||
suggestionCache.clear()
|
||||
nominatimNextSlot = 0
|
||||
}
|
||||
|
||||
function cacheGet<T>(cache: Map<string, CacheEntry<T>>, key: string): CacheEntry<T> | undefined {
|
||||
const entry = cache.get(key)
|
||||
if (!entry) return undefined
|
||||
if (entry.expires < Date.now()) {
|
||||
cache.delete(key)
|
||||
return undefined
|
||||
}
|
||||
return entry
|
||||
}
|
||||
|
||||
function cacheSet<T>(cache: Map<string, CacheEntry<T>>, key: string, value: T, ttlMs: number): void {
|
||||
if (cache.size >= CACHE_MAX_ENTRIES) {
|
||||
// Maps iterate in insertion order; dropping the first key evicts the oldest.
|
||||
const oldest = cache.keys().next().value
|
||||
if (oldest !== undefined) cache.delete(oldest)
|
||||
}
|
||||
cache.set(key, { value, expires: Date.now() + ttlMs })
|
||||
}
|
||||
|
||||
function normalizeQuery(value: string): string {
|
||||
return value.trim().toLowerCase().replace(/\s+/g, ' ')
|
||||
}
|
||||
|
||||
// In-instance politeness spacing for live Nominatim calls. Serverless
|
||||
// instances cannot guarantee a global rate, but combined with the
|
||||
// click-triggered lookup and the 24h caches this keeps a single instance
|
||||
// well under the 1 req/s policy. The queue is bounded: rather than holding
|
||||
// request handlers open, a lookup that would wait longer than
|
||||
// MAX_QUEUE_WAIT_MS bails out (uncached, so a later retry can succeed)
|
||||
// without reserving a slot.
|
||||
let nominatimNextSlot = 0
|
||||
const MAX_QUEUE_WAIT_MS = 3_000
|
||||
|
||||
async function waitForNominatimSlot(minIntervalMs: number): Promise<void> {
|
||||
const now = Date.now()
|
||||
const waitMs = nominatimNextSlot - now
|
||||
if (waitMs > MAX_QUEUE_WAIT_MS) throw new Error('geocoding queue saturated')
|
||||
nominatimNextSlot = Math.max(now, nominatimNextSlot) + minIntervalMs
|
||||
if (waitMs > 0) await new Promise((resolve) => setTimeout(resolve, waitMs))
|
||||
}
|
||||
|
||||
async function geocode(
|
||||
query: string,
|
||||
fetchImpl: typeof fetch,
|
||||
minIntervalMs: number
|
||||
): Promise<GeocodeHit | null> {
|
||||
const key = normalizeQuery(query)
|
||||
const cached = cacheGet(geocodeCache, key)
|
||||
if (cached) return cached.value
|
||||
|
||||
await waitForNominatimSlot(minIntervalMs)
|
||||
|
||||
const params = new URLSearchParams({ q: query.trim(), format: 'jsonv2', limit: '1' })
|
||||
const res = await fetchImpl(`${NOMINATIM_URL}?${params}`, {
|
||||
headers: { 'User-Agent': USER_AGENT, 'Accept-Language': 'sv' },
|
||||
signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS),
|
||||
})
|
||||
if (!res.ok) throw new Error(`Nominatim responded ${res.status}`)
|
||||
|
||||
const body = (await res.json()) as GeocodeHit[]
|
||||
const hit = body[0]?.lat && body[0]?.lon ? body[0] : null
|
||||
cacheSet(geocodeCache, key, hit, hit ? HIT_TTL_MS : MISS_TTL_MS)
|
||||
return hit
|
||||
}
|
||||
|
||||
async function fetchRouteKm(
|
||||
from: GeocodeHit,
|
||||
to: GeocodeHit,
|
||||
fetchImpl: typeof fetch
|
||||
): Promise<number | null> {
|
||||
const coords = `${from.lon},${from.lat};${to.lon},${to.lat}`
|
||||
const res = await fetchImpl(`${OSRM_URL}/${coords}?overview=false`, {
|
||||
headers: { 'User-Agent': USER_AGENT },
|
||||
signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS),
|
||||
})
|
||||
if (!res.ok) throw new Error(`OSRM responded ${res.status}`)
|
||||
|
||||
const body = (await res.json()) as { code?: string; routes?: Array<{ distance?: number }> }
|
||||
const meters = body.code === 'Ok' ? body.routes?.[0]?.distance : undefined
|
||||
if (typeof meters !== 'number') return null
|
||||
const km = Math.round(meters / 100) / 10
|
||||
// Guard the ROUNDED value: 30 m rounds to 0.0 km, which the trip form
|
||||
// rejects (km must be > 0), so it must never be suggested.
|
||||
return km > 0 ? km : null
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve a one-way driving-distance suggestion between two free-text
|
||||
* locations. Returns null when either endpoint cannot be geocoded, no route
|
||||
* exists, or an upstream call fails: the form simply shows no suggestion.
|
||||
*/
|
||||
export async function fetchDistanceSuggestion(
|
||||
from: string,
|
||||
to: string,
|
||||
options: DistanceFetchOptions = {}
|
||||
): Promise<DistanceSuggestion | null> {
|
||||
const fetchImpl = options.fetchImpl ?? fetch
|
||||
const minIntervalMs = options.minIntervalMs ?? DEFAULT_MIN_INTERVAL_MS
|
||||
|
||||
const routeCacheKey = normalizeQuery(from) + ROUTE_CACHE_SEPARATOR + normalizeQuery(to)
|
||||
const cached = cacheGet(suggestionCache, routeCacheKey)
|
||||
if (cached) return cached.value
|
||||
|
||||
try {
|
||||
// Sequential on purpose: Nominatim's policy forbids parallel requests.
|
||||
const fromHit = await geocode(from, fetchImpl, minIntervalMs)
|
||||
const toHit = fromHit ? await geocode(to, fetchImpl, minIntervalMs) : null
|
||||
if (!fromHit || !toHit) {
|
||||
cacheSet(suggestionCache, routeCacheKey, null, MISS_TTL_MS)
|
||||
return null
|
||||
}
|
||||
|
||||
const km = await fetchRouteKm(fromHit, toHit, fetchImpl)
|
||||
if (km === null) {
|
||||
cacheSet(suggestionCache, routeCacheKey, null, MISS_TTL_MS)
|
||||
return null
|
||||
}
|
||||
|
||||
const suggestion: DistanceSuggestion = {
|
||||
distance_km: km,
|
||||
from_label: fromHit.display_name,
|
||||
to_label: toHit.display_name,
|
||||
}
|
||||
cacheSet(suggestionCache, routeCacheKey, suggestion, HIT_TTL_MS)
|
||||
return suggestion
|
||||
} catch (error) {
|
||||
// Upstream hiccups are not cached: the next debounced attempt may succeed.
|
||||
log.warn('suggestion lookup failed', { error: String(error) })
|
||||
return null
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user