From 78366a902492ebc00e9b6d4fe6b98281ee9eb211 Mon Sep 17 00:00:00 2001 From: Maurice Date: Tue, 20 Oct 2026 12:59:07 +0200 Subject: [PATCH] fix(tours): long rides save, the horse route store stays bounded A ride sends its full line with heights and every horse route section's own line besides, about fifty bytes a point, so a ride along a few thousand points of bridleway hit the API's 100kb body ceiling and the save failed with a 413. Creating and editing a tour are now measured against TOUR_WRITE_MAX_BYTES (2 MB), like the book and list file routes; every other route, the tour copy among them, keeps 100kb. The horse routes kept in memory are now bounded by section points as well as by count (64 answers, 500k points), so panning a dense map can no longer grow the server by hundreds of megabytes. A search that found no connection can be asked again on the view whose routes are shown, and a ride with a recorded time shows the horseshoe beside its riding time instead of footprints. --- .../components/Tours/TourDayRowFacts.test.tsx | 8 ++++ .../src/components/Tours/TourDayRowFacts.tsx | 4 +- .../Tours/horse/HorseRoutesCard.test.tsx | 15 +++++- .../Tours/horse/HorseRoutesCard.tsx | 2 +- server/src/bootstrap.ts | 19 ++++++-- .../nest/tours/tour-horse-routes.service.ts | 46 ++++++++++++++++--- server/tests/e2e/tours-horse.e2e.test.ts | 28 ++++++++++- server/tests/integration/misc.test.ts | 34 +++++++++++++- .../nest/tour-horse-routes.service.test.ts | 44 +++++++++++++++--- shared/src/tours/tours.schema.ts | 7 +++ 10 files changed, 182 insertions(+), 25 deletions(-) diff --git a/client/src/components/Tours/TourDayRowFacts.test.tsx b/client/src/components/Tours/TourDayRowFacts.test.tsx index d80ae7cf1..4050ea598 100644 --- a/client/src/components/Tours/TourDayRowFacts.test.tsx +++ b/client/src/components/Tours/TourDayRowFacts.test.tsx @@ -35,6 +35,14 @@ describe('RC-01 Tour day-row facts', () => { expect(screen.getByTestId('plan-tour-icon').querySelector('.lucide-mountain')).toBeTruthy(); }); + it('HOR-01: a ride with a recorded time shows the horseshoe beside its riding time', () => { + render(); + const time = screen.getByTestId('trip-plan-tour-walking-time'); + expect(time.getAttribute('aria-label')).toMatch(/^Riding time/); + expect(time.querySelector('.lucide-horseshoe')).toBeTruthy(); + expect(time.querySelector('.lucide-footprints')).toBeNull(); + }); + it.each([false, true])('renders compact facts with accessible meaning (mobile=%s)', (mobile) => { render(); expect(screen.getByTestId('plan-tour-distance')).toHaveTextContent('4'); diff --git a/client/src/components/Tours/TourDayRowFacts.tsx b/client/src/components/Tours/TourDayRowFacts.tsx index 4bd61421b..3fae230c3 100644 --- a/client/src/components/Tours/TourDayRowFacts.tsx +++ b/client/src/components/Tours/TourDayRowFacts.tsx @@ -22,9 +22,9 @@ function knownType(type: string): boolean { return importableTourTypeSchema.safeParse(type).success; } -/** The icon the time on the move goes by: a ride, a run or a paddle by its type's own, any walk by footprints. */ +/** The icon the time on the move goes by: a bike tour, a ride on horseback, a run or a paddle by its type's own, any walk by footprints. */ function movingIconType(type: string): string { - return type === 'kayak' || type === 'bike' || type === 'run' ? type : 'walk'; + return type === 'kayak' || type === 'bike' || type === 'run' || type === 'horse' ? type : 'walk'; } export function TourDayRowIcon({ type, size = 27 }: { type: string; size?: number }) { diff --git a/client/src/components/Tours/horse/HorseRoutesCard.test.tsx b/client/src/components/Tours/horse/HorseRoutesCard.test.tsx index 5ead03a28..654eaabcc 100644 --- a/client/src/components/Tours/horse/HorseRoutesCard.test.tsx +++ b/client/src/components/Tours/horse/HorseRoutesCard.test.tsx @@ -2,6 +2,7 @@ import { HORSE_ROUTES_ATTRIBUTION, type HorseRoute } from '@trek/shared'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { act, fireEvent, render, screen, waitFor, within } from '../../../../tests/helpers/render'; import { hikePlannerDefaults } from '../../../../tests/helpers/tourPlannerStub'; +import { OnlineOnlyError } from '../../../repo/onlineOnly'; import { publishTourMapViewport } from '../../../store/tourMapViewportStore'; import type { TourPlannerWaypoint } from '../planner/tourPlannerWaypoints'; import type { TourPlannerController } from '../planner/useTourPlanner'; @@ -17,7 +18,7 @@ vi.mock('../../Map/MapViewAuto', () => ({ ), })); -// FE-HORSE-CARD-001 to FE-HORSE-CARD-007 +// FE-HORSE-CARD-001 to FE-HORSE-CARD-008 const VIEW = { south: 47.1, west: 11.2, north: 47.3, east: 11.5 }; const ROUTE: HorseRoute = { @@ -181,4 +182,16 @@ describe('HorseRoutesCard (HOR-01)', () => { fireEvent.click(within(dialog).getByRole('button', { name: 'Cancel' })); expect(stub.editWaypoints).not.toHaveBeenCalled(); }); + + it('FE-HORSE-CARD-008: a search without a connection can be asked again on the view found before', async () => { + repo.list.mockResolvedValueOnce(reply([ROUTE])).mockRejectedValueOnce(new OnlineOnlyError()); + render(); + fireEvent.click(screen.getByRole('button', { name: 'Find in map view' })); + await screen.findByTestId('horse-route-relation/42'); + act(() => publishTourMapViewport({ ...VIEW, north: 47.31 })); + fireEvent.click(await screen.findByRole('button', { name: 'Search this view' })); + expect(await screen.findByText('Horse routes need a connection.')).toBeInTheDocument(); + act(() => publishTourMapViewport(VIEW)); + expect(screen.getByRole('button', { name: 'Search this view' })).toBeEnabled(); + }); }); diff --git a/client/src/components/Tours/horse/HorseRoutesCard.tsx b/client/src/components/Tours/horse/HorseRoutesCard.tsx index 46ee68748..fa986468c 100644 --- a/client/src/components/Tours/horse/HorseRoutesCard.tsx +++ b/client/src/components/Tours/horse/HorseRoutesCard.tsx @@ -97,7 +97,7 @@ function HorseRoutesBody({ planner }: { planner: TourPlannerController }) { const found = useHorseRoutes(planner.tripId); const [open, setOpen] = useState(null); const searched = found.searched !== null; - const again = searched && (found.stale || found.status === 'failed'); + const again = searched && (found.stale || found.status === 'failed' || found.status === 'offline'); return ( } diff --git a/server/src/bootstrap.ts b/server/src/bootstrap.ts index d84ddf90b..509dfbbe0 100644 --- a/server/src/bootstrap.ts +++ b/server/src/bootstrap.ts @@ -20,7 +20,7 @@ import type { INestApplication } from '@nestjs/common'; import type { ConfigType } from '@nestjs/config'; import { NestFactory } from '@nestjs/core'; import { ExpressAdapter } from '@nestjs/platform-express'; -import { MAX_COLLECTION_FILE_BYTES } from '@trek/shared'; +import { MAX_COLLECTION_FILE_BYTES, TOUR_WRITE_MAX_BYTES } from '@trek/shared'; import express from 'express'; import type { Request, Response, NextFunction, RequestHandler } from 'express'; @@ -171,7 +171,7 @@ export async function buildApp(): Promise { if (Buffer.isBuffer(buffer)) (req as Request & { rawBody?: Buffer }).rawBody = buffer; }; /* - * ── Why one route gets a larger body than the rest ─────────────────── + * ── Why a few routes get a larger body than the rest ───────────────── * * A hundred kilobytes is the right ceiling for an API of forms and ids, and * it is the wrong one for a photo book. A Studio document is sent WHOLE on @@ -191,21 +191,32 @@ export async function buildApp(): Promise { * GPX of anybody who uses OsmAnd (#2301). The three routes that carry one, * the import into a new list, the GPX reader and the import into a list that * already exists, are measured against that megabyte, doubled for the JSON - * escaping of a document full of quotes. Everything else keeps the tighter - * limit. + * escaping of a document full of quotes. + * + * A tour is the third. Creating or editing one sends it whole: the full line + * with its heights, and every fixed leg's own line besides, about fifty bytes + * a point. A ride along a signposted horse route (HOR-01) is a few thousand + * points from OpenStreetMap, and one section of it alone can pass a hundred + * kilobytes, so those two routes are measured against TOUR_WRITE_MAX_BYTES. + * Everything else keeps the tighter limit. */ const bookBody = express.json({ limit: '8mb', verify: rawBodyKeeper }); const listFileBody = express.json({ limit: MAX_COLLECTION_FILE_BYTES * 2, verify: rawBodyKeeper }); + const tourBody = express.json({ limit: TOUR_WRITE_MAX_BYTES, verify: rawBodyKeeper }); const json = express.json({ limit: '100kb', verify: rawBodyKeeper }); const urlencoded = express.urlencoded({ limit: '100kb', extended: true, verify: rawBodyKeeper }); const isMcp = (req: Request) => req.path === '/mcp' || req.path === '/mcp/'; const isBookWrite = (req: Request) => req.method === 'PUT' && /^\/api\/journeys\/\d+\/book$/.test(req.path); const isListFile = (req: Request) => req.method === 'POST' && /^\/api\/addons\/collections\/(import|gpx\/read|\d+\/import)$/.test(req.path); + const isTourWrite = (req: Request) => + (req.method === 'POST' && /^\/api\/trips\/\d+\/tours$/.test(req.path)) || + (req.method === 'PUT' && /^\/api\/trips\/\d+\/tours\/\d+$/.test(req.path)); instance.use(function jsonParser(req: Request, res: Response, next: NextFunction) { if (isBookWrite(req)) return bookBody(req, res, next); if (isListFile(req)) return listFileBody(req, res, next); + if (isTourWrite(req)) return tourBody(req, res, next); return isMcp(req) ? next() : json(req, res, next); }); instance.use(function urlencodedParser(req: Request, res: Response, next: NextFunction) { diff --git a/server/src/nest/tours/tour-horse-routes.service.ts b/server/src/nest/tours/tour-horse-routes.service.ts index 831bf16bb..ea5d1a98c 100644 --- a/server/src/nest/tours/tour-horse-routes.service.ts +++ b/server/src/nest/tours/tour-horse-routes.service.ts @@ -18,7 +18,14 @@ import { export const HORSE_ROUTES_KEPT_MS = 12 * 60 * 60 * 1000; /** The most answers kept at once; the one kept longest goes first. */ -const HORSE_ROUTES_KEPT_MAX = 200; +export const HORSE_ROUTES_KEPT_MAX = 64; + +/** + * The most section points kept across all answers. One dense view can carry + * hundreds of thousands, and a point held as a JS tuple costs some sixty bytes, + * so this keeps the whole store near thirty megabytes however the map is panned. + */ +export const HORSE_ROUTES_KEPT_POINTS_MAX = 500_000; export const HORSE_ROUTES_UNAVAILABLE_MESSAGE = 'Horse route data is unavailable right now'; @@ -28,15 +35,18 @@ type Answer = Omit; * HOR-01: the signposted horse routes around a map view, from OpenStreetMap * through the operator's Overpass mirrors. The view is grown to the 0.05 * degree grid, so nearby views share one answer, and an answer is kept for - * HORSE_ROUTES_KEPT_MS in memory; two requests for the same box at once share - * one query. Read only: nothing is stored, and nothing about the trip is sent. + * HORSE_ROUTES_KEPT_MS in memory, within a budget of answers and of section + * points so a busy map cannot grow the store without end; two requests for + * the same box at once share one query. Read only: nothing is stored, and nothing about the trip is sent. * * When no mirror answers, the client gets a 502 `{ error, code }` it can show, * and whatever it had loaded before stays as it was. */ @Injectable() export class TourHorseRoutesService { - private readonly kept = new Map(); + private readonly kept = new Map(); + /** The section points of every answer in `kept`, against HORSE_ROUTES_KEPT_POINTS_MAX. */ + private keptPoints = 0; private readonly pending = new Map>(); constructor(private readonly overpass: OverpassElementsClient) {} @@ -78,9 +88,31 @@ export class TourHorseRoutesService { } private keep(key: string, answer: Answer, at: number): void { - this.kept.delete(key); - this.kept.set(key, { at, answer }); + this.drop(key); + const points = sectionPoints(answer); + this.kept.set(key, { at, answer, points }); + this.keptPoints += points; // A map keeps its insertion order: the first key is the one kept longest. - if (this.kept.size > HORSE_ROUTES_KEPT_MAX) this.kept.delete(this.kept.keys().next().value as string); + // The answer just kept stays even when it alone is past the budget. + while ( + this.kept.size > 1 && + (this.kept.size > HORSE_ROUTES_KEPT_MAX || this.keptPoints > HORSE_ROUTES_KEPT_POINTS_MAX) + ) { + this.drop(this.kept.keys().next().value as string); + } + } + + private drop(key: string): void { + const known = this.kept.get(key); + if (!known) return; + this.kept.delete(key); + this.keptPoints -= known.points; } } + +function sectionPoints(answer: Answer): number { + return answer.routes.reduce( + (sum, route) => sum + route.sections.reduce((inRoute, section) => inRoute + section.geometry.length, 0), + 0, + ); +} diff --git a/server/tests/e2e/tours-horse.e2e.test.ts b/server/tests/e2e/tours-horse.e2e.test.ts index f159303a8..bd9fee185 100644 --- a/server/tests/e2e/tours-horse.e2e.test.ts +++ b/server/tests/e2e/tours-horse.e2e.test.ts @@ -32,8 +32,9 @@ import { FakeRealtimeService } from '../helpers/fake-realtime'; import { createTestMikroOrmModule, createTestOrm, type TestOrm } from '../helpers/test-orm'; import { TestUnitOfWorkModule } from '../helpers/test-uow'; import { sessionCookie } from './harness'; +import type { NestExpressApplication } from '@nestjs/platform-express'; import { Test } from '@nestjs/testing'; -import { FERRATA_BBOX_TOO_LARGE_MESSAGE, HORSE_ROUTES_ATTRIBUTION } from '@trek/shared'; +import { FERRATA_BBOX_TOO_LARGE_MESSAGE, HORSE_ROUTES_ATTRIBUTION, TOUR_WRITE_MAX_BYTES } from '@trek/shared'; import cookieParser from 'cookie-parser'; import type { Server } from 'http'; @@ -157,7 +158,9 @@ describe('Horse tours e2e (real guards + temp SQLite)', () => { .overrideProvider(OverpassElementsClient) .useValue({ query: overpassQuery }) .compile(); - const nest = moduleRef.createNestApplication(); + const nest = moduleRef.createNestApplication(); + // The ceiling bootstrap.ts gives a tour create or edit, in place of the 100kb default. + nest.useBodyParser('json', { limit: TOUR_WRITE_MAX_BYTES }); nest.use(cookieParser()); nest.useGlobalPipes(new ZodValidationPipe()); nest.useGlobalFilters(new TrekExceptionFilter()); @@ -266,6 +269,27 @@ describe('Horse tours e2e (real guards + temp SQLite)', () => { expect(await countRows(orm, Tours)).toBe(0); }); + it('saves and opens again a ride along a long section, a body far past 100kb', async () => { + // A few thousand points at OSM's seven decimals, as a section of bridleway comes. + const line = Array.from({ length: 4000 }, (_, i) => [ + Number((47 + i * 0.00001).toFixed(7)), + Number((11 + i * 0.0000013).toFixed(7)), + ]); + const end = line[line.length - 1]; + const long = rideBody({ + route_geometry: line.map(([lat, lng], i) => [lat, lng, 600 + (i % 40)]), + waypoints: [ + { lat: 47, lng: 11, role: 'start', sequence: 0, fixed_leg: { ...SECTION, geometry: line } }, + { lat: end[0], lng: end[1], role: 'end', sequence: 1 }, + ], + }); + expect(JSON.stringify(long).length).toBeGreaterThan(150 * 1024); + const placeId = await create(long); + const res = await request(server).get(`/api/trips/5/tours/${placeId}`).set('Cookie', sessionCookie(OWNER)); + expect(res.status).toBe(200); + expect(res.body.waypoints[0].fixed_leg.geometry).toEqual(line); + }); + it('a member who may only read cannot save one', async () => { expect((await send('post', '', rideBody(), VIEWER)).status).toBe(403); }); diff --git a/server/tests/integration/misc.test.ts b/server/tests/integration/misc.test.ts index ce704d18b..45124699a 100644 --- a/server/tests/integration/misc.test.ts +++ b/server/tests/integration/misc.test.ts @@ -1,6 +1,6 @@ /** * Miscellaneous integration tests. - * Covers MISC-001, 002, 004, 007, 008, 013, 015. + * Covers MISC-001, 002, 004, 007, 008, 009 to 014. */ import { buildApp } from '../../src/bootstrap'; import { db as testDb } from '../../src/db/database'; @@ -8,6 +8,7 @@ import { authCookie } from '../helpers/auth'; import { createUser } from '../helpers/factories'; import { resetTestDb, resetRateLimits } from '../helpers/test-db'; import type { INestApplication } from '@nestjs/common'; +import { TOUR_WRITE_MAX_BYTES } from '@trek/shared'; import type { Application } from 'express'; import request from 'supertest'; @@ -158,4 +159,35 @@ describe('Request body ceiling', () => { .send({ name: 'x'.repeat(200 * 1024) }); expect(elsewhere.status).toBe(413); }); + + // A tour is sent whole on create and edit, its line and every fixed leg's + // line besides: a ride along a horse route passes 100kb (HOR-01). Only those + // two routes take the larger ceiling, and only up to TOUR_WRITE_MAX_BYTES. + it('MISC-013: a tour create or edit over 100kb reaches the router', async () => { + const created = await request(app) + .post('/api/trips/5/tours') + .set('Content-Type', 'application/json') + .send({ name: 'x'.repeat(300 * 1024) }); + expect(created.status).not.toBe(413); + + const edited = await request(app) + .put('/api/trips/5/tours/9') + .set('Content-Type', 'application/json') + .send({ name: 'x'.repeat(300 * 1024) }); + expect(edited.status).not.toBe(413); + }); + + it('MISC-014: a tour past its own ceiling is refused, and the other tour routes keep 100kb', async () => { + const huge = await request(app) + .post('/api/trips/5/tours') + .set('Content-Type', 'application/json') + .send({ name: 'x'.repeat(TOUR_WRITE_MAX_BYTES + 1) }); + expect(huge.status).toBe(413); + + const copy = await request(app) + .post('/api/trips/5/tours/9/copy') + .set('Content-Type', 'application/json') + .send({ name: 'x'.repeat(200 * 1024) }); + expect(copy.status).toBe(413); + }); }); diff --git a/server/tests/unit/nest/tour-horse-routes.service.test.ts b/server/tests/unit/nest/tour-horse-routes.service.test.ts index a7ff8fabd..1c34969f6 100644 --- a/server/tests/unit/nest/tour-horse-routes.service.test.ts +++ b/server/tests/unit/nest/tour-horse-routes.service.test.ts @@ -1,7 +1,9 @@ import { DomainError } from '../../../src/nest/common/domain-error'; import { OverpassElementsClient } from '../../../src/nest/overpass/overpass-elements.service'; import { + HORSE_ROUTES_KEPT_MAX, HORSE_ROUTES_KEPT_MS, + HORSE_ROUTES_KEPT_POINTS_MAX, HORSE_ROUTES_UNAVAILABLE_MESSAGE, TourHorseRoutesService, } from '../../../src/nest/tours/tour-horse-routes.service'; @@ -10,7 +12,7 @@ import { HORSE_ROUTES_ATTRIBUTION, HORSE_ROUTES_UNAVAILABLE_CODE, horseRoutesRes import { beforeEach, describe, expect, it, vi } from 'vitest'; -// HORSE-SVC-001 to HORSE-SVC-005 +// HORSE-SVC-001 to HORSE-SVC-006 const query = vi.fn<(q: string, caller: string) => Promise>(); let service: TourHorseRoutesService; @@ -87,14 +89,42 @@ describe('TourHorseRoutesService (HOR-01)', () => { expect((await service.list(box, NOW)).routes).toHaveLength(1); }); - it('HORSE-SVC-005: keeps at most two hundred answers, letting the one kept longest go', async () => { + it('HORSE-SVC-005: keeps a bounded number of answers, letting the one kept longest go', async () => { query.mockResolvedValue([]); const nth = (n: number) => ({ south: 40 + n * 0.1, west: 5, north: 40.05 + n * 0.1, east: 5.05 }); - for (let n = 0; n <= 200; n += 1) await service.list(nth(n), NOW); - expect(query).toHaveBeenCalledTimes(201); - await service.list(nth(200), NOW); - expect(query).toHaveBeenCalledTimes(201); + for (let n = 0; n <= HORSE_ROUTES_KEPT_MAX; n += 1) await service.list(nth(n), NOW); + expect(query).toHaveBeenCalledTimes(HORSE_ROUTES_KEPT_MAX + 1); + await service.list(nth(HORSE_ROUTES_KEPT_MAX), NOW); + await service.list(nth(1), NOW); + expect(query).toHaveBeenCalledTimes(HORSE_ROUTES_KEPT_MAX + 1); await service.list(nth(0), NOW); - expect(query).toHaveBeenCalledTimes(202); + expect(query).toHaveBeenCalledTimes(HORSE_ROUTES_KEPT_MAX + 2); + }); + + it('HORSE-SVC-006: keeps no more section points than its budget, letting the one kept longest go', async () => { + // Ways of ten thousand points, together just over half the budget: two such answers never fit at once. + const perWay = 10_000; + const wayCount = Math.ceil((HORSE_ROUTES_KEPT_POINTS_MAX / 2 + 1) / perWay); + const ids = Array.from({ length: wayCount }, (_, w) => w + 1); + const dense = [ + { type: 'relation', id: 7, tags: { route: 'horse' }, members: ids.map((ref) => ({ type: 'way', ref, role: '' })) }, + ...ids.map((id) => ({ + type: 'way', + id, + tags: { horse: 'designated' }, + nodes: Array.from({ length: perWay }, (_, i) => id * perWay + i + 1), + geometry: Array.from({ length: perWay }, (_, i) => ({ lat: 47.15 + i * 1e-5, lon: 11 + id * 0.001 })), + })), + ]; + query.mockResolvedValue(dense); + const other = { south: 48.12, west: 11.21, north: 48.18, east: 11.29 }; + await service.list(box, NOW); + await service.list(other, NOW); + expect(query).toHaveBeenCalledTimes(2); + // The second answer pushed the first out, and stays itself. + await service.list(other, NOW); + expect(query).toHaveBeenCalledTimes(2); + await service.list(box, NOW); + expect(query).toHaveBeenCalledTimes(3); }); }); diff --git a/shared/src/tours/tours.schema.ts b/shared/src/tours/tours.schema.ts index a6b8e1fe5..0b7924716 100644 --- a/shared/src/tours/tours.schema.ts +++ b/shared/src/tours/tours.schema.ts @@ -308,6 +308,13 @@ export type TourWaypointRole = z.infer; export const TOUR_FIXED_LEG_MAX_POINTS = 5000; /** How far a fixed leg's ends may lie from the waypoints it joins, in metres. */ export const TOUR_FIXED_LEG_END_TOLERANCE_M = 60; +/** + * The largest body a tour create or edit may send, in bytes. A tour goes whole, + * its full line with heights and every fixed leg's line besides, at roughly + * fifty bytes a point: one horse route section at TOUR_FIXED_LEG_MAX_POINTS is + * a quarter of a megabyte on its own, far past the API's hundred kilobytes. + */ +export const TOUR_WRITE_MAX_BYTES = 2 * 1024 * 1024; const fixedLegGeometrySchema = z .array(z.tuple([z.number().finite().min(-90).max(90), z.number().finite().min(-180).max(180)]))