Issue #3624476: Machine-name columns are stored as if they could hold any character
Closes #3624476.
14 columns gain is_ascii, from 7 field definitions.
state, onorchestra_instance,orchestra_token,orchestra_incidentandorchestra_work_item— 14 index positions between them. Every value is an interface constant:StateTransitions::claimState()is the only thing that moves the column, all 14 of its call sites name a*Interface::STATE_*, and so does each entity'ssetState().tenant, one definition inTenantField::getDefinition()reaching 7 entity types and 10 index positions. A tenant id is a config entity id.definitiononorchestra_instanceandworkflowonorchestra_workflow_version— both documented as "Machine name of the workflow", both config entity ids.entity_type_idonorchestra_attachment— core constrains entity type IDs.
What stays wide, and why the test says so
correlation_key, idempotency_key, caller and origin are the caller's own words. node_id, source_flow, status, attachment_key, outcome and label are named by whoever authored the workflow. entity_id is explicitly "string to allow non-integer IDs". note is prose. An ASCII column answers a value it cannot hold with a failed write, so the line is drawn by who produces the value.
Four of the wide entries read like candidates and are deliberately not taken: resolution and mode do come from interface constants and hash is a digest, but each has a write site that takes the value from a variable rather than from a constant, and candidates carries audience tokens like org:legal whose second half nobody here mints. Naming them in wideFields() is what makes the next person's call a decision rather than an oversight.
On the cost
InnoDB stores a varchar in a B-tree record with its actual bytes, in the index as in the table, so this makes no index smaller on disk. What the declaration decides is InnoDB's 3072-byte index key limit, computed from maximum widths, and the cost of every comparison, ascii_general_ci being the simpler collation.
The test
MachineNameColumnsTest asserts both lists, and testEveryStringColumnIsAccountedFor() walks the storage definitions and fails on any single-property string base field in neither. That third method is the reason this MR is 14 columns and not 5: it found orchestra_token.source_flow, orchestra_attachment.tenant and orchestra_variable.tenant among nine others that the issue's hand-written list had missed. It reports every offender in one failure rather than the first, because a list like this gets filled in one sitting.
33 tests / 65 assertions green with the change; 14 failures without it, one per declared column.
Upgrading
The narrower columns reach a site installing orchestra now. An existing site keeps what it has: changing a populated column is a separate question and is not attempted here.
Second commit: the state guard.
Declaring state ASCII was the one entry here that could not be defended on the same terms as the others. definition and workflow are lookup-guarded by loadDefinition(), entity_type_id is minted by core and tenant comes from a context that answers with a config entity id — but setState(string $state) is public on three interfaces, sets the column directly and validated nothing. "Every call site names a constant" is a fact about today's callers, not about the column.
It also cut the wrong way: with the column declared ASCII, a caller passing a non-ASCII state would have got a failed write on the column that moves a run forward, which is the same failure mode used to exclude correlation_id from the sibling audit_trail change.
So the set is now stated once and enforced at both doors:
StatefulInterfacedeclaresconst STATES = [], andTokenInterface,ProcessInstanceInterface,WorkItemInterfaceandIncidentInterfaceextend it and override it with their own states.- Each entity's
setState()refuses anything outsideself::STATES, naming the states in the message. StateTransitions::claimState()refuses both$expectedand$new. It is a rawUPDATEand the only route an incident has, so guarding only the setter would state the rule at one of two doors.$expectedmatters as much as$new: an unknown one matches no row, andclaimState()then answers FALSE, which a caller reads as "another actor won it" rather than as a typo.
ASCII is then a consequence of the constants rather than a second rule, and a wrong state fails loudly at the door it came through instead of quietly at an insert.
claimState() is retyped from ContentEntityInterface to StatefulInterface, which is also what makes $entity::STATES legal at phpstan level 5.
testAnUnknownStateIsRefused() drives both doors and both ends of claimState(), and asserts an ordinary flip still applies so the guard refuses the wrong thing and nothing else. It fails without the change ("setState() accepted a state the token does not have"). StateTransitionEventTest is green at 5 tests / 64 assertions, MachineNameColumnsTest at 33 / 65, and phpcs is clean across src and tests.