Problem/Motivation
HoldEngine::hold() was 443 lines, 169 of them code, written as one unbroken run. Read by what it does it is four things and then the locked section: parse the lines and confirm the group may be held, work out which held lines are being given up, ask every rule about it before any lock, work out the rows it will wait on. Three of those four had no name, so the try block that matters most read as one more paragraph a hundred and eighty lines down.
The second half is vocabulary. Much of this engine is named in words this project invented where Drupal core already had one, so a contributor arriving has to learn a vocabulary for things they already know. An audit of every method-name verb against core found the inventions: prime, primer and primed (about seventy occurrences, and zero in core), tally (zero in core), and forget, where core says reset. Class suffixes were already core's and needed nothing.
Proposed resolution
Name the three phases, as private methods on the same class. Nothing leaves HoldEngine and the class does not shrink: what changes is that hold() reads as four steps and then the transaction, and each phase's paragraph comment becomes its docblock. All three run before the first lock, which is what makes it safe; the try block and its catch are not split.
Then say what Drupal says. preload for reading a set up front, as preloadPathAlias() does. add for recording into running state, as addCacheableDependency() and addConstraint() do. reset for dropping what was read, in core's own shape: one reset(?int $slot_id = NULL) where NULL means everything, rather than a scoped method beside a wholesale one.
Two of the methods called forget() are not caches at all: they run DELETE against a table for one slot. Those become deleteForSlot(), with no default, because core says delete for deleting rows and because a NULL-means-everything reset() there would empty the table.
GroupPrimedConstraintInterface becomes LineGroupConstraintInterface. Group alone never said group of what, and Group is a contrib module a Drupal developer already knows as groups of users. The interface has three moments, only one of which preloads: the other two count each line as it is saved, so a request naming one seat twice cannot pass twice, and drop everything at the lock boundary. Naming it after preloading would have named a third of it.
Remaining tasks
Behaviour is identical, which for the extraction means the statements move without being rewritten, and for the renames means no signature changes but the names. Two @api interfaces and a service tag are renamed; every implementation is in this project and moves with them.
User interface changes
None.
API changes
HoldPassPrimerInterface becomes HoldPreloaderInterface and GroupPrimedConstraintInterface becomes LineGroupConstraintInterface, with the methods they declare and the yoyaku.hold_pass_primer tag. BookingClientInterface::holdGroup() becomes holdLines(). A module implementing any of them renames its methods to match.
Data model changes
None.
AI-Generated: Yes (Claude Code was used to run the vocabulary audit 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.)
Issue fork yoyaku-3618881
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
Comment #3
mably commentedComment #4
mably commentedComment #6
mably commented