Problem/Motivation

The write path is a chain: BookingManager is constructed with ConstraintPolicyManager, and TransactionManager is constructed with BookingManager. TransactionManager is also where "which lines does this order hold" lives, alongside lineBookings() and lineCounts().

So a read-only caller must depend on the whole write path, and one caller cannot: ConstraintPolicyManager asks that question on every hold and sits inside that chain, so injecting TransactionManager there is a circular service reference the container refuses.

Every caller therefore answered the question itself. The same query appears in four places, twice as byte-identical private methods in different modules (BookingScheduleResolver and PaymentPolicyResolver, docblock included), and the same predicate as an in-PHP filter in four more (TransactionSummary, TransactionManager::lineCounts(), WorkflowProfileResolver, Arrival). BookingInterface::CONSUMING_STATES reaches a query condition in twelve places module-wide.

That is not only repetition. #3615260: An order's charged amount ignores the line quantity, so a two-ticket line is charged as one was an arithmetic bug in one of these traversals which survived because the traversal beside it was separate code with its own tests: the gateway and the summary answered about the same order and only one multiplied by the line quantity. The duplication is the symptom; the cycle is the cause.

Three of the eight also load rows they discard. Reading every line to keep the ones that count hydrates the whole tail of released and expired holds a booker leaves behind while choosing seats. One production order carries 302 lines for 1 consuming one.

Proposed resolution

Give the read its own service, TransactionLookup, depending on entity_type.manager and nothing else. Nothing can cycle through it, so every reader can simply ask, including the ones the write path constructs.

consumingLines() and cancelledLines() live there, both scoping the state in the query. They take an id rather than the transaction, as lineCounts() does: taking the entity costs four extra queries per call, because loading an order pulls its base table plus three booker address field tables.

Measured on that 302-line order: before, load-all-then-filter, 2 queries and 302 booking entities hydrated; after, scoped query, 2 queries and 1.

No query condition is dropped anywhere; two readers gain one. An order holding nothing pays one extra cheap query for the cancelled fallback, and still loads only cancelled rows.

Remaining tasks

Follow-up: move lineBookings(), lineCounts() and allLinesPast() onto the lookup too, leaving TransactionManager write-only by construction. Separate, because it touches many callers across yoyaku_order and yoyaku_orchestra.

User interface changes

None.

API changes

TransactionLookup is new. consumingLines() and cancelledLines() are not on TransactionManagerInterface.

Data model changes

None.

Issue fork yoyaku-3615268

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

mably created an issue. See original summary.

mably’s picture

Title: Nothing owns "the lines of an order that count", so it is re-derived eight times, twice as identical copy-paste » Reading an order's lines depends on the service that writes them, so eight places re-derive it and three hydrate every abandoned hold
Issue summary: View changes

  • mably committed 43ab0877 on 1.x
    task: #3615268 Reading an order's lines depends on the service that...
mably’s picture

Status: Active » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • mably committed 48534dcc on 1.x
    follow-up: #3615268 Reading an order's lines depends on the service that...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.