feat(backend): concurrency-safe appointment booking API with slot reservation (#24)

[balisdev]

Closes #24

The problem

Two gaps, both visible from the frontend side of this issue.

Nothing stopped a double-booking. POST /api/v1/client/bookAppointment inserted an appointment unconditionally. Two clients posting the same skilledWorkerId and scheduleTime at the same moment both succeeded, and the worker found out later.

Nothing could see a worker's calendar. GET /api/v1/client/viewAllAppointment returns the calling client's own bookings, so a booking UI had no way to ask what is already taken on a worker's calendar. The calendar in guildworkman-web#34 worked around that with a client-side lock (localStorage + BroadcastChannel, 5-minute TTL) in src/lib/slotLock.ts — which stops one visitor double-booking themselves across tabs, but cannot stop two different visitors racing, because neither browser can see the other's state.

How double-booking is prevented

A new slot_reservations table and a reserve → confirm flow under /api/v1/booking. Three layers, in order — the first is the guarantee, the other two exist so it stays true as the code changes.

1. A per-worker lock, taken before anything is read. SELECT id FROM skilled_workers WHERE id = ? FOR UPDATE. The ordering is the entire point: an overlap query run before the lock reads a snapshot from before the competing writer committed, which is exactly how two concurrent bookings both conclude a slot is free. Taking the lock first means the second request blocks inside Postgres until the first commits, then re-checks and sees what the winner took.

Per worker, so different workers never contend. Held across one query and one insert, with no network calls inside the critical section. Cannot deadlock — a booking locks exactly one worker row and never a second.

2. A half-open [start, end) overlap check, under that lock. This is what catches partial overlaps (10:00–11:00 vs 10:30–11:30), which no equality-based constraint can express. Half-open means back-to-back slots are fine.

3. A unique index as a backstop. active_slot_key holds "<workerId>@<slotStart>" while a reservation occupies the slot and is NULL the moment it stops; Postgres treats NULLs in a unique index as distinct, so any number of released rows can coexist for one slot while at most one active row can. Under the lock it should never fire — it's there so a future path that forgets the lock is rejected by the database instead of silently double-booking.

Why pessimistic rather than optimistic locking: an optimistic/CAS scheme needs an existing row to contend over. The race here is between two inserts — there is no shared row whose @Version could clash, so a version check has nothing to fail on.

Holds

A hold is authoritative and short-lived (5 minutes, matching the TTL the web calendar already used). It's what lets a visitor fill in booking details without racing anyone. Reclaimed two ways, neither the only line of defence: inline on the reserve path (under the lock, before the overlap check — so a slot is never blocked by a dead hold regardless of when the sweep ran) and by a background sweep every 30s, which claims rows FOR UPDATE ... ORDER BY id so two app instances can't process the same row or deadlock.

reserve is idempotent on a caller-supplied key, including the subtle case: a concurrent retry queues behind the original on the worker lock and would otherwise be told it lost a race against itself. SlotReservationService.reserve re-reads the key when a claim fails and replays the winner if the row turns out to be the caller's own; anything else stays a 409. It's deliberately not @Transactional — a unique violation marks the surrounding transaction rollback-only, so the recovery read has to happen after it ends.

Every path that changes a slot goes through the guard

A guard that only covers creation isn't a guard. So:

• bookAppointment (the one-step endpoint) claims through the same lock and overlap check, writing a reservation that's CONFIRMED from the start. The claim happens before the appointment is saved — not just for early failure, but because an appointment saved first shows up in its own overlap query and conflicts with itself.
• updateAppointment with a startTime is a booking of a different slot, so it claims the new one and gives the old one back. The appointment is excluded from its own conflict check (it still carries its old scheduleTime, so a 10:00 → 10:30 shift would otherwise conflict with itself). Fails 409 if the new time is taken, and the whole update rolls back rather than half-applying.
• updateAppointment to CANCELLED/DECLINED, cancelAppointment, deleteAppointment release the reservation — otherwise a declined job would block the worker's time forever.

Bookings with no skilledWorkerId are unguarded, because there's no calendar to double-book.

The availability endpoint

GET /api/v1/booking/workers/{workerId}/availability?from=&to= — the counterpart to viewAllAppointment the calendar was missing.

It reports what is taken, not what is free: free time is whatever a worker's published working hours leave over, which is a frontend (and future working-hours feature) concern. Live holds come back as state: "HELD" with a holdExpiresAt, so a calendar can distinguish "booked" from "someone is mid-checkout".

It unions reservations with appointments that predate this feature — those have no reservation row but still occupy the calendar. An Appointment stores only a scheduleTime, so each legacy row is treated as occupying [scheduleTime, scheduleTime + slot-duration). A slot already covered by a CONFIRMED reservation is reported once, from the reservation.

Endpoints

Method	Path	Purpose
POST	/api/v1/booking/reservations	Hold a slot (201; X-Idempotent-Replay header)
GET	/api/v1/booking/reservations/{id}	Current state
POST	/api/v1/booking/reservations/{id}/confirm	Turn a hold into an appointment
DELETE	/api/v1/booking/reservations/{id}	Release a hold early
GET	/api/v1/booking/workers/{id}/availability	A worker's taken slots in a window

All documented in OpenAPI. Errors follow the existing RFC 7807 contract — losing a race is 409 slot-unavailable, not a 400: the request was well-formed, it just arrived second.

Schema

No migration file, and none needed — this codebase has no Flyway/Liquibase and manages schema via ddl-auto=update. slot_reservations is declared as JPA annotations like every other table here, same as #21 and #22 did for their new tables. It's an entirely new table, so the first deploy creates it and every deploy after is a no-op for it; no existing table or column is altered.

Two things worth flagging

Booking is public. /api/v1/booking/** mirrors the flow it replaces: /api/v1/client/bookAppointment is already public, and a calendar has to read taken slots before anyone signs in. Tightening the booking surface means tightening the client surface with it — a deliberate auth-scope change rather than something to fold into a concurrency fix. Noted as a follow-up in the docs.

One unrelated fix. db/data.sql (used by SkilledWorkerServiceTest via @Sql) seeds hard-coded ids — skilled_workers 201-204, clients 301-304 — without realigning the identity sequences. Postgres GENERATED BY DEFAULT AS IDENTITY columns don't advance their sequence for an explicit id, and TRUNCATE without RESTART IDENTITY doesn't reset it, so any later test that lets Hibernate generate an id gets a duplicate-key violation once the counter walks into the seeded range. The booking tests hit it because they create their own workers and clients. The script now ends with setval(pg_get_serial_sequence(...), MAX(id)) per seeded table. It was previously invisible only because CI starts from an empty database each run.

Testing

166 tests pass; ./mvnw verify and ./mvnw test are green against a fresh postgres:16-alpine, and the suite was also run twice consecutively against a dirty database to check it isn't order-dependent.

• SlotReservationIntegrationTest (28) runs against a real Postgres, because that's the only place the guarantee lives — a mocked repository would happily "pass" while double-booking in production. Covers the 12-thread race for one slot (exactly one winner, eleven 409s), the same race through bookAppointment, 8 concurrent retries of one request taking a single hold, per-worker isolation, partial and adjacent overlaps, legacy appointments, expiry, cancel/decline/reschedule, and the availability read.
• BookingControllerTest (13) — HTTP contract: status codes, the replay header, problem-JSON shape for every failure.
• SlotReservationServiceTest (12) — replay, race recovery, confirm/release state checks, mocked.
• ReserveSlotRequestValidationTest (7) — bean-validation rules.

CI already caches Maven dependencies via setup-java's cache: maven in .github/workflows/test.yml, so that task was already satisfied. The sweep's poll delay is pinned high in the Surefire config alongside the existing chain-event and escrow pollers, so it can't race tests through the shared test database.

New dependencies

None.

Full design write-up: backend-api/docs/APPOINTMENT_BOOKING.md.

Follow-ups (out of scope)

• Retire src/lib/slotLock.ts in guildworkman-web once the calendar reads /availability and reserves through this API.
• Worker working hours — the server knows what's taken, not when a worker is available.
• Authentication scope, as above.
• A Postgres EXCLUDE USING gist constraint would make overlaps impossible at the schema level rather than under an application-held lock, but ddl-auto=update can't express it — needs the Flyway discussion the escrow doc already raises.

[meshackyaro]

Well done and thanks for your contribution