diff --git a/client/src/components/Studio/applyTemplate.ts b/client/src/components/Studio/applyTemplate.ts index 307079514..efb1e4f1c 100644 --- a/client/src/components/Studio/applyTemplate.ts +++ b/client/src/components/Studio/applyTemplate.ts @@ -1,4 +1,5 @@ import type { BookElement, BookPageSetup, BookSpread, JourneyStats } from '@trek/shared' +import { MAX_TEXT_LENGTH } from '@trek/shared' import type { SpreadTemplate } from './bookTemplates.data' import { formatBookCoords, formatBookDate } from './entryText' import { coordValue } from './resolveBindings' @@ -109,8 +110,10 @@ export function applyTemplate( const PH = page.pageHeight let nextPhoto = 0 - const story = (entry.story || '').trim() - const heading = entry.title || entry.location || '' + // A journal entry has no length limit and a text element does — see the note + // in the contract. Poured in whole, one long day is a book that will not save. + const story = (entry.story || '').trim().slice(0, MAX_TEXT_LENGTH) + const heading = (entry.title || entry.location || '').slice(0, MAX_TEXT_LENGTH) const elements = template.elements.map(el => { const out = { diff --git a/client/src/components/Studio/autoLayout.ts b/client/src/components/Studio/autoLayout.ts index 6c0214d98..ea76c7a99 100644 --- a/client/src/components/Studio/autoLayout.ts +++ b/client/src/components/Studio/autoLayout.ts @@ -1,6 +1,11 @@ import type { BookDocument, BookElement, BookMetric, BookPageSetup, BookSpread, JourneyStats, } from '@trek/shared' +import { + MAX_BOOK_COUNTRIES, MAX_BOOK_TITLE, MAX_COUNTRY_NAME, MAX_MAP_POINTS, + MAX_MAP_POINT_LABEL, MAX_PATH_POINTS, MAX_PATH_SEGMENTS, MAX_SPREADS, + MAX_SPREAD_ELEMENTS, MAX_TEXT_LENGTH, +} from '@trek/shared' import { SPREAD_TEMPLATES } from './bookTemplates.data' import { applyTemplate, templateFit } from './applyTemplate' import { formatBookCoords, formatBookDate } from './entryText' @@ -119,7 +124,7 @@ function text( rotation: 0, opacity: 1, locked: false, - text: value, + text: value.slice(0, MAX_TEXT_LENGTH), font: 'sans', size: 11, weight: 400, @@ -260,9 +265,12 @@ function mapEl( routeArc: 'bow', routeDash: 'arcs', pinStyle: 'photo', - countries: stats.countries.map(c => c.code), - points: stats.points.map(pt => ({ - lat: pt.lat, lng: pt.lng, label: pt.label, photoId: pt.photoId ?? null, + countries: stats.countries.slice(0, MAX_BOOK_COUNTRIES).map(c => c.code), + points: stats.points.slice(0, MAX_MAP_POINTS).map(pt => ({ + lat: pt.lat, + lng: pt.lng, + label: pt.label.slice(0, MAX_MAP_POINT_LABEL), + photoId: pt.photoId ?? null, })), /* * The fields the contract would default for a parsed element, spelled out @@ -289,8 +297,8 @@ function countriesEl( id: uid('co'), kind: 'countries', frame, - codes: stats.countries.map(c => c.code), - names, + codes: stats.countries.slice(0, MAX_BOOK_COUNTRIES).map(c => c.code), + names: names.slice(0, MAX_BOOK_COUNTRIES).map(n => n.slice(0, MAX_COUNTRY_NAME)), layout: 'list', showOutline: true, showFlag: false, @@ -550,8 +558,23 @@ function hasSubstance(entry: AutoEntry): boolean { return entry.photos.length > 0 || !!(entry.story || '').trim() } -/** How many stations one page holds before the type gets too tight. */ -const STATIONS_PER_PAGE = 13 +/** What one station row costs: its date, its name, and the rule under it. */ +const STATION_ELEMENTS = 3 +/** And what the heading above the list costs: an accent rule and a word. */ +const STATIONS_HEADING = 2 + +/** + * How many stations one page holds. + * + * Thirteen is where the type gets too tight — but the contract caps a spread at + * MAX_SPREAD_ELEMENTS, and a spread is two of these pages, so thirteen rows a + * page is eighty elements and a book the server refuses to store. Whichever + * limit runs out first is the answer. + */ +const STATIONS_PER_PAGE = Math.min( + 13, + Math.floor((MAX_SPREAD_ELEMENTS - STATIONS_HEADING) / STATION_ELEMENTS / 2), +) /** * The stops that had nothing to say, as a list. @@ -1311,7 +1334,9 @@ function summarySpread(input: AutoInput): BookSpread | null { if (stats.points.length >= 2) { // With the recorded track, when the journey has one: the editor fetches it // and this was the line that threw it away. - els.push(mapEl({ x: m, y: m, w: W - m * 2, h: H - m * 2 }, stats, { path: input.path ?? [] })) + els.push(mapEl({ x: m, y: m, w: W - m * 2, h: H - m * 2 }, stats, { + path: (input.path ?? []).slice(0, MAX_PATH_SEGMENTS).map(seg => seg.slice(0, MAX_PATH_POINTS)), + })) } // Which figures are worth the space: everything the journey actually has. @@ -1550,9 +1575,9 @@ export function buildBook(input: AutoInput): BookDocument { spreads.push(backSpread(input)) return { version: 1, - title: input.title, + title: input.title.slice(0, MAX_BOOK_TITLE), page: input.page, - spreads: spreads.slice(0, 150), + spreads: spreads.slice(0, MAX_SPREADS), } } @@ -1575,7 +1600,7 @@ export function emptyBook(input: AutoInput): BookDocument { }) return { version: 1, - title: input.title, + title: input.title.slice(0, MAX_BOOK_TITLE), page: input.page, spreads: [blank('cover'), blank('inner'), blank('back')], } diff --git a/client/src/components/Studio/resolveBindings.ts b/client/src/components/Studio/resolveBindings.ts index 76b0bc862..2a39f58da 100644 --- a/client/src/components/Studio/resolveBindings.ts +++ b/client/src/components/Studio/resolveBindings.ts @@ -1,4 +1,5 @@ import type { BookDocument } from '@trek/shared' +import { MAX_TEXT_LENGTH } from '@trek/shared' import { formatBookCoords, formatBookDate } from './entryText' /** @@ -73,16 +74,20 @@ export function resolveBindings(doc: BookDocument, src: BindingSource, locale: s if (el.kind !== 'text' || !el.binding || el.overridden) return el const next = resolveOne(el.binding, src, locale) + // A journal entry has no length limit and a text element does. Written + // back whole, a long story is a book the save route refuses — and the + // editor can only report that as a book that will not save. + const value = next?.text.slice(0, MAX_TEXT_LENGTH) // An empty answer is not an answer — see the first rule above. A journey // with no subtitle, an entry whose title has been cleared and a photo // that lost its caption all arrive here as an empty string, and none of // them is a reason to blank a line somebody has set. - if (!next || !next.text || next.text === el.text) return el + if (!next || !value || value === el.text) return el spreadChanged = true return next.value === undefined - ? { ...el, text: next.text } - : { ...el, text: next.text, binding: { ...el.binding, value: next.value } } + ? { ...el, text: value } + : { ...el, text: value, binding: { ...el.binding, value: next.value } } }) if (!spreadChanged) return spread diff --git a/client/tests/unit/studio/autoLayout.contract.test.ts b/client/tests/unit/studio/autoLayout.contract.test.ts new file mode 100644 index 000000000..ca417bcc2 --- /dev/null +++ b/client/tests/unit/studio/autoLayout.contract.test.ts @@ -0,0 +1,95 @@ +import { describe, it, expect } from 'vitest' +import type { BookPageSetup, JourneyStats } from '@trek/shared' +import { bookDocumentSchema, bookPageSetupSchema } from '@trek/shared' +import { buildBook, emptyBook, type AutoEntry, type AutoInput } from '../../../src/components/Studio/autoLayout' + +/** + * The auto layout writes journey data straight into a book document, and the + * save route parses that document against the contract before storing it — so + * anything the layout produces that the contract will not take is a book that + * cannot be saved at all, for the rest of the session (#2085). + * + * These cases feed it journeys past every cap the contract sets and assert the + * one thing that matters: whatever comes out, the server would take it. + */ + +const page: BookPageSetup = bookPageSetupSchema.parse({ + preset: 'square-210', pageWidth: 210, pageHeight: 210, bleed: 3, safe: 5, +}) + +const long = (n: number) => 'x'.repeat(n) + +const entry = (over: Partial = {}): AutoEntry => ({ + id: 1, title: 'A day', story: 'Something happened.', location: 'Reykjavík', + date: '2026-06-02', photos: [], ...over, +}) + +const stats = (over: Partial = {}): JourneyStats => ({ + journeyId: 6, distance: 1_189_000, days: 14, steps: 14, photos: 57, places: 21, furthest: 408_000, + countries: [{ code: 'IS', name: 'Iceland', places: 14, firstVisit: '2026-06-02' }], + points: [ + { lat: 64.14, lng: -21.94, label: 'Reykjavík', date: '2026-06-02', country: 'IS', tripId: null, photoId: null }, + { lat: 65.68, lng: -18.12, label: 'Akureyri', date: '2026-06-06', country: 'IS', tripId: null, photoId: null }, + ], + trips: [], start: '2026-06-02', end: '2026-06-15', + ...over, +}) + +const input = (over: Partial = {}): AutoInput => ({ + locale: 'en', title: 'Iceland', subtitle: null, coverPhotoId: null, + stationsLabel: 'Stations', dayLabel: 'DAY', summaryLabel: 'Trip summary', countriesLabel: 'Countries', + entries: [entry()], page, stats: stats(), ...over, +}) + +/** The contract's own verdict, with the offending fields named when it refuses. */ +const savable = (doc: unknown) => { + const parsed = bookDocumentSchema.safeParse(doc) + return parsed.success + ? { ok: true as const } + : { ok: false as const, at: parsed.error.issues.map(i => i.path.join('.')) } +} + +describe('a laid-out book is a book the server will take', () => { + it('when an entry runs longer than a text element may hold', () => { + expect(savable(buildBook(input({ entries: [entry({ story: long(20_000) })] })))).toEqual({ ok: true }) + }) + + it('when a stop carries a place name longer than a map label may print', () => { + const points = [ + { lat: 64.14, lng: -21.94, label: long(300), date: '2026-06-02', country: 'IS', tripId: null, photoId: null }, + { lat: 65.68, lng: -18.12, label: 'Akureyri', date: '2026-06-06', country: 'IS', tripId: null, photoId: null }, + ] + expect(savable(buildBook(input({ stats: stats({ points }) })))).toEqual({ ok: true }) + }) + + it('when the journey visited more countries than a page may name', () => { + const countries = Array.from({ length: 150 }, () => ({ + code: 'IS', name: long(150), places: 1, firstVisit: '2026-06-02', + })) + expect(savable(buildBook(input({ stats: stats({ countries }) })))).toEqual({ ok: true }) + }) + + it('when the journey is called something longer than a title may be', () => { + expect(savable(buildBook(input({ title: long(400) })))).toEqual({ ok: true }) + expect(savable(emptyBook(input({ title: long(400) })))).toEqual({ ok: true }) + }) + + /* + * The one that is not about a runaway string: a run of stops with nothing on + * them becomes a two-column list, and the rows cost three elements each. At + * thirteen rows a page that is eighty elements on a spread the contract caps + * at sixty — an ordinary journey of dated stops, not a pathological one. + */ + it('when a long run of bare stops becomes an itinerary list', () => { + const bare = Array.from({ length: 60 }, (_, i) => entry({ + id: i + 1, title: 'Stop ' + i, story: '', location: 'Place ' + i, date: '2026-06-01', + })) + expect(savable(buildBook(input({ entries: bare })))).toEqual({ ok: true }) + }) + + it('when a recorded track is longer than the map may draw', () => { + const path = Array.from({ length: 60 }, () => + Array.from({ length: 2000 }, () => [64, -21] as [number, number])) + expect(savable(buildBook(input({ path })))).toEqual({ ok: true }) + }) +}) diff --git a/client/tests/unit/studio/autoLayout.stations.test.ts b/client/tests/unit/studio/autoLayout.stations.test.ts index 0978fb071..66a822a89 100644 --- a/client/tests/unit/studio/autoLayout.stations.test.ts +++ b/client/tests/unit/studio/autoLayout.stations.test.ts @@ -7,7 +7,7 @@ import { describe, it, expect, vi } from 'vitest' * in Studio — so the set is empty here on purpose. */ vi.mock('../../../src/components/Studio/bookTemplates.data', () => ({ SPREAD_TEMPLATES: [] })) -import { bookPageSetupSchema, type BookSpread } from '@trek/shared' +import { bookDocumentSchema, bookPageSetupSchema, type BookSpread } from '@trek/shared' import { buildBook, type AutoEntry, type AutoInput } from '../../../src/components/Studio/autoLayout' /** @@ -34,12 +34,13 @@ const rich = (id: number): AutoEntry => ({ location: 'Berlin', date: '2026-07-08', photos: [], }) +const document = (entries: AutoEntry[]) => buildBook({ + locale: 'en', title: 'T', subtitle: null, coverPhotoId: null, + entries, page, stats: null, stationsLabel: 'Stations', dayLabel: 'DAY', summaryLabel: 'Trip summary', countriesLabel: 'Countries', +}) + function build(entries: AutoEntry[]): BookSpread[] { - const input: AutoInput = { - locale: 'en', title: 'T', subtitle: null, coverPhotoId: null, - entries, page, stats: null, stationsLabel: 'Stations', dayLabel: 'DAY', summaryLabel: 'Trip summary', countriesLabel: 'Countries', - } - return buildBook(input).spreads + return document(entries).spreads } /** The inner spreads, which is where entries land. */ @@ -75,6 +76,18 @@ describe('entries with nothing on them', () => { expect(inner(build(many))).toHaveLength(2) }) + /* + * And the list it runs onto is one the server will take (#2085). + * + * This fixture already built the document that could not be saved; it only + * ever counted the spreads. The save route parses against the contract, not + * against normalizeBookDocument, so that is what a layout has to satisfy. + */ + it('produces a document the save route accepts, however long the run', () => { + const many = Array.from({ length: 200 }, (_, i) => bare(i + 1)) + expect(bookDocumentSchema.safeParse(document(many)).success).toBe(true) + }) + /* * The old behaviour, restated as the thing that must not come back: one * spread per stop, each of them nearly empty. diff --git a/server/src/bootstrap.ts b/server/src/bootstrap.ts index 02debeff5..b1d158387 100644 --- a/server/src/bootstrap.ts +++ b/server/src/bootstrap.ts @@ -122,7 +122,7 @@ export async function buildApp(): Promise { * it is the wrong one for a photo book. A Studio document is sent WHOLE on * every autosave — deliberately, see book-store.schema.ts, because a patch * protocol would need an ordering guarantee and a merge rule per field — and - * the contract already caps it at 150 spreads of 60 elements. A real book of + * the contract already caps it at 150 spreads of 90 elements. A real book of * a fortnight's journey is well past a hundred kilobytes before anybody asks * for road geometry, and what the ceiling produces is not an error message * but a save that quietly fails and an editor that says "not saved" without diff --git a/server/tests/e2e/journey.e2e.test.ts b/server/tests/e2e/journey.e2e.test.ts index a77f52927..8e049a89b 100644 --- a/server/tests/e2e/journey.e2e.test.ts +++ b/server/tests/e2e/journey.e2e.test.ts @@ -64,6 +64,7 @@ const { booksvc } = vi.hoisted(() => ({ }, })); import { JourneyBookService } from '../../src/nest/journey/journey-book.service'; +import { MAX_SPREAD_ELEMENTS } from '@trek/shared'; import { JourneyModule } from '../../src/nest/journey/journey.module'; import { AddonsService } from '../../src/nest/addons/addons.service'; @@ -300,6 +301,37 @@ describe('Journey e2e (real auth guard + temp SQLite)', () => { expect(booksvc.saveBook).not.toHaveBeenCalled(); }); + /* + * The refusal Studio hit on its own auto layout (#2085). + * + * A spread past MAX_SPREAD_ELEMENTS is refused whole, and the editor can only + * report that as "couldn't save" — for the rest of the session, because + * autosave keeps offering the same document. The layout is what was fixed; + * this pins the other half, that the route does refuse such a book, so the + * client-side guarantee is a guarantee about something. + */ + it('400 for a spread carrying more elements than the contract allows', async () => { + const element = (i: number) => ({ + id: 'e' + i, kind: 'shape', frame: { x: 0, y: 0, w: 10, h: 10 }, shape: 'rect', + }); + const res = await request(server) + .put('/api/journeys/9/book') + .set('Cookie', sessionCookie(1)) + .send({ + title: 'T', + document: { + version: 1, + spreads: [{ + id: 'sp1', + role: 'inner', + elements: Array.from({ length: MAX_SPREAD_ELEMENTS + 1 }, (_, i) => element(i)), + }], + }, + }); + expect(res.status).toBe(400); + expect(booksvc.saveBook).not.toHaveBeenCalled(); + }); + it('204 on delete, and 404 when the journey is out of reach', async () => { booksvc.deleteBook.mockReturnValue(true); const ok = await request(server).delete('/api/journeys/9/book').set('Cookie', sessionCookie(1)); diff --git a/shared/src/book/book.schema.spec.ts b/shared/src/book/book.schema.spec.ts index 936b48aa8..dbbc38e1e 100644 --- a/shared/src/book/book.schema.spec.ts +++ b/shared/src/book/book.schema.spec.ts @@ -171,9 +171,11 @@ describe('normalizeBookDocument', () => { expect(out.spreads[0]!.parked.map((el) => el.id)).toEqual(['parked-kept']); }); - it('trims a spread of seventy elements to the cap rather than losing the spread', () => { - const seventy = Array.from({ length: 70 }, (_, i) => text({ id: `t${i}` })); - const out = normalizeBookDocument(doc(seventy)); + it('trims a spread past the cap to it rather than losing the spread', () => { + // Counted off the cap, not written out: as a literal this fixture stopped + // being over the cap the moment the cap moved, and passed by not trimming. + const over = Array.from({ length: MAX_SPREAD_ELEMENTS + 10 }, (_, i) => text({ id: `t${i}` })); + const out = normalizeBookDocument(doc(over)); expect(out.spreads[0]!.elements).toHaveLength(MAX_SPREAD_ELEMENTS); // Back to front, so the ones that go are the ones on top. diff --git a/shared/src/book/book.schema.ts b/shared/src/book/book.schema.ts index 3a2e23610..0de9064be 100644 --- a/shared/src/book/book.schema.ts +++ b/shared/src/book/book.schema.ts @@ -38,6 +38,31 @@ const mm = z.number().finite().min(-10000).max(10000).transform(v => Math.round( const hex = z.string().regex(/^#[0-9a-fA-F]{6}$/, 'expected #rrggbb'); +/** + * The limits a producer has to build to, named rather than written inline. + * + * Studio's auto layout is a producer: it pours journey data straight into a + * document, and a document over any one of these is refused whole by the save + * route — which the editor can only report as "couldn't save", for the rest of + * the session, because autosave keeps offering the same rejected book. So the + * caps are exported and the layout builds against them, instead of the two + * being written down twice and drifting apart. + * + * `MAX_SPREAD_ELEMENTS` belongs to this set and is declared with the spread it + * bounds, where the salvage pass reads it too. + */ +export const MAX_SPREADS = 150; +export const MAX_BOOK_TITLE = 200; +export const MAX_TEXT_LENGTH = 8000; +/** Codes on a map or a country page, and the names printed beside them. */ +export const MAX_BOOK_COUNTRIES = 80; +export const MAX_COUNTRY_NAME = 80; +export const MAX_MAP_POINTS = 400; +export const MAX_MAP_POINT_LABEL = 120; +/** A recorded track: segments, and points within one segment. */ +export const MAX_PATH_SEGMENTS = 40; +export const MAX_PATH_POINTS = 1200; + /** * Every shape the book can draw — and, because a picture frame *is* a shape with * a photograph inside it, every mask a photo can be cut to. @@ -147,7 +172,7 @@ export const bookPhotoElementSchema = z.object({ export const bookTextElementSchema = z.object({ ...elementBase, kind: z.literal('text'), - text: z.string().max(8000).default(''), + text: z.string().max(MAX_TEXT_LENGTH).default(''), font: z.enum(BOOK_FONTS_IDS).default('sans'), /** Points. */ size: z.number().min(4).max(200).default(11), @@ -365,7 +390,7 @@ export const bookMapElementSchema = z.object({ */ pinStyle: z.enum(['dot', 'photo']).default('dot'), /** ISO-3166-1 alpha-2, the countries whose outlines to draw. */ - countries: z.array(z.string().length(2)).max(80).default([]), + countries: z.array(z.string().length(2)).max(MAX_BOOK_COUNTRIES).default([]), /** * The route, in order. Capped where a printed line stops gaining from more * points — a book page cannot resolve four hundred stops anyway. @@ -374,7 +399,7 @@ export const bookMapElementSchema = z.object({ .array(z.object({ lat: z.number().min(-90).max(90), lng: z.number().min(-180).max(180), - label: z.string().max(120).default(''), + label: z.string().max(MAX_MAP_POINT_LABEL).default(''), /** * One photograph from this stop, for a marker that shows it. * @@ -384,7 +409,7 @@ export const bookMapElementSchema = z.object({ */ photoId: z.number().int().positive().nullable().default(null), })) - .max(400) + .max(MAX_MAP_POINTS) .default([]), /** * The way the trip was actually travelled, where the trip knows it. @@ -405,8 +430,8 @@ export const bookMapElementSchema = z.object({ * three-week trip is a document nobody can save. */ path: z - .array(z.array(z.tuple([z.number().min(-90).max(90), z.number().min(-180).max(180)])).max(1200)) - .max(40) + .array(z.array(z.tuple([z.number().min(-90).max(90), z.number().min(-180).max(180)])).max(MAX_PATH_POINTS)) + .max(MAX_PATH_SEGMENTS) .default([]), /** * The roads each leg actually took, when somebody asked for them. @@ -521,9 +546,9 @@ export const bookCountriesElementSchema = z.object({ ...typeset, kind: z.literal('countries'), /** ISO-3166-1 alpha-2, in visit order. */ - codes: z.array(z.string().length(2)).max(80).default([]), + codes: z.array(z.string().length(2)).max(MAX_BOOK_COUNTRIES).default([]), /** Names as resolved when placed, so the page does not depend on a lookup. */ - names: z.array(z.string().max(80)).max(80).default([]), + names: z.array(z.string().max(MAX_COUNTRY_NAME)).max(MAX_BOOK_COUNTRIES).default([]), layout: z.enum(['list', 'grid', 'column']).default('list'), /** The silhouette behind each name — the thing that makes the page read as a map. */ showOutline: z.boolean().default(true), @@ -666,8 +691,19 @@ export const bookListElementSchema = z.object({ * * A limit rather than a taste: the document is saved whole on every autosave, * and a page nobody could design is a page somebody's browser has to serialise. + * + * Sixty was below what this app's own layouts draw. A full itinerary list is + * two columns of thirteen stops, and a stop costs three elements — its date, + * its name and the rule under it — so the spread the auto layout produces is + * eighty, and it was refused every time (#2085). A limit the product cannot + * build to is not a limit, it is a bug with a constant in front of it. + * + * Ninety rather than eighty, so the next row of anything does not land here + * again. It stays a serialisation limit: an element of this kind measures a few + * hundred bytes, which puts the largest document the contract admits at under + * four megabytes against the book route's eight (server/src/bootstrap.ts). */ -export const MAX_SPREAD_ELEMENTS = 60; +export const MAX_SPREAD_ELEMENTS = 90; export const bookElementSchema = z.discriminatedUnion('kind', [ bookPhotoElementSchema, @@ -770,9 +806,9 @@ export type BookPageSetup = z.infer; export const bookDocumentSchema = z.object({ /** In the document, not in a column: a format bump must not need a migration. */ version: z.literal(1).catch(1).default(1), - title: z.string().max(200).default(''), + title: z.string().max(MAX_BOOK_TITLE).default(''), page: bookPageSetupSchema.default(() => bookPageSetupSchema.parse({})), - spreads: z.array(bookSpreadSchema).max(150).default([]), + spreads: z.array(bookSpreadSchema).max(MAX_SPREADS).default([]), }); export type BookDocument = z.infer;