Move availability reading out of BookingManager, so a read does not cost the reader the locking

Problem/Motivation

BookingManager is 2,529 lines across 54 methods, and its size is the problem rather than a symptom of one: it is where a reader has to go to answer any question about the engine, so every question costs the whole file. #3618847: Let BookingManager read through the services it already injects, instead of keeping private copies took 113 lines out of it by giving the quota rule and the consuming sum owners of their own, which is deduplication and was never going to move this number. What moves it is that whole groups of methods in here belong to something other than the manager.

Sixteen of the 54 methods are public, and they are 603 lines. The other 38 are 1,801. Counting the methods by what they actually do, rather than by where they sit, gives four groups.

Reading availability, about 632 lines. availability(), availabilityMap(), findAvailability() and consumed(), with the fourteen private methods only they use: boundedAvailability(), boundedAtAll(), hasAvailableUnits(), lowerOf(), limitFor(), isBookableAt(), tariffsByResource(), allotmentFor(), allotmentOf(), withheldOn(), consumedFor() and consumedOnSlot(). None of it writes anything or takes a lock. It is a read, and it is the read every booking surface is built on.

Keeping the counters, about 296 lines, and this one turned out not to be extractable. anchorScopes(), scopesBounding(), startCounting(), tally(), countedOnSlot(), heldOn(), lockRow() and forgetReads() are each called by exactly one caller, and all eight callers are in the hold path. Three of them mutate the class's own consumption map, whose entire purpose is that what was read above the slot locks never answers below them. They are not a thing the engine has; they are how hold() works, and putting indirection through them to save 250 lines would run it through the most safety-critical code in the module. So this half is a finding rather than a task, and the estimate below is corrected accordingly.

The hold path, about 1,217 lines. hold() alone is 429, with holdInTransaction() at 122, placeUnderLocks() at 102, assertFits() at 98 and assignPartitions() at 82. This is the engine, and it is the part that has to stay in one place and stay readable in order.

The lifecycle transitions, about 216 lines. confirm(), release(), cancel(), complete(), markUnused(), expireLine() and the two settlement variants, most of which are a handful of lines around lockedMutate(), plus refusalFor() at 59.

Proposed resolution

Move the availability reading into a class of its own, so the reader asking how availability is computed no longer opens the file that holds the locking. That is 2,060 lines rather than 2,529, not the 1,600 first estimated here, because the counter bookkeeping is staying where it is for the reason above.

BookingManagerInterface and its 15 methods do not change. The four public reads stay on the manager as delegations, so nothing outside has to know the reading moved, and the availability reader is then usable on its own by anything that only reads.

Three of the nine private helpers the reads use are also asked by the hold path, which checks the same bounds before it takes a lock. Those become public on the reader rather than living on both sides.

What must not happen is a class that is a bag of the methods that were left over. An extraction has to be nameable as a thing the engine has, and what is available is one. Where a method does not belong to it, it stays, which is what the counters turned out to be.

Remaining tasks

Every step has to hold or improve the query count, proven with Database::startLog() and with the performance budgets the project already pins on real pages rather than argued. Moving a read behind a new collaborator is exactly the change that can add a lookup per call, and the budgets on the booking page and the place map are what would catch it.

hold() at 429 lines is not addressed by this and should not be: breaking it up is a change to the order things happen in under locks, which wants its own issue and its own load-rig evidence rather than riding along with a move.

There is a second cut this exposes, and it is an architectural decision rather than a task, so it is recorded here rather than assumed. Everything reachable from hold(), holdRequests() and holdInTransaction() is 33 methods and about 1,696 lines, and it touches the rest of the class at exactly two points: lockedMutate(), which five lifecycle transitions call, and assertNotInCheckout(), which cancel() calls. Moving that cluster as one unit would leave the manager at roughly 300 lines, a facade implementing the interface over a reader and a writer, with the lock ordering untouched because nothing inside the unit moves relative to anything else. What it would not do is make the engine smaller: the mass would sit in one cohesive class of about 1,750 lines instead of this one. Whether that is worth doing is a call about what the engine should be, not a cleanup.

User interface changes

None.

API changes

None to the engine's own interface. One class is added, and the availability reader becomes callable directly by a consumer that only reads.

Data model changes

None.

AI-Generated: Yes (Claude Code was used to take the inventory this issue quotes, to help draft this summary, and to write the code on the merge request. The linters and the named test classes were run before it was pushed; a maintainer reviews it before it merges.)