From e316ccf3bbb1bad7d065dddf888b0e38fff83b05 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sat, 10 Oct 2026 12:37:51 +0200 Subject: [PATCH] perf(db): the calendar export, the journey reads and the stop reorders read and write in batches The ICS export reads the stops and notes of all dated days in two statements instead of two per day, the journey's full read and entry list take the source trip titles in one join instead of one read per entry, and reordering a day's stops or sorting it by time writes every position in one CASE update. Results are unchanged: each day keeps its order (the id settles a tie), a missing trip still names null, and an id listed twice keeps its last position. --- server/scripts/size-baseline.json | 4 +-- .../repositories/DayAssignments.repository.ts | 24 +++++++++++++ .../repositories/JourneyEntries.repository.ts | 16 +++++++++ .../repositories/Reservations.repository.ts | 27 +++++++------- .../nest/assignments/assignments.service.ts | 21 +++++------ server/src/nest/calendar/calendar.service.ts | 32 +++++++++++------ .../nest/journey/journey-domain.service.ts | 35 ++++++++----------- .../DayAssignments.repository.test.ts | 26 ++++++++++++++ .../Reservations.repository.test.ts | 4 +-- .../tests/unit/nest/calendar.service.test.ts | 18 ++++++++++ .../unit/nest/journey-domain.service.test.ts | 17 +++++++++ 11 files changed, 163 insertions(+), 61 deletions(-) diff --git a/server/scripts/size-baseline.json b/server/scripts/size-baseline.json index 034ce75db..91ba58ac6 100644 --- a/server/scripts/size-baseline.json +++ b/server/scripts/size-baseline.json @@ -2,7 +2,7 @@ "scripts/db-call-graph.mjs": 1036, "scripts/generate-entities.ts": 2259, "src/db/repositories/Places.repository.ts": 1440, - "src/db/repositories/Reservations.repository.ts": 1676, + "src/db/repositories/Reservations.repository.ts": 1675, "src/db/repositories/Trips.repository.ts": 1121, "src/db/repositories/Users.repository.ts": 1710, "src/nest/atlas/atlas-geo.ts": 1005, @@ -12,7 +12,7 @@ "src/nest/collections/collections.service.ts": 1488, "src/nest/doc-sync/doc-sync.service.ts": 1060, "src/nest/doc-sync/providers/synology-drive.client.ts": 1160, - "src/nest/journey/journey-domain.service.ts": 1804, + "src/nest/journey/journey-domain.service.ts": 1799, "src/nest/maps/maps.service.ts": 3252, "src/nest/places/places.service.ts": 2020, "src/nest/plugins/plugin-runtime.service.ts": 1463, diff --git a/server/src/db/repositories/DayAssignments.repository.ts b/server/src/db/repositories/DayAssignments.repository.ts index 83208d5a4..94480d24e 100644 --- a/server/src/db/repositories/DayAssignments.repository.ts +++ b/server/src/db/repositories/DayAssignments.repository.ts @@ -461,6 +461,30 @@ export class DayAssignmentsRepository extends TrekRepository { await this.nativeUpdate(where, { order_index }); } + /** + * {@link setOrderIndex} for many rows in one statement: `UPDATE + * day_assignments SET order_index = CASE id WHEN ? THEN ? ... END WHERE id IN + * (...) [AND day_id = ?]`. The result is what the writes one by one in the + * given order left: an id listed twice keeps its last index, and with + * `day_id` a row on another day is not touched. + */ + async setOrderIndexes(order: ReadonlyArray<{ id: number; order_index: number }>, day_id?: number): Promise { + const last = new Map(); + for (const { id, order_index } of order) last.set(id, order_index); + const [first, ...rest] = [...last]; + if (!first) return; + let query = this.kysely>() + .updateTable('day_assignments') + .set((eb) => { + let index = eb.case().when('id', '=', first[0]).then(first[1]); + for (const [id, orderIndex] of rest) index = index.when('id', '=', id).then(orderIndex); + return { order_index: index.else(eb.ref('order_index')).end() }; + }) + .where('id', 'in', [...last.keys()]); + if (day_id !== undefined) query = query.where('day_id', '=', day_id); + await query.execute(); + } + /** AS13 (`moveAssignment`'s source-day read) — `SELECT day_id FROM day_assignments WHERE id = ?`. */ async getDayId(id: number): Promise { const platform = this.getEntityManager().getPlatform(); diff --git a/server/src/db/repositories/JourneyEntries.repository.ts b/server/src/db/repositories/JourneyEntries.repository.ts index b99f27934..ed2ab956a 100644 --- a/server/src/db/repositories/JourneyEntries.repository.ts +++ b/server/src/db/repositories/JourneyEntries.repository.ts @@ -115,6 +115,22 @@ export class JourneyEntriesRepository extends TrekRepository { .execute('all', false); } + /** + * JG16/JG73 — the titles of the trips a journey's entries came from, keyed by + * trip id, in one read (the journey reads used to ask once per entry). A trip + * that is gone has no entry, so its entries read a null name as before. + */ + async sourceTripTitles(journeyId: number): Promise> { + const rows = await this.kysely>() + .selectFrom('journey_entries as je') + .innerJoin('trips as t', 't.id', 'je.source_trip_id') + .select(['t.id', 't.title']) + .distinct() + .where('je.journey_id', '=', journeyId) + .execute(); + return new Map(rows.map((r) => [r.id, r.title ?? null])); + } + /** * JS13 (Plan 4 Task 8b relocation) — the public `getPublicJourney` route's * entry list: `SELECT je.* FROM journey_entries je WHERE je.journey_id=? diff --git a/server/src/db/repositories/Reservations.repository.ts b/server/src/db/repositories/Reservations.repository.ts index c1b8abaff..fcad8bd5e 100644 --- a/server/src/db/repositories/Reservations.repository.ts +++ b/server/src/db/repositories/Reservations.repository.ts @@ -171,19 +171,16 @@ export interface CalendarReservationRow extends ReservationAllColumnsRow { type CalendarReservationKyselyDB = Pick; /** - * CL4 (`CalendarService.buildTripCalendar`'s per-day assignment read, - * previously run once per dated day) — `SELECT da.*, p.name as place_name, - * p.address as place_address, p.lat as place_lat, p.lng as place_lng, + * CL4 (`CalendarService.buildTripCalendar`'s assignment read, for every dated + * day at once) — `SELECT da.*, p.name as place_name, p.address as + * place_address, p.lat as place_lat, p.lng as place_lng, * COALESCE(da.assignment_time, p.place_time) as effective_time, * COALESCE(da.assignment_end_time, p.end_time) as effective_end_time FROM - * day_assignments da JOIN places p ON da.place_id = p.id WHERE da.day_id = - * ? AND da.accommodation_id IS NULL ORDER BY da.order_index ASC, - * da.created_at ASC` — the booked-night stop (its own accommodation-linked - * `day_assignments` row) is excluded, matching the legacy comment ("that - * stop is the booking … reading it here as well would put the hotel on the - * day a second time"). Kept per-day (the caller's own loop), not batched — - * result-identical either way per the inventory's own note; per-day is the - * smaller diff against the legacy statement shape. + * day_assignments da JOIN places p ON da.place_id = p.id WHERE da.day_id IN + * (...) AND da.accommodation_id IS NULL ORDER BY da.order_index, da.created_at, + * da.id` — the booked-night stop (its own accommodation-linked row) is + * excluded: it is the booking, and reading it here would put the hotel on the + * day a second time. Each day's rows keep the per-day order; the id settles a tie. */ export interface CalendarStopRow extends DayAssignmentRow { place_name: string; @@ -994,8 +991,9 @@ export class ReservationsRepository extends TrekRepository { return rows as CalendarReservationRow[]; } - /** CL4 — see {@link CalendarStopRow}'s docstring. */ - async listCalendarStops(day_id: number): Promise { + /** CL4 — see {@link CalendarStopRow}'s docstring. No days, no query. */ + async listCalendarStopsForDays(day_ids: number[]): Promise { + if (day_ids.length === 0) return []; const rows = await this.kysely() .selectFrom('day_assignments as da') .innerJoin('places as p', 'p.id', 'da.place_id') @@ -1008,10 +1006,11 @@ export class ReservationsRepository extends TrekRepository { eb.fn.coalesce('da.assignment_time', 'p.place_time').as('effective_time'), eb.fn.coalesce('da.assignment_end_time', 'p.end_time').as('effective_end_time'), ]) - .where('da.day_id', '=', day_id) + .where('da.day_id', 'in', day_ids) .where('da.accommodation_id', 'is', null) .orderBy('da.order_index', 'asc') .orderBy('da.created_at', 'asc') + .orderBy('da.id', 'asc') .execute(); return rows as CalendarStopRow[]; } diff --git a/server/src/nest/assignments/assignments.service.ts b/server/src/nest/assignments/assignments.service.ts index 64061d4f5..e923644a7 100644 --- a/server/src/nest/assignments/assignments.service.ts +++ b/server/src/nest/assignments/assignments.service.ts @@ -325,17 +325,13 @@ export class AssignmentsService { } /** - * AS11 — `DayAssignmentsRepository.setOrderIndex`, day-scoped, one row per - * id, sequentially and in the legacy's own order (not `Promise.all` — the - * program's transaction-ordering rule). + * AS11 — `DayAssignmentsRepository.setOrderIndexes`, day-scoped: every id + * gets its position in one statement, with the result the per-row writes in + * the given order had (an id listed twice keeps its last position). */ async reorderAssignments(dayId: string | number, orderedIds: number[]): Promise { const dayIdNum = toRowId(dayId)!; - await this.uow.transactional(async () => { - for (const [index, id] of orderedIds.entries()) { - await this.dayAssignmentsRepo.setOrderIndex(id, dayIdNum, index); - } - }); + await this.dayAssignmentsRepo.setOrderIndexes(orderedIds.map((id, index) => ({ id, order_index: index })), dayIdNum); } /** @@ -457,11 +453,10 @@ export class AssignmentsService { // with their gaps would put the day notes and bookings that sort between stops in // one place for the writer, who reads the day back, and in another for everyone // else. Only a stop whose key changes is written. AS19 — no day scoping, - // unlike AS11's `reorderAssignments` (`setOrderIndex`'s `day_id` left - // `undefined`), sequentially and in the legacy's own order. - for (const [i, row] of sorted.entries()) { - if (row.order_index !== i) await this.dayAssignmentsRepo.setOrderIndex(row.id, undefined, i); - } + // unlike AS11's `reorderAssignments` (`setOrderIndexes`'s `day_id` left + // `undefined`), in one statement. + const moved = sorted.flatMap((row, i) => (row.order_index !== i ? [{ id: row.id, order_index: i }] : [])); + await this.dayAssignmentsRepo.setOrderIndexes(moved); return { dayId, orderedIds: sorted.map(row => row.id), viasMoved: await this.reanchorVias(dayId, rows, sorted) }; } diff --git a/server/src/nest/calendar/calendar.service.ts b/server/src/nest/calendar/calendar.service.ts index 471ea803a..6532430bc 100644 --- a/server/src/nest/calendar/calendar.service.ts +++ b/server/src/nest/calendar/calendar.service.ts @@ -127,6 +127,17 @@ interface ZoneTransition { } const DAY_MS = 86_400_000; + +/** Rows of several days, by day id; each day keeps the order the rows came in. */ +function groupByDay(rows: T[]): Map { + const byDay = new Map(); + for (const row of rows) { + const list = byDay.get(row.day_id); + if (list) list.push(row); + else byDay.set(row.day_id, [row]); + } + return byDay; +} /** Ten years at most: a span longer than that is a typo, not a trip. */ const MAX_SCAN_DAYS = 3660; @@ -318,19 +329,20 @@ export class CalendarService { events.push(ev); } - // Days with assignments and notes + // Days with assignments and notes, read for all dated days at once. const days = await this.daysRepo.listByTrip(trip.id); + const datedDayIds = days.filter((d) => d.date).map((d) => d.id); + // A booked night puts a stop of its own on its check-in day, so the route + // can reach the hotel. That stop is the booking, not a place the traveller + // planned to visit, and the booking already comes through below as the + // stay block or its check-in and check-out markers. Read here as well it + // would put the hotel on the day a second time. + const stopsByDay = groupByDay(await this.reservationsRepo.listCalendarStopsForDays(datedDayIds)); + const notesByDay = groupByDay(await this.dayNotesRepo.listByDayIds(datedDayIds)); for (const day of days) { if (!day.date) continue; - - // A booked night puts a stop of its own on its check-in day, so the route - // can reach the hotel. That stop is the booking, not a place the traveller - // planned to visit, and the booking already comes through below as the - // stay block or its check-in and check-out markers. Read here as well it - // would put the hotel on the day a second time. - const assignments = await this.reservationsRepo.listCalendarStops(day.id); - - const notes = await this.dayNotesRepo.listByDayIds([day.id]); + const assignments = stopsByDay.get(day.id) ?? []; + const notes = notesByDay.get(day.id) ?? []; const timed = assignments.filter(a => a.effective_time); const untimed = assignments.filter(a => !a.effective_time); diff --git a/server/src/nest/journey/journey-domain.service.ts b/server/src/nest/journey/journey-domain.service.ts index 8ee01f715..d5cacea67 100644 --- a/server/src/nest/journey/journey-domain.service.ts +++ b/server/src/nest/journey/journey-domain.service.ts @@ -115,8 +115,7 @@ export class JourneyDomainService { // R7 — AP1's `this.db.canAccessTrip(...)` becomes a direct call to the // same repository `DatabaseService.canAccessTrip` already delegates to // (Task 0's confirmation: `canAccessTrip` IS `findAccessible`, not - // merely equivalent to it). Also reused by `getJourneyFull`'s per-entry - // `source_trip_name` lookup (JG16, via the already-existing `getTitle`). + // merely equivalent to it). @InjectRepository(Trips) private readonly tripsRepo: TripsRepository, // Plan 3g Task 2 (Part B) — the full photos surface (JG19/JG87-116), // finishing the two sites Task 1 left raw (JG15/JG19) because their @@ -259,16 +258,13 @@ export class JourneyDomainService { // .galleryRead`). const gallery = await this.photosRepo.galleryRead(journeyId); - const enrichedEntries = await Promise.all( - entries.map(async (e) => ({ - ...decodeEntryRow(e), - photos: photosByEntry[e.id] || [], - // JG16 — reuses `TripsRepository.getTitle` (already public, 3c; same - // statement text as this site's legacy `SELECT title FROM trips - // WHERE id = ?`) rather than a new method. - source_trip_name: e.source_trip_id ? await this.tripsRepo.getTitle(e.source_trip_id) : null, - })), - ); + // JG16 — the source trips' titles in one read instead of one per entry. + const tripTitles = await this.entriesRepo.sourceTripTitles(journeyId); + const enrichedEntries = entries.map((e) => ({ + ...decodeEntryRow(e), + photos: photosByEntry[e.id] || [], + source_trip_name: e.source_trip_id ? (tripTitles.get(e.source_trip_id) ?? null) : null, + })); // linked trips (JG17) const trips = await this.journeyTripsRepo.listForJourney(journeyId); @@ -1174,14 +1170,13 @@ export class JourneyDomainService { (photosByEntry[p.entry_id] ||= []).push(p); } - // JG73 — same N+1-by-design shape as JG16, reusing `TripsRepository.getTitle`. - return await Promise.all( - entries.map(async (e) => ({ - ...decodeEntryRow(e), - photos: photosByEntry[e.id] || [], - source_trip_name: e.source_trip_id ? await this.tripsRepo.getTitle(e.source_trip_id) : null, - })), - ); + // JG73 — the source trips' titles in one read, as JG16. + const tripTitles = await this.entriesRepo.sourceTripTitles(journeyId); + return entries.map((e) => ({ + ...decodeEntryRow(e), + photos: photosByEntry[e.id] || [], + source_trip_name: e.source_trip_id ? (tripTitles.get(e.source_trip_id) ?? null) : null, + })); } async createEntry( diff --git a/server/tests/unit/db/repositories/DayAssignments.repository.test.ts b/server/tests/unit/db/repositories/DayAssignments.repository.test.ts index 0cc01427d..714b83653 100644 --- a/server/tests/unit/db/repositories/DayAssignments.repository.test.ts +++ b/server/tests/unit/db/repositories/DayAssignments.repository.test.ts @@ -407,6 +407,32 @@ describe('DayAssignmentsRepository — AS10/AS11/AS13/AS14/AS19 (delete / order expect(await orderOf(a.id)).toBe(3); }); + it('ASSIGNREPO-009b (setOrderIndexes): one statement leaves what the per-row writes left, day scope included', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const otherDay = createDay(testDb, trip.id); + const place = createPlace(testDb, trip.id); + const a = createDayAssignment(testDb, day.id, place.id, { order_index: 0 }); + const b = createDayAssignment(testDb, day.id, place.id, { order_index: 1 }); + const c = createDayAssignment(testDb, day.id, place.id, { order_index: 2 }); + const foreign = createDayAssignment(testDb, otherDay.id, place.id, { order_index: 5 }); + + // b twice: the later index wins, as two writes in a row would leave it; foreign is on another day. + await withRequestContext(t.orm, () => + assignments.setOrderIndexes([{ id: c.id, order_index: 0 }, { id: b.id, order_index: 9 }, { id: a.id, order_index: 2 }, { id: b.id, order_index: 1 }, { id: foreign.id, order_index: 0 }], day.id), + ); + expect([await orderOf(a.id), await orderOf(b.id), await orderOf(c.id), await orderOf(foreign.id)]).toEqual([2, 1, 0, 5]); + + // Unscoped: by id alone, and only the listed rows. + await withRequestContext(t.orm, () => assignments.setOrderIndexes([{ id: foreign.id, order_index: 1 }])); + expect([await orderOf(foreign.id), await orderOf(a.id)]).toEqual([1, 2]); + + // Nothing to write is no statement at all. + await withRequestContext(t.orm, () => assignments.setOrderIndexes([], day.id)); + expect(await orderOf(c.id)).toBe(0); + }); + it('ASSIGNREPO-010 (AS13, getDayId): the assignment\'s day_id, undefined for a missing id', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); diff --git a/server/tests/unit/db/repositories/Reservations.repository.test.ts b/server/tests/unit/db/repositories/Reservations.repository.test.ts index 4854b61ee..3fca19c70 100644 --- a/server/tests/unit/db/repositories/Reservations.repository.test.ts +++ b/server/tests/unit/db/repositories/Reservations.repository.test.ts @@ -185,7 +185,7 @@ describe('ReservationsRepository — CL2 (listForCalendar)', () => { }); }); -describe('ReservationsRepository — CL4 (listCalendarStops)', () => { +describe('ReservationsRepository — CL4 (listCalendarStopsForDays)', () => { it('CAL-CL4-001: matches the legacy per-day statement, excluding a booked-night stop', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id, { start_date: '2026-06-01', end_date: '2026-06-03' }); @@ -197,7 +197,7 @@ describe('ReservationsRepository — CL4 (listCalendarStops)', () => { const accId = await insertRow(await orm(), DayAccommodations, { trip: trip.id, place: hotelPlace.id, startDay: day.id, endDay: day.id }); await insertRow(await orm(), DayAssignments, { day: day.id, place: hotelPlace.id, order_index: 1, accommodation_id: accId }); - const typed = await reservationsRepo.listCalendarStops(day.id); + const typed = await reservationsRepo.listCalendarStopsForDays([day.id]); // test-sql-allow: the legacy statement is the oracle the repository read is held to. const legacy = testDb.prepare(` SELECT da.*, p.name as place_name, p.address as place_address, diff --git a/server/tests/unit/nest/calendar.service.test.ts b/server/tests/unit/nest/calendar.service.test.ts index 4c8194d89..2964f6a4f 100644 --- a/server/tests/unit/nest/calendar.service.test.ts +++ b/server/tests/unit/nest/calendar.service.test.ts @@ -488,6 +488,24 @@ describe('exportICS', () => { expect(ics).toContain('LOCATION:Rue de Rivoli'); }); + it('CAL-016b: read in one go for all days, each day keeps its own stops and notes in their order', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { title: 'Two Days' }); + const first = createDay(testDb, trip.id, { date: '2025-06-02', title: 'First' }); + const second = createDay(testDb, trip.id, { date: '2025-06-03', title: 'Second' }); + const place = (name: string) => createPlace(testDb, trip.id, { name }); + createDayAssignment(testDb, first.id, place('Later').id, { order_index: 1 }); + createDayAssignment(testDb, first.id, place('Sooner').id, { order_index: 0 }); + createDayAssignment(testDb, second.id, place('Elsewhere').id, { order_index: 0 }); + createDayNote(testDb, first.id, trip.id, { text: 'second note', sort_order: 1 }); + createDayNote(testDb, first.id, trip.id, { text: 'first note', sort_order: 0 }); + + const ics = (await svc.exportICS(trip.id)).ics.replace(/\r\n /g, ''); + + expect(ics).toContain('SUMMARY:First\r\nDESCRIPTION:• Sooner\\n• Later\\n\\nNotes:\\n• first note\\n• second note\r\n'); + expect(ics).toContain('SUMMARY:Second\r\nDESCRIPTION:• Elsewhere\r\n'); + }); + it('CAL-016: a day without a date is skipped and a titled day uses its own title', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id, { title: 'Mixed Days' }); diff --git a/server/tests/unit/nest/journey-domain.service.test.ts b/server/tests/unit/nest/journey-domain.service.test.ts index c555b20c7..8f00eed8b 100644 --- a/server/tests/unit/nest/journey-domain.service.test.ts +++ b/server/tests/unit/nest/journey-domain.service.test.ts @@ -2729,6 +2729,23 @@ describe('entry enrichment', () => { expect((entry as any).source_trip_name).toBe('Japan 2026'); }); + it('JOURNEY-SVC-ENRICH-001b: entries from two trips each carry their own trip name, in the full read and the list alike', async () => { + const { user } = createUser(testDb); + const a = tripWithPlace(user.id).trip; + const b = tripWithPlace(user.id).trip; + await updateRows(await orm(), Trips, { id: a.id }, { title: 'Alps' }); + await updateRows(await orm(), Trips, { id: b.id }, { title: 'Baltic' }); + const journey = await svc.createJourney(user.id, { title: 'J', trip_ids: [a.id, b.id] }); + await svc.createEntry(journey.id, user.id, { entry_date: '2026-05-01', title: 'Solo' }); + + const names = (rows: Array<{ source_trip_id?: number | null; source_trip_name?: string | null }>) => + rows.map((e) => [e.source_trip_id ?? null, e.source_trip_name]).sort((x, y) => Number(x[0]) - Number(y[0])); + const listed = names((await svc.listEntries(journey.id, user.id))!); + expect(listed).toEqual([[null, null], [a.id, 'Alps'], [b.id, 'Baltic']].sort((x, y) => Number(x[0]) - Number(y[0]))); + const full = (await svc.getJourneyFull(journey.id, user.id))! as unknown as { entries: Array<{ source_trip_id?: number | null; source_trip_name?: string | null }> }; + expect(names(full.entries)).toEqual(listed); + }); + it('JOURNEY-SVC-ENRICH-002: an entry with no trip and no tags gets [] and null, not undefined', async () => { const { user } = createUser(testDb); const journey = await svc.createJourney(user.id, { title: 'J' });