Follow-up to #3010055: Multiple Event Triggers.

When multiple event support was added in the UI, a new ReactionRuleConfig::removeEvent() method was introduced to remove an event from a reaction rule.

ReactionRuleConfig uses a numerically indexed array variable as an internal data structure to store event information for a reaction rule. The event names are input/output from configuration, where they are stored in a sequence.

Conceptually, sequences should have an order but no explicit indices. Configuration does not import or export indices, or even require them, for sequences. However, this is done by assuming all sequences are stored in PHP arrays with numerical indices *starting with 0* with no gaps in the sequence. If these assumptions are not observed, then the configuration will import/export explicit numerical indices.

ReactionRuleConfig::removeEvent() works by unsetting an array index to delete the event. This leaves gaps in the sequence. Using rules.reaction.react_on_two_events.yml as an example, the configuration contains the following when two events are defined:

events:
  -
    event_name: 'rules_entity_insert:node'
  -
    event_name: 'rules_entity_update:node'

If the first is deleted, we expect to see:

events:
  -
    event_name: 'rules_entity_update:node'

Instead, what we currently see is:

events:
  1:
    event_name: 'rules_entity_update:node'

This behavior was recognized in #3010055: Multiple Event Triggers and a test was added to check that Rules still worked even with explicit indices like this. I'm not sure this is actually wrong, because it does pass schema checks, but it looks bad and the indices are irrelevant so it is a source of potential error if the configuration contains wrong (invalid) indices instead of no indices.

The fix is to ensure that when an event is removed, the internal array structure used by ReactionRuleConfig is re-indexed to start at 0 and have no gaps. This is a simple 1-line fix. The test has to be changed as well, so that if a configuration is created with incorrect indices we should be testing to ensure that the indices get corrected.

CommentFileSizeAuthor
#3 3259498-3-array-values.patch1.98 KBtr

Comments

TR created an issue. See original summary.

tr’s picture

Issue summary: View changes
tr’s picture

Status: Active » Needs review
StatusFileSize
new1.98 KB
tr’s picture

Component: Rules Core » Events

  • TR committed d70724f on 8.x-3.x
    Issue #3259498 by TR: Fix indices in exports of Rules with multiple...
tr’s picture

Status: Needs review » Fixed

Committed #3.

Status: Fixed » Closed (fixed)

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