[SalesForce] Prevent duplicates: Trigger – Workflow Field Update- Trigger w/ Callout – AllOrNone = false

Looking for a generalized solution (for all sobjects/ all trigger-driven services) to this use case:

  1. After update trigger invoked; decides a callout via future is needed (could be related record DML too)
  2. Workflow field update causes trigger to refire. Per SFDC doc, last bullet, the trigger is presented with the original Trigger.old and decides the callout needs to be sent again (bad).

Trigger will execute in many use cases:

  • From standard UI or VF pages (AllOrNone = true)
  • From Data Loader (AllOrNone = false)
  • From Bulk API (AllOrNone = false)

Although static variables can be used to control WF-iniated recursion in an AllOrNone = true use case they don't work when AllOrNone = false and 1+ records succeed in the batch and 1+ records fail. The retry (see second bullet in doc) will roll back the updates, pass the successes back through the trigger again but, as the static variables aren't rolled back, the trigger will do nothing – no callout will be made.

Obviously, I could get rid of the workflow field update and move into the trigger the field update but what happens when some admin inadvertently adds back a new workflow with field update? Any batches with AllOrNone=false and partial errors will leave the database in an inconsistent state with respect to these callouts (or other trigger-driven DML).

This won't work in Use Case: AllOrNone=false with 1+ successes, 1+ failures in a batch

public class LeadTriggerHandler {
    static set<ID> leadIdsAlreadySentToFuture = new set<ID>(); // recursion control
    public void onAfterUpdate(Lead[] leads, map<ID,Lead> oldLeads) {
        
         set<ID> leadIdsToCallout = new set<ID>();
         for (Lead l: leads) { 
            Lead oldLead = oldLeads.get(l.Id);
            if (l.Company != oldLead.Company && 
                !leadIdsAlreadySentToFuture.contains(l.Id)) { // have we already done this?
                leadIdsToCallout.add(l.Id);
                leadIdsAlreadySentToFuture.add(l.Id);    
            }
        }
    }
      
    @future
    static void doCallout(set<ID> leadIds) {
        // .. callout details not important
    }
}

Ideas considered:

Option 1: Custom Transaction_State__c Sobject to preserve state, but that doesn't need to be immediately cleared

In the trigger, once it decides it needs to callout, save in a custom Transaction_State__c object with key = some application context + SobjectId + Transaction Id to record whether the trigger has done its work already. Inspectable (via SOQL) in the trigger when running after a field update so acts like a rollback-able static variable in an AllOrNone = false retry use case. Thus, in retry, the trigger's first execution queries and finds nothing in Transaction_State__c for the context + row + transaction Id, sets the Sobject and when the WF field update comes around, the trigger bypasses the second callout because now there will be a row in Transaction_State__c.

