11

Here I'm talking about a handler that covers all the TriggerOperations and that accepts SObjects of arbitrary type. And the recursion problem is where a change caused by a trigger results in that trigger getting called again and so on creating an infinite loop.

So an example trigger would be:

trigger ExampleTrigger on Example__c (before insert, before update, before delete,
        after insert, after update, after delete, after undelete) {

    SObjectType t = Example__c.SObjectType;
    MyHandler.handle(t, Trigger.operationType, Trigger.oldMap, Trigger.newMap);
}

with the handler:

public inherited sharing MyHandler {

    public void handle(SObjectType t, TriggerOperation op,
            Map<Id, SObject> oldMap, Map<Id, SObject> newMap) {

        // Logic conditional on t and op that does updates that cause recursion
        ...
    }
}

Often single static Boolean flags are talked about to block trigger recursion. (Though an approach based on change checking that is designed to settle to the correct state can be needed instead to produce the correct business results.)

But for the simple case of complete blocking, (and now I'm writing this I think the answer is "yes"), is this pattern sufficient?

public inherited sharing MyHandler {

    // Implements equals and hashCode
    private class Kind {
        ...
    }

    private static final Set<Kind> BLOCKED = new Set<Kind>();

    public void handle(SObjectType t, TriggerOperation op,
            Map<Id, SObject> oldMap, Map<Id, SObject> newMap) {

        if (BLOCKED.contains(new Kind(t, op))) return;
        ...
        BLOCKED.add(new Kind(t, TriggerOperation.BEFORE_INSERT));
        BLOCKED.add(new Kind(t, TriggerOperation.AFTER_INSERT));
        insert sobs;
        ...
    }
}
5
  • What about when you create/update/delete more than 200 instances in one go? The trigger will be called repeatedly and items 200+ won't be processed... right?
    – Phil W
    Mar 4 '20 at 8:20
  • 4
    I think you need to track the object IDs against operation here to resolve that issue.
    – Phil W
    Mar 4 '20 at 8:22
  • Thanks Phil, I think you are right so worth posting as an answer. So are you thinking something like Map< TriggerOperation, Set<Id>> for the blocking?
    – Keith C
    Mar 4 '20 at 8:23
  • 1
    Phil, can you post that as an answer and I'll accept when I can. You just saved me great pain in 2 years time when some bulk import is done.
    – Keith C
    Mar 4 '20 at 8:24
  • One question on this: should the "insert sobs;" line be a list of of objects that gets added to and then "insert <list of objects>;" be called? I'm assuming it was left out for conciseness, or am I missing something (and it's quite possible I'm missing something!)?
    – Jeff
    Sep 22 '20 at 14:24
10

Remember that triggers will be called in chunks if you create/update/delete (generally in chunks of 200 items). Perhaps you could adjust this to track operations against ID, something like:

static Map<TriggerOperation, Set<Id>> blockedObjectIdsByOperation;

That way if the object has already been through the trigger for the given operation type the execution can be suppressed but this won't interfere with chunked trigger calls.

I don't think there's an issue with before insert since such a trigger shouldn't get invoked iteratively on an object (though there could be some edge cases).

2
  • 1
    And BEFORE_INSERT where there is no Id can be skipped as an insert can only happen once?
    – Keith C
    Mar 4 '20 at 8:29
  • 1
    Right - in this case you can't insert the same object twice and the trigger is invoked with update for additional processing. Right?
    – Phil W
    Mar 4 '20 at 8:30
7

The way I've come to accept recursive blocking is to block only during operations where I feel I may end up with recursion. I use the blocks only during known recursive updates. For example:

public class AccountTriggerHandler {
  static Boolean isInAccountUpdate = false;
  ...
  public static void afterUpdate(Account[] oldValues, Account[] newValues) {
    if(!isInAccountUpdate) {
      if(shouldDoUpdate()) {
        isInAccountUpdate = true;
        updateRecords(oldValues, newValues);
        isInAccountUpdate = false;
      }
    }
  }
}

Note that an even better idea is to make your triggers "rising edge" when possible:

public static void afterUpdate(Account[] oldValues, Account[] newValues) {
  Account[] oldChanges = new Account[0], newChanges = new Account[0];
  for(Integer i = 0, s = newValues.size(); i < s; i++) {
    if(recordChanged(oldValues[i], newValues[i])) {
      oldChanges.add(oldValues[i]);
      newChanges.add(newValues[i]);
    }
  }
  processChangedRecords(oldChanges, newChanges);
}

This pattern almost always (but exceptions obviously exist) eliminates recursion without any extra variables at all.

If you want to avoid recursions in the face of potential updates to multiple records of the same type, you can then use a set. However, one specific caveat here is that you should always reset this set at the end of the trigger to avoid logic bugs when dealing with retries/workflow/process updates.

public class AccountTriggerHandler {
  static Set<Id> accountIds = new Set<id>();
  public static void afterUpdate(Account[] oldValues, Account[] newValues, Set<Id> accountIdSet) {
    if(accountIds.containsAll(accountIdSet)) {
      return;
    }
    accountIds.addAll(accountIdSet);
    doMainLogicHere();
    accountIds.removeAll(accountIdSet);
  }
}

If you don't do this, you're blocking the ability to react to workflow field updates, process builder updates, approval process updates, valid recursive updates, updates involving more than 200 records, etc. You'll also restrict the ability for unit tests to perform multiple DML operations.

A proper strategy must include the ability to handle partial updates (I've specifically had this happen to me once before I started doing this), as well as only performing necessary updates (e.g. rising edge triggers, checking if any data changed).

There is no single magic bullet to stopping "recursion", and often all of the strategies above (or variants) should be employed, all possible consequences considered. Always "unlock" your trigger at the end of a trigger context, even if you don't think you'll need it. Always minimize the "lock time", and definitely clear out the lock when the trigger ends.

2
  • Thanks, great advice. I don't quite follow the calling the change checking "rising edge" but agree that it is the best way to go where the logic can be reasoned through. And the set/dml/unset seems like a core piece of advice that also allows simple booleans to be used while still supporting more than 200 records?
    – Keith C
    Mar 4 '20 at 20:15
  • 1
    @KeithC Just noticed I missed this. Rising edge trigger is a electrical engineering term, regarding things like "logic happens when a clock signal goes from 0 to 1". The "turn off the boolean" idea works well even when you have more than 200 records in a list, or multiple, separate updates, etc. I had one project where all of my triggers had to have this logic to keep my sanity, it's when I figured it out.
    – sfdcfox
    Mar 9 '20 at 23:53

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.