fix: ( bookings ) prevent double booking by claiming seats under a row lock

This commit is contained in:
Abubeker Yasin
2026-09-08 10:05:47 +03:00
parent 70b1f75158
commit 888bf20072
10 changed files with 1946 additions and 187 deletions

View File

@@ -8,7 +8,7 @@ import { PrismaService } from '../../common/prisma.service';
* the same person can book that train again. PENDING_PAYMENT counts — otherwise the whole check
* is bypassable by simply never finishing the first payment.
*/
const ACTIVE_BOOKING_STATUSES: BookingStatus[] = [
export const ACTIVE_BOOKING_STATUSES: BookingStatus[] = [
BookingStatus.DRAFT,
BookingStatus.PENDING_PAYMENT,
BookingStatus.CONFIRMED,

View File

@@ -2,7 +2,7 @@ import { Injectable, NotFoundException, BadRequestException, Logger } from '@nes
import { InjectDataSource } from '@nestjs/typeorm';
import { DataSource } from 'typeorm';
import { PrismaService } from '../../common/prisma.service';
import { SeatsService } from '../seats/seats.service';
import { SeatsService, SeatClaim } from '../seats/seats.service';
import { TicketsService } from '../tickets/tickets.service';
import { EventEmitter2 } from '@nestjs/event-emitter';
import { CreateBookingDto } from './bookings.dto';
@@ -873,15 +873,10 @@ export class BookingsService {
return this.createOneWayBooking(dto);
}
private validateSeatIdsAgainstHold(holdId: string, holdSeatIds: string[], requestedSeatIds: string[]) {
for (const seatId of requestedSeatIds) {
if (!holdSeatIds.includes(seatId)) {
throw new BadRequestException(
`Seat ${seatId} is not part of hold ${holdId}. Use seat IDs returned from POST /seats/hold.`,
);
}
}
}
// validateSeatIdsAgainstHold lived here. Every caller now goes through
// SeatsService.assertSeatsClaimable, which performs the same seat-in-hold check alongside
// the hold-is-live, hold-is-for-this-schedule and no-conflict checks — and, at write time,
// re-runs all of them inside the seat lock.
/** Resolves contactEmail/contactPhone for an IAM-authenticated passenger booking. */
private async resolveIamContact(passengerId?: string): Promise<{ contactEmail: string | null; contactPhone: string | null }> {
@@ -896,11 +891,18 @@ export class BookingsService {
}
private async createOneWayBooking(dto: CreateBookingDto) {
const hold = await this.prisma.seatHold.findUnique({ where: { id: dto.holdId } });
if (!hold || hold.expiresAt < new Date()) throw new BadRequestException('Seat hold expired');
const requestedSeatIds = (dto.passengers as any[]).map(p => p.seatId);
this.validateSeatIdsAgainstHold(dto.holdId, hold.seatIds, requestedSeatIds);
// Hold live, on this departure, covering these seats, and nobody else holding or booked
// on them. Advisory here — re-run under the seat lock at the write below.
const requestedSeatIds = (dto.passengers as any[]).map(p => p.seatId).filter((id: any): id is string => !!id);
const claims: SeatClaim[] = [{
holdId: dto.holdId,
seatIds: requestedSeatIds,
scheduleId: dto.scheduleId,
originStationId: dto.originStationId,
destinationStationId: dto.destinationStationId,
journeyDirection: JourneyDirection.ONE_WAY,
}];
await this.seatsService.assertSeatsClaimableAll(claims);
const schedule = await this.prisma.trainSchedule.findUnique({
where: { id: dto.scheduleId },
@@ -912,14 +914,6 @@ export class BookingsService {
const destStop = schedule.stopTimes.find(s => s.stationId === dto.destinationStationId);
if (!originStop || !destStop) throw new NotFoundException('Origin or destination not found');
await this.seatsService.assertNoRouteSeatConflict({
scheduleId: dto.scheduleId,
seatIds: requestedSeatIds,
originStationId: dto.originStationId,
destinationStationId: dto.destinationStationId,
journeyDirection: JourneyDirection.ONE_WAY,
});
const [passengersData, iamContact] = await Promise.all([
this.processPassengers(dto.passengers as any[]),
this.resolveIamContact(dto.passengerId),
@@ -1029,7 +1023,9 @@ export class BookingsService {
// still pass; the tolerance absorbs FX-conversion rounding.
this.assertTotalNotUnderAuthoritative(resolvedTotalMinor, fareCalculation.totalMinor, 'createOneWayBooking');
const booking = await this.prisma.booking.create({
// Locked and re-validated inside the lock — see SeatsService.claimSeatsAndWrite. All the
// slow work (fare engine, FX, identity) is already done, so this transaction stays short.
const booking = await this.seatsService.claimSeatsAndWrite(claims, async (tx) => tx.booking.create({
data: {
bookingRef: generateRef(),
passengerId: dto.passengerId,
@@ -1068,7 +1064,7 @@ export class BookingsService {
}
},
include: { seats: { include: { seat: true } }, schedule: { include: { originStation: true, destinationStation: true, train: true } } }
});
}));
await this.seatsService.confirmSeats(passengersData.map(p => p.seatId));
if (dto.packageId && dto.priceTierId) {
@@ -1087,18 +1083,31 @@ export class BookingsService {
throw new BadRequestException('Return trip details required for round-trip booking');
}
const [outboundHold, returnHold] = await Promise.all([
this.prisma.seatHold.findUnique({ where: { id: dto.holdId } }),
this.prisma.seatHold.findUnique({ where: { id: dto.returnHoldId } })
]);
if (!outboundHold || outboundHold.expiresAt < new Date()) throw new BadRequestException('Outbound seat hold expired');
if (!returnHold || returnHold.expiresAt < new Date()) throw new BadRequestException('Return seat hold expired');
const holdObSeatIds = (dto.passengers as any[]).map((p: any) => p.seatId ?? p.outboundSeatId).filter(Boolean);
const holdRetSeatIds = (dto.passengers as any[]).map((p: any) => p.returnSeatId).filter(Boolean);
if (holdObSeatIds.length) this.validateSeatIdsAgainstHold(dto.holdId, outboundHold.seatIds, holdObSeatIds);
if (holdRetSeatIds.length) this.validateSeatIdsAgainstHold(dto.returnHoldId!, returnHold.seatIds, holdRetSeatIds);
// Advisory pass; re-run under the seat lock at the write.
const claims: SeatClaim[] = [
{
holdId: dto.holdId,
seatIds: holdObSeatIds,
scheduleId: dto.scheduleId,
originStationId: dto.originStationId,
destinationStationId: dto.destinationStationId,
journeyDirection: JourneyDirection.OUTBOUND,
legLabel: 'Outbound',
},
{
holdId: dto.returnHoldId!,
seatIds: holdRetSeatIds,
scheduleId: dto.returnScheduleId!,
originStationId: dto.returnOriginStationId,
destinationStationId: dto.returnDestinationStationId,
journeyDirection: JourneyDirection.RETURN,
legLabel: 'Return',
},
];
await this.seatsService.assertSeatsClaimableAll(claims);
const [outboundSchedule, returnSchedule] = await Promise.all([
this.prisma.trainSchedule.findUnique({
@@ -1122,21 +1131,6 @@ export class BookingsService {
throw new NotFoundException('Origin or destination stops not found');
}
await this.seatsService.assertNoRouteSeatConflict({
scheduleId: dto.scheduleId,
seatIds: holdObSeatIds,
originStationId: dto.originStationId,
destinationStationId: dto.destinationStationId,
journeyDirection: JourneyDirection.OUTBOUND,
});
await this.seatsService.assertNoRouteSeatConflict({
scheduleId: dto.returnScheduleId,
seatIds: holdRetSeatIds,
originStationId: dto.returnOriginStationId,
destinationStationId: dto.returnDestinationStationId,
journeyDirection: JourneyDirection.RETURN,
});
const [passengersData, iamContact] = await Promise.all([
this.processRoundTripPassengers(dto.passengers as any[]),
this.resolveIamContact(dto.passengerId),
@@ -1252,7 +1246,8 @@ export class BookingsService {
// C-1 guard: never charge less than the server-recomputed authoritative round-trip fare.
this.assertTotalNotUnderAuthoritative(totalMinor, authoritativeTotalMinor, 'createRoundTripBooking');
const booking = await this.prisma.booking.create({
// Locked across both legs' seats and re-validated inside the lock.
const booking = await this.seatsService.claimSeatsAndWrite(claims, async (tx) => tx.booking.create({
data: {
bookingRef: generateRef(),
passengerId: dto.passengerId,
@@ -1314,7 +1309,7 @@ export class BookingsService {
},
} as any,
include: { seats: { include: { seat: true } }, schedule: { include: { originStation: true, destinationStation: true, train: true } } }
});
}));
const outboundSeatIds = passengersData.map(p => p.outboundSeatId);
const returnSeatIds = passengersData.map(p => p.returnSeatId);
@@ -1355,17 +1350,31 @@ export class BookingsService {
throw new BadRequestException('leg2ScheduleId, leg2HoldId, transitStationId and leg2DestinationStationId are required for TRANSIT bookings');
}
const [leg1Hold, leg2Hold] = await Promise.all([
this.prisma.seatHold.findUnique({ where: { id: dto.holdId } }),
this.prisma.seatHold.findUnique({ where: { id: dto.leg2HoldId } }),
]);
if (!leg1Hold || leg1Hold.expiresAt < new Date()) throw new BadRequestException('Leg-1 seat hold expired');
if (!leg2Hold || leg2Hold.expiresAt < new Date()) throw new BadRequestException('Leg-2 seat hold expired');
const leg1SeatIds = (dto.passengers as any[]).map(p => p.seatId).filter(Boolean);
const leg2SeatIds = (dto.passengers as any[]).map(p => p.leg2SeatId ?? p.seatId).filter(Boolean);
const leg1SeatIds = (dto.passengers as any[]).map(p => p.seatId);
const leg2SeatIds = (dto.passengers as any[]).map(p => p.leg2SeatId ?? p.seatId);
this.validateSeatIdsAgainstHold(dto.holdId, leg1Hold.seatIds, leg1SeatIds);
this.validateSeatIdsAgainstHold(dto.leg2HoldId!, leg2Hold.seatIds, leg2SeatIds);
// Advisory pass; re-run under the seat lock at the write.
const claims: SeatClaim[] = [
{
holdId: dto.holdId,
seatIds: leg1SeatIds,
scheduleId: dto.scheduleId,
originStationId: dto.originStationId,
destinationStationId: dto.transitStationId,
journeyDirection: JourneyDirection.ONE_WAY,
legLabel: 'Leg-1',
},
{
holdId: dto.leg2HoldId!,
seatIds: leg2SeatIds,
scheduleId: dto.leg2ScheduleId!,
originStationId: dto.transitStationId,
destinationStationId: dto.leg2DestinationStationId,
journeyDirection: JourneyDirection.ONE_WAY,
legLabel: 'Leg-2',
},
];
await this.seatsService.assertSeatsClaimableAll(claims);
const [leg1Schedule, leg2Schedule] = await Promise.all([
this.prisma.trainSchedule.findUnique({
@@ -1387,21 +1396,6 @@ export class BookingsService {
if (!leg1OriginStop || !leg1DestStop) throw new NotFoundException('Leg-1 origin or transit station not found on schedule');
if (!leg2OriginStop || !leg2DestStop) throw new NotFoundException('Transit or leg-2 destination station not found on leg-2 schedule');
await this.seatsService.assertNoRouteSeatConflict({
scheduleId: dto.scheduleId,
seatIds: leg1SeatIds,
originStationId: dto.originStationId,
destinationStationId: dto.transitStationId,
journeyDirection: JourneyDirection.ONE_WAY,
});
await this.seatsService.assertNoRouteSeatConflict({
scheduleId: dto.leg2ScheduleId,
seatIds: leg2SeatIds,
originStationId: dto.transitStationId,
destinationStationId: dto.leg2DestinationStationId,
journeyDirection: JourneyDirection.ONE_WAY,
});
const [passengersData, iamContact] = await Promise.all([
this.processPassengers(dto.passengers as any[]),
this.resolveIamContact(dto.passengerId),
@@ -1467,7 +1461,8 @@ export class BookingsService {
});
// Single booking — leg-1 seats at leg=1, leg-2 seats at leg=2
const booking = await this.prisma.booking.create({
// Locked across both legs' seats and re-validated inside the lock.
const booking = await this.seatsService.claimSeatsAndWrite(claims, async (tx) => tx.booking.create({
data: {
bookingRef: generateRef(),
passengerId: dto.passengerId,
@@ -1526,7 +1521,7 @@ export class BookingsService {
},
} as any,
include: { seats: { include: { seat: true } }, schedule: { include: { originStation: true, destinationStation: true, train: true } } },
});
}));
await Promise.all([
this.seatsService.confirmSeats(passengersData.map(p => p.seatId)),
@@ -1561,23 +1556,49 @@ export class BookingsService {
);
}
// Validate all 4 holds
const [obL1Hold, obL2Hold, retL1Hold, retL2Hold] = await Promise.all([
this.prisma.seatHold.findUnique({ where: { id: dto.holdId } }),
this.prisma.seatHold.findUnique({ where: { id: dto.leg2HoldId } }),
this.prisma.seatHold.findUnique({ where: { id: dto.returnHoldId } }),
this.prisma.seatHold.findUnique({ where: { id: dto.returnLeg2HoldId } }),
]);
const now = new Date();
if (!obL1Hold || obL1Hold.expiresAt < now) throw new BadRequestException('Outbound leg-1 seat hold expired');
if (!obL2Hold || obL2Hold.expiresAt < now) throw new BadRequestException('Outbound leg-2 seat hold expired');
if (!retL1Hold || retL1Hold.expiresAt < now) throw new BadRequestException('Return leg-1 seat hold expired');
if (!retL2Hold || retL2Hold.expiresAt < now) throw new BadRequestException('Return leg-2 seat hold expired');
this.validateSeatIdsAgainstHold(dto.holdId, obL1Hold.seatIds, (dto.passengers as any[]).map(p => p.seatId));
this.validateSeatIdsAgainstHold(dto.leg2HoldId!, obL2Hold.seatIds, (dto.passengers as any[]).map(p => p.leg2SeatId ?? p.seatId));
this.validateSeatIdsAgainstHold(dto.returnHoldId!, retL1Hold.seatIds, (dto.passengers as any[]).map(p => p.returnSeatId));
this.validateSeatIdsAgainstHold(dto.returnLeg2HoldId!, retL2Hold.seatIds, (dto.passengers as any[]).map(p => p.returnLeg2SeatId ?? p.returnSeatId));
// All 4 holds: live, on their own departure, covering their leg's seats, unconflicted.
// Advisory pass; re-run under the seat lock at the write.
const seatsOf = (pick: (p: any) => string | undefined) =>
(dto.passengers as any[]).map(pick).filter((id): id is string => !!id);
const claims: SeatClaim[] = [
{
holdId: dto.holdId,
seatIds: seatsOf(p => p.seatId),
scheduleId: dto.scheduleId,
originStationId: dto.originStationId,
destinationStationId: dto.transitStationId,
journeyDirection: JourneyDirection.OUTBOUND,
legLabel: 'Outbound leg-1',
},
{
holdId: dto.leg2HoldId!,
seatIds: seatsOf(p => p.leg2SeatId ?? p.seatId),
scheduleId: dto.leg2ScheduleId!,
originStationId: dto.transitStationId,
destinationStationId: dto.leg2DestinationStationId,
journeyDirection: JourneyDirection.OUTBOUND,
legLabel: 'Outbound leg-2',
},
{
holdId: dto.returnHoldId!,
seatIds: seatsOf(p => p.returnSeatId),
scheduleId: dto.returnScheduleId!,
originStationId: dto.returnOriginStationId,
destinationStationId: dto.returnTransitStationId,
journeyDirection: JourneyDirection.RETURN,
legLabel: 'Return leg-1',
},
{
holdId: dto.returnLeg2HoldId!,
seatIds: seatsOf(p => p.returnLeg2SeatId ?? p.returnSeatId),
scheduleId: dto.returnLeg2ScheduleId!,
originStationId: dto.returnTransitStationId,
destinationStationId: dto.returnLeg2DestinationStationId,
journeyDirection: JourneyDirection.RETURN,
legLabel: 'Return leg-2',
},
];
await this.seatsService.assertSeatsClaimableAll(claims);
// Load all 4 schedules
const [obL1Sched, obL2Sched, retL1Sched, retL2Sched] = await Promise.all([
@@ -1604,35 +1625,6 @@ export class BookingsService {
if (!retL1Origin || !retL1Dest) throw new NotFoundException('Return leg-1: origin or transit station not found');
if (!retL2Origin || !retL2Dest) throw new NotFoundException('Return leg-2: transit or destination not found');
await this.seatsService.assertNoRouteSeatConflict({
scheduleId: dto.scheduleId,
seatIds: (dto.passengers as any[]).map(p => p.seatId),
originStationId: dto.originStationId,
destinationStationId: dto.transitStationId,
journeyDirection: JourneyDirection.OUTBOUND,
});
await this.seatsService.assertNoRouteSeatConflict({
scheduleId: dto.leg2ScheduleId,
seatIds: (dto.passengers as any[]).map(p => p.leg2SeatId ?? p.seatId),
originStationId: dto.transitStationId,
destinationStationId: dto.leg2DestinationStationId,
journeyDirection: JourneyDirection.OUTBOUND,
});
await this.seatsService.assertNoRouteSeatConflict({
scheduleId: dto.returnScheduleId,
seatIds: (dto.passengers as any[]).map(p => p.returnSeatId),
originStationId: dto.returnOriginStationId,
destinationStationId: dto.returnTransitStationId,
journeyDirection: JourneyDirection.RETURN,
});
await this.seatsService.assertNoRouteSeatConflict({
scheduleId: dto.returnLeg2ScheduleId,
seatIds: (dto.passengers as any[]).map(p => p.returnLeg2SeatId ?? p.returnSeatId),
originStationId: dto.returnTransitStationId,
destinationStationId: dto.returnLeg2DestinationStationId,
journeyDirection: JourneyDirection.RETURN,
});
const [passengersData, iamContact] = await Promise.all([
this.processRoundTripPassengers(dto.passengers as any[]),
this.resolveIamContact(dto.passengerId),
@@ -1718,7 +1710,8 @@ export class BookingsService {
displayCurrency,
});
const booking = await this.prisma.booking.create({
// Locked across all four legs' seats and re-validated inside the lock.
const booking = await this.seatsService.claimSeatsAndWrite(claims, async (tx) => tx.booking.create({
data: {
bookingRef: generateRef(),
passengerId: dto.passengerId,
@@ -1759,7 +1752,7 @@ export class BookingsService {
},
} as any,
include: { seats: { include: { seat: true } }, schedule: { include: { originStation: true, destinationStation: true, train: true } } },
});
}));
await Promise.all([
this.seatsService.confirmSeats(passengersData.map(p => p.outboundSeatId)),

View File

@@ -2,7 +2,7 @@ import { Injectable, BadRequestException, NotFoundException, Logger } from '@nes
import { InjectDataSource } from '@nestjs/typeorm';
import { DataSource } from 'typeorm';
import { PrismaService } from '../../common/prisma.service';
import { SeatsService } from '../seats/seats.service';
import { SeatsService, SeatClaim } from '../seats/seats.service';
import { VerifaydaService } from '../verifayda/verifayda.service';
import { CurrencyService } from '../currency/currency.service';
import { PassengerAuthService } from '../auth/passenger-auth.service';
@@ -146,11 +146,20 @@ export class GuestBookingService {
private async createGuestOneWayBooking(dto: CreateGuestBookingDto, req?: any) {
const authUserId: string | null = req?.user?.id ?? null;
// Validate hold
const hold = await this.prisma.seatHold.findUnique({ where: { id: dto.holdId } });
if (!hold || hold.expiresAt < new Date()) {
throw new BadRequestException('Seat hold expired or not found');
}
const claimedSeatIds = dto.passengers.map((p) => p.seatId).filter((id): id is string => !!id);
// Fail fast, before any fare or identity work: hold is live, belongs to this departure,
// covers the seats asked for, and nobody else holds or has booked them. This is a
// courtesy check for a clean early error — the authoritative one runs under the seat
// lock at the write below, because anything checked out here can change before we write.
const claims: SeatClaim[] = [{
holdId: dto.holdId,
seatIds: claimedSeatIds,
scheduleId: dto.scheduleId,
originStationId: dto.originStationId,
destinationStationId: dto.destinationStationId,
journeyDirection: JourneyDirection.ONE_WAY,
}];
await this.seatsService.assertSeatsClaimableAll(claims);
// Get schedule
const schedule = await this.prisma.trainSchedule.findUnique({
@@ -371,8 +380,15 @@ export class GuestBookingService {
}
}
// Create booking
const booking = await this.prisma.booking.create({
// Create booking.
//
// Locked, and re-validated inside the lock. Everything slow — fare engine, FX, Verifayda,
// guest-passenger resolution — is already done above, so this transaction is two reads and
// a write. Re-checking here is the whole point: the courtesy check at the top of the method
// ran hundreds of milliseconds ago, and between then and now another request holding the
// same hold (a double-submit, a retry after a timeout) could have booked these seats.
const booking = await this.seatsService.claimSeatsAndWrite(claims, async (tx) => {
return tx.booking.create({
data: {
bookingRef: generateRef(),
passengerId: guestPassengerId,
@@ -418,6 +434,7 @@ export class GuestBookingService {
seats: { include: { seat: { include: { coach: true } } } },
schedule: { include: { originStation: true, destinationStation: true, train: true } },
},
});
});
// Save passenger details as traveler profiles — guest bookings only.
@@ -702,19 +719,35 @@ export class GuestBookingService {
throw new BadRequestException('returnScheduleId, returnHoldId, returnOriginStationId and returnDestinationStationId are required for ROUND_TRIP');
}
// Validate both holds
const [outboundHold, returnHold] = await Promise.all([
this.prisma.seatHold.findUnique({ where: { id: dto.holdId } }),
this.prisma.seatHold.findUnique({ where: { id: dto.returnHoldId } }),
]);
if (!outboundHold || outboundHold.expiresAt < new Date()) throw new BadRequestException('Outbound seat hold expired or not found');
if (!returnHold || returnHold.expiresAt < new Date()) throw new BadRequestException('Return seat hold expired or not found');
// Validate passengers have returnSeatId
for (const p of dto.passengers) {
if (!p.returnSeatId) throw new BadRequestException(`returnSeatId is required for each passenger in a ROUND_TRIP booking (missing for ${p.passengerName})`);
}
// Each leg's hold must be live, belong to that leg's departure, cover that leg's seats,
// and clear the conflict check. Advisory here; re-run under the seat lock at the write.
const claims: SeatClaim[] = [
{
holdId: dto.holdId,
seatIds: dto.passengers.map((p) => p.seatId).filter((id): id is string => !!id),
scheduleId: dto.scheduleId,
originStationId: dto.originStationId,
destinationStationId: dto.destinationStationId,
journeyDirection: JourneyDirection.OUTBOUND,
legLabel: 'Outbound',
},
{
holdId: dto.returnHoldId,
seatIds: dto.passengers.map((p) => p.returnSeatId).filter((id): id is string => !!id),
scheduleId: dto.returnScheduleId,
originStationId: dto.returnOriginStationId,
destinationStationId: dto.returnDestinationStationId,
journeyDirection: JourneyDirection.RETURN,
legLabel: 'Return',
},
];
await this.seatsService.assertSeatsClaimableAll(claims);
// Load both schedules
const [outboundSchedule, returnSchedule] = await Promise.all([
this.prisma.trainSchedule.findUnique({
@@ -921,7 +954,10 @@ export class GuestBookingService {
const outboundSeatIds = dto.passengers.map(p => p.seatId).filter((id): id is string => !!id);
const returnSeatIds = dto.passengers.map(p => p.returnSeatId!);
const booking = await this.prisma.booking.create({
// Locked across BOTH legs' seats, with each leg's claim re-checked inside the lock —
// so a round trip is all-or-nothing: it never commits with the outbound seat secured
// and the return seat sold out from under it.
const booking = await this.seatsService.claimSeatsAndWrite(claims, async (tx) => tx.booking.create({
data: {
bookingRef: generateRef(),
passengerId: guestPassengerId,
@@ -987,7 +1023,7 @@ export class GuestBookingService {
seats: { include: { seat: { include: { coach: true } } } },
schedule: { include: { originStation: true, destinationStation: true, train: true } },
},
});
}));
// Traveler profiles: guest bookings only (authenticated passengers already have one).
if (!authUserId) await this.createTravelerProfiles(guestPassengerId, passengersData);
@@ -1032,17 +1068,32 @@ export class GuestBookingService {
throw new BadRequestException('leg2ScheduleId, leg2HoldId, transitStationId and leg2DestinationStationId are required for TRANSIT bookings');
}
const [leg1Hold, leg2Hold] = await Promise.all([
this.prisma.seatHold.findUnique({ where: { id: dto.holdId } }),
this.prisma.seatHold.findUnique({ where: { id: dto.leg2HoldId } }),
]);
if (!leg1Hold || leg1Hold.expiresAt < new Date()) throw new BadRequestException('Leg-1 seat hold expired or not found');
if (!leg2Hold || leg2Hold.expiresAt < new Date()) throw new BadRequestException('Leg-2 seat hold expired or not found');
for (const p of dto.passengers) {
if (!p.leg2SeatId) throw new BadRequestException(`leg2SeatId is required for each passenger in a TRANSIT booking (missing for ${p.passengerName})`);
}
const claims: SeatClaim[] = [
{
holdId: dto.holdId,
seatIds: dto.passengers.map((p) => p.seatId).filter((id): id is string => !!id),
scheduleId: dto.scheduleId,
originStationId: dto.originStationId,
destinationStationId: dto.transitStationId,
journeyDirection: JourneyDirection.ONE_WAY,
legLabel: 'Leg-1',
},
{
holdId: dto.leg2HoldId,
seatIds: dto.passengers.map((p) => p.leg2SeatId).filter((id): id is string => !!id),
scheduleId: dto.leg2ScheduleId,
originStationId: dto.transitStationId,
destinationStationId: dto.leg2DestinationStationId,
journeyDirection: JourneyDirection.ONE_WAY,
legLabel: 'Leg-2',
},
];
await this.seatsService.assertSeatsClaimableAll(claims);
const [leg1Schedule, leg2Schedule] = await Promise.all([
this.prisma.trainSchedule.findUnique({
where: { id: dto.scheduleId },
@@ -1140,8 +1191,9 @@ export class GuestBookingService {
const { guestPassengerId, iamUserId, createdAccount } = await this.resolveGuestPassenger(dto, passengersData[0], req);
const contact = await this.resolveActorContact(req, passengersData[0]);
// Single booking — leg-1 seats at leg=1, leg-2 seats at leg=2
const booking = await this.prisma.booking.create({
// Single booking — leg-1 seats at leg=1, leg-2 seats at leg=2.
// Locked across both legs' seats and re-checked inside the lock.
const booking = await this.seatsService.claimSeatsAndWrite(claims, async (tx) => tx.booking.create({
data: {
bookingRef: generateRef(),
passengerId: guestPassengerId,
@@ -1204,7 +1256,7 @@ export class GuestBookingService {
seats: { include: { seat: { include: { coach: true } } } },
schedule: { include: { originStation: true, destinationStation: true, train: true } },
},
});
}));
// Traveler profiles: guest bookings only (authenticated passengers already have one).
if (!authUserId) await this.createTravelerProfiles(guestPassengerId, passengersData);
@@ -1247,17 +1299,48 @@ export class GuestBookingService {
if (!p.returnLeg2SeatId) throw new BadRequestException(`returnLeg2SeatId required for ${p.passengerName}`);
}
const now = new Date();
const [obL1Hold, obL2Hold, retL1Hold, retL2Hold] = await Promise.all([
this.prisma.seatHold.findUnique({ where: { id: dto.holdId } }),
this.prisma.seatHold.findUnique({ where: { id: dto.leg2HoldId } }),
this.prisma.seatHold.findUnique({ where: { id: dto.returnHoldId } }),
this.prisma.seatHold.findUnique({ where: { id: dto.returnLeg2HoldId } }),
]);
if (!obL1Hold || obL1Hold.expiresAt < now) throw new BadRequestException('Outbound leg-1 hold expired');
if (!obL2Hold || obL2Hold.expiresAt < now) throw new BadRequestException('Outbound leg-2 hold expired');
if (!retL1Hold || retL1Hold.expiresAt < now) throw new BadRequestException('Return leg-1 hold expired');
if (!retL2Hold || retL2Hold.expiresAt < now) throw new BadRequestException('Return leg-2 hold expired');
const seatsOf = (pick: (p: (typeof dto.passengers)[number]) => string | undefined) =>
dto.passengers.map(pick).filter((id): id is string => !!id);
const claims: SeatClaim[] = [
{
holdId: dto.holdId,
seatIds: seatsOf((p) => p.seatId),
scheduleId: dto.scheduleId,
originStationId: dto.originStationId,
destinationStationId: dto.transitStationId,
journeyDirection: JourneyDirection.OUTBOUND,
legLabel: 'Outbound leg-1',
},
{
holdId: dto.leg2HoldId,
seatIds: seatsOf((p) => p.leg2SeatId),
scheduleId: dto.leg2ScheduleId,
originStationId: dto.transitStationId,
destinationStationId: dto.leg2DestinationStationId,
journeyDirection: JourneyDirection.OUTBOUND,
legLabel: 'Outbound leg-2',
},
{
holdId: dto.returnHoldId,
seatIds: seatsOf((p) => p.returnSeatId),
scheduleId: dto.returnScheduleId,
originStationId: dto.returnOriginStationId,
destinationStationId: dto.returnTransitStationId,
journeyDirection: JourneyDirection.RETURN,
legLabel: 'Return leg-1',
},
{
holdId: dto.returnLeg2HoldId,
seatIds: seatsOf((p) => p.returnLeg2SeatId),
scheduleId: dto.returnLeg2ScheduleId,
originStationId: dto.returnTransitStationId,
destinationStationId: dto.returnLeg2DestinationStationId,
journeyDirection: JourneyDirection.RETURN,
legLabel: 'Return leg-2',
},
];
await this.seatsService.assertSeatsClaimableAll(claims);
const [obL1Sched, obL2Sched, retL1Sched, retL2Sched] = await Promise.all([
this.prisma.trainSchedule.findUnique({ where: { id: dto.scheduleId }, include: { originStation: true, destinationStation: true, stopTimes: { include: { station: true }, orderBy: { sequence: 'asc' } }, route: { include: { stops: true } } } }),
@@ -1371,7 +1454,8 @@ export class GuestBookingService {
displayCurrency,
});
const booking = await this.prisma.booking.create({
// Locked across all four legs' seats and re-checked inside the lock.
const booking = await this.seatsService.claimSeatsAndWrite(claims, async (tx) => tx.booking.create({
data: {
bookingRef: generateRef(),
passengerId: guestPassengerId,
@@ -1410,7 +1494,7 @@ export class GuestBookingService {
seats: { include: { seat: { include: { coach: true } } } },
schedule: { include: { originStation: true, destinationStation: true, train: true } },
},
});
}));
// Traveler profiles: guest bookings only (authenticated passengers already have one).
if (!authUserId) await this.createTravelerProfiles(guestPassengerId, passengersData);

View File

@@ -0,0 +1,217 @@
import { Test, TestingModule } from '@nestjs/testing';
import { SeatsService } from './seats.service';
import { PrismaService } from '../../common/prisma.service';
import { SegmentsService } from '../segments/segments.service';
import { SystemConfigService } from '../system-config/system-config.service';
import { AuditService } from '../../common/audit.service';
import { SmsClientService } from '../notifications/sms-client.service';
/**
* Regression guard for the double-booking fix.
*
* Every case below was reproduced against the running API before the fix and must stay
* closed. These are unit tests: they prove the guards exist, are wired in the right order,
* and read through the transaction client. They do NOT prove concurrency safety — only a
* real database can, and that proof lives in scripts/stress-booking-concurrency.cjs.
*/
describe('SeatsService — double-booking guards', () => {
let service: SeatsService;
const seatHold = { findUnique: jest.fn() };
const bookingSeat = { findMany: jest.fn() };
const tripStopTime = { findMany: jest.fn() };
const seat = { findMany: jest.fn() };
const queryRaw = jest.fn();
// SET LOCAL lock_timeout — see SeatsService.lockSeatsForUpdate.
const executeRawUnsafe = jest.fn();
const tx: any = { seatHold, bookingSeat, tripStopTime, seat, $queryRaw: queryRaw, $executeRawUnsafe: executeRawUnsafe };
const mockPrisma: any = {
seatHold, bookingSeat, tripStopTime, seat, $queryRaw: queryRaw, $executeRawUnsafe: executeRawUnsafe,
$transaction: jest.fn((fn: any) => fn(tx)),
};
const mockSegments = { getSeatAvailabilityMap: jest.fn() };
const LIVE_HOLD = {
id: 'hold-1',
scheduleId: 'sched-1',
seatIds: ['seat-1', 'seat-2'],
passengerId: 'pax-1',
expiresAt: new Date(Date.now() + 60_000),
};
const baseArgs = {
holdId: 'hold-1',
seatIds: ['seat-1'],
scheduleId: 'sched-1',
originStationId: 'a',
destinationStationId: 'c',
};
/** A BookingSeat row owned by someone else, spanning a -> c on this schedule. */
const ownedByOther = [{
seatId: 'seat-1',
seat: { seatNumber: '7' },
booking: {
bookingRef: 'AAA111', scheduleId: 'sched-1',
originStationId: 'a', destinationStationId: 'c',
returnScheduleId: null, returnOriginStationId: null, returnDestinationStationId: null,
leg2ScheduleId: null, leg2OriginStationId: null, leg2DestinationStationId: null,
returnLeg2ScheduleId: null, returnLeg2OriginStationId: null, returnLeg2DestStationId: null,
},
}];
beforeEach(async () => {
const module: TestingModule = await Test.createTestingModule({
providers: [
SeatsService,
{ provide: PrismaService, useValue: mockPrisma },
{ provide: SegmentsService, useValue: mockSegments },
{ provide: SystemConfigService, useValue: { getNumber: jest.fn().mockResolvedValue(5) } },
{ provide: AuditService, useValue: { log: jest.fn() } },
{ provide: SmsClientService, useValue: { send: jest.fn() } },
],
}).compile();
service = module.get<SeatsService>(SeatsService);
jest.clearAllMocks();
seatHold.findUnique.mockResolvedValue(LIVE_HOLD);
bookingSeat.findMany.mockResolvedValue([]);
tripStopTime.findMany.mockResolvedValue([
{ stationId: 'a', sequence: 1 },
{ stationId: 'b', sequence: 2 },
{ stationId: 'c', sequence: 3 },
]);
seat.findMany.mockResolvedValue([{ id: 'seat-1', seatNumber: '7' }]);
mockSegments.getSeatAvailabilityMap.mockResolvedValue(new Map());
queryRaw.mockResolvedValue([]);
executeRawUnsafe.mockResolvedValue(0);
});
describe('assertSeatsClaimable', () => {
it('accepts a seat the presented hold actually covers', async () => {
await expect(service.assertSeatsClaimable(baseArgs)).resolves.toBeUndefined();
});
it('rejects a seat that is not part of the hold (hold laundering)', async () => {
await expect(service.assertSeatsClaimable({ ...baseArgs, seatIds: ['seat-99'] }))
.rejects.toThrow(/not part of hold/i);
});
it('rejects a hold taken on a different departure — one Seat row is reused across schedules', async () => {
await expect(service.assertSeatsClaimable({ ...baseArgs, scheduleId: 'sched-OTHER' }))
.rejects.toThrow(/different departure/i);
});
it('rejects an expired hold', async () => {
seatHold.findUnique.mockResolvedValue({ ...LIVE_HOLD, expiresAt: new Date(Date.now() - 1) });
await expect(service.assertSeatsClaimable(baseArgs)).rejects.toThrow(/expired or not found/i);
});
it('rejects a hold that does not exist', async () => {
seatHold.findUnique.mockResolvedValue(null);
await expect(service.assertSeatsClaimable(baseArgs)).rejects.toThrow(/expired or not found/i);
});
});
describe('active-booking conflict — enabled for booking writes only', () => {
it('rejects a seat an active booking already owns on an overlapping leg', async () => {
bookingSeat.findMany.mockResolvedValue(ownedByOther);
await expect(service.assertSeatsClaimable({ ...baseArgs, alsoRejectActiveBookings: true }))
.rejects.toThrow(/just booked by someone else/i);
});
it('allows a seat whose existing booking covers a non-overlapping stretch', async () => {
// existing leg b->c is [2,3); requested a->b is [1,2) — they only touch, so no conflict
bookingSeat.findMany.mockResolvedValue([{
...ownedByOther[0],
booking: { ...ownedByOther[0].booking, originStationId: 'b', destinationStationId: 'c' },
}]);
await expect(service.assertSeatsClaimable({
...baseArgs, destinationStationId: 'b', alsoRejectActiveBookings: true,
})).resolves.toBeUndefined();
});
it('does NOT run for plain holds — reschedule/upgrade legitimately re-hold their own seat', async () => {
bookingSeat.findMany.mockResolvedValue(ownedByOther);
await expect(service.assertSeatsClaimable(baseArgs)).resolves.toBeUndefined();
expect(bookingSeat.findMany).not.toHaveBeenCalled();
});
it('blocks conservatively when the existing booking leg cannot be resolved', async () => {
bookingSeat.findMany.mockResolvedValue([{
...ownedByOther[0],
booking: { ...ownedByOther[0].booking, originStationId: null, destinationStationId: null },
}]);
await expect(service.assertSeatsClaimable({ ...baseArgs, alsoRejectActiveBookings: true }))
.rejects.toThrow(/just booked by someone else/i);
});
it('reads BookingSeat through the transaction client, not the pool', async () => {
await service.claimSeatsAndWrite([baseArgs], async () => ({ id: 'b1' }) as any);
expect(bookingSeat.findMany).toHaveBeenCalled();
// same mock object is shared by tx and pool here, so assert the availability map —
// whose last positional argument is the client — received the transaction client.
const call = mockSegments.getSeatAvailabilityMap.mock.calls[0];
expect(call[call.length - 1]).toBe(tx);
});
});
describe('claimSeatsAndWrite', () => {
it('locks the seats FOR UPDATE, then re-checks, then writes — in that order', async () => {
const order: string[] = [];
queryRaw.mockImplementation(() => { order.push('lock'); return Promise.resolve([]); });
seatHold.findUnique.mockImplementation(() => { order.push('check'); return Promise.resolve(LIVE_HOLD); });
const write = jest.fn(async () => { order.push('write'); return { id: 'b1' } as any; });
await service.claimSeatsAndWrite([baseArgs], write);
expect(order).toEqual(['lock', 'check', 'write']);
expect(mockPrisma.$transaction).toHaveBeenCalledTimes(1);
});
it('never runs the write when a leg fails its claim check', async () => {
seatHold.findUnique.mockResolvedValue({ ...LIVE_HOLD, expiresAt: new Date(Date.now() - 1) });
const write = jest.fn();
await expect(service.claimSeatsAndWrite([baseArgs], write as any)).rejects.toThrow();
expect(write).not.toHaveBeenCalled();
});
it('takes one lock covering every leg of a multi-leg booking, in one transaction', async () => {
const write = jest.fn(async () => ({ id: 'b1' }) as any);
await service.claimSeatsAndWrite(
[baseArgs, { ...baseArgs, seatIds: ['seat-2', 'seat-1'], legLabel: 'Return' }],
write,
);
expect(mockPrisma.$transaction).toHaveBeenCalledTimes(1);
expect(queryRaw).toHaveBeenCalledTimes(1);
expect(write).toHaveBeenCalledTimes(1);
});
it('skips the lock statement when there are no seats to lock (free-child-only booking)', async () => {
const write = jest.fn(async () => ({ id: 'b1' }) as any);
await service.claimSeatsAndWrite([{ ...baseArgs, seatIds: [] }], write);
expect(queryRaw).not.toHaveBeenCalled();
expect(write).toHaveBeenCalledTimes(1);
});
});
describe('withSeatsLocked', () => {
it('de-duplicates and sorts seat ids so two overlapping requests cannot deadlock', async () => {
await service.withSeatsLocked(['s-b', 's-a', 's-b'], async () => null);
const sql = queryRaw.mock.calls[0][0];
// Prisma.sql carries the interpolated values in `values`; order proves the sort.
expect(sql.values).toEqual(['s-a', 's-b']);
});
it('bounds the lock wait so a deep queue returns 409, not a transaction timeout', async () => {
await service.withSeatsLocked(['s-a'], async () => null);
expect(executeRawUnsafe).toHaveBeenCalledWith(expect.stringMatching(/SET LOCAL lock_timeout/i));
});
it('reports a berth being claimed elsewhere as a conflict, not a 500', async () => {
queryRaw.mockRejectedValue(Object.assign(new Error('raw query failed'), { meta: { code: '55P03' } }));
await expect(service.withSeatsLocked(['s-a'], async () => null))
.rejects.toThrow(/being booked by someone else/i);
});
});
});

View File

@@ -1,16 +1,101 @@
import { Injectable, ConflictException, NotFoundException, BadRequestException, Logger } from '@nestjs/common';
import { randomUUID } from 'crypto';
import { Prisma } from '@prisma/client';
import { PrismaService } from '../../common/prisma.service';
import { BlockSeatDto, HoldSeatsDto, JourneyDirection, SeatBlockReasonCategory } from './seats.dto';
import { ActingUser } from '../../common/acting-user';
import { Cron, CronExpression } from '@nestjs/schedule';
import { SegmentsService } from '../segments/segments.service';
import { SegmentsService, PrismaClientLike } from '../segments/segments.service';
import { SystemConfigService, CONFIG_KEYS } from '../system-config/system-config.service';
import { AuditService } from '../../common/audit.service';
import { AUDIT_ACTIONS, AUDIT_ENTITIES } from '../../common/audit.actions';
import { SmsClientService } from '../notifications/sms-client.service';
import { computePaymentDeadline } from '../../common/utils/payment-deadline.utils';
import { checkDirectionConflict } from '../../common/utils/journey-direction.utils';
// Same definition the one-ticket-per-identity check uses: the states that still hold a
// traveller's place. CANCELLED / REFUNDED / NO_SHOW must free the berth immediately.
import { ACTIVE_BOOKING_STATUSES } from '../bookings/booking-identity.util';
/**
* How long a request will queue for a berth's row lock before giving up and reporting the
* seat as taken. Must stay comfortably under Prisma's interactive-transaction timeout
* (5000 ms by default) so this is what fires, not the transaction ceiling — the latter
* surfaces as an opaque 500 instead of an actionable 409.
*/
const SEAT_LOCK_TIMEOUT_MS = 3000;
/** Postgres 55P03 lock_not_available, as surfaced through Prisma's raw-query wrapper. */
function isLockTimeout(err: unknown): boolean {
const meta = (err as any)?.meta;
if (meta?.code === '55P03') return true;
const text = `${(err as any)?.message ?? ''} ${meta?.message ?? ''}`;
return /55P03|lock timeout|canceling statement due to lock timeout/i.test(text);
}
/**
* One leg's claim on some seats: the hold being presented, the seats it must cover, and the
* stretch of that schedule they are wanted for. A one-way booking has one; a round-trip
* transit booking has four, each with its own hold.
*/
export interface SeatClaim {
holdId: string;
seatIds: string[];
scheduleId: string;
originStationId?: string;
destinationStationId?: string;
journeyDirection?: JourneyDirection;
/** Label used in error messages when a booking presents several holds (one per leg). */
legLabel?: string;
}
/**
* The stretch of track a booking occupies on one schedule.
*
* Resolved by matching the SCHEDULE rather than the leg number. `BookingSeat.leg` is
* numbered per booking type (1=outbound, 2=return/leg-2, 3-4=return transit legs), so
* reading it correctly means re-deriving the same branching PaymentsService.createJourneySegments
* does. Matching on scheduleId asks the question we actually care about — "what stretch of
* THIS departure does this booking hold?" — and stays correct if leg numbering ever changes.
*
* Leg boundaries mirror createJourneySegments exactly: on a transit booking the outbound leg
* ends at the transit station (leg2OriginStationId), not at the booking's final destination.
*
* A booking that somehow lands on one schedule twice yields the union of both stretches,
* which is the conservative reading.
*/
function resolveBookingLegRange(
booking: {
scheduleId: string;
originStationId: string | null;
destinationStationId: string | null;
returnScheduleId: string | null;
returnOriginStationId: string | null;
returnDestinationStationId: string | null;
leg2ScheduleId: string | null;
leg2OriginStationId: string | null;
leg2DestinationStationId: string | null;
returnLeg2ScheduleId: string | null;
returnLeg2OriginStationId: string | null;
returnLeg2DestStationId: string | null;
},
scheduleId: string,
): { originStationId: string | null; destinationStationId: string | null } | null {
const b = booking;
const legs = [
// Outbound. On a transit booking this leg stops at the transit station.
{ scheduleId: b.scheduleId, from: b.originStationId, to: b.leg2OriginStationId ?? b.destinationStationId },
{ scheduleId: b.leg2ScheduleId, from: b.leg2OriginStationId, to: b.leg2DestinationStationId },
// Return. Likewise stops at the return transit station when there is one.
{ scheduleId: b.returnScheduleId, from: b.returnOriginStationId, to: b.returnLeg2OriginStationId ?? b.returnDestinationStationId },
{ scheduleId: b.returnLeg2ScheduleId, from: b.returnLeg2OriginStationId, to: b.returnLeg2DestStationId },
].filter((l) => l.scheduleId === scheduleId && l.from && l.to);
if (!legs.length) return null;
if (legs.length === 1) {
return { originStationId: legs[0].from, destinationStationId: legs[0].to };
}
return { originStationId: legs[0].from, destinationStationId: legs[legs.length - 1].to };
}
@Injectable()
export class SeatsService {
@@ -206,6 +291,239 @@ export class SeatsService {
return legacyMap[col?.toUpperCase()] ?? null;
}
/**
* The single gate every booking path must pass before it may write a BookingSeat.
*
* It answers both halves of "may this request claim these seats": that the hold it
* presents actually covers them, and that nobody else has them for this leg. Those two
* checks used to live only in BookingsService, so POST /bookings/guest — a public,
* unauthenticated endpoint — validated nothing beyond "some hold exists and hasn't
* expired" and would happily connect any seat id the caller sent, held or not, free or
* not. Keep both booking services routed through here; a path that skips it can sell
* the same berth twice.
*/
async assertSeatsClaimable(args: SeatClaim & {
/**
* Read through this client. Callers inside `withSeatsLocked` pass their `tx` so the
* check runs on the connection that owns the row locks; everyone else gets the pool.
*/
db?: PrismaClientLike;
/** See assertNoRouteSeatConflict — booking writes turn this on, holds do not. */
alsoRejectActiveBookings?: boolean;
}): Promise<void> {
const { holdId, seatIds, legLabel } = args;
const db = args.db ?? this.prisma;
const prefix = legLabel ? `${legLabel} ` : '';
const hold = await db.seatHold.findUnique({ where: { id: holdId } });
if (!hold || hold.expiresAt < new Date()) {
throw new BadRequestException(`${prefix}seat hold expired or not found`.trim());
}
// A hold is scoped to one departure, but the same physical Seat row is reused across
// every schedule its coach is assigned to. Without this check a hold taken on a quiet
// departure redeems that seat on a busy one — and the resulting booking is then
// "protected" by a hold on the wrong schedule, i.e. not protected at all.
if (hold.scheduleId !== args.scheduleId) {
throw new BadRequestException(
`${prefix}hold ${holdId} belongs to a different departure. ` +
`Hold the seats on this schedule before booking them.`.trim(),
);
}
const heldSeatIds = new Set(hold.seatIds);
const stray = seatIds.filter((id) => id && !heldSeatIds.has(id));
if (stray.length) {
throw new BadRequestException(
`${prefix}seat(s) ${stray.join(', ')} are not part of hold ${holdId}. ` +
`Use seat IDs returned from POST /seats/hold.`.trim(),
);
}
await this.assertNoRouteSeatConflict({
scheduleId: args.scheduleId,
seatIds,
originStationId: args.originStationId,
destinationStationId: args.destinationStationId,
journeyDirection: args.journeyDirection,
requestingPassengerId: hold.passengerId,
db,
alsoRejectActiveBookings: args.alsoRejectActiveBookings,
legLabel,
});
}
/**
* Runs `work` inside a transaction that holds an exclusive row lock on every seat it
* touches — the write-side twin of the lock `holdSeats` takes.
*
* Validating seat availability and then writing the BookingSeat rows in two separate
* statements is a read-then-write with nothing between them: N concurrent requests all
* read "free" and all write, which is exactly how one berth was sold six times over.
* Wrapping both in this makes the second request block on the lock, then re-read and see
* the first request's committed booking.
*
* `ORDER BY id` matches holdSeats, so a hold and a booking contending for the same two
* seats take them in the same sequence and queue instead of deadlocking.
*
* The transaction must stay short: do fare calculation, FX conversion and identity
* verification BEFORE calling this. Slow I/O in here holds seat locks open for the whole
* round trip.
*/
/**
* Takes the exclusive row locks for `seatIds` inside the caller's transaction.
*
* Bounded on purpose. FOR UPDATE serialises everyone contending for a berth, so under a
* burst the Nth contender waits for the N-1 transactions ahead of it. Left unbounded that
* wait runs past Prisma's 5s interactive-transaction ceiling and the request dies with
* P2028 — a 500 reading "Internal server error" for what is really "someone else got the
* seat". lock_timeout fires first and turns that into the honest answer.
*
* Waiting at all is still right: a fast hand-off (the common case) succeeds normally. Only
* a queue deep enough to mean the berth is genuinely being taken gets the 409.
*/
private async lockSeatsForUpdate(tx: Prisma.TransactionClient, seatIds: string[]): Promise<void> {
if (!seatIds.length) return;
// SET LOCAL — scoped to this transaction, reset on commit/rollback. Must stay below the
// interactive-transaction timeout so it is the one that fires.
await tx.$executeRawUnsafe(`SET LOCAL lock_timeout = '${SEAT_LOCK_TIMEOUT_MS}ms'`);
try {
await tx.$queryRaw(
Prisma.sql`SELECT id FROM passenger."Seat" WHERE id IN (${Prisma.join(seatIds)}) ORDER BY id FOR UPDATE`,
);
} catch (err) {
// 55P03 lock_not_available: another transaction is mid-claim on one of these berths.
if (isLockTimeout(err)) {
throw new ConflictException(
'Those seats are being booked by someone else right now. Please pick another seat.',
);
}
throw err;
}
}
async withSeatsLocked<T>(
seatIds: string[],
work: (tx: Prisma.TransactionClient) => Promise<T>,
): Promise<T> {
// Sorted here as well as in the SQL. ORDER BY is the intent, but Postgres is only
// guaranteed to apply it before locking when the plan produces rows in that order — a
// bitmap heap scan can lock first and sort after. Handing the IN-list in id order costs
// nothing and makes the ordering independent of the planner.
const distinct = [...new Set(seatIds.filter(Boolean))].sort();
return this.prisma.$transaction(async (tx) => {
await this.lockSeatsForUpdate(tx, distinct);
return work(tx);
});
}
/**
* The two-line contract every booking-creation path uses.
*
* const booking = await seatsService.claimSeatsAndWrite(claims, (tx) => tx.booking.create(…));
*
* Locks every seat across every leg, re-runs the full claim gate for each leg inside that
* lock (including the active-booking check), then runs `write` — all in one transaction, so
* a concurrent request either blocks and then sees this booking, or is seen by it.
*
* Call `assertSeatsClaimableAll` first, before the fare and identity work, so an obviously
* doomed request fails early instead of paying for a Verifayda round trip it will discard.
* That earlier pass is advisory only; this one decides.
*/
async claimSeatsAndWrite<T>(
claims: SeatClaim[],
write: (tx: Prisma.TransactionClient) => Promise<T>,
): Promise<T> {
const allSeatIds = claims.flatMap((c) => c.seatIds);
return this.withSeatsLocked(allSeatIds, async (tx) => {
for (const claim of claims) {
await this.assertSeatsClaimable({ ...claim, db: tx, alsoRejectActiveBookings: true });
}
return write(tx);
});
}
/** Advisory pre-flight for the same claims, outside any lock. See claimSeatsAndWrite. */
async assertSeatsClaimableAll(claims: SeatClaim[]): Promise<void> {
for (const claim of claims) {
await this.assertSeatsClaimable(claim);
}
}
/**
* Rejects seats that an active booking already owns on an overlapping stretch of this
* schedule.
*
* This is deliberately NOT part of assertNoRouteSeatConflict. That check is also run by
* holdSeats, and some flows (the backoffice reservation-issue path, reschedule, upgrade)
* legitimately re-hold a seat for a booking that already owns it — folding this in there
* would break them. Here it guards only the moment of writing a NEW booking, where no
* legitimate caller can already hold the seat.
*
* It exists because getSeatAvailabilityMap cannot see a pending booking: it reads
* SeatHolds and JourneySegments, and JourneySegments are only written on payment success
* (PaymentsService.createJourneySegments). Until then a PENDING_PAYMENT booking is
* represented by nothing but its hold — so any request whose own hold is skipped as
* "its own" saw a free seat and booked straight over it.
*/
async assertNoPendingBookingConflict(
db: PrismaClientLike,
args: { scheduleId: string; seatIds: string[]; reqFrom: number; reqTo: number; legLabel?: string },
): Promise<void> {
const { scheduleId, seatIds, reqFrom, reqTo, legLabel } = args;
if (!seatIds.length) return;
const rows = await db.bookingSeat.findMany({
where: {
scheduleId,
seatId: { in: seatIds },
booking: { status: { in: ACTIVE_BOOKING_STATUSES } },
},
select: {
seatId: true,
seat: { select: { seatNumber: true } },
booking: {
select: {
bookingRef: true, scheduleId: true,
originStationId: true, destinationStationId: true,
returnScheduleId: true, returnOriginStationId: true, returnDestinationStationId: true,
leg2ScheduleId: true, leg2OriginStationId: true, leg2DestinationStationId: true,
returnLeg2ScheduleId: true, returnLeg2OriginStationId: true, returnLeg2DestStationId: true,
},
},
},
});
if (!rows.length) return;
const stopTimes = await db.tripStopTime.findMany({
where: { scheduleId },
select: { stationId: true, sequence: true },
});
const seqOf = (id?: string | null) =>
id ? stopTimes.find(s => s.stationId === id)?.sequence : undefined;
const conflicts = new Map<string, string>();
for (const row of rows) {
const leg = resolveBookingLegRange(row.booking as any, scheduleId);
const from = seqOf(leg?.originStationId);
const to = seqOf(leg?.destinationStationId);
// Conservative: a row whose leg we cannot resolve blocks. Selling a berth twice is
// far worse than refusing one booking we could not prove safe.
const overlaps = from === undefined || to === undefined || (from < reqTo && reqFrom < to);
if (overlaps) {
conflicts.set(row.seatId, row.seat?.seatNumber ?? row.seatId);
}
}
if (conflicts.size > 0) {
const prefix = legLabel ? `${legLabel} ` : '';
throw new ConflictException(
`${prefix}seat(s) ${[...conflicts.values()].join(', ')} were just booked by someone else. ` +
`Pick another seat.`.trim(),
);
}
}
// Delegates the actual "is this seat held/booked for this leg" determination to
// SegmentsService.getSeatAvailabilityMap — the same canonical check search results
// (availabilityByClass) use — so the seatmap and search results can never disagree
@@ -217,11 +535,28 @@ export class SeatsService {
originStationId?: string;
destinationStationId?: string;
journeyDirection?: JourneyDirection;
/**
* Whoever is asking. OUTBOUND/RETURN on one schedule only stop conflicting when both
* legs belong to this same passenger — see getSeatAvailabilityMap.
*/
requestingPassengerId?: string;
/** Read through this client — a locking caller passes its own `tx`. */
db?: PrismaClientLike;
/**
* Also reject seats an active booking already owns. Off by default: holdSeats and the
* reschedule/upgrade/reservation flows legitimately re-hold a seat whose booking already
* exists. Only the moment of writing a NEW booking turns this on — see
* assertNoPendingBookingConflict.
*/
alsoRejectActiveBookings?: boolean;
/** Label used in error messages when a booking presents several holds (one per leg). */
legLabel?: string;
}): Promise<void> {
const { scheduleId, seatIds, originStationId, destinationStationId, journeyDirection = JourneyDirection.ONE_WAY } = args;
if (!seatIds.length) return;
const db = args.db ?? this.prisma;
const stopTimes = await this.prisma.tripStopTime.findMany({
const stopTimes = await db.tripStopTime.findMany({
where: { scheduleId },
select: { stationId: true, sequence: true },
});
@@ -245,11 +580,19 @@ export class SeatsService {
reqFrom,
reqTo,
journeyDirection,
args.requestingPassengerId,
db,
);
if (args.alsoRejectActiveBookings) {
await this.assertNoPendingBookingConflict(db, {
scheduleId, seatIds, reqFrom, reqTo, legLabel: args.legLabel,
});
}
if (availability.size === 0) return;
const seats = await this.prisma.seat.findMany({
const seats = await db.seat.findMany({
where: { id: { in: seatIds } },
select: { id: true, seatNumber: true },
});
@@ -413,6 +756,16 @@ export class SeatsService {
}
const hold = await this.prisma.$transaction(async (tx) => {
// Everything below is read-then-write — check no hold/segment covers these seats,
// then insert a hold — with no unique constraint behind it. Without a lock, N
// simultaneous requests all read "free" and all insert, which is how one berth ends
// up held (and then sold) several times over. Locking the Seat rows serializes those
// requests; ORDER BY id keeps two overlapping multi-seat requests taking the rows in
// the same sequence, so they queue instead of deadlocking. Held only for this
// transaction, so cross-schedule contention on a shared coach is negligible.
// Sorted so holds and booking writes take the same rows in the same sequence.
await this.lockSeatsForUpdate(tx, [...new Set(seatIds)].sort());
const seats = await tx.seat.findMany({
where: { id: { in: seatIds } },
select: { id: true, status: true, seatNumber: true },
@@ -483,6 +836,16 @@ export class SeatsService {
originStationId: dto.originStationId,
destinationStationId: dto.destinationStationId,
journeyDirection: currentDirection,
// Matches how SeatHold.passengerId is written below, so this passenger's own
// outbound hold doesn't block them from holding their return leg.
requestingPassengerId: dto.passengers[0]?.passengerId,
// MUST be tx, not the pool. This runs while the transaction already owns a pooled
// connection; reading through `this.prisma` would check out a SECOND one. With N
// concurrent holds that is 2N connections against a pool of N_max, so past roughly
// half the pool every transaction ends up waiting for a connection only another
// transaction can release — a pool deadlock that fails every request with a 500,
// not just the losers. Reproduced at 25 concurrent holds before this was passed.
db: tx,
});
const activeHolds = await tx.seatHold.findMany({
@@ -511,7 +874,14 @@ export class SeatsService {
const legsOverlap = legUnknown || (holdFrom < reqTo && reqFrom < holdTo);
if (!legsOverlap) continue;
const directionsConflict = checkDirectionConflict(currentDirection, holdDirection);
// OUTBOUND and RETURN coexist on one schedule only for a single traveller holding
// both legs of their own turnaround trip. Between two different travellers that
// exemption is just a double-sold berth, so it applies only to the requester's
// own holds.
const requestPassengerIds = new Set(dto.passengers.map((p) => p.passengerId));
const isOwnHold = passengerIds.some((id) => requestPassengerIds.has(id));
const directionsConflict = !isOwnHold || checkDirectionConflict(currentDirection, holdDirection);
if (!directionsConflict) continue;
for (const { passengerId, seatId } of dto.passengers) {
@@ -1364,12 +1734,15 @@ export class SeatsService {
});
const occupiedIds = new Set(journeySegments.map(js => js.seatId!));
// Group BookingSeat rows by seatId::leg to find candidate duplicates,
// then filter to only those whose booking segments actually overlap.
// Group BookingSeat rows by seatId to find candidate duplicates, then filter to
// only those whose booking segments actually overlap.
type BS = (typeof bookingSeats)[number];
const groups = new Map<string, BS[]>();
// Grouped by seat alone. `leg` is a per-booking notion — one booking's leg 1 and
// another's leg 2 are the same physical berth on this departure — so keying on it
// hid every round-trip-vs-one-way collision from this report.
for (const bs of bookingSeats) {
const key = `${bs.seatId}::${bs.leg}`;
const key = bs.seatId;
if (!groups.has(key)) groups.set(key, []);
groups.get(key)!.push(bs);
}
@@ -1463,16 +1836,20 @@ export class SeatsService {
}
if (overlapping.length <= 1) continue;
const [seatId] = key.split('::');
const seatId = key;
const seat = coach.seats.find(s => s.id === seatId);
duplicates.push({
seatId,
seatNumber: seat?.seatNumber ?? seatId,
// A group can now span legs (one booking's outbound against another's return),
// so the authoritative leg is the per-booking one below; this stays for
// backwards compatibility with existing callers.
leg: overlapping[0].leg,
bookings: overlapping.map(bs => ({
bookingSeatId: bs.id,
bookingId: bs.booking.id,
bookingRef: bs.booking.bookingRef,
leg: bs.leg,
passengerName: bs.passengerName,
contactPhone: bs.booking.contactPhone,
createdAt: bs.booking.createdAt,

View File

@@ -1,7 +1,15 @@
import { Injectable, BadRequestException } from '@nestjs/common';
import { Prisma } from '@prisma/client';
import { PrismaService } from '../../common/prisma.service';
import { JourneyDirection } from '../seats/seats.dto';
import { checkDirectionConflict } from '../../common/utils/journey-direction.utils';
/**
* Either the pooled client or a transaction-scoped one. A caller that has taken row locks
* must pass its own `tx`, or its "is this seat still free" read runs on a second connection
* outside the lock's transaction — burning a pool slot per in-flight booking and reading a
* snapshot the lock does not actually govern.
*/
export type PrismaClientLike = PrismaService | Prisma.TransactionClient;
export interface Segment {
fromStationId: string;
@@ -73,9 +81,10 @@ export class SegmentsService {
* P3: A(1) → D(4) reqFrom=1, reqTo=4
* Check P3 vs P2: 1 < 4 AND 2 < 4 → true AND true → CONFLICT ✓
*
* journeyDirection lets a round-trip's OUTBOUND and RETURN holds coexist on the
* same schedule without blocking each other (see checkDirectionConflict) — omit it
* for one-way contexts, where it defaults to ONE_WAY (conflicts with anything).
* A round-trip's OUTBOUND and RETURN holds coexist on the same schedule because they
* belong to the same traveller — pass requestingPassengerId so that traveller's own
* holds are skipped. Direction alone never grants that exemption: between two different
* travellers an OUTBOUND and a RETURN hold on one berth is a double sale.
*
* Sources checked:
* 1. Active SeatHolds — leg + direction decoded from createdBy JSON
@@ -93,7 +102,24 @@ export class SegmentsService {
stopTimesForSeqLookup: ReadonlyArray<{ stationId: string; sequence: number }>,
reqFrom: number,
reqTo: number,
/**
* Retained for call-site compatibility (this is a positional signature) and for the
* seatmap/search callers that still describe their leg. Hold conflicts no longer turn
* on it — see the own-hold rule below.
*/
journeyDirection: JourneyDirection = JourneyDirection.ONE_WAY,
/**
* The passenger asking. Their own active holds are skipped, which is what lets one
* traveller keep a berth across both legs of a turnaround round trip. Omit it in
* display contexts (seatmap, search) so every hold shows as taken.
*/
requestingPassengerId?: string,
/**
* Transaction client to read through. Defaults to the pooled client, which is right for
* every display caller; a caller holding `FOR UPDATE` locks must pass its own `tx` so the
* read happens on the locked connection.
*/
db: PrismaClientLike = this.prisma,
): Promise<Map<string, 'HELD' | 'BOOKED'>> {
const result = new Map<string, 'HELD' | 'BOOKED'>();
if (seatIds.length === 0) return result;
@@ -105,11 +131,11 @@ export class SegmentsService {
const now = new Date();
const [allHolds, bookedLegs] = await Promise.all([
this.prisma.seatHold.findMany({
db.seatHold.findMany({
where: { scheduleId, expiresAt: { gt: now } },
select: { seatIds: true, createdBy: true },
select: { seatIds: true, createdBy: true, passengerId: true },
}),
this.prisma.journeySegment.findMany({
db.journeySegment.findMany({
where: {
scheduleId,
seatId: { in: seatIds },
@@ -123,22 +149,29 @@ export class SegmentsService {
for (const hold of allHolds) {
let holdFrom: number | undefined;
let holdTo: number | undefined;
let holdDirection = JourneyDirection.ONE_WAY;
try {
if (hold.createdBy) {
const meta = JSON.parse(hold.createdBy as string);
holdFrom = seqOf(meta.originStationId);
holdTo = seqOf(meta.destinationStationId);
holdDirection = meta.journeyDirection || JourneyDirection.ONE_WAY;
}
} catch { /* ignore */ }
// A hold reserves the seat *for* whoever placed it, so it must never be read as an
// obstacle to that same person's own booking — including the other leg of their own
// round trip, which is what the OUTBOUND/RETURN exemption used to cover. Anyone
// else's hold blocks on leg overlap alone: direction is irrelevant between two
// different travellers, and treating OUTBOUND and RETURN as compatible there is
// exactly how one berth got sold to two people.
const isOwnHold =
requestingPassengerId != null && hold.passengerId === requestingPassengerId;
if (isOwnHold) continue;
for (const sid of hold.seatIds) {
if (!seatIdSet.has(sid)) continue;
// Conservative block if leg can't be resolved; otherwise check overlap.
const legsOverlap = holdFrom === undefined || holdTo === undefined || (holdFrom < reqTo && reqFrom < holdTo);
if (!legsOverlap) continue;
if (!checkDirectionConflict(journeyDirection, holdDirection)) continue;
result.set(sid, 'HELD');
}
}