BookingManager::hold() reserved one line through reserve(), and reserve() never ran the group policy check. Only the group path did, twice: once before taking the slot locks and once under them. So a hold placed through the single-line entry point was subject to the booking constraints, the capacity check and the vetoable event, but to no attached constraint policy at all.

A limit configured on a resource therefore simply did not apply to a single-line hold, and nothing noticed, because every surface books through the request booker and only tests reached the narrow method.

Correction to the original summary

This issue first claimed that the same method was also at fault for writing a line that belonged to no transaction. That is not a defect. The booking entity's transaction field is not required, a line without one is a supported shape, and the engine deliberately creates none: grouping lines is the business of the method that groups them. The claim was read off a nullable column rather than off a real requirement, and pursuing it would have made a transaction mandatory for every hold, which it must not be.

Resolution

The two methods differed in one thing, and it was not the number of lines: five of the seven things the group path did that the narrow one did not were about the transaction. So the narrow method is now the bare one, taking lines and returning them, and the group method is renamed holdInTransaction(), keeping its signature and the parameter that lets a cart accumulate.

holdInTransaction() calls hold() and adds only the transaction: it instantiates one unsaved, which touches no database, and hold() saves it once it has validated and is inside the database transaction it already opens. So the order of database operations is unchanged from before the split, and because the wrapper contains no rule logic it cannot skip a constraint or a policy.

book() is removed. It was a hold followed by a confirm, and nothing called it.

Fixed as part of #3614532: Constraint policies should declare what they need rather than when they run, and stop assuming a tariff, which reworks how a policy declares what it needs.

Issue fork yoyaku-3614745

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: The single-hold path enforces no constraint policy and leaves a line belonging to no order » The single-hold path enforces no constraint policy
Issue summary: View changes
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.

Status: Fixed » Closed (fixed)

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