From 64b362ff51227d7fed8a9d1c9ae5136b49b5ef3e Mon Sep 17 00:00:00 2001 From: Roba Boru Date: Wed, 15 Jul 2026 08:13:23 +0300 Subject: [PATCH] Fix expired seat --- .../src/modules/seats/seats.service.ts | 87 +++++++++++++++---- .../segments/enhanced-seats.service.ts | 72 ++++++++++++--- 2 files changed, 132 insertions(+), 27 deletions(-) diff --git a/apps/edr-passenger-api/src/modules/seats/seats.service.ts b/apps/edr-passenger-api/src/modules/seats/seats.service.ts index 3d485d22c..ab0791352 100644 --- a/apps/edr-passenger-api/src/modules/seats/seats.service.ts +++ b/apps/edr-passenger-api/src/modules/seats/seats.service.ts @@ -1,4 +1,4 @@ -import { Injectable, ConflictException, NotFoundException, BadRequestException } from '@nestjs/common'; +import { Injectable, ConflictException, NotFoundException, BadRequestException, Logger } from '@nestjs/common'; import { PrismaService } from '../../common/prisma.service'; import { HoldSeatsDto, JourneyDirection } from './seats.dto'; import { Cron, CronExpression } from '@nestjs/schedule'; @@ -7,6 +7,8 @@ import { SystemConfigService, CONFIG_KEYS } from '../system-config/system-config @Injectable() export class SeatsService { + private readonly logger = new Logger(SeatsService.name); + constructor( private prisma: PrismaService, private segmentsService: SegmentsService, @@ -891,30 +893,83 @@ export class SeatsService { ); } + // Runs every minute, but is also safe to call on-demand (e.g. right after a hold's + // TTL is read back to the client) — expiresAt/now are both absolute UTC instants + // (Date objects, not wall-clock strings), so this is correct regardless of the + // server's or a client's local timezone; there's no wall-clock parsing involved. @Cron(CronExpression.EVERY_MINUTE) async expireHolds() { + try { + const result = await this.expireHoldsCore(); + if (result.expiredHolds > 0) { + this.logger.log( + `Expired ${result.expiredHolds} hold(s): released ${result.releasedSeatIds.length} seat(s), ` + + `skipped ${result.skippedSeatIds.length} still held by another active hold on the same schedule`, + ); + } + } catch (error) { + // A failed run must not crash the process or silently go unnoticed — the next + // scheduled run one minute later will retry the same (still-expired) holds, + // since nothing here is deleted/updated until the queries above succeed. + this.logger.error('Failed to expire seat holds', error instanceof Error ? error.stack : error); + } + } + + async expireHoldsCore(now: Date = new Date()): Promise<{ + expiredHolds: number; + releasedSeatIds: string[]; + skippedSeatIds: string[]; + }> { const expired = await this.prisma.seatHold.findMany({ - where: { expiresAt: { lt: new Date() } }, - select: { id: true, seatIds: true }, + where: { expiresAt: { lt: now } }, + select: { id: true, scheduleId: true, seatIds: true }, }); - if (expired.length === 0) return; + if (expired.length === 0) { + return { expiredHolds: 0, releasedSeatIds: [], skippedSeatIds: [] }; + } - const expiredSeatIds = expired.flatMap(h => h.seatIds as string[]); - - // Only reset seats that have no remaining active holds - const stillHeld = await this.prisma.seatHold.findMany({ - where: { expiresAt: { gte: new Date() }, seatIds: { hasSome: expiredSeatIds } }, - select: { seatIds: true }, + // Still-active holds — scoped per (scheduleId, seatId), not just seatId. The same + // physical Seat row is reused across every recurring date a coach runs, so the + // same seatId legitimately appears in unrelated holds for other schedules; without + // this scoping, an unrelated active hold on a DIFFERENT schedule would wrongly + // block release of a seat whose hold expired on THIS schedule, leaving it stuck at + // status 'HELD' indefinitely. + const activeHolds = await this.prisma.seatHold.findMany({ + where: { expiresAt: { gte: now } }, + select: { scheduleId: true, seatIds: true }, }); - const stillHeldIds = new Set(stillHeld.flatMap(h => h.seatIds as string[])); - const toRelease = expiredSeatIds.filter(id => !stillHeldIds.has(id)); + const stillHeldKeys = new Set( + activeHolds.flatMap(h => (h.seatIds as string[]).map(seatId => `${h.scheduleId}:${seatId}`)), + ); - if (toRelease.length > 0) { + const releasedSeatIds = new Set(); + const skippedSeatIds = new Set(); + for (const hold of expired) { + for (const seatId of hold.seatIds as string[]) { + if (stillHeldKeys.has(`${hold.scheduleId}:${seatId}`)) { + skippedSeatIds.add(seatId); + } else { + releasedSeatIds.add(seatId); + } + } + } + + if (releasedSeatIds.size > 0) { await this.prisma.seat.updateMany({ - where: { id: { in: toRelease }, status: 'HELD' }, - data: { status: 'AVAILABLE' }, + where: { id: { in: Array.from(releasedSeatIds) }, status: 'HELD' }, + // heldUntil is cleared alongside status — leaving a stale (past) heldUntil on an + // AVAILABLE seat is stale data that any future code reading heldUntil directly + // (instead of re-deriving availability live) would misinterpret. + data: { status: 'AVAILABLE', heldUntil: null }, }); } - await this.prisma.seatHold.deleteMany({ where: { expiresAt: { lt: new Date() } } }); + + await this.prisma.seatHold.deleteMany({ where: { expiresAt: { lt: now } } }); + + return { + expiredHolds: expired.length, + releasedSeatIds: Array.from(releasedSeatIds), + skippedSeatIds: Array.from(skippedSeatIds), + }; } } diff --git a/apps/edr-passenger-api/src/modules/segments/enhanced-seats.service.ts b/apps/edr-passenger-api/src/modules/segments/enhanced-seats.service.ts index 406c9e61b..bd99f1ceb 100644 --- a/apps/edr-passenger-api/src/modules/segments/enhanced-seats.service.ts +++ b/apps/edr-passenger-api/src/modules/segments/enhanced-seats.service.ts @@ -1,4 +1,4 @@ -import { Injectable, BadRequestException, ConflictException } from '@nestjs/common'; +import { Injectable, BadRequestException, ConflictException, Logger } from '@nestjs/common'; import { PrismaService } from '../../common/prisma.service'; import { SegmentsService, Segment } from '../segments/segments.service'; import { EventEmitter2 } from '@nestjs/event-emitter'; @@ -18,6 +18,8 @@ export interface BookingConfirmRequest { @Injectable() export class EnhancedSeatsService { + private readonly logger = new Logger(EnhancedSeatsService.name); + constructor( private prisma: PrismaService, private segmentsService: SegmentsService, @@ -57,6 +59,15 @@ export class EnhancedSeatsService { }, }); + // Mirrors SeatsService.holdSeats() — without this, a seat held through this path + // reads back as status 'AVAILABLE' in the DB despite being actively held, which is + // wrong for any consumer that trusts `status` directly instead of re-deriving + // availability live from SeatHold. + await tx.seat.updateMany({ + where: { id: { in: request.seatIds } }, + data: { status: 'HELD', heldUntil: expiresAt }, + }); + this.eventEmitter.emit('seats.held', { holdId: seatHold.id, scheduleId: request.scheduleId, seatIds: request.seatIds, segments }); return { holdId: seatHold.id, expiresAt, segments, seats: request.seatIds }; }); @@ -157,19 +168,58 @@ export class EnhancedSeatsService { }); } - async expireHolds() { - return this.prisma.$transaction(async (tx) => { - const expiredHolds = await tx.seatHold.findMany({ where: { expiresAt: { lt: new Date() } } }); - const expiredSeatIds = expiredHolds.flatMap(h => h.seatIds); + // now/expiresAt are absolute UTC instants (Date objects), not wall-clock strings, so + // this comparison is correct regardless of the server's local timezone. + async expireHolds(now: Date = new Date()) { + try { + const result = await this.prisma.$transaction(async (tx) => { + const expiredHolds = await tx.seatHold.findMany({ where: { expiresAt: { lt: now } } }); + if (expiredHolds.length === 0) { + return { expiredHolds: 0, releasedSeats: [] as string[] }; + } - if (expiredSeatIds.length > 0) { - await tx.seat.updateMany({ where: { id: { in: expiredSeatIds } }, data: { status: 'AVAILABLE', heldUntil: null } }); - await tx.seatHold.deleteMany({ where: { expiresAt: { lt: new Date() } } }); - this.eventEmitter.emit('holds.expired', { expiredHolds: expiredHolds.length, releasedSeats: expiredSeatIds }); + // Still-active holds — scoped per (scheduleId, seatId). The same physical Seat + // row is reused across every recurring date a coach runs, so the same seatId can + // legitimately appear in an unrelated hold for a different schedule; without this + // scoping, that unrelated hold would wrongly be treated as covering THIS + // schedule's seat too, and a seat still genuinely held (same schedule, a newer + // non-expired hold) could be released out from under it. + const activeHolds = await tx.seatHold.findMany({ where: { expiresAt: { gte: now } } }); + const stillHeldKeys = new Set( + activeHolds.flatMap(h => h.seatIds.map(seatId => `${h.scheduleId}:${seatId}`)), + ); + + const releasedSeatIds = new Set(); + for (const hold of expiredHolds) { + for (const seatId of hold.seatIds) { + if (!stillHeldKeys.has(`${hold.scheduleId}:${seatId}`)) releasedSeatIds.add(seatId); + } + } + + if (releasedSeatIds.size > 0) { + await tx.seat.updateMany({ + where: { id: { in: Array.from(releasedSeatIds) } }, + data: { status: 'AVAILABLE', heldUntil: null }, + }); + } + await tx.seatHold.deleteMany({ where: { expiresAt: { lt: now } } }); + + return { expiredHolds: expiredHolds.length, releasedSeats: Array.from(releasedSeatIds) }; + }); + + if (result.expiredHolds > 0) { + this.logger.log(`Expired ${result.expiredHolds} hold(s), released ${result.releasedSeats.length} seat(s)`); + this.eventEmitter.emit('holds.expired', { expiredHolds: result.expiredHolds, releasedSeats: result.releasedSeats }); } - return { expiredHolds: expiredHolds.length, releasedSeats: expiredSeatIds }; - }); + return result; + } catch (error) { + // A failed run must not go unnoticed — nothing is deleted/updated until the + // transaction commits, so the next caller/scheduled run simply retries the same + // still-expired holds. + this.logger.error('Failed to expire seat holds', error instanceof Error ? error.stack : error); + return { expiredHolds: 0, releasedSeats: [] as string[] }; + } } async getSeatAvailability(scheduleId: string, originStationId: string, destinationStationId: string) {