>>> [!note] Migrated issue <!-- Drupal.org comment --> <!-- Migrated from issue #3618847. --> Reported by: [mably](https://www.drupal.org/user/3375160) Related to !301 >>> <h3>Problem/Motivation</h3> <p>The same duplication audit as <span class="drupalorg-gitlab-issue-link drupalorg-gitlab-link-wrapper"><a href="https://git.drupalcode.org/project/yoyaku/-/work_items/3618835" class="drupalorg-gitlab-link">https://git.drupalcode.org/project/yoyaku/-/work_items/3618835</a></span> found that <code>BookingManager</code> keeps private copies of knowledge that services it already injects own. This is why the class is 2,644 lines, but the size is the symptom: a capacity rule written four times is a rule three of whose copies can be missed.</p> <p><strong>A tariff's effective quota on a slot is stated four times and owned nowhere.</strong> A slot tariff row overrides the resource tariff's quota, and where there is no such row the resource tariff's own quota stands. That one sentence is written out in four places: <code>BookingConsumption::quotaFor()</code> and <code>CapacityLockAnchorScopes::quotaFor()</code> are byte-identical, docblock included; <code>BookingManager::limitFor()</code> is the same expression behind a NULL-tariff branch; and <code>BookingManager</code> states it again inline where it walks a page of slots. So a correction to the fallback has to be found in three files to be complete, and this is the number every capacity decision is taken against.</p> <p>Two things nearby look like the same rule and are not, which is worth saying so they are not folded in by mistake. The allotment is resolved by both <code>BookingManager::allotmentFor()</code> and yoyaku_placement's <code>PinnedPlaceHold::allotmentFor()</code>, but not the same way: the second guards on <code>hasField('allotment')</code> and falls back to the raw <code>target_id</code> where the first calls <code>getAllotmentId()</code>. Reconciling those two is a decision about behavior rather than a move. And the two price reads in yoyaku_payment's <code>PaymentPolicyResolver</code> take the override alone, with no fallback at all, because the tariff's own price is applied further on inside <code>policyFor()</code>. They are a different shape and are not part of this.</p> <p><strong>What the booking rows have taken is assembled six times.</strong> <code>sumConsuming()</code>, <code>sumConsumingGrouped()</code> and <code>sumConsumingByTariffAndAllotment()</code> in <code>BookingManager</code>, then <code>CapacityLockAnchorCounts</code>, and then twice more in yoyaku_placement, in <code>PlacementLockAnchorCounts</code> and <code>PinnedPlaceHold</code>. Every one of them needs the same three facts, the booking table, the <code>SUM(quantity)</code> measure and the filter on <code>CONSUMING_STATES</code>, and they differ only in what they group by. So a fact about what a consuming line is has to be found in six places across three modules, and two of those six are summing something the engine already sums.</p> <p><strong>Two private methods that only forward.</strong> <code>BookingManager::forTariff()</code> and <code>BookingScheduleResolver::consumingLines()</code> each wrap one call to an injected service under the same name it already has, so they add nothing but a second place to look. <code>bookingStorage()</code> and <code>transactionStorage()</code> read the same way at a glance but are not the same thing: they have four callers and two, so removing them would repeat a storage name four times and twice, which is the duplication this is about. They stay.</p> <h3>Proposed resolution</h3> <p>Give the effective quota one owner on <code>SlotTariffLookup</code>, which already owns the <code>forTariff()</code> lookup three of the four sites call first. Two entry points, because the callers differ in what they hold: one taking the slot and the tariff, for a caller that has to look the row up, and one taking an already-loaded row and the tariff, for the caller that walks a page of slots with the rows preloaded. The second is what keeps this from costing a lookup per tariff per slot, which is the way a change like this silently makes things worse. The rule itself lives in one of them and the other calls it. Keep the return type honest about a quota that is absent on both sides, since an unset quota means unbounded and not nought.</p> <p>Give the sum one owner stating those three facts once, with the groupings the engine needs on it. A module bounding a scope of its own groups these rows by a column the engine has no business naming, so the statement itself is reachable for a reader to narrow and group, rather than restated. Then nothing outside that owner says what a consuming line is.</p> <p>Drop the forwarding methods and call the collaborator.</p> <h3>Remaining tasks</h3> <p>Every step has to hold or improve the query count, proven with <code>Database::startLog()</code> rather than argued, because collapsing four inline reads into one shared method is exactly the kind of change that can add a lookup per call. The existing performance test classes cover the paths involved and stay the check on this.</p> <p>Whether to reconcile the two allotment resolutions is left open here rather than assumed, since the two differ in what they do when a tariff has no allotment field at all.</p> <p>Decomposing the class along the seams this leaves, into reading availability, keeping the capacity accounting, and the booking lifecycle, is a follow-up rather than part of this: what is here is the reuse the title names, and it is what makes those seams visible. <code>BookingManagerInterface</code> and its 15 methods do not move either way.</p> <h3>User interface changes</h3> <p>None.</p> <h3>API changes</h3> <p>None to the engine's own interface. <code>SlotTariffLookup</code> gains the effective-quota read in two forms, and the sum's owner is new; both are additions.</p> <h3>Data model changes</h3> <p>None.</p> <p>AI-Generated: Yes (Claude Code was used to run the duplication audit behind this issue, to help draft this summary, and to write the code and tests on the merge request. The linters and the named test classes were run before it was pushed; a maintainer reviews it before it merges.)</p>