feat(server): adopt zod DTOs for the reservation body contracts

ReservationsController's 4 grandfathered @Body() params now validate through
the global ZodValidationPipe (createZodDto over the existing @trek/shared
schemas); their allow-list entries are removed (the boot gate rejects stale
entries). The bespoke 400 bodies (Title is required / positions must be an
array / user_ids must be an array) become the pipe's field envelope, and
non-numeric traveler ids now 400 instead of being silently dropped.

reservationPositionsRequestSchema is loosened to the real wire contract:
day_plan_position is optional (the legacy route never validated position
items and an absent value binds NULL) — pinned by RESV-006.

AccommodationsController stays grandfathered until the dayService migration.
This commit is contained in:
jubnl
2026-07-27 19:45:41 +02:00
parent fbc773386f
commit c27fc99220
7 changed files with 108 additions and 81 deletions
@@ -119,10 +119,6 @@ export const BODY_CONTRACT_ALLOW_LIST: string[] = [
'PluginsController.setEgressHosts',
'PluginsController.uninstall',
'PluginsController.updateConfig',
'ReservationsController.create',
'ReservationsController.update',
'ReservationsController.updatePositions',
'ReservationsController.updateTravelers',
'SynologyMemoriesController.putSettings',
'SynologyMemoriesController.search',
'SynologyMemoriesController.test',
@@ -15,6 +15,12 @@ import { ReservationsService } from './reservations.service';
import { JwtAuthGuard } from '../auth/jwt-auth.guard';
import { CurrentUser } from '../auth/current-user.decorator';
import { pushReservationToAirtrail } from '../../services/airtrail/airtrailSync';
import {
ReservationCreateDto,
ReservationUpdateDto,
ReservationPositionsDto,
ReservationTravelersDto,
} from './reservations.dto';
type ReservationBody = Record<string, unknown> & {
title?: string;
@@ -27,9 +33,11 @@ type ReservationBody = Record<string, unknown> & {
*
* Byte-identical to the legacy Express route (server/src/routes/reservations.ts):
* trip access (404), 'reservation_edit' permission (403), create 201 / rest 200,
* the bespoke 400/404 bodies, the accommodation + budget side effects, the
* booking notifications, and all WebSocket broadcasts with the forwarded
* X-Socket-Id. /positions is declared before /:id so it wins over the param.
* the bespoke 404 bodies, the accommodation + budget side effects, the booking
* notifications, and all WebSocket broadcasts with the forwarded X-Socket-Id.
* Invalid bodies (missing title, non-array positions/user_ids) now 400 through
* the global ZodValidationPipe envelope instead of the bespoke strings.
* /positions is declared before /:id so it wins over the param.
*/
@Controller('api/trips/:tripId/reservations')
@UseGuards(JwtAuthGuard)
@@ -60,14 +68,12 @@ export class ReservationsController {
create(
@CurrentUser() user: User,
@Param('tripId') tripId: string,
@Body() body: ReservationBody,
@Body() rawBody: ReservationCreateDto,
@Headers('x-socket-id') socketId?: string,
) {
const body = rawBody as ReservationBody & { title: string };
const trip = this.requireTrip(tripId, user);
this.requireEdit(trip, user);
if (!body.title) {
throw new HttpException({ error: 'Title is required' }, 400);
}
const { reservation, accommodationCreated } = this.reservations.create(tripId, body as never);
if (accommodationCreated) {
this.reservations.broadcast(tripId, 'accommodation:created', {}, socketId);
@@ -82,15 +88,14 @@ export class ReservationsController {
updatePositions(
@CurrentUser() user: User,
@Param('tripId') tripId: string,
@Body() body: { positions?: unknown; day_id?: unknown },
@Body() body: ReservationPositionsDto,
@Headers('x-socket-id') socketId?: string,
) {
const trip = this.requireTrip(tripId, user);
this.requireEdit(trip, user);
if (!Array.isArray(body.positions)) {
throw new HttpException({ error: 'positions must be an array' }, 400);
}
this.reservations.updatePositions(tripId, body.positions, body.day_id);
// The legacy signature declares day_plan_position required, but the wire
// contract tolerates absent values (bind NULL) — see the shared schema.
this.reservations.updatePositions(tripId, body.positions as { id: number; day_plan_position: number }[], body.day_id);
this.reservations.broadcast(tripId, 'reservation:positions', { positions: body.positions, day_id: body.day_id }, socketId);
return { success: true };
}
@@ -100,9 +105,10 @@ export class ReservationsController {
@CurrentUser() user: User,
@Param('tripId') tripId: string,
@Param('id') id: string,
@Body() body: ReservationBody,
@Body() rawBody: ReservationUpdateDto,
@Headers('x-socket-id') socketId?: string,
) {
const body = rawBody as ReservationBody;
const trip = this.requireTrip(tripId, user);
this.requireEdit(trip, user);
const current = this.reservations.getReservation(id, tripId);
@@ -130,15 +136,12 @@ export class ReservationsController {
@CurrentUser() user: User,
@Param('tripId') tripId: string,
@Param('id') id: string,
@Body('user_ids') userIds: unknown,
@Body() body: ReservationTravelersDto,
@Headers('x-socket-id') socketId?: string,
) {
const trip = this.requireTrip(tripId, user);
this.requireEdit(trip, user);
if (!Array.isArray(userIds)) {
throw new HttpException({ error: 'user_ids must be an array' }, 400);
}
const result = this.reservations.setTravelers(id, tripId, userIds as number[]);
const result = this.reservations.setTravelers(id, tripId, body.user_ids);
if (!result) {
throw new HttpException({ error: 'Reservation not found' }, 404);
}
@@ -0,0 +1,21 @@
import { createZodDto } from 'nestjs-zod';
import {
reservationCreateRequestSchema,
reservationUpdateRequestSchema,
reservationPositionsRequestSchema,
reservationTravelersRequestSchema,
} from '@trek/shared';
/**
* Server-side createZodDto wrappers over the @trek/shared reservation
* contracts. The global ZodValidationPipe (APP_PIPE in app.module.ts)
* validates any @Body() parameter typed with one of these classes by
* metatype — the Zod schemas in shared/ remain the single source of truth
* for the wire contract. Create/update stay open records on purpose: the
* legacy route accepted the whole Reservation shape as an arbitrary
* partial, and parity forbids tightening it here.
*/
export class ReservationCreateDto extends createZodDto(reservationCreateRequestSchema) {}
export class ReservationUpdateDto extends createZodDto(reservationUpdateRequestSchema) {}
export class ReservationPositionsDto extends createZodDto(reservationPositionsRequestSchema) {}
export class ReservationTravelersDto extends createZodDto(reservationTravelersRequestSchema) {}
+49 -45
View File
@@ -1,12 +1,14 @@
/**
* Reservations + accommodations module e2e — exercises both migrated mounts
* through the real JwtAuthGuard against a temp SQLite db. The reservation/day/
* budget services, the permission check, canAccessTrip and the WebSocket
* broadcast are mocked.
* through the real JwtAuthGuard against a temp SQLite db. Reservation SQL runs
* for real (ReservationsService is DI-native, no mock — the temp db carries the
* full schema via createTables + runMigrations); the day/budget services, the
* permission check and the WebSocket broadcast stay mocked.
*/
import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter';
import { ZodValidationPipe } from '../../src/nest/common/zod-validation.pipe';
import { ReservationsModule } from '../../src/nest/reservations/reservations.module';
import { seedUser, sessionCookie } from './harness';
import { sessionCookie } from './harness';
import { DatabaseModule } from '../../src/nest/database/database.module';
import { Test } from '@nestjs/testing';
@@ -20,9 +22,6 @@ const { db } = vi.hoisted(() => {
const Database = require('better-sqlite3');
const tmp = new Database(':memory:');
tmp.exec('PRAGMA journal_mode = WAL');
tmp.exec(`CREATE TABLE users (id INTEGER PRIMARY KEY AUTOINCREMENT, username TEXT NOT NULL,
email TEXT NOT NULL UNIQUE, role TEXT NOT NULL DEFAULT 'user', password_version INTEGER NOT NULL DEFAULT 0);`);
tmp.exec('CREATE TABLE trips (id INTEGER PRIMARY KEY AUTOINCREMENT, title TEXT);');
return { db: tmp };
});
@@ -36,23 +35,13 @@ vi.mock('../../src/db/database', () => ({
reinitialize: () => {},
}));
vi.mock('../../src/websocket', () => ({ broadcast: vi.fn() }));
vi.mock('../../src/services/notificationService', () => ({ send: vi.fn().mockResolvedValue(undefined) }));
const { notificationSend } = vi.hoisted(() => ({ notificationSend: vi.fn().mockResolvedValue(undefined) }));
vi.mock('../../src/services/notificationService', () => ({ send: notificationSend }));
const { checkPermission } = vi.hoisted(() => ({ checkPermission: vi.fn() }));
vi.mock('../../src/services/permissions', () => ({ checkPermission }));
const { resv, budget, day } = vi.hoisted(() => ({
resv: {
verifyTripAccess: vi.fn(),
listReservations: vi.fn(),
createReservation: vi.fn(),
updatePositions: vi.fn(),
getReservation: vi.fn(),
updateReservation: vi.fn(),
deleteReservation: vi.fn(),
getUpcomingReservations: vi.fn(),
notifyBookingChange: vi.fn(),
},
const { budget, day } = vi.hoisted(() => ({
budget: {
createBudgetItem: vi.fn(),
updateBudgetItem: vi.fn(),
@@ -68,39 +57,47 @@ const { resv, budget, day } = vi.hoisted(() => ({
deleteAccommodation: vi.fn(),
},
}));
vi.mock('../../src/services/reservationService', () => resv);
vi.mock('../../src/services/budgetService', () => budget);
vi.mock('../../src/services/dayService', () => day);
describe('Reservations + accommodations e2e (real auth guard + temp SQLite)', () => {
import { createTables } from '../../src/db/schema';
import { runMigrations } from '../../src/db/migrations';
describe('Reservations + accommodations e2e (real auth guard + temp SQLite, real reservation SQL)', () => {
let server: Server;
let app: Awaited<ReturnType<typeof build>>;
let tripId: number;
async function build() {
const moduleRef = await Test.createTestingModule({ imports: [DatabaseModule, ReservationsModule] }).compile();
const nest = moduleRef.createNestApplication();
nest.use(cookieParser());
nest.useGlobalFilters(new TrekExceptionFilter());
nest.useGlobalPipes(new ZodValidationPipe());
await nest.init();
return nest;
}
beforeAll(async () => {
seedUser(db as never, { id: 1 });
createTables(db);
runMigrations(db);
// The temp db carries the real schema (password_hash NOT NULL), so seed the
// auth user directly instead of via the trimmed-DDL seedUser helper.
db.prepare(
"INSERT INTO users (id, username, email, password_hash, role, password_version) VALUES (1, 'e2e-user', 'e2e@example.test', 'x', 'user', 0)",
).run();
tripId = Number(db.prepare("INSERT INTO trips (user_id, title) VALUES (1, 'E2E Trip')").run().lastInsertRowid);
app = await build();
server = app.getHttpServer();
resv.listReservations.mockReturnValue([{ id: 1, title: 'Hotel' }]);
resv.createReservation.mockReturnValue({ reservation: { id: 9, title: 'Hotel' }, accommodationCreated: false });
day.listAccommodations.mockReturnValue([{ id: 1 }]);
day.validateAccommodationRefs.mockReturnValue([]);
day.createAccommodation.mockReturnValue({ id: 9 });
resv.getUpcomingReservations.mockReturnValue([{ id: 1, trip_id: 5, title: 'Flight' }]);
});
beforeEach(() => {
resv.verifyTripAccess.mockReturnValue({ id: 5, user_id: 1 });
canAccessTrip.mockReturnValue({ id: 5, user_id: 1 });
canAccessTrip.mockImplementation((id: unknown) => db.prepare('SELECT * FROM trips WHERE id = ?').get(id));
checkPermission.mockReturnValue(true);
notificationSend.mockClear();
});
afterAll(async () => {
@@ -108,13 +105,15 @@ describe('Reservations + accommodations e2e (real auth guard + temp SQLite)', ()
});
it('401 without a cookie (reservations)', async () => {
expect((await request(server).get('/api/trips/5/reservations')).status).toBe(401);
expect((await request(server).get(`/api/trips/${tripId}/reservations`)).status).toBe(401);
});
it('200 list reservations', async () => {
const res = await request(server).get('/api/trips/5/reservations').set('Cookie', sessionCookie(1));
it('200 list reservations (real SQL, joins attached)', async () => {
const rid = db.prepare("INSERT INTO reservations (trip_id, title, type) VALUES (?, 'Hotel', 'hotel')").run(tripId).lastInsertRowid;
const res = await request(server).get(`/api/trips/${tripId}/reservations`).set('Cookie', sessionCookie(1));
expect(res.status).toBe(200);
expect(res.body).toEqual({ reservations: [{ id: 1, title: 'Hotel' }] });
const row = res.body.reservations.find((r: { id: number }) => r.id === Number(rid));
expect(row).toMatchObject({ title: 'Hotel', type: 'hotel', endpoints: [], travelers: [] });
});
it('401 without a cookie (upcoming feed)', async () => {
@@ -122,37 +121,42 @@ describe('Reservations + accommodations e2e (real auth guard + temp SQLite)', ()
});
it('200 cross-trip upcoming reservations feed', async () => {
db.prepare("INSERT INTO reservations (trip_id, title, type, reservation_time) VALUES (?, 'Flight', 'flight', '2999-01-01T10:00:00')").run(tripId);
const res = await request(server).get('/api/reservations/upcoming').set('Cookie', sessionCookie(1));
expect(res.status).toBe(200);
expect(res.body).toEqual({ reservations: [{ id: 1, trip_id: 5, title: 'Flight' }] });
expect(res.body.reservations.map((r: { title: string }) => r.title)).toContain('Flight');
});
it('404 when trip not accessible (reservations)', async () => {
resv.verifyTripAccess.mockReturnValue(undefined);
const res = await request(server).get('/api/trips/5/reservations').set('Cookie', sessionCookie(1));
canAccessTrip.mockReturnValue(undefined);
const res = await request(server).get(`/api/trips/${tripId}/reservations`).set('Cookie', sessionCookie(1));
expect(res.status).toBe(404);
expect(res.body).toEqual({ error: 'Trip not found' });
});
it('201 create reservation, 400 without title', async () => {
it('201 create reservation (real insert + booking notification), 400 without title', async () => {
const ok = await request(server)
.post('/api/trips/5/reservations')
.post(`/api/trips/${tripId}/reservations`)
.set('Cookie', sessionCookie(1))
.send({ title: 'Hotel' });
expect(ok.status).toBe(201);
expect(ok.body).toEqual({ reservation: { id: 9, title: 'Hotel' } });
expect(resv.notifyBookingChange).toHaveBeenCalledWith('5', 1, 'Hotel', '');
const bad = await request(server).post('/api/trips/5/reservations').set('Cookie', sessionCookie(1)).send({});
expect(ok.body.reservation).toMatchObject({ title: 'Hotel', type: 'other', status: 'pending' });
expect(db.prepare('SELECT title FROM reservations WHERE id = ?').get(ok.body.reservation.id)).toEqual({ title: 'Hotel' });
// The fire-and-forget booking notification reaches the (mocked) notification service.
await vi.waitFor(() => expect(notificationSend).toHaveBeenCalled());
expect(notificationSend).toHaveBeenCalledWith(expect.objectContaining({ event: 'booking_change', actorId: 1 }));
const bad = await request(server).post(`/api/trips/${tripId}/reservations`).set('Cookie', sessionCookie(1)).send({});
expect(bad.status).toBe(400);
expect(bad.body).toEqual({ error: 'Title is required' });
expect(bad.body.error).toContain('title');
});
it('200 list accommodations + 201 create', async () => {
const list = await request(server).get('/api/trips/5/accommodations').set('Cookie', sessionCookie(1));
const list = await request(server).get(`/api/trips/${tripId}/accommodations`).set('Cookie', sessionCookie(1));
expect(list.status).toBe(200);
expect(list.body).toEqual({ accommodations: [{ id: 1 }] });
const create = await request(server)
.post('/api/trips/5/accommodations')
.post(`/api/trips/${tripId}/accommodations`)
.set('Cookie', sessionCookie(1))
.send({ place_id: 2, start_day_id: 10, end_day_id: 11 });
expect(create.status).toBe(201);
@@ -161,14 +165,14 @@ describe('Reservations + accommodations e2e (real auth guard + temp SQLite)', ()
it('404 when trip not accessible (accommodations)', async () => {
canAccessTrip.mockReturnValue(undefined);
const res = await request(server).get('/api/trips/5/accommodations').set('Cookie', sessionCookie(1));
const res = await request(server).get(`/api/trips/${tripId}/accommodations`).set('Cookie', sessionCookie(1));
expect(res.status).toBe(404);
expect(res.body).toEqual({ error: 'Trip not found' });
});
it('400 accommodation create without refs', async () => {
const res = await request(server)
.post('/api/trips/5/accommodations')
.post(`/api/trips/${tripId}/accommodations`)
.set('Cookie', sessionCookie(1))
.send({ place_id: 2 });
expect(res.status).toBe(400);
@@ -45,9 +45,8 @@ describe('ReservationsController (parity with the legacy /api/trips/:tripId/rese
expect(thrown(() => new ReservationsController(svc).create(user, '5', { title: 'Hotel' }))).toEqual({ status: 403, body: { error: 'No permission' } });
});
it('400 without a title', () => {
expect(thrown(() => new ReservationsController(makeService()).create(user, '5', {}))).toEqual({ status: 400, body: { error: 'Title is required' } });
});
// The bespoke 'Title is required' 400 moved to the global ZodValidationPipe
// (ReservationCreateDto) — covered by the e2e suite.
it('creates, runs budget sync, broadcasts accommodation + reservation, notifies', () => {
const create = vi.fn().mockReturnValue({ reservation: { id: 9 }, accommodationCreated: true });
@@ -63,9 +62,8 @@ describe('ReservationsController (parity with the legacy /api/trips/:tripId/rese
});
describe('PUT /positions', () => {
it('400 when positions is not an array', () => {
expect(thrown(() => new ReservationsController(makeService()).updatePositions(user, '5', { positions: 'no' }))).toEqual({ status: 400, body: { error: 'positions must be an array' } });
});
// The 'positions must be an array' 400 moved to the global
// ZodValidationPipe (ReservationPositionsDto).
it('updates positions and broadcasts', () => {
const updatePositions = vi.fn(); const broadcast = vi.fn();
@@ -96,13 +94,12 @@ describe('ReservationsController (parity with the legacy /api/trips/:tripId/rese
});
describe('PUT /:id/travelers', () => {
it('400 when user_ids is not an array', () => {
expect(thrown(() => new ReservationsController(makeService()).updateTravelers(user, '5', '9', 'nope'))).toEqual({ status: 400, body: { error: 'user_ids must be an array' } });
});
// The 'user_ids must be an array' 400 moved to the global
// ZodValidationPipe (ReservationTravelersDto).
it('404 when the reservation is off-trip / missing', () => {
const svc = makeService({ setTravelers: vi.fn().mockReturnValue(null) } as Partial<ReservationsService>);
expect(thrown(() => new ReservationsController(svc).updateTravelers(user, '5', '9', [1]))).toEqual({ status: 404, body: { error: 'Reservation not found' } });
expect(thrown(() => new ReservationsController(svc).updateTravelers(user, '5', '9', { user_ids: [1] }))).toEqual({ status: 404, body: { error: 'Reservation not found' } });
});
it('assigns travelers, broadcasts, and returns { travelers, reservation }', () => {
@@ -111,7 +108,7 @@ describe('ReservationsController (parity with the legacy /api/trips/:tripId/rese
const setTravelers = vi.fn().mockReturnValue({ travelers, reservation });
const broadcast = vi.fn();
const svc = makeService({ setTravelers, broadcast } as Partial<ReservationsService>);
expect(new ReservationsController(svc).updateTravelers(user, '5', '9', [2], 'sock')).toEqual({ travelers, reservation });
expect(new ReservationsController(svc).updateTravelers(user, '5', '9', { user_ids: [2] }, 'sock')).toEqual({ travelers, reservation });
expect(setTravelers).toHaveBeenCalledWith('9', '5', [2]);
expect(broadcast).toHaveBeenCalledWith('5', 'reservation:travelers-updated', { reservationId: 9, travelers }, 'sock');
});
@@ -20,14 +20,17 @@ describe('reservationCreateRequestSchema', () => {
});
describe('reservationPositionsRequestSchema', () => {
it('requires positions with id + day_plan_position', () => {
it('requires positions with an id; day_plan_position stays optional (legacy wire tolerance)', () => {
expect(
reservationPositionsRequestSchema.safeParse({
positions: [{ id: 1, day_plan_position: 0 }],
day_id: 3,
}).success,
).toBe(true);
expect(reservationPositionsRequestSchema.safeParse({ positions: [{ id: 1 }] }).success).toBe(false);
// The legacy route accepted items without day_plan_position (binds NULL) —
// RESV-006 pins this on the server side.
expect(reservationPositionsRequestSchema.safeParse({ positions: [{ id: 1 }] }).success).toBe(true);
expect(reservationPositionsRequestSchema.safeParse({ positions: [{}] }).success).toBe(false);
});
});
+4 -1
View File
@@ -162,7 +162,10 @@ export const reservationTravelersRequestSchema = z.object({
export type ReservationTravelersRequest = z.infer<typeof reservationTravelersRequestSchema>;
export const reservationPositionsRequestSchema = z.object({
positions: z.array(z.object({ id: z.number(), day_plan_position: z.number() })),
// day_plan_position is optional on the wire: the legacy route never
// validated position items, and an absent value binds NULL (clears the
// stored position) — pinned by the server integration suite (RESV-006).
positions: z.array(z.object({ id: z.number(), day_plan_position: z.number().optional() })),
day_id: z.union([z.number(), z.string()]).nullable().optional(),
});
export type ReservationPositionsRequest = z.infer<typeof reservationPositionsRequestSchema>;