task: #3615268 Reading an order's lines depends on the service that writes them, so eight places re-derive it and three hydrate every abandoned hold

Reading an order's lines now has an owner, and it is a read-only service that depends on nothing.

The cycle this fixes

The write path is a chain: BookingManager is constructed with ConstraintPolicyManager, and TransactionManager is constructed with BookingManager. So a service that both writes orders and answers "which lines does this order hold" cannot be reached from inside that chain. ConstraintPolicyManager asks that question on every hold and sits squarely inside it, so injecting TransactionManager there is a circular reference the container refuses.

Every caller therefore answered the question itself: the same query in four places, twice as byte-identical private methods in different modules (BookingScheduleResolver and PaymentPolicyResolver), and the same predicate as an in-PHP filter in four more. That is how the amount a gateway charged and the amount a summary showed came to be computed by two separate traversals of one order, and to disagree ([#3615260]).

TransactionReader depends on entity_type.manager and nothing else, so nothing can cycle through it. consumingLines() and cancelledLines() move off TransactionManagerInterface onto it, and all six readers ask it, including ConstraintPolicyManager. Documented in docs/architecture.md, including the rule not to give the reader a dependency that writes.

Performance, measured on a production order

The worst real order carries 302 lines for 1 consuming one: picking seats leaves a long tail of released and expired holds.

queries booking entities hydrated
before, load-all-then-filter 2 302
after, scoped query 2 1

The state is in the query now instead of a filter after the load, so nothing hydrates a row it will discard. Three readers had that shape: TransactionSummary::currentLines(), WorkflowProfileResolver::typeOf() and the Arrival variable provider. cancelledLines() exists so the settled-order path is scoped too rather than reading the whole tail.

The accessors take an id, not the transaction, as lineCounts() already does: taking the entity cost 4 extra queries per call, because loading an order pulls its base table plus three booker address field tables.

transactionAmountDueNow() is unchanged in cost, with identical SQL (transaction = ? AND state IN (...)). No query condition was dropped anywhere; two readers gained one.

Residual cost, stated plainly: an order holding nothing pays one extra cheap query for the cancelled fallback, and still loads only cancelled rows.

Tests

TransactionReaderTest pins the set, the churn exclusion and the zero-indexing. Green locally: it plus PaymentPolicyResolverTest, TransactionSummaryTest, WorkflowProfileResolutionTest and TransactionQuantityLimitTest.

Not in scope

lineBookings(), lineCounts() and allLinesPast() still sit on TransactionManager. Moving them makes it write-only and is a follow-up, because it touches many callers across yoyaku_order and yoyaku_orchestra.

Edited by Frank Mably

Merge request reports

Loading