Move the hold path into an engine of its own, so BookingManager is the contract rather than the implementation

Problem/Motivation

#3618860: Move availability reading out of BookingManager, so a read does not cost the reader the locking moved the availability reading out, and BookingManager is 2,060 lines. What is left is not a manager with some methods on it: it is one large thing and eleven small ones. Everything reachable from hold(), holdRequests() and holdInTransaction() is 33 methods and 1,696 lines, and the other twelve methods are 210 lines, most of them a handful of lines each around a single call.

So the class is really two things wearing one name: the contract the rest of the site holds, which is BookingManagerInterface and its fifteen methods, and the engine that takes a hold under locks, which is what almost all of the file is. Anything wanting the first has to open the second.

The two touch at exactly two points, which is what makes this a move rather than a rewrite. lockedMutate() is called by five lifecycle transitions (confirm(), complete(), markUnused(), expireLine() and cancelForSettlement()), and assertNotInCheckout() is called by cancel(). Nothing else crosses.

Proposed resolution

Move the cluster into an engine of its own, as one unit. Nothing inside it moves relative to anything else, so the order things happen in under locks is exactly what it is today: the same statements in the same sequence, in a file that contains only them. That is the property that makes this safe to do and the reason it must be done as a move and not as a tidy-up.

The two crossing methods become public on the engine, and the transitions that call them call them there. BookingManagerInterface and its fifteen methods do not change, and the manager keeps implementing it, so nothing outside the engine has to know where the work went. On the figures above the manager is then roughly 300 lines.

What this does not do is make the engine smaller. The mass sits in one cohesive class of about 1,750 lines rather than in this one, and that is the honest description of the change: it separates the contract from the implementation, and it puts everything that writes a booking under locks in one place, beside the class that already owns everything that reads one. It is not a way of making hold() shorter.

Remaining tasks

hold() is 429 lines and this does not touch it. Breaking it up is a change to the order things happen in under locks, which needs its own issue and its own evidence from the load rig rather than riding along with a move. Doing the move first is what leaves it in a file where it is the subject rather than a passage.

The move has to be checked rather than trusted: every method, constant and property that exists before has to exist in exactly one of the two files after, and the performance budgets pinned on real pages are what would catch a delegation that started costing a query.

User interface changes

None.

API changes

None to the engine's own interface. One class is added, and two of its methods are public because the lifecycle transitions call them.

Data model changes

None.

AI-Generated: Yes (Claude Code was used to take the call-graph measurements this issue quotes and to help draft this summary. I reviewed it before posting; there is no code on this issue yet.)