Prevent duplicates: Trigger - Workflow Field Update- Trigger w/ Callout - AllOrNone = false

Question

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

Answer 1

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.

Answer 2

Just a thought... Queueables can be queued, inspected, and then de-queued again. Each time you queue it, though, you count against the transaction limit. But let's set that aside for a moment and hope it doesn't hit the use case here.

A rollback also rolls back placing anything in the async queue, right? So the job ID would drop out of AsyncApexJob.

Let's take your scenario where allOrNone = false and there's a workflow rule, and it's going to run the trigger twice. Your difficulty with the "static variable solution" is that the statics don't reset. OK.

So instead of @future let's pass the set of IDs to a Queueable and enqueue it. Then use the job ID returned from it as the key to store the Queueable instance in a static Map<Id, YourQueueableClass>.

Now on the next pass through the trigger, instead of just looking at a static list of which IDs have already been seen, pull the .keySet() of the static Map of Queueables and query AsyncApexJob to see who's actually in the queue. Inspect only the Queueables that are actually in the queue to see which record IDs were passed into them.

If this works, it serves as an answer to one of your big dilemmas, "Clearly, you need to have state that: Persists across the trigger – workflow field update – trigger sequence, Is rolled back when SFDC retries in the AllorNone = false (partial success) use case". The state of whether the Queueable is still in the queue plays that role while also giving you a more transactional way to handle your async calls.

Answer 3

Example usage

public class LeadTriggerHandler {

 public void onAfterUpdate(Lead[] leads, map<ID,Lead> oldLeads) {
     set<ID> newToTxnIds = TransactionService.getUnvisitedIdsThisTxn
          ('usecaseCallout',Trigger.newMap.keyset()); 
     set<ID> leadIdsToCallout = new set<ID>();
     for (Lead l: leads) { 
        Lead oldLead = oldLeads.get(l.Id);
        if (l.Company != oldLead.Company && 
            unvisitedIdsThisTxn.contains(l.Id)) { // work not prev done
            leadIdsToCallout.add(l.Id);     
        }
     }
     doCallout(leadIdsToCallout);
 }

 @future
 static void doCallout(set<ID> leadIds) {
    // .. callout details not important
 }
}

I created an Sobject

