diff --git a/client/src/components/Map/RouteCalculator.test.ts b/client/src/components/Map/RouteCalculator.test.ts index dc92b877d..40752bc36 100644 --- a/client/src/components/Map/RouteCalculator.test.ts +++ b/client/src/components/Map/RouteCalculator.test.ts @@ -13,6 +13,10 @@ import { parsePluginProfile, withHotelBookends, calculateAlternatives, + avoidedClasses, + routeEngineFor, + sameRoad, + furthestFrom, } from './RouteCalculator' // Every route now goes to the FOSSGIS per-profile hosts. The car-only project-osrm.org @@ -1049,3 +1053,196 @@ describe('turning round at a stop', () => { useSettingsStore.setState(st => ({ settings: { ...st.settings, routing_base_url: '' } })) }) }) + +// ── a trip that avoids something ────────────────────────────────────────────── + +/** + * The rail drives a trip with avoided classes through Valhalla, and everything that talks + * about one of its legs has to use the same engine. The offers used to come from OSRM with + * nothing avoided, so the picker described a different drive from the rail's; and an OSRM + * stand-in for a Valhalla that did not answer was filed as the avoided road for the rest + * of the session, without a word on the rail. + */ +describe('a trip that avoids something', () => { + const VALHALLA = 'https://valhalla1.openstreetmap.de/route' + + /** Polyline6, the way Valhalla sends its shapes. */ + const encode6 = (points: [number, number][]): string => { + const part = (value: number): string => { + let n = value < 0 ? ~(value << 1) : value << 1 + let out = '' + while (n >= 0x20) { out += String.fromCharCode((0x20 | (n & 0x1f)) + 63); n >>= 5 } + return out + String.fromCharCode(n + 63) + } + let lat = 0 + let lng = 0 + return points.map(([pLat, pLng]) => { + const a = Math.round(pLat * 1e6) + const b = Math.round(pLng * 1e6) + const piece = part(a - lat) + part(b - lng) + lat = a + lng = b + return piece + }).join('') + } + const trip = (points: [number, number][], summary: Record = {}) => ({ + legs: [{ shape: encode6(points) }], + summary: { length: 100, time: 4000, has_toll: false, has_highway: false, has_ferry: false, ...summary }, + }) + const NORTH: [number, number][] = [[61, 10], [61.5, 11], [61, 12]] + const SOUTH: [number, number][] = [[61, 10], [60.5, 11], [61, 12]] + const DIRECT: [number, number][] = [[61, 10], [61, 11], [61, 12]] + const from = { lat: 61, lng: 10 } + const to = { lat: 61, lng: 12 } + + it('FE-COMP-ROUTECALCULATOR-059: the ways come from Valhalla with the trip classes, and OSRM is never asked', async () => { + let osrm = 0 + const bodies: Array> = [] + server.use( + http.get(`${FOSSGIS.driving}/:coords`, () => { osrm++; return HttpResponse.json(buildOsrmRouteResponse()) }), + http.post(VALHALLA, async ({ request }) => { + bodies.push(await request.json() as Record) + return HttpResponse.json({ + trip: trip(DIRECT, { time: 3600 }), + alternates: [{ trip: trip(NORTH, { time: 4200, has_ferry: true }) }, { trip: trip(SOUTH, { time: 4800 }) }], + }) + }), + ) + + const routes = await calculateAlternatives(from, to, 'driving', { avoid: ['toll'] }) + + expect(osrm).toBe(0) + expect(bodies).toHaveLength(1) + expect(bodies[0]).toMatchObject({ alternates: 2, costing_options: { auto: { use_tolls: 0 } } }) + expect(bodies[0].locations).toHaveLength(2) + expect(routes.map(r => r.engine)).toEqual(['valhalla', 'valhalla', 'valhalla']) + expect(routes.map(r => r.duration)).toEqual([3600, 4200, 4800]) + expect(routes.map(r => r.hasFerry)).toEqual([false, true, false]) + // Pinnable like any other offer: measured against the preferred one. + expect(routes[0].divergence).toBeNull() + expect(routes[1].divergence).toEqual({ lat: 61.5, lng: 11 }) + }) + + it('FE-COMP-ROUTECALCULATOR-060: a Valhalla with nothing to say is the router not answering, not the OSRM roads', async () => { + let osrm = 0 + server.use( + http.get(`${FOSSGIS.driving}/:coords`, () => { osrm++; return HttpResponse.json(buildOsrmRouteResponse()) }), + http.post(VALHALLA, () => new HttpResponse(null, { status: 503 })), + ) + + await expect(calculateAlternatives({ lat: 62, lng: 10 }, { lat: 62, lng: 12 }, 'driving', { avoid: ['ferry'] })).rejects.toThrow() + expect(osrm).toBe(0) + }) + + it('FE-COMP-ROUTECALCULATOR-061: one way back is offered a second only for a class the trip does not avoid yet', async () => { + const asked: Array> = [] + server.use(http.post(VALHALLA, async ({ request }) => { + const body = await request.json() as { alternates?: number; costing_options: { auto: Record } } + asked.push(body.costing_options.auto) + if (body.alternates) return HttpResponse.json({ trip: trip(DIRECT, { has_toll: true }) }) + // Asked to leave out the tolls as well: a road that really is without them. + return HttpResponse.json({ trip: trip(NORTH, { time: 5000, has_toll: false }) }) + })) + + const routes = await calculateAlternatives({ lat: 61, lng: 10.5 }, { lat: 61, lng: 12 }, 'driving', { avoid: ['motorway'] }) + + // Motorways are avoided already, so the first extra question is about tolls, and it + // keeps the trip's own class in. + expect(asked).toEqual([{ use_highways: 0 }, { use_highways: 0, use_tolls: 0 }]) + expect(routes).toHaveLength(2) + expect(routes[1]).toMatchObject({ avoids: 'toll', engine: 'valhalla', duration: 5000 }) + }) + + it('FE-COMP-ROUTECALCULATOR-062: only a driving leg is weighed; a walk is asked of OSRM as before', async () => { + let valhalla = 0 + server.use( + http.get(`${FOSSGIS.walking}/:coords`, () => HttpResponse.json({ code: 'Ok', routes: [{ geometry: { coordinates: [[10, 61], [12, 61]] }, distance: 1000, duration: 900 }] })), + http.post(VALHALLA, () => { valhalla++; return HttpResponse.json({ trip: trip(DIRECT) }) }), + ) + + const routes = await calculateAlternatives(from, to, 'walking', { avoid: ['toll'] }) + + expect(valhalla).toBe(0) + expect(routes).toHaveLength(1) + expect(routes[0].engine).toBeUndefined() + expect(avoidedClasses('walking', ['toll'])).toEqual([]) + expect(avoidedClasses('driving', ['toll', 'ferry'])).toEqual(['ferry', 'toll']) + expect(routeEngineFor('driving', ['toll'])).toBe('valhalla') + expect(routeEngineFor('driving', [])).toBe('osrm') + expect(routeEngineFor('plugin:ev/fast', ['toll'])).toBe('plugin') + }) + + it('FE-COMP-ROUTECALCULATOR-063: an OSRM stand-in says nothing was avoided, and is not filed as the avoided road', async () => { + const [a, b] = freshWaypoints() + let osrm = 0 + let valhallaUp = false + server.use( + http.get(`${FOSSGIS.driving}/:coords`, () => { osrm++; return HttpResponse.json(buildLegsResponse()) }), + http.post(VALHALLA, () => (valhallaUp + ? HttpResponse.json({ trip: { legs: [{ shape: encode6([[a.lat, a.lng], [b.lat, b.lng]]), summary: { length: 5, time: 700, has_toll: false, has_ferry: true } }] } }) + : new HttpResponse(null, { status: 503 }))), + ) + + const fallback = await calculateRouteWithLegs([a, b], { avoid: ['toll'] }) + expect(osrm).toBe(1) + expect(fallback.avoidance).toEqual({ asked: ['toll'], achieved: [], fellBack: true }) + expect(fallback.distance).toBe(4200) + + // Valhalla is back: the same drive asks it again instead of keeping the OSRM answer. + valhallaUp = true + const weighed = await calculateRouteWithLegs([a, b], { avoid: ['toll'] }) + expect(weighed.avoidance).toEqual({ asked: ['toll'], achieved: ['toll'] }) + expect(weighed.distance).toBe(5000) + expect(weighed.hasFerry).toBe(true) + + // The stand-in was filed where it is true: as the plain OSRM answer, which an ordinary + // caller now gets without asking again, and without the avoidance attached. + const plain = await calculateRouteWithLegs([a, b]) + expect(osrm).toBe(1) + expect(plain.distance).toBe(4200) + expect(plain.avoidance).toBeUndefined() + }) + + it('FE-COMP-ROUTECALCULATOR-064: a road that runs the whole of another and turns off to a point and back is not that road', () => { + // The shape of a leg bent by a via: every point of the plain road lies on it, so + // asked only one way round the two read as one. + const plainRoad = { coordinates: [[61, 10], [61, 11], [61, 12]] as [number, number][], distance: 100_000, duration: 3600 } + const spur = { coordinates: [[61, 10], [61, 11], [61.3, 11], [61, 11], [61, 12]] as [number, number][], distance: 166_000, duration: 5400 } + + expect(sameRoad(plainRoad, spur)).toBe(false) + expect(sameRoad(spur, plainRoad)).toBe(false) + expect(sameRoad(plainRoad, { ...plainRoad, distance: 104_000, duration: 3900 })).toBe(true) + }) + + it('FE-COMP-ROUTECALCULATOR-065: one road drawn with a different spacing of points is still one road', () => { + // Two engines' lines of one long road share almost no vertex. Measured to the nearest + // sampled vertex, a point on the same road sat half a sample off it: kilometres here. + // The dense line is sampled every tenth vertex, 0.05 degrees apart; the sparse one sits + // exactly between those samples, 1.8 km from the nearest, on the very same road. + const dense = Array.from({ length: 2001 }, (_, i) => [50, i * 0.005] as [number, number]) + const sparse: [number, number][] = [ + [50, 0], + ...Array.from({ length: 200 }, (_, i) => [50, 0.025 + i * 0.05] as [number, number]), + [50, 10], + ] + const a = { coordinates: dense, distance: 700_000, duration: 25_000 } + const b = { coordinates: sparse, distance: 712_000, duration: 27_000 } + + expect(sameRoad(a, b)).toBe(true) + const off = furthestFrom([[50.02, 5]], dense) + expect(off?.index).toBe(0) + expect(off?.km).toBeCloseTo(2.22, 1) + }) + + it('FE-COMP-ROUTECALCULATOR-066: the avoidance offer beside an OSRM leg says when it crosses by ferry', async () => { + server.use( + http.get(`${FOSSGIS.driving}/:coords`, () => HttpResponse.json({ code: 'Ok', routes: [{ geometry: { coordinates: [[10, 63], [11, 63], [12, 63]] }, distance: 100000, duration: 3600 }] })), + http.post(VALHALLA, () => HttpResponse.json({ trip: trip([[63, 10], [63.6, 11], [63, 12]], { has_highway: false, has_ferry: true }) })), + ) + + const routes = await calculateAlternatives({ lat: 63, lng: 10 }, { lat: 63, lng: 12 }) + + expect(routes).toHaveLength(2) + expect(routes[1]).toMatchObject({ avoids: 'motorway', engine: 'valhalla', hasFerry: true }) + }) +}) diff --git a/client/src/components/Map/RouteCalculator.ts b/client/src/components/Map/RouteCalculator.ts index d201ee2de..e8eb59bbe 100644 --- a/client/src/components/Map/RouteCalculator.ts +++ b/client/src/components/Map/RouteCalculator.ts @@ -4,7 +4,7 @@ import type { DistanceUnit, RouteResult, RouteSegment, RouteWithLegs, SnappedWay import { haversineKm } from '../../utils/geo' import { formatDistance } from '../../utils/units' import { countRoute } from './routeUsageCounter' -import { valhallaRouteAvoiding, valhallaRun, valhallaAvailable, legAvoids, type AvoidClass } from './valhallaRoute' +import { valhallaRouteAvoiding, valhallaRun, valhallaAvailable, valhallaAlternates, valhallaSpacing, legAvoids, type AvoidClass, type ValhallaLeg } from './valhallaRoute' import type { RouteUsageSurface } from '@trek/shared' // FOSSGIS hosts OSRM with real per-profile routing (car/foot/bike) — the @@ -216,6 +216,33 @@ export function parsePluginProfile(profile: string): { pluginId: string; profile return { pluginId: rest.slice(0, slash), profileId: rest.slice(slash + 1) } } +/** + * Which engine priced a route: OSRM, the second engine that weighs road classes away, or + * a route provider plugin. Their figures are three different speed models, so a figure + * from one read against a figure from another measures the models, not the roads. + */ +export type RouteEngine = 'osrm' | 'valhalla' | 'plugin' + +/** + * The classes a request really weighs away, in a stable order, and none when it cannot. + * + * Only the driving profile has classes to leave out and only Valhalla can leave them out, + * so every other request is asked plainly whatever the trip says. One function for the + * day routing and for the offers beside it, so the two cannot disagree about which engine + * a leg belongs to. + */ +export function avoidedClasses(profile: RouteProfileKey, avoid: readonly AvoidClass[]): AvoidClass[] { + return profile === 'driving' && avoid.length > 0 && valhallaAvailable() + ? [...avoid].sort((a, b) => a.localeCompare(b)) + : [] +} + +/** The engine `calculateRouteWithLegs` hands a request with this profile and avoidance to. */ +export function routeEngineFor(profile: RouteProfileKey, avoid: readonly AvoidClass[]): RouteEngine { + if (parsePluginProfile(profile)) return 'plugin' + return avoidedClasses(profile, avoid).length ? 'valhalla' : 'osrm' +} + /** Fetches a full route via OSRM and returns coordinates, distance, and duration estimates for driving/walking. */ export async function calculateRoute( waypoints: Waypoint[], @@ -480,9 +507,7 @@ export async function calculateRouteWithLegs( return { coordinates: [], distance: 0, duration: 0, legs: [] } } - const avoiding = profile === 'driving' && avoid.length > 0 && valhallaAvailable() - ? [...avoid].sort((a, b) => a.localeCompare(b)) - : [] + const avoiding = avoidedClasses(profile, avoid) const coords = waypoints.map((p) => `${p.lng},${p.lat}`).join(';') // The cached result carries formatted leg distances, so the active distance unit is @@ -497,23 +522,30 @@ export async function calculateRouteWithLegs( // way is worse: this Map is shared with the ordinary day planner, the studio and the // booking geometry, none of which ask for avoidance, so an avoid-routed answer stored // under the plain key would hand them a road nobody drives and a second engine's - // times. `avoiding` is empty whenever the request goes to OSRM after all, so the key - // also separates the two engines. + // times. So the avoiding key only ever holds a Valhalla answer: when Valhalla cannot + // answer and OSRM drives the request after all, that answer is filed under the plain + // key, where it is exactly what an ordinary caller would have got. Filed under the + // avoiding key it stood in for the avoided road until the next reload, however soon + // Valhalla came back. const pluginScope = profile.startsWith('plugin:') ? `:${tripId ?? ''}:${dayId ?? ''}` : '' const avoidScope = avoiding.length ? `:avoid=${avoiding.join(',')}` : '' - const cacheKey = `${profile}:${getDistanceUnit()}:${coords}${pluginScope}${avoidScope}` + const plainKey = `${profile}:${getDistanceUnit()}:${coords}${pluginScope}` + const cacheKey = `${plainKey}${avoidScope}` const cached = routeCache.get(cacheKey) if (cached) return cached if (avoiding.length) { const routed = await routeAvoidingWithLegs(waypoints, avoiding, signal) - // Null is a Valhalla that could not answer at all. Falling through to OSRM keeps a - // day drawn rather than blank, and `avoidance` stays absent on that result so the - // rail can say the avoidance did not happen instead of implying it did. if (routed) { cacheRoute(cacheKey, routed) return routed } + // Null is a Valhalla that could not answer at all. OSRM keeps the day drawn rather + // than blank, and the answer says that nothing was avoided, with the reason, so the + // rail flags the day instead of implying the setting held on a road nobody weighed. + const plain = routeCache.get(plainKey) ?? await osrmWithLegs(waypoints, coords, profile, signal) + cacheRoute(plainKey, plain) + return { ...plain, avoidance: { asked: avoiding, achieved: [], fellBack: true } } } // Plugin profile (`plugin:/`): the server invokes that routeProvider @@ -558,6 +590,18 @@ export async function calculateRouteWithLegs( return result } + const result = await osrmWithLegs(waypoints, coords, profile, signal) + cacheRoute(cacheKey, result) + return result +} + +/** The OSRM answer for a chain, one leg per waypoint pair. Throws like every OSRM call here. */ +async function osrmWithLegs( + waypoints: Waypoint[], + coords: string, + profile: RouteProfileKey, + signal?: AbortSignal, +): Promise { // Written as literals rather than narrowing `profile`: its type is an open string union // (plugins name their own modes), which no comparison narrows to the three OSRM knows. const osrmProfile: 'driving' | 'walking' | 'cycling' = @@ -592,9 +636,7 @@ export async function calculateRouteWithLegs( ) const snapped = readSnapped(data, waypoints) - const result: RouteWithLegs = { coordinates, distance: route.distance, duration: route.duration, legs, ...(snapped ? { snapped } : {}) } - cacheRoute(cacheKey, result) - return result + return { coordinates, distance: route.distance, duration: route.duration, legs, ...(snapped ? { snapped } : {}) } } /** Store a route and drop the oldest once the cache is over its cap. */ @@ -659,6 +701,7 @@ async function routeAvoidingWithLegs( legs, snapped: run.snapped, avoidance: { asked: avoid, achieved: avoid.filter(cls => legAvoids(run.total, cls)) }, + hasFerry: run.total.hasFerry, } } @@ -714,7 +757,7 @@ function formatDuration(seconds: number): string { return `${m} min` } -/** One way of driving a leg, as OSRM offers it. */ +/** One way of driving a leg, as a router offers it. */ export interface RouteAlternative { coordinates: [number, number][] distance: number @@ -724,20 +767,24 @@ export interface RouteAlternative { /** Set when this way exists because a road class was left out of it. */ avoids?: AvoidClass /** - * Which engine priced this route, when it was not the one that priced the rest. + * Which engine priced this route. Absent means OSRM, which is what answers every route + * of a trip that avoids nothing, except the avoidance offer on a default install. * - * Absent for everything OSRM answered, which is every route in a list except the - * avoidance offer on a default install. It exists because the two engines do not - * agree on speed: measured over twelve European legs the per-leg drive times run - * from 14.7 % under OSRM's to 13.2 % over, and the sign depends on the region: - * Valhalla is faster on Spanish autovía and slower through a city. So a figure from - * one of them subtracted from a figure from the other is not a difference in - * driving time, it is the gap between two speed models, and the reader has no way - * of telling the two apart. + * It exists because the engines do not agree on speed: measured over twelve European + * legs the per-leg drive times run from 14.7 % under OSRM's to 13.2 % over, and the + * sign depends on the region: Valhalla is faster on Spanish autovía and slower through + * a city. So a figure from one of them subtracted from a figure from the other is not a + * difference in driving time, it is the gap between two speed models, and the reader + * has no way of telling the two apart. */ - engine?: 'valhalla' + engine?: RouteEngine + /** Whether the way crosses by ferry, where the engine that drew it says so. */ + hasFerry?: boolean } +/** What two routes are compared by: their line, and their figures where there is none. */ +export type RoadLine = Pick + /** * Road classes worth asking the router to leave out, in the order they are tried. * @@ -780,10 +827,16 @@ const SAME_ROAD_KM = 0.5 * * The geometry does not care which engine drew it, so it decides whenever there is one. * The numbers stay as the fallback for a line with no shape to compare. + * + * Measured both ways round. Asked only whether `b` stays near `a`, a road that runs the + * whole of `b` and then turns off to a point and back passed as `b`, because every point + * of `b` is on it; that is exactly the shape of a leg bent by a via, and the picker and + * the check behind a choice both have to tell it from the plain road. */ -function sameRoad(a: RouteAlternative, b: RouteAlternative): boolean { - const stray = furthestFrom(b.coordinates, a.coordinates) - if (stray) return stray.km < SAME_ROAD_KM +export function sameRoad(a: RoadLine, b: RoadLine): boolean { + const there = furthestFrom(b.coordinates, a.coordinates) + const back = furthestFrom(a.coordinates, b.coordinates) + if (there && back) return Math.max(there.km, back.km) < SAME_ROAD_KM return Math.abs(a.distance - b.distance) < 200 && Math.abs(a.duration - b.duration) < 60 } @@ -862,12 +915,18 @@ async function valhallaExcluding( // and every routing request an instance makes still lands in the one counter. if (!signal?.aborted) countRoute({ ...sample, failed: !leg }) if (!leg || !legAvoids(leg, exclude)) return null + return valhallaOffer(leg) +} + +/** A Valhalla leg as an offer, carrying the engine that priced it and what it crosses. */ +function valhallaOffer(leg: ValhallaLeg): RouteAlternative { return { coordinates: leg.coordinates, distance: leg.distance, duration: leg.duration, divergence: null, engine: 'valhalla', + hasFerry: leg.hasFerry, } } @@ -917,12 +976,89 @@ async function osrmExcluding( * It is what makes a choice persistable: saving that point as a via forces the router back * onto this road on every future request, without storing a polyline that would go stale * with the next OSM update. + * + * `avoid` is the classes the trip weighs away. A driving leg of such a trip is routed by + * Valhalla, so its offers come from Valhalla too, weighed the same way, and never from + * OSRM: a list holding both engines is what offered a road the rail was not on as the + * quicker one, and clicking it changed nothing. */ export async function calculateAlternatives( from: Waypoint, to: Waypoint, profile: 'driving' | 'walking' | 'cycling' = 'driving', - { signal, limit = 3 }: { signal?: AbortSignal; limit?: number } = {}, + { signal, limit = 3, avoid = [] }: { signal?: AbortSignal; limit?: number; avoid?: readonly AvoidClass[] } = {}, +): Promise { + const avoiding = avoidedClasses(profile, avoid) + const routes = avoiding.length + ? await alternativesAvoiding(from, to, avoiding, limit, signal) + : await alternativesFromOsrm(from, to, profile, limit, signal) + if (!routes.length) return routes + + // The first route is what the router would have given anyway; the others are measured + // against it so each one can be pinned by the point that makes it different. + const [primary, ...rest] = routes + for (const alt of rest) alt.divergence = furthestPointFrom(alt.coordinates, primary.coordinates) + return routes +} + +/** + * The offers for a leg of a trip that weighs classes away, every one of them Valhalla's. + * + * Throws when Valhalla has nothing to say, so the picker reports a router that is not + * answering. Quietly offering OSRM's roads instead is the very mix this path exists to end. + * + * One way back gets the question the OSRM path asks too: the same drive with one more + * class left out, only of those the trip does not already avoid, and only where the road + * that comes back really is without it. Paced like the pieces of a split day, because the + * public instance allows one request a second. + */ +async function alternativesAvoiding( + from: Waypoint, + to: Waypoint, + avoiding: AvoidClass[], + limit: number, + signal?: AbortSignal, +): Promise { + const sample = { + profile: 'driving' as const, + surface: 'alternatives' as const, + selfHosted: !!useSettingsStore.getState().settings.routing_base_url?.trim(), + waypoints: 2, + km: haversineKm(from, to), + } + const answered = await valhallaAlternates(from, to, 'driving', avoiding, Math.max(0, limit - 1), signal) + if (!signal?.aborted) countRoute({ ...sample, failed: !answered }) + if (!answered?.length) throw new Error('No route found') + + const routes: RouteAlternative[] = [] + for (const leg of answered) { + const offer = valhallaOffer(leg) + if (!routes.some(r => sameRoad(r, offer))) routes.push(offer) + } + if (routes.length >= 2) return routes + + for (const cls of EXCLUDABLE_CLASSES) { + if (avoiding.includes(cls)) continue + await valhallaSpacing(signal) + if (signal?.aborted) break + const run = await valhallaRun([from, to], 'driving', [...avoiding, cls], signal) + if (!signal?.aborted) countRoute({ ...sample, failed: !run }) + if (!run || !legAvoids(run.total, cls)) continue + const detour = valhallaOffer(run.total) + if (routes.some(r => sameRoad(r, detour))) continue + routes.push({ ...detour, avoids: cls }) + break + } + return routes +} + +/** The offers for a leg of a trip that avoids nothing: OSRM's, and one avoidance offer. */ +async function alternativesFromOsrm( + from: Waypoint, + to: Waypoint, + profile: 'driving' | 'walking' | 'cycling', + limit: number, + signal?: AbortSignal, ): Promise { const coords = `${from.lng},${from.lat};${to.lng},${to.lat}` const url = `${routeBaseFor(profile)}/${coords}?alternatives=${limit}&overview=full&geometries=geojson` @@ -960,11 +1096,6 @@ export async function calculateAlternatives( } } } - - // The first route is what the router would have given anyway; the others are measured - // against it so each one can be pinned by the point that makes it different. - const [primary, ...rest] = routes - for (const alt of rest) alt.divergence = furthestPointFrom(alt.coordinates, primary.coordinates) return routes } @@ -982,31 +1113,78 @@ function furthestPointFrom(line: [number, number][], reference: [number, number] * nothing for it. The winner is converted once at the end, because `sameRoad` needs a * real distance rather than a rank, and half a degree is a different number of metres in * Andalusia than in Lapland. + * + * `index` is where on `line` the point sits, so points taken off one line can be put back + * in the order that line drives through them. */ -function furthestFrom( +export function furthestFrom( line: [number, number][], reference: [number, number][], -): { lat: number; lng: number; km: number } | null { +): { index: number; lat: number; lng: number; km: number } | null { if (!line.length || !reference.length) return null // Every tenth vertex is plenty: alternatives differ over kilometres, not metres, and a // full cross product of two thousand-point lines is not worth the milliseconds. const step = Math.max(1, Math.floor(reference.length / 200)) - let best: { lat: number; lng: number } | null = null + let bestIndex = -1 let bestNeighbour: [number, number] | null = null let bestDist = -1 for (let i = 0; i < line.length; i += Math.max(1, Math.floor(line.length / 200))) { - const [lat, lng] = line[i] - let nearest = Infinity - let nearestPoint: [number, number] | null = null - for (let j = 0; j < reference.length; j += step) { - const dLat = lat - reference[j][0] - const dLng = (lng - reference[j][1]) * Math.cos((lat * Math.PI) / 180) - const d = dLat * dLat + dLng * dLng - if (d < nearest) { nearest = d; nearestPoint = reference[j] } - } - if (nearest > bestDist) { bestDist = nearest; best = { lat, lng }; bestNeighbour = nearestPoint } + const near = nearestOn(line[i], reference, step) + if (near.d > bestDist) { bestDist = near.d; bestIndex = i; bestNeighbour = near.at } } - if (!best) return null - const km = bestNeighbour ? haversineKm(best, { lat: bestNeighbour[0], lng: bestNeighbour[1] }) : 0 - return { ...best, km } + if (bestIndex < 0) return null + const [lat, lng] = line[bestIndex] + const km = bestNeighbour ? haversineKm({ lat, lng }, { lat: bestNeighbour[0], lng: bestNeighbour[1] }) : 0 + return { index: bestIndex, lat, lng, km } +} + +/** + * Where `reference` passes closest to `point`, as a squared distance in the same scaled + * degrees and the spot on the line it was measured to. + * + * Coarse, then fine. The sampled vertices find the stretch, and the segments either side + * of the nearest one find the road itself. Measured to the sampled vertex alone, a point + * on the very same road sat up to half a sample apart from it, which on a thousand + * kilometre leg is kilometres: two engines' lines of one motorway then read as two roads. + */ +function nearestOn( + point: [number, number], + reference: [number, number][], + step: number, +): { d: number; at: [number, number] } { + const cos = Math.cos((point[0] * Math.PI) / 180) + const squared = (p: [number, number]): number => { + const dLat = point[0] - p[0] + const dLng = (point[1] - p[1]) * cos + return dLat * dLat + dLng * dLng + } + let nearestIndex = 0 + let nearest = Infinity + for (let j = 0; j < reference.length; j += step) { + const d = squared(reference[j]) + if (d < nearest) { nearest = d; nearestIndex = j } + } + let at = reference[nearestIndex] + const last = Math.min(reference.length - 1, nearestIndex + step) + for (let k = Math.max(0, nearestIndex - step); k < last; k++) { + const on = closestOnSegment(point, reference[k], reference[k + 1], cos) + const d = squared(on) + if (d < nearest) { nearest = d; at = on } + } + return { d: nearest, at } +} + +/** The point of segment a-b closest to `p`, with longitude scaled by `cos` so the plane is fair. */ +function closestOnSegment( + p: [number, number], + a: [number, number], + b: [number, number], + cos: number, +): [number, number] { + const dx = (b[1] - a[1]) * cos + const dy = b[0] - a[0] + const length = dx * dx + dy * dy + if (length === 0) return a + const t = Math.max(0, Math.min(1, (((p[1] - a[1]) * cos) * dx + (p[0] - a[0]) * dy) / length)) + return [a[0] + (b[0] - a[0]) * t, a[1] + (b[1] - a[1]) * t] } diff --git a/client/src/components/Map/valhallaRoute.test.ts b/client/src/components/Map/valhallaRoute.test.ts index 14526e08e..49b1c7f5b 100644 --- a/client/src/components/Map/valhallaRoute.test.ts +++ b/client/src/components/Map/valhallaRoute.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect, afterEach } from 'vitest' import { http, HttpResponse } from 'msw' import { server } from '../../../tests/helpers/msw/server' import { useSettingsStore } from '../../store/settingsStore' -import { valhallaBase, valhallaAvailable, valhallaRouteAvoiding, valhallaRun, runFrom, legAvoids } from './valhallaRoute' +import { valhallaBase, valhallaAvailable, valhallaRouteAvoiding, valhallaRun, valhallaAlternates, alternatesFrom, runFrom, legAvoids } from './valhallaRoute' const FOSSGIS_VALHALLA = 'https://valhalla1.openstreetmap.de/route' @@ -365,3 +365,62 @@ describe('valhallaRun over a whole day', () => { await expect(valhallaRun(stops(6), 'driving', [])).resolves.toBeNull() }) }) + +describe('valhallaAlternates', () => { + /** The same leg read three ways, the second of them crossing by ferry. */ + const withAlternates = (alternates: unknown[]) => ({ + ...answer({ time: 11_460 }), + alternates, + }) + + it('VALHALLA-ALT-001: reads the preferred way, then every alternate, each with its own flags', () => { + const ways = alternatesFrom(withAlternates([ + answer({ time: 12_000, has_ferry: true }), + answer({ time: 13_000, has_toll: true }), + ])) + + expect(ways?.map(w => w.duration)).toEqual([11_460, 12_000, 13_000]) + expect(ways?.map(w => w.hasFerry)).toEqual([false, true, false]) + expect(ways?.map(w => w.hasToll)).toEqual([false, false, true]) + expect(ways?.[1].coordinates).toEqual(SHAPE_POINTS) + }) + + it('VALHALLA-ALT-002: an alternate that cannot be read is dropped, a preferred way that cannot is no answer', () => { + // One bad entry is no reason to lose the leg's other ways, but the others are offered + // against the preferred one and mean nothing without it. + expect(alternatesFrom(withAlternates([{ trip: { legs: [] } }, answer({ time: 13_000 })]))?.map(w => w.duration)) + .toEqual([11_460, 13_000]) + expect(alternatesFrom(answer())).toHaveLength(1) + expect(alternatesFrom({ alternates: [answer()] })).toBeNull() + expect(alternatesFrom({ error_code: 442, error: 'No path could be found for input' })).toBeNull() + }) + + it('VALHALLA-ALT-003: asks for the leg between exactly its two ends, weighed the way the trip is, with the number of alternates', async () => { + const bodies: Record[] = [] + server.use(http.post(FOSSGIS_VALHALLA, async ({ request }) => { + bodies.push(await request.json() as Record) + return HttpResponse.json(withAlternates([answer({ time: 12_000 })])) + })) + + const ways = await valhallaAlternates(HAMBURG, BERLIN, 'driving', ['toll', 'ferry'], 2) + await valhallaAlternates(HAMBURG, BERLIN, 'driving', ['toll'], 0) + + expect(ways).toHaveLength(2) + expect(bodies[0]).toMatchObject({ alternates: 2, costing: 'auto', costing_options: { auto: { use_tolls: 0, use_ferry: 0 } } }) + expect(bodies[0].locations).toHaveLength(2) + // No alternates asked for, none named: the request stays the plain one. + expect(bodies[1]).not.toHaveProperty('alternates') + }) + + it('VALHALLA-ALT-004: a refusal is asked once more, then is no answer; an instance with its own OSRM asks nothing', async () => { + let calls = 0 + server.use(http.post(FOSSGIS_VALHALLA, () => { calls++; return new HttpResponse(null, { status: 429 }) })) + + await expect(valhallaAlternates(HAMBURG, BERLIN, 'driving', ['toll'], 2)).resolves.toBeNull() + expect(calls).toBe(2) + + setSettings({ routing_base_url: 'https://osrm.example.org' }) + await expect(valhallaAlternates(HAMBURG, BERLIN, 'driving', ['toll'], 2)).resolves.toBeNull() + expect(calls).toBe(2) + }) +}) diff --git a/client/src/components/Map/valhallaRoute.ts b/client/src/components/Map/valhallaRoute.ts index fe736c0fd..5c4369327 100644 --- a/client/src/components/Map/valhallaRoute.ts +++ b/client/src/components/Map/valhallaRoute.ts @@ -204,14 +204,7 @@ export async function valhallaRun( // rate limit on the last piece of a split day would otherwise throw away the pieces // that already answered and ask for all of them again — three times over, against a // host that allows one request a second. - let run = await requestRun(base, chunk, profile, avoid, signal) - for (let attempt = 0; !run && attempt < CHUNK_RETRIES && !signal?.aborted; attempt++) { - // Not when the host just proved it is not a Valhalla at all: that answer will not - // change on a second ask, and asking anyway is the request the memo exists to save. - if (notValhalla.has(base)) break - await pause(CHUNK_PAUSE_MS, signal) - run = await requestRun(base, chunk, profile, avoid, signal) - } + const run = await askWithRetry(base, () => requestRun(base, chunk, profile, avoid, signal), signal) // A day is one answer or none. Half a day of legs would be worse than no answer: // the schedule chains leg times blindly and cannot tell that it is missing some, so // it would print a complete, plausible, wrong timetable rather than a gap. @@ -260,14 +253,39 @@ function pause(ms: number, signal?: AbortSignal): Promise { }) } -/** One request, at most ten locations. */ -async function requestRun( - base: string, +/** + * Waits out the host's one request a second before the next question is put. + * + * For a caller that asks this engine several things in a row on its own, such as the + * offers for one leg: the same pause the pieces of a split day keep, so a picker cannot + * fire at a pace the day routing was careful never to. + */ +export function valhallaSpacing(signal?: AbortSignal): Promise { + return pause(CHUNK_PAUSE_MS, signal) +} + +/** + * One question, and one more try after a pause when it earned no answer. + * + * Not when the host just proved it is not a Valhalla at all: that answer will not change + * on a second ask, and asking anyway is the request the memo exists to save. + */ +async function askWithRetry(base: string, ask: () => Promise, signal?: AbortSignal): Promise { + let answer = await ask() + for (let attempt = 0; !answer && attempt < CHUNK_RETRIES && !signal?.aborted; attempt++) { + if (notValhalla.has(base)) break + await pause(CHUNK_PAUSE_MS, signal) + answer = await ask() + } + return answer +} + +/** The request body for a chain of waypoints, weighted away from `avoid`. */ +function routeBody( waypoints: Waypoint[], profile: 'driving' | 'walking' | 'cycling', avoid: readonly AvoidClass[], - signal?: AbortSignal, -): Promise | null> { +): Record { const costing = COSTING[profile] // All of them in one request: the options are independent weightings, so asking to // leave out tolls and ferries together is one question rather than two routes to @@ -275,13 +293,16 @@ async function requestRun( // away together answered 656.9 km against 339.2 km for the plain route. const options: Record = {} for (const cls of avoid) options[AVOID_OPTION[cls]] = 0 - const body = { + return { locations: waypoints.map(w => ({ lat: w.lat, lon: w.lng, radius: SNAP_RADIUS_M })), costing, costing_options: { [costing]: options }, directions_options: { units: 'kilometers' }, } +} +/** The parsed answer to one POST, or null for every failure. */ +async function postRoute(base: string, body: Record, signal?: AbortSignal): Promise { try { const response = await fetch(`${base}/route`, { method: 'POST', @@ -299,12 +320,66 @@ async function requestRun( if (response.status === 404 || response.status === 405) notValhalla.add(base) return null } - return runFrom(await response.json()) + return await response.json() } catch { return null } } +/** One request, at most ten locations. */ +async function requestRun( + base: string, + waypoints: Waypoint[], + profile: 'driving' | 'walking' | 'cycling', + avoid: readonly AvoidClass[], + signal?: AbortSignal, +): Promise | null> { + const data = await postRoute(base, routeBody(waypoints, profile, avoid), signal) + return data === null ? null : runFrom(data) +} + +/** + * The ways of driving one leg this engine would offer, its preferred one first, or null. + * + * The offers for a trip that avoids something have to come from the engine that routes + * that trip's days, or the list and the rail describe two different drives: OSRM's + * quickest road beside a day Valhalla had routed was offered as the faster way, and + * taking it changed nothing. Valhalla answers `alternates` between exactly two locations, + * which is all one leg is, and weighs every one of them the way it weighed the day. + */ +export async function valhallaAlternates( + from: Waypoint, + to: Waypoint, + profile: 'driving' | 'walking' | 'cycling', + avoid: readonly AvoidClass[], + count: number, + signal?: AbortSignal, +): Promise { + const base = valhallaBase() + if (!base || notValhalla.has(base)) return null + const body = { ...routeBody([from, to], profile, avoid), ...(count > 0 ? { alternates: count } : {}) } + return askWithRetry(base, async () => { + const data = await postRoute(base, body, signal) + return data === null ? null : alternatesFrom(data) + }, signal) +} + +/** + * Every way in an answer that asked for alternates: `trip` first, then each + * `alternates[].trip`, read the way `runFrom` reads a single leg. + * + * Null when the preferred one cannot be read, because the others are offered against it. + * An alternate that cannot be read is dropped on its own: one bad entry is no reason to + * lose the leg's other ways. + */ +export function alternatesFrom(data: unknown): ValhallaLeg[] | null { + const primary = runFrom(data) + if (!primary) return null + const extra = (data as { alternates?: unknown })?.alternates + const others = Array.isArray(extra) ? extra.map(alt => runFrom(alt)?.total ?? null) : [] + return [primary.total, ...others.filter((leg): leg is ValhallaLeg => leg !== null)] +} + /** * Several legs read as one, the way a caller that only wants the whole drive sees it. * diff --git a/client/src/components/Roadtrip/RoadtripAlternativesBar.test.tsx b/client/src/components/Roadtrip/RoadtripAlternativesBar.test.tsx index fa9034809..3bdc18cbd 100644 --- a/client/src/components/Roadtrip/RoadtripAlternativesBar.test.tsx +++ b/client/src/components/Roadtrip/RoadtripAlternativesBar.test.tsx @@ -5,9 +5,10 @@ import { useSettingsStore } from '../../store/settingsStore' import RoadtripAlternativesBar from './RoadtripAlternativesBar' import type { AlternativeOverlay } from './alternativeOverlays' import type { LegAlternatives } from './useRouteAlternatives' +import { openLeg } from '../../../tests/helpers/legAlternatives' /** - * FE-ALTBAR-001..009 — the ways of driving one leg, offered over the map. + * FE-ALTBAR-001..012: the ways of driving one leg, offered over the map. * * Every value shown comes from the same overlay the map draws. Working them out * here separately is what once made the list label the driven route "Fastest" @@ -31,9 +32,7 @@ const overlay = (over: Partial = {}): AlternativeOverlay => ...over, }) -const leg = (over: Partial = {}): LegAlternatives => ({ - dayId: 4, index: 1, routes: [], loading: false, error: false, ...over, -}) +const leg = (over: Partial = {}): LegAlternatives => openLeg({ dayId: 4, index: 1, ...over }) beforeEach(() => { useSettingsStore.setState({ settings: { distance_unit: 'metric' } as never }) @@ -158,4 +157,36 @@ describe('RoadtripAlternativesBar', () => { ) expect(screen.getAllByLabelText(/avoidance router/i)).toHaveLength(1) }) + + it('FE-ALTBAR-011: while a choice is checked and saved the list stands still, and the one in hand says so', () => { + // A second click would race the first, and the router's answer to it could land on + // the leg after it. + const onChoose = vi.fn() + render( + , + ) + const [, current, checked, other] = screen.getAllByRole('button') + expect([current, checked, other].every(chip => (chip as HTMLButtonElement).disabled)).toBe(true) + expect(checked).toHaveAttribute('aria-busy', 'true') + expect(other).toHaveAttribute('aria-busy', 'false') + expect(checked.parentElement).toHaveAttribute('aria-busy', 'true') + fireEvent.click(other) + expect(onChoose).not.toHaveBeenCalled() + // Closing stays available: it is how a check nobody wants to wait for is abandoned. + expect(screen.getByRole('button', { name: /close/i })).not.toBeDisabled() + }) + + it('FE-ALTBAR-012: with nothing being checked every offer can be taken', () => { + render( + , + ) + const chips = screen.getAllByRole('button').slice(1) + expect(chips.every(chip => !(chip as HTMLButtonElement).disabled)).toBe(true) + expect(chips[0].parentElement).toHaveAttribute('aria-busy', 'false') + }) }) diff --git a/client/src/components/Roadtrip/RoadtripAlternativesBar.tsx b/client/src/components/Roadtrip/RoadtripAlternativesBar.tsx index 02cb319be..9de8a62ac 100644 --- a/client/src/components/Roadtrip/RoadtripAlternativesBar.tsx +++ b/client/src/components/Roadtrip/RoadtripAlternativesBar.tsx @@ -1,11 +1,11 @@ import React from 'react' -import { Shuffle, X, AlertTriangle, Info } from 'lucide-react' +import { Shuffle, X, AlertTriangle, Info, Loader2 } from 'lucide-react' import { useTranslation } from '../../i18n/TranslationContext' import { Tooltip } from '../shared/Tooltip' import { useSettingsStore } from '../../store/settingsStore' import { formatDistance } from '../../utils/units' import type { LegAlternatives } from './useRouteAlternatives' -import { alternativesPhase, alternativeSubline, type AlternativeOverlay } from './alternativeOverlays' +import { alternativesBusy, alternativesPhase, alternativeSubline, type AlternativeOverlay } from './alternativeOverlays' interface RoadtripAlternativesBarProps { open: LegAlternatives | null @@ -35,6 +35,9 @@ export default function RoadtripAlternativesBar({ const distanceUnit = useSettingsStore(s => s.settings.distance_unit) if (!open) return null const phase = alternativesPhase(open, overlays) + // While a choice is checked against the router and written, the list stands still: a + // second click would race the first, and its answer could land on the leg after it. + const busy = alternativesBusy(open) return (
@@ -64,25 +67,31 @@ export default function RoadtripAlternativesBar({ // One route back means there is genuinely only one sensible way to drive it.

{t('roadtrip.alt.onlyOne')}

) : ( -
+
{overlays.map(alt => (