3

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

  • It's hard to prove a negative, but I don't think there's a better route to take than your first listed option. Door #1 seems to be your best case scenario. – Adrian Larson Dec 21 '17 at 18:32
1

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.

  • This is intriguing. The queueables self clean/delete themselves when the transaction ends. Because there is a limit on # queueables per txn and I need headroom for legitimate queueables then a fallback to the sobject approach might be needed for maximum robustness. – cropredy Dec 22 '17 at 5:26
  • It would be certainly be nicer if cancelled queueables didn't count towards the limits. Or if it were possible to add data to a queueable's instance after queueing but before it executes. – Charles T Dec 22 '17 at 5:58
  • actually, the above gave me an idea -- the presence of a single queueable could indicate whether the static variables are trustworthy - if yes, use them; if not clear them. I will experiment today – cropredy Dec 22 '17 at 16:02
1

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
0

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.

  • Glad this led you to a helpful solution. Too bad about the unit test issue. Maybe you can factor that enqueueJob call out to a method can be better mocked in unit tests. Incidentally I had another thought which may or may not be of use - Platform Events don't roll back. So if you ever had a context where you want to trigger something async that will definitely rollback, you could set it off in a Platform Event. (But PEs have their own unit testing quirks I understand...) – Charles T Dec 23 '17 at 6:46
  • @charlesT - I too, thought of PEs as an alternative to Queueables. They do roll back on allofnone=false retry but they have the same unit test issues and can't be queried (you need a trigger to consume and persist somewhere). The mocking idea for the system.enqueueJob is also meritorious . – cropredy Dec 23 '17 at 20:23

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.