TCOSMM-37: Key trigger data cache by trigger class

[hitesh-jain]

Overview

CiviRules silently stops running case rules. When a rule creates an activity on a case, the rules that should react to that activity never run: nothing is created, nothing is sent, and nothing appears in the rule log. From the outside it looks as though those rules were never configured at all.

Any site with case rules alongside at least one plain activity rule is affected.

Where it was introduced

Commit	4b571b2d "cache trigger data", Jon Goldberg, 2025-02-28
Upstream MR	extensions/civirules!284
First affected release	3.17.0 (2025-03-06)
Last clean release	3.16.0 (2025-02-26)
Still present in	every release through 3.41.0, and master

That commit added 9 lines and nothing else. Its stated premise is the defect:

the trigger data shouldn't change between rules

It does change, whenever the rules use different trigger classes, because each subclass attaches different entities. The MR's scope was "multiple rules with the same trigger", but the cache was keyed by nothing at all, so it also leaked across unrelated classes. This PR narrows it to that stated intent.

Upgrading does not fix this. The faulty cache is byte-identical in 3.32, 3.41.0 and master. 3.18+ also needs CiviCRM 5.82+, so it is not an option on the 4.x line (CiviCRM 5.75) anyway.

Before

Two probe rules, both enabled, both triggered by the same event. Neither has any actions, so the only thing measured is whether each rule runs at all.

ID	Rule	Trigger	Enabled	Last Triggered
2	probe A, plain Activity, no conditions	Activity is added	Yes	5 Aug 2026 6:24 AM
3	probe B, CaseActivity + case_type condition	Case activity is added	Yes	(empty)

Blue marks the rule under test. Red marks the Last Triggered column: populated for ID 2, empty for ID 3, even though one event fired both.

After

Same two rules, same event, on a build carrying this fix. Last Triggered is now populated for both.

Both screenshots come from validating the identical change on the 3.17.1 line (see the companion PR below). The patched post() is byte-identical on both branches, so they demonstrate this change too, but they were not captured on a 3.32 build. See Testing.

Technical Details

What goes wrong

1. A case activity is created. post() gathers every rule whose trigger matches Activity + create. That set mixes plain Activity rules and CaseActivity rules.
2. The first rule in the set builds its trigger data, and post() stores that object in one static slot.
3. Every later rule is handed that same object before it runs.
4. triggerTrigger() only builds its own data when none is set. A handed-down object therefore skips getTriggerDataFromPost() — the only place CaseActivity attaches the Case entity.
5. getEntityData('Case') returns [], because there is no database fallback for Case. case_type and case_status compare against nothing and fail, so the rule does not run. A rule that fails its conditions writes no log row, which is why the failure is completely silent.

The fix

• Key the cache by trigger class. Data is only interchangeable between triggers of the same class, so each class builds its own once and reuses it. The speedup MR !284 wanted is kept.
• Park the cache across nested events. A rule action can fire another post hook and re-enter post(). The cache is pushed onto a stack and restored in a finally, so a nested event cannot clear or overwrite the cache of the event still running underneath it. Raised in review; nesting was then observed reaching depth 2 on a real chain.

A single slot plus a class name is not sufficient: ??= assigns once, so whichever class runs first claims the slot and every other class rebuilds, silently losing the optimisation. Hence a per-class map.

What is and is not affected

Two conditions must both hold for a trigger class to be affected:

Condition	Consequence
Two different trigger classes share the same object_name + op	post() filters on these, so a class that is the sole handler for its object can never be handed another class's data
The class routes through the hasTriggerData() guard	Only CivirulesPostTrigger/Activity.php and Civirules/Trigger/Post.php consult it

Not affected, despite overriding getTriggerDataFromPost(): ContributionSoft, Participant, Event, ActionLog, Relationship — each is the only class for its object.

Immune, because they override triggerTrigger() and rebuild unconditionally: ContactRestored, ContactTrashed, MembershipRenewed, RelatedParticipantWhenActivityChanged, RelatedParticipantWhenActivityIsTagged.

So in the standard trigger set the only affected class is CaseActivity, on Activity/create and Activity/edit. It overrides triggerTrigger() but delegates to Activity::triggerTrigger(), which carries the guard.

Note findRulesByObjectNameAndOp() has no ORDER BY, so which class runs first is not deterministic. Two sites with identical configuration can behave differently, and the same site can differ between runs. Both orderings were seen while testing.

Risk to check when reviewing

$triggerDataCache changes type from ?CRM_Civirules_TriggerData_TriggerData to array. It is public static, so this is technically an API change, though it is an implementation detail added in 3.17.0. A search across ~90 CiviCRM extensions, custom modules and core found no external consumers, but downstream custom code cannot be ruled out.

Testing

The runtime validation for this change was done on the 3.17.1 line, where the environments available run CiviCRM 5.75. Summarised there:

• Reproduced 3 runs out of 3 on a real case rule chain; Case empty on every CaseActivity trigger before the fix, populated after, downstream rules firing.
• Reproduced broken then verified fixed on a deployed QA build of the core product.
• Cache still used within a class, verified by spl_object_id.
• Nested events: depth 2 observed, outer event resumes with cache intact, stack drains to 0.
• Exercised on Activity/create and Activity/edit, both class orderings.

What has and has not been verified on this branch specifically:

Check	Status
post() byte-identical to the validated 3.17.1 branch	verified by diff, exit 0
Bug mechanism unchanged on this line	verified: same hasTriggerData() guard sites, same Activity::triggerTrigger() else-branch, CaseActivity still attaches Case
php -l	clean
PHPCS	86 errors / 8 warnings, against 87 / 10 on the unmodified file

Comments

• Upstream MR: https://lab.civicrm.org/extensions/civirules/-/merge_requests/376, targeting upstream master. Referenced as Core PR: in the commits. Landing it upstream is what stops a future version bump re-introducing this.
• Companion PR for the 3.17.1 line: TCOSMM-37: Key trigger data cache by trigger class #8. The patched post() is byte-identical on both branches; they should land together.
• No info.xml change and no tag here: version metadata is left to whoever cuts the release.

[gemini-code-assist]

Code Review

This pull request refactors the trigger data caching mechanism in CRM_Civirules_Trigger_Post to cache data per trigger class and introduces a stack ($triggerDataCacheStack) to handle nested post events safely. A potential runtime issue was identified in the finally block, where array_pop() could return null if the stack is empty, leading to a TypeError when assigned to the strictly typed array property self::$triggerDataCache. It is recommended to use the null coalescing operator to default to an empty array.

[hitesh-jain]

Upstream MR raised: https://lab.civicrm.org/extensions/civirules/-/merge_requests/376

Core PR: added as line 2 of every commit on this branch, matching the convention in 7e80bcf and 6b7653c.

Companion PR for the 3.17.1 line: #8.