Issue #3622548: Let each node feature declare the schema of the node key it owns

A node's configuration schema named eight keys one at a time, and each one is the private shape of a node feature. Three of the eight described an optional module's data in the kernel's own file — interaction, park_notify and notify_to, belonging to orchestra_interaction, orchestra_interaction_notification and orchestra_notification — and nothing on the page said which three.

The kernel already knew better everywhere else: a node feature declares the key it owns through getConfigKey(), and NodeFeatureManager maps that key back to the providing module, which is how a workflow records its dependencies. The schema was the one place the kernel still wrote those keys out by hand, so a module adding a feature had to patch this module's schema for its own key or its settings failed validation on any workflow carrying them.

The container

Node feature settings now share one container, typed by the feature that owns each entry:

features:
  type: sequence
  label: 'Node feature settings'
  sequence:
    type: orchestra.node_feature.[%key]

The kernel declares the container once and its own five features; the three submodule keys move to their owners, which gives orchestra_interaction_notification and orchestra_notification their first schema file. A ninth feature from a new module needs no change here at all, which is the point — the one thing none of the cheaper answers gave.

This is the shape core uses for the same problem twice over: Views types a display's handlers by plugin id and lets each module declare views.filter.* for its own, and third party settings are a sequence keyed by module name, [%key] resolving to the owning module's type. Both keep the container with the owner of the structure and the entry with the owner of the data.

Two decisions the issue left open

A new features key, not the eight at the top of the node with a marker. The node keeps a strict FullyValidatable mapping over its own six keys, so a typo in label is still refused and only the feature area is open. Making the whole node a sequence would have opened the kernel's own keys too and lost that check — it would erase the boundary rather than draw it.

No dual-shape shim. A shim that reads both shapes would duplicate the shape logic at every read site indefinitely.

orchestra.node_feature.* catches a feature whose module declares nothing: the workflow schema is FullyValidatable, so without it a workflow carrying such a feature would be invalid outright, and that pressure belongs on a feature being written now rather than retroactively on ones written before anything asked. It is typed ignore, not mapping — a bare mapping types no children, so an undeclared feature carrying any data at all would still have been refused. NodeSettingSchemaTest covers both halves.

Tests

NodeSettingSchemaTest::testEveryNodeFeatureKeyIsTypedByItsOwnModule() is the contract this issue exists for: for every node feature the engine has, the shape of its key is declared, and declared in the module that provides the feature rather than in the kernel. Nothing else reports it — the wildcard keeps such a workflow valid, so a missing declaration is silent at runtime. ::testAnUndeclaredFeatureKeyStillValidates() pins the wildcard itself.

Three fixture fixes are worth naming, because each is a way this shape can move without anything failing loudly:

  • ChainTest's long-chain helper merged $act_extra + [...], which used to combine sibling top-level keys and now has a features key on both sides, where + lets the left one win whole. A timer passed in replaced the interaction that made the step interactive, and the test read zero continuations rather than an error.
  • WorkflowDependenciesTest assigned its feature key through a variable and NodeSettingSchemaTest walked a typed-config path as a list of strings — neither is a node literal, so neither is findable by the key's name.
  • InteractionResolver read its key through self::CONFIG_KEY, the one production read no search for the key's name finds.

The modeler's restoreNodeData() filter was the one real behaviour bug: it preserves node-level data across the component rebuild and filters it by the features the node's task type still has, so a retype sheds the previous type's data. That filter is keyed by feature, so it now runs inside the container; left at the top level it matched nothing and the round-trip silently dropped every feature it was meant to preserve.

Checks

phpcs Drupal and DrupalPractice clean over the whole module, phpstan level 5 [OK] No errors, cspell clean on all 121 changed files, scripts/check-translations.php exit 0. Twenty-five impacted kernel classes green against this branch in a lean root, including the modeler round-trips, the chain tests and the schema contract.

The eleven shipped workflows were rewritten in place rather than through a YAML round-trip, because they are hand-written and carry seventy-six comments between them that a re-dump would discard; each comment travels with the key it explains, and the parsed data is identical.

Edited by Frank Mably

Merge request reports

Loading
Loading