The transaction Id (which can be a static variable as we don't want this to roll back) could be the currentDateTimeInMs + running userId.

Drawbacks

  • Costly, a DML call has to be made (in the first, pre-WF trigger invocation); a SOQL has to be made (twice – before and after the workflow)
  • Eventually, the rows in this Sobject need to be deleted so some Schedulable needs to be written (minor, but a pain)

Option 2 Have workflows that unconditionally always set, via Field Update, an Is_WF_Processed__c = true on each SObject.

Being an SObject field, it will get rolled back in the AllOrNone = false retry use case. However, the field needs to be cleared once the transaction is done and that yields another DML call in the after trigger with consequent new trigger firing

Drawbacks

  • Cost of clearing the field
  • Every object that has a trigger will need this workflow as a guard against the duplicate work. Triggers will always fire twice, even in transactions where the rule criteria for all other workflows on the Sobject evaluate to false.
  • The duplicate trigger execution issue should be solved exclusively in apex (separation of concerns – you shouldn't have to orchestrate workflow to aid in the solution)
  • It all has to be repeated for Process Builder

Is there a lighter-weight solution than option 1 (which I'm inclined
to go with)?

Lengthy blog post with proofs and proposed solution

Best Answer

Recent conversation on another question prompted me to take a look into this.

The documentation has changed since this question was originally asked, but a key observation here is that when allOrNone = false, any dml retrying that Salesforce does will not contain the record(s) that previously failed.

That fact, combined with the fact that we are unable to add or remove records from trigger context variables (TCVs from here on) gives us another method to detect recursion.

In normal operation, the TCVs for each trigger chunk will have absolutely no overlap with one another. By storing the Ids given in the TCVs in a separate static Set<Id>, if we detect that there is any overlap, we can say with confidence that we're working in an allOrNone = false context, and use that to remove records previously marked as having been handled.

I threw together a class to act as a proof of concept

public class AntiRecursion{
    // An extension of the normal "static set<Id>" that allows finer ganularity
    // With this, we say "this Id has been processed by classes with these names"
    private static Map<Id, Set<String>> recIdToWorkDone = new Map<Id, Set<String>>();
    private static Set<Id> bulkIds = new Set<Id>();

    public static void track(Map<Id, SObject> sobjMap){
        Set<Id> localBulkIds = bulkIds.clone();
        localBulkIds.retainAll(sobjMap.keySet());

        // Any overlap in the bulkIds means we're in allOrNone = false territory
        //   and there was at least one failure
        // Remove the Ids from our encountered Ids collection so we don't think we're
        //   recursing
        if(!localBulkIds.isEmpty()){
            recIdToWorkDone.keySet().removeAll(sobjMap.keySet());
        }

        bulkIds.addAll(sobjMap.keySet());
    }

    public static Boolean check(Id sobjId){
        return check(sobjId, null);
    }

    // Returns true if the sobjId has not been encountered yet
    // Returns false if the sobjId has been encountered (i.e. we're in recursive execution)    
    public static Boolean check(Id sobjId, String className){
        if(sobjId == null){
            throw new System.IllegalArgumentException('Id parameter cannot be null');
        }

        Set<String> result = recIdToWorkDone.get(sobjId);
        Boolean wasNull = result == null;

        if(wasNull){
            recIdToWorkDone.put(sobjId, new Set<String>{className});
        }else{
            result.add(className);
        }

        if(String.isBlank(className)){
            return wasNull;
        }else{
            return wasNull || !result.contains(className);
        }
    }
}

Example usage

trigger testTrigger on Physical_Inventory__c (before update) {    
    AntiRecursion.track(trigger.newMap);

    Integer i = 0;
    for(Physical_Inventory__c pi :trigger.new){
        system.debug(pi);
        if(AntiRecursion.check(pi.Id)){
            system.debug('record has not undergone recursion');
        }else{
            system.debug('record has recursed');
        }

        if(i == 0){
            pi.addError('just unlucky, I guess');
            i++;
        }
    }
}

...and the anonymous apex to test it

List<Physical_Inventory__c> piList = new List<Physical_Inventory__c>();
for(Integer i = 0; i < 10; i++){
    // In my org Phyiscal_Inventory__c is a custom object with an autonumber name, so
    //  all I need to do is add a blank instance
    piList.add(new Physical_Inventory__c(        
    ));
}

List<Database.SaveResult> srList;
srList = Database.insert(piList, true);

// Test the behavior by changing this between allOrNone = true and = false
srList = Database.update(piList, true);

// clean up after ourselves
delete piList;

Specifying allOrNone = false, and calling AntiRecursion.track() in the trigger results in us removing the previously encountered Ids from recIdToWorkDone (the desired behavior).

If the call to AntiRecursion.track() is commented out, then it's equivalent to the simple static Set<Id> recursion prevention mechanism (which results in us mistakenly marking rolled-back and retried records as having been previously handled when allOrNone = false)

Benefits of this approach:

  • No queries required
  • No abuse of Queueable
  • No need for a new custom object
  • No cleanup
  • Relatively simple extension to an established approach

Drawbacks of this approach:

  • Takes up more heap space than other approaches
  • Can't be used with insert triggers

On that last point, I learned that partial failures on inserts cause entirely new record Ids to be assigned on the retries (meaning we'd never mark them as being part of a re-entrant case). Re-entrancy is mostly an issue with updates though, so it might not matter too much.

Related Topic