Issue #3618873: Give a hold line a type, so its shape is stated once instead of in seven docblocks

A line of a hold was an array with six string keys, described by hand in seven docblocks across two modules and assembled in the engine. Nothing checked that the seven agreed, and every reader went back to keys that might be wrong. HoldLine states the shape once.

Where the parse happens

HoldLine::fromArray() does what hold() used to do inline: refuse a line with no slot, a quantity below one, a tariff that is not a tariff, fields that are not LineFields, flags that are not an array. What may be booked stays in the engine, because the tariff belonging to the resource, the channel it is offered on and the tenant the group is in all take more than the line to know.

What a caller passes is still an array. BookingManagerInterface, RequestBooker, holdRequests() and the placement's own answer all keep the loose shape a producer composes. The engine parses each of them once, and everything downstream of that parse holds the type.

The seams that now say what they receive

PartitionProviderInterface, HoldPassPrimerInterface, LockAnchorScopeProviderInterface and GroupPrimedConstraintInterface take HoldLine[]. Their implementations in yoyaku_placement read properties rather than keys, and ConstraintPolicyManager::normalizeProposed() converts at the one point where a line becomes what a policy is handed.

assignPartitions() no longer writes two keys into a line it was lent. A line given a partition is a new line, and givenPartition() carries the mark saying the booker did not ask for it, because those were one fact written as two statements.

What phpstan could not see, and what caught it

Declaring the element type on the seams let phpstan enumerate most of the work: it found 12 sites across 4 files and, once those were converted, went green.

Green was not enough. Three sites build a line-shaped array literal and append it to a list that already holds parsed lines, which no type declaration can catch: PlacementLockAnchorScopes::scopesFor() appending the held bookings it was given, HoldEngine::anchorScopes() doing the same for the lines being released, and the fallback in placeUnderLocks() for a slot with no resource. The kernel tests are what found them, as Cannot use object of type HoldLine as array. All three now build the type.

That is worth saying plainly: on a change like this, a green static analyser means the declared types agree, not that the program works.

Testing

phpstan level 3 [OK], phpcs Drupal,DrupalPractice over 1,216 files clean, cspell adding no new word, the whole unit suite at 2,477 tests, and 26 kernel classes covering the hold path, the placement seams, the lock anchors and every query budget.

HoldEngine is 2,002 lines, from 2,047. The point is not the 45 lines; it is that the record has a name and one definition.

Merge request reports

Loading
Loading