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)]))