Repository navigation
feat(booking): create bookings and hold seats, closing US-003, US-004 and EPIC-02 - #25
Merged
Merged
Conversation
This was
linked to
issues
Aug 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds booking-service and gives flight-service the write endpoint it needs to
answer. A passenger asks booking-service for seats on a flight; booking-service
asks flight-service to hold them, and only once they are held does a booking
exist. Both services keep the hexagonal shape of ADR-006, with three Maven
modules and a framework-free domain and application layer, and both are
exercised together by a new end-to-end module.
Note on the history: the first five commits of this branch reached
maindirectly by mistake and the branch was cut from there, so they appear here too.
They are the seat-blocking work this PR builds on.
Why this closes both stories
US-003, create a booking.
POST /api/v1/bookingstakes a passenger, aflight and a number of seats. The seats are held at flight-service first, and the
booking is written only if that succeeds, so "a booking is created for the user"
never means a booking without seats. It lands in
PENDING, which here means theseats are held and the fare is unpaid: the only state a booking can be in until
payment exists.
US-004, no overselling. The answer lives in flight-service, because that is
where the inventory is.
POST /api/v1/flights/{id}/seat-blocksreads the flightFOR UPDATE, reduces it and records the hold inside one transaction, so a secondrequest waits rather than reading a seat count that is about to change.
ConcurrentSeatBlockTestis what earns the claim: eight passengers after nineseats, two each, all released at once. Four are served, four are refused with a
409, and the flight never owes seats it does not have.
That test is also the only one that fails if
@UnitOfWorkis removed. Everyother test runs one request at a time, and a lock that is never contended is
indistinguishable from no lock.
Why they ship together. US-004 is not a feature with its own endpoint; it is
the correctness condition of US-003. Delivering the booking first would have
meant merging a seat count that two passengers could both spend, and the fix
would have touched the same transaction, the same lock and the same port. With
both merged, EPIC-02 has no remaining scope: every child story is closed, and
each was implementable and testable on its own. The seat blocking has its own
slice and concurrency tests in flight-service, and the booking has its own in
booking-service with the outbound port mocked.
Closes #4 (US-003)
Closes #5 (US-004)
Closes #6 (EPIC-02)
Ricardo's two issues
Final parameters. Applied across both services: 95 parameters in production
code. Left off interfaces and abstract methods, where
finalpromises somethingthe language does not keep, since the implementer decides. Local variables were
briefly caught by the same inspection and then stripped back out, since the issue
asked about parameters.
JSpecify. Applied as
@NullMarkedon sixteenpackage-info.javafilesrather than
@NonNullon each signature: with the package marked, every type isnon-null by default and
@Nullablemarks the exceptions. There are five, all onthe same path: an optional search filter travelling from the query string to the
Specification. That so few exist is itself the finding.
Objects.requireNonNullwas not added, because the records already validate intheir compact constructors and throw better messages than a bare NPE.
ADR-006 gains a criterion for what may live in the inner modules, since JSpecify
is the first annotation to sit near that line: an annotation may if it
describes code, may not if it generates code, changes an object's
lifecycle, or needs a runtime to interpret it. JSpecify passes; Lombok, Bean
Validation and MapStruct do not, for three different reasons.
Worth knowing before this is trusted: the annotations document, they do not
enforce. Nothing in the build checks them. NullAway would make them binding and
is the obvious next step.
Closes #23
Closes #24
Changes
SeatBlockas its own aggregate,BlockSeatsUseCase, awrite adapter under a pessimistic lock, and
POST /flights/{id}/seat-blocks.The response carries the fare, so the caller charges what it reserved.
Bookingwith its fare and total,
CreateBookingUseCase, a Feign client toflight-service with Resilience4j, and
POST /bookings.building the images from the same Dockerfiles the compose file uses.
Beyond the stories
Three things went in that neither story asked for:
response means either seats held twice or two bookings for one passenger.
Both take an
Idempotency-Keyheader; ADR-011 records what the full patternlooks like and which parts of it are missing here, chiefly that keys are
never purged.
of one instance. Splitting it makes "a service cannot read another's tables" a
fact rather than a convention, at the cost of one container each.
first run (below).
Notes
The bug the end-to-end tests found. booking-service was answering 502 where
flight-service had said 422. Spring 7 deprecated
HttpStatus.UNPROCESSABLE_ENTITYin favour of
UNPROCESSABLE_CONTENT, following the rename in RFC 9110;resolve()returns the new constant, so a decoder naming the old one nevermatched and fell through to the default. Every other test passed, because every
other test mocks the port the decoder sits behind. Status codes crossing a
service boundary are now compared as numbers, which do not get renamed.
Boot 4 and its neighbours moved a great deal. Recorded because the remaining
four services will hit the same wall:
spring-boot-starter-aopspring-boot-starter-aspectjSpringDataWebAutoConfigurationorg.springframework.boot.data.autoconfigure.web.DataWebAutoConfigurationorg.testcontainers:postgresqlorg.testcontainers:testcontainers-postgresqlarchunit-junit5archunit-junit6hibernate-jpamodelgenhibernate-processorcom.fasterxml.jackson..tools.jackson..(Jackson 3)flyway-corespring-boot-starter-flyway+flyway-database-postgresqlresilience4j-spring-boot3resilience4j-spring-boot4, and not in its own BOM, so the version is pinned by handHttpStatus.UNPROCESSABLE_ENTITYHttpStatus.UNPROCESSABLE_CONTENTSpring Cloud's release train is named for the year it opened, not the year of the
release:
2025.1.x(Oakwood) is what carries compatibility with Boot 4.1.Two decisions that a reviewer may want to push back on.
@UnitOfWorkis an annotation in the application layer, satisfied by an aspectin infrastructure delegating to a
TransactionRunner. ADR-009 explains why thealternatives failed: a boundary on the adapter releases the lock between port
calls, and pushing the whole operation behind one port method puts business rules
in the persistence layer.
CreateBookingServicedeliberately has no suchannotation, because its only local write must not span the HTTP call before it.
Bookinghas no behaviour: it is constructed and validated and nothing more.Confirming and expiring are the state changes it will grow, and both belong to
payment.
Testing
./mvnw -B clean verifygives 164 tests.Plus ten end-to-end, which are skipped by default:
The
packagefirst is not optional: the images copy a jar Maven has alreadybuilt.
Known gaps
save then failed leaves a hold nobody will claim. The expiry sweep ADR-008
anticipates is what would release it, and it arrives with payment.
key reused after any length of time returns the original booking.
once per service, and can drift silently. Contract testing is the intended
mitigation (ADR-003) and does not exist yet.
front of it; until then
PassengerIdis whatever the caller says. It is avalue object already, so only its source changes.