• Transaction_State__c with three relevant fields:
• Name (the transactionId)
• Scope__c (user-defined scope as one ID may be involved in multiple operations you don't want to do twice)
• SObjectId__c (an objectId as String)

I created the following class to manage Transaction State

public class TransactionService {
  private static String transactionId {
        get {
            if (transactionId == null)
                transactionId = String.valueOf(System.currentTimeMillis()) + 
                               '_' + UserInfo.getName();
            return transactionId;   
        }
        set;
    }

    /**
    *   get(set)TransactionId - use to record some identifier for the transaction. 
    **/

    public String getTransactionId() {return transactionId;}

    public void setTransactionId(String xactId) {transactionId = xactId;}

    /**
    *   getUnvisitedIdsThisTxn
    *   
    * Avoids reprocessing the same ID more than once in a 
    * service method for a single transaction
    * This is particularly relevant in SFDC WF use cases 
    * where a field update will cause a trigger
    * to refire using the before value of field(s) as they were 
    * when the Txn started - and not their current vals!
    *
    * Thus, domain classes that check on delta for field F
    * will re-execute. If these do callouts
    * or publish platform events, there is duplicate traffic
    *
    * Static variables can't be used because in a Bulk 
    * AllOrNone = false (partial success) use case where
    * 1+ recs succeed and 1+ recs fail; SFDC will retry the successes
    * after rolling back DB updates but 
    * SFDC does not rollback static vbls.
    *
    * Hence, when the retry occurs, the trigger work may end 
    * up doing nothing if they relied on static variables 
    * to prevent recursion
    **/

    public virtual set<ID> getUnvisitedIdsThisTxn(String scopeKey, set<ID> proposedIds) {
        set<ID> unvisitedIds = new set<ID> (proposedIds);
        set<ID> visitedIds = new set<ID>();


        //  Get the complement of proposedIds and IDs we've already seen 
        //  in some prior call this Txn.
        //  Example: previous calls had us visiting  IDs 1, 3, 8
        //  Proposed Ids to see if "new" are 0, 3, 5
        //  Should return 0, 5 and also record 0, 5 in database in case trigger called a third time

        for (Transaction_State__c  txnState : 
          [select ID, Name, SObjectId__c, Scope__c from Transaction_State__c
                where   Name = :TransactionServiceImpl.transactionId and 
                        Scope__c = :scopeKey and
                        SObjectId__c IN: proposedIds]) {
            visitedIds.add((ID)txnState.SobjectId__c) ;                                 
        }

        unvisitedIds.removeAll(visitedIds); // do the complement

        //  persist the Transaction State as SObjects so 
        //  if allOrNone = false retry use case occurs, 
        //  these get rolled back. 
        //  We want them to roll back because the trigger should re-execute 
        //  as if it had never executed at all

        Transaction_State__c[] newlyVisitedTxnStates = new list<Transaction_State__c>();         
        for (ID unvisitedId : unvisitedIds)    
            newlyVisitedTxnStates.add(new Transaction_State__c(Name = TransactionServiceImpl.transactionId, Scope__c = scopeKey, SObjectId__c = unvisitedId));
        insert newlyVisitedTxnStates;  // save that we've been to these ids in this scopeKey       
        return unvisitedIds;

    }
}

Not shown

• Schedulable class to delete Transaction_State__c older than today
• Testmethods for the above

Answer 4

Here's an improved answer inspired by @CharlesT

The secret:

You need some indication that you are in a AllOrNone = false retry use case. We already know that you can't use static variables as they don't get rolled back on AllOrNone = false retry. Hence we need Sobjects. But, we really only need one Sobject - the sobject represented by a Queueable job (i.e. an AsyncApexJob record)

If a static variable points at a queued job, and that job no longer exists, we know that we are in a retry use case - because SFDC rolls back sobjects on retry. So ... the static variable map can be reinitialized

If the static variable points at a queued job that does exist, then we can use static variables to maintain state and catch recursion issues across trigger-workflow-field update-trigger. Or, for that matter, triggers calling themselves via CRUD.

This answer is superior to the DML answer in that no cleanup job is required and hundreds of records don't have to be queried or inserted. Thanks Charles T!

Total cost: 1 queueable start and n SOQL queries for each place in your code you need to do recursion control for a given transaction.

Solution details

• No custom object required

Apex class

public class TransactionService implements Queueable{
    static String txnId {
        get {
            if (txnId == null) {
                txnId = String.valueOf(System.currentTimeMillis()) + '_' + UserInfo.getName();
            }
            return txnId;
        }
        set;
    }

    static ID jobIdAsProxyForStateTrust; // an AsyncApexjob

    /**
    * establishStateTrust - invoked on every call; tells us whether static
    *                       variable state map is reliable
    **/ 
    static void establishStateTrust() {
        if (jobIdAsProxyForStateTrust == null) { // no trust yet setup
            resetStateTrust();
        }
        else {
            //  if we have a jobId, has it been rolled back because we are 
            //  in an AllOrNone = false (partial success) SFDC-initiated 
            //  retry use case on the "successes"?
            AsyncApexJob[] jobs = [select Id from AsyncApexJob 
                                   where ID = :jobIdAsProxyForStateTrust];
            if (jobs.isEmpty()) { // static vbl points at job that has been rolled back!
                resetStateTrust();
            }
            else {} // if the static variable we established points at an existing job,
                    // that means we are not in an AllOrNone = false retry step 
                    // and the static variables maintaining state can be 
                    // relied on. Thus, triggers re-executed
                    // as part of a workflow/Process Builder can avoid 
                    // repeating logic.
        }
    }

    static void resetStateTrust(){
       jobIdAsProxyForStateTrust = System.enqueueJob(new TransactionServiceQ(txnId));
       clearVisitedCaches(); 
    }

    static map<String,Set<ID>>  visitedIdsThisTxnByScopeKey = new map<String,set<ID>> ();        

    public static set<ID> getVisitedIdsThisContext(String scopeKey, set<ID> proposedIds) {

        establishStateTrust();

        if (visitedIdsThisTxnByScopeKey.containsKey(scopeKey)) {
           // start with proposedIds as unvisited   
           set<ID> unvisitedIds = new set<ID>(proposedIds);                 
           // remove any Ids we've already seen
           unvisitedIds.removeAll(visitedIdsThisTxnByScopeKey.get(scopeKey));
           // update visited set    
            visitedIdsThisTxnByScopeKey.get(scopeKey).addAll(proposedIds);      
            return unvisitedIds;
        }
        else {                                                                  
            // new scopeKey, hence all ids are unvisited
            visitedIdsThisTxnByScopeKey.put(scopeKey,new set<ID>(proposedIds));
            return proposedIds;
        }
    }

    /**
    *   clearVisitedCaches()    - Clears all visited ID caches
    **/
    public static void clearVisitedCaches() {
        visitedIdsThisTxnByScopeKey.clear();
    }

    /**
    *  Methods and variables when class is invoked as a Queueable
    **/  
    String txnIdUsedToEstablishState;

    public TransactionServiceQ(String txnIdUsedToEstablishState) {
        this.txnIdUsedToEstablishState = txnIdUsedToEstablishState;
    }

    public void execute(QueueableContext qc) {
        System.debug(LoggingLevel.INFO,'queueable starts using '+ this.txnIdUsedToEstablishState + 
                    ' no work need be done');
    }
}

Example Usage

public class LeadTriggerHandler {

 public void onAfterUpdate(Lead[] leads, map<ID,Lead> oldLeads) {
     set<ID> newToTxnIds = TransactionService.getUnvisitedIdsThisTxn
          ('usecaseCallout',Trigger.newMap.keyset()); 
     set<ID> leadIdsToCallout = new set<ID>();
     for (Lead l: leads) { 
        Lead oldLead = oldLeads.get(l.Id);
        if (l.Company != oldLead.Company && 
            unvisitedIdsThisTxn.contains(l.Id)) { // work not prev done
            leadIdsToCallout.add(l.Id);     
        }
     }
     doCallout(leadIdsToCallout);
 }

 @future
 static void doCallout(set<ID> leadIds) {
    // .. callout details not important
 }
}

UPDATE: While the above works; it has a problem that makes it unworkable for those of you who rigorously unit test.

Specifically, in a testmethod context, the line:

jobIdAsProxyForStateTrust = System.enqueueJob(new TransactionServiceQ(txnId));

in method

static void resetStateTrust(){
           jobIdAsProxyForStateTrust = System.enqueueJob(new TransactionServiceQ(txnId));
           clearVisitedCaches(); 
        }

returns null as the enqueueJob method won't execute until Test.stopTest();. Thus in test contexts, the service always thinks that state is just being established and thus you may not always be able to verify that your code-under-test is prevented from doing work twice.

The workaround is to replace the system.enqueueJob with an insert of an sobject Transaction_State__c with Name = transactionId. Then, look for that record with that id; if not found, then we know that allornone retry has commenced.