10

One of the most common questions on this forum is around trigger recursion. There are a few standard ways to prevent recursion. My method of choice when it comes to checking for field changes, is Dan Appleman's 'Correct' old Value method found in Chapter 6 of his book Advanced Apex Programming. I have used it many times and like the concept.

My question revolves around how to use it correctly and most efficiently in an org with multiple complex pieces of separate logic within a single trigger. Let's say I have the simple Trigger Framework shown below

Trigger

trigger OpportunityTrigger on Opportunity (after delete, after insert, after undelete, after update, before delete, before insert, before update) {

    //Other Scenarios (Before insert, before update, etc.)

    if(Trigger.isAfter && Trigger.isUpdate){
          OpportunityTriggerHandler.OppAfterUpdate(trigger.new, trigger.old, trigger.newMap, trigger.oldMap);
    }
}

Let's say this is my simplified handler class.

public class OpportunityTriggerHandler {

      public void OppAfterUpdate(list<Opportunity> newOpps, list<Opportunity> oldOpps, map<Id,Opportunity> newMap, map<Id,Opportunity> oldMap){
             myMethod1(newOpps, oldOpps, newMap, oldMap);
      }

      private void myMethod1(List<Opportunity> newOpps, List<Opportunity> oldOpps, map<Id, Opportunity> newMap, map<Id, Opportunity> oldMap){
             //MY LOGIC HERE
      }

}

If I wanted to check for opps that have just changed to 'Won' using Dan Appleman's method, I would add a static map to the handler class and add this logic to myMethod1

public class OpportunityTriggerHandler {

       Private static Map <Id,boolean> oldIsWonMap = null;

       //................

       private void myMethod1(List<Opportunity> newOpps, List<Opportunity> oldOpps, map<Id, Opportunity> newMap, map<Id, Opportunity> oldMap){

               if(oldIsWonMap == null) {
                     oldIsWonMap = new map<Id,boolean >();
               }
               set<Id> oppIds = new set<Id>();
               for(Opportunity o : newOpps){
                     boolean oldIsWon = (oldIsWonMap.containsKey(o.id)) ? oldIsWonMap.get(o.id) : oldmap.get(o.id).isWon;
                     if(o.isWon && !oldIsWon){
                           oppIds.add(o.Id);
                     }
                     if(oldIsWon != o.isWon) {
                           oldIsWonMap.put(o.id,o.isWon);
                     }
                }     
                //MY LOGIC HERE USING THE OPP IDS OF JUST WON OPPS
         }
}

This works great and now I have a map that holds the 'correct' old values and even if a workflow causes this trigger to fire a 2nd time, the logic will not repeat as I am checking against the correct old values from the map.

My question

How do I use this method if I have multiple, separate methods that need to check for a field change. Suppose I have 2 (or more) methods in my handler class that need to perform logic based on only opps that were just won. How would I best implement this paradigm

public class OpportunityTriggerHandler {

          public void OppBeforeUpdate(list<Opportunity> newOpps){
                 myMethod1(newOpps, oldOpps, newMap, oldMap);
          }

          public void OppAfterUpdate(list<Opportunity> newOpps, list<Opportunity> oldOpps, map<Id,Opportunity> newMap, map<Id,Opportunity> oldMap){
                 myMethod2(newOpps, oldOpps, newMap, oldMap);
                 myMethod3(newOpps, oldOpps, newMap, oldMap);
          }

          private void myMethod1(List<Opportunity> newOpps, List<Opportunity> oldOpps, map<Id, Opportunity> newMap, map<Id, Opportunity> oldMap){
                 //MY LOGIC HERE
          }

          private void myMethod2(List<Opportunity> newOpps, List<Opportunity> oldOpps, map<Id, Opportunity> newMap, map<Id, Opportunity> oldMap){
                 //MY LOGIC HERE
          }

          private void myMethod3(List<Opportunity> newOpps, List<Opportunity> oldOpps, map<Id, Opportunity> newMap, map<Id, Opportunity> oldMap){
                 //MY LOGIC HERE
          }

}
3
+100

After going over this question rather thoroughly, I think I came up with a utility that should do the trick. It's basically what @greenstork suggested, just implemented a little differently.

Below is the utility code along with implementation close to the original example:

public with sharing class BucketingUtility 
{
    public interface ITriggerComparisonFilter
    {
        Set<sObject> FilterResults(List<sObject> newsObjectList, Map<Id, sObject> oldMapForsObjects);
    }

    //Simple marker interface
    public interface IFilterable { }

    //Should be in in the IsWonOpportunityFilter class, but since I wrote it in one file, it is outside of it.
    public static Map<Id, Boolean> OldOpportunitiesThatWon = new Map<Id, Boolean>();

    public class IsWonOpportunityFilter implements ITriggerComparisonFilter, IFilterable
    {
        public Set<sObject> FilterResults(List<sObject> newsObjectList, Map<Id, sObject> oldMapForsObjects)
        {
            List<Opportunity> newOpportunities = (List<Opportunity>)newsObjectList;
            Map<Id, Opportunity> oldOpportunityMap = (Map<Id, Opportunity>)oldMapForsObjects;

            Set<sObject> wonOpportunities = new Set<sObject>();

            for(Opportunity singleOpportunity : newOpportunities)
            {
                Boolean oldHasWon = (OldOpportunitiesThatWon.containsKey(singleOpportunity.Id)) ? 
                    OldOpportunitiesThatWon.get(singleOpportunity.Id) : oldOpportunityMap.get(singleOpportunity.Id).isWon;

                if(singleOpportunity.isWon && !oldHasWon)
                    wonOpportunities.add(singleOpportunity);

                if(oldHasWon != singleOpportunity.isWon)
                    OldOpportunitiesThatWon.put(singleOpportunity.Id,singleOpportunity.isWon);
            }

            return wonOpportunities;
        }
    }

    public enum Bucket
    {
        OpportunitiesThatWon
    }

    private static Map<Bucket, IFilterable> BucketMethods = 
        new Map<Bucket, IFilterable>{ Bucket.OpportunitiesThatWon => new IsWonOpportunityFilter() };   

    private static Map<Bucket, Set<sObject>> BucketResults = new Map<Bucket, Set<sObject>>();

    private static Set<sObject> ProviderTriggerFilterResults(Bucket methodToBucketBy, List<sObject> newList, Map<Id, sObject> oldMap)
    {
        //I'm assuming the bucket will exist in BucketMethods
        ITriggerComparisonFilter triggerFilterMethod = (ITriggerComparisonFilter)BucketMethods.get(methodToBucketBy);
        BucketResults.put(methodToBucketBy, triggerFilterMethod.FilterResults(newList, oldMap));
        return BucketResults.get(methodToBucketBy);
    }

    public static Set<sObject> GetTriggerFilterResults(Bucket methodToBucketBy, List<sObject> newList, Map<Id, sObject> oldMap)
    {
        if(!BucketResults.containsKey(methodToBucketBy))
            return ProviderTriggerFilterResults(methodToBucketBy, newList, oldMap);
        if(BucketResults.get(methodToBucketBy) == null)
            return ProviderTriggerFilterResults(methodToBucketBy, newList, oldMap);
        return BucketResults.get(methodToBucketBy);
    }

    public static void ClearFilteredResults(Bucket bucketMethodToClearResults)
    {
        BucketResults.put(bucketMethodToClearResults, null);
    }

    public static Map<Id, sObject> GetTriggerFilterResultMap(Bucket methodToBucketBy, List<sObject> newList, Map<Id, sObject> oldMap)
    {
        return new Map<Id, SObject>(new List<sObject>(GetTriggerFilterResults(methodToBucketBy, newList, oldMap)));
    }

    public static Set<Id> GetTriggerFilterResultIds(Bucket methodToBucketBy, List<sObject> newList, Map<Id, sObject> oldMap)
    {
        return GetTriggerFilterResultMap(methodToBucketBy, newList, oldMap).keySet();
    }
}

Now you can share your results with however many methods within your particular trigger handler. Here is an example of how to use it:

public with sharing class OpportunityHandler 
{
    public static void OpportunityAfterUpdate(List<Opportunity> newOpportunities, List<Opportunity> oldOpportunities, Map<Id,Opportunity> newMap, 
        Map<Id,Opportunity> oldMap)
    {
        ExampleMethodForDemonstration(newOpportunities, oldOpportunities, newMap, oldMap);
        ExampleMethodIIForDemonstration(newOpportunities, oldOpportunities, newMap, oldMap);
        BucketingUtility.ClearFilteredResults(BucketingUtility.Bucket.OpportunitiesThatWon);
    }

    private static void ExampleMethodForDemonstration(List<Opportunity> newOpportunities, List<Opportunity> oldOpportunities, 
        Map<Id, Opportunity> newMap, Map<Id, Opportunity> oldMap)
    {
        Set<Id> opportunityIdsForWonSet = BucketingUtility.GetTriggerFilterResultIds(BucketingUtility.Bucket.OpportunitiesThatWon, 
            newOpportunities, oldMap);

        //Operate on won Opportunities Won Id Set in Example Method
    }

    private static void ExampleMethodIIForDemonstration(List<Opportunity> newOpportunities, List<Opportunity> oldOpportunities, 
        Map<Id, Opportunity> newMap, Map<Id, Opportunity> oldMap)
    {
        Set<Id> opportunityIdsForWonSet = BucketingUtility.GetTriggerFilterResultIds(BucketingUtility.Bucket.OpportunitiesThatWon, 
            newOpportunities, oldMap);

        //Operate on won Opportunities Won Id Set in Example Method II
    }
}

As you can see, I can now share the Id Set of all won Opportunities between method calls inside of the OpportunityHandler's OpportunityAfterUpdate method. I also only retrieve the Id Set once per call to OpportunityAfterUpdate. Once OpportunityAfterUpdate's methods are all complete, I clear the results using the BucketingUtility.ClearFilteredResults. This specifies that we should recalculate, using our filter to retrieve our Ids again, if another call occurs - like a workflow rule kicks off the trigger again. We still have the original static map to check against recursion still in place and should still guard against that.

|improve this answer|||||
  • 1
    I have not yet had a chance to implement this or play with it, but at first look, it looks like a pretty elegant solution. Thanks for this. – Chris Duncombe Oct 27 '14 at 13:14
3

Instead of populating oldIsWonMap in your method1, abstract out that logic into a separate method called populateIsWonMap and then within your processing methods, just call your method at the top top:

populateIsWonMap(newMap, oldMap);
//continue with your logic here

Since the map is declared outside of your methods, it can be used by each method and held in memory for the duration of the transaction. By abstracting the populating of the map to its own method, then you minimize duplicative code and you can reuse the method at the top of each processing class that needs that map.

EDIT: Expanded answer

public class OpportunityTriggerHandler {

   Private static Map <Id,boolean> oldIsWonMap;

   Private void populateIsWonMap(map<Id, Opportunity> newMap, map<Id, Opportunity> oldMap) {
         if(oldIsWonMap == null) {
              oldIsWonMap = new map<Id,boolean >();
              set<Id> oppIds = new set<Id>();
              for(Opportunity o : newMap.values()){
                boolean oldIsWon = (oldIsWonMap.containsKey(o.id)) ? oldIsWonMap.get(o.id) : oldmap.get(o.id).isWon;
                 if(o.isWon && !oldIsWon){
                       oppIds.add(o.Id);
                 }
                 if(oldIsWon != o.isWon) {
                    oldIsWonMap.put(o.id,o.isWon);
                 }
              }
          }
   }

   public void OppBeforeUpdate(list<Opportunity> newOpps){
          myMethod1(newOpps, oldOpps, newMap, oldMap);
   }

  ...

   private void myMethod1(List<Opportunity> newOpps, List<Opportunity> oldOpps, map<Id, Opportunity> newMap, map<Id, Opportunity> oldMap){
          populateIsWonMap(newMap, oldMap);
          //MY LOGIC HERE
   }
|improve this answer|||||
  • Can you expand a bit on how you would do it. You very well could be right, but I'm a bit unsure if it will work correctly in between methods as well as for a second (recursive) call in the same context. – Chris Duncombe Oct 10 '14 at 15:56
  • I've added to my answer – greenstork Oct 10 '14 at 16:20
  • Ok so if I call this populateIsWonMap again in method2, then none of the opps will look they have just closed, as we are filling the map in method1 with the trigger.new value of isWon, which is true. It seems this will only work when called for the first method. – Chris Duncombe Oct 10 '14 at 16:25
  • If you need triggered methods to happen in sequence, then you should call the second method from the first method and control what you pass into that method. – greenstork Oct 10 '14 at 16:59
  • It doesn't have to be in sequence, it just needs to be able to accurately gather the opportunities that have just become won. The logic is not at all dependent on each other. Just need the correct opps and need to use this paradigm to prevent duplication due to recursion. – Chris Duncombe Oct 10 '14 at 17:14
0

@sfdc_ninja, please excuse me if I'm over-simplifying the problem. I feel that what you're really trying to do is to prevent the same handler method from executing a second time within the same context. If this is the case, would the problem be solved by a solution that just "remembers" when a specific code context has already been processed?

Building on the example you provided:

public class OpportunityTriggerHandler {

    ...

    /*
     * The number of times business logic has been processed in the
     * after update context
     */
    private Boolean numberOfAfterUpdateOperations {
        get {
            if (numberOfAfterUpdateOperations == null)
                numberOfAfterUpdateOperations = 0;

            return numberOfAfterUpdateOperations;
        }
        set;
    }

    ...

    public void OppAfterUpdate(list<Opportunity> newOpps, list<Opportunity> oldOpps, map<Id,Opportunity> newMap, map<Id,Opportunity> oldMap){
        if (isFirstAfterUpdate()) {
            myMethod2(newOpps, oldOpps, newMap, oldMap);
            myMethod3(newOpps, oldOpps, newMap, oldMap);
        }
        rememberAfterUpdate();
    }

    /*
     * @return whether this is the first time processing business logic 
     *         in the after update context
     */
    public Boolean isFirstAfterUpdate() {
        return numberOfAfterUpdateOperations < 1;
    }

    /*
     * Remember that business logic was processed once more
     * in the after update context
     */
    public void rememberAfterUpdate() {
        numberOfAfterUpdateOperations++;
    }

    ...
}
|improve this answer|||||
  • I'm not quite sure this will work if a workflow update makes a change that you want to show up as a field change in your trigger. This strictly enforces the trigger only fires once, but what about certain cases where you want it fire again due to a change from a workflow. In those cases, you want it to fire again. This is why I am keeping track of values, not number of times executed. – Chris Duncombe Oct 21 '14 at 12:45
  • @sfdc_ninja, the idea is that for operations that you want to run every time the trigger executes, you move it outside the if (isFirstAfterUpdate) block. This should hopefully give you enough flexibility to separate one-time operations with all-the-time operations. Can you give an example of a situation where this paradigm would fall short? It would help clarify the question and maybe spur a better answer. – Marty C. Oct 21 '14 at 14:52
  • This would likely fail in a field change detection. If I have certain logic that fire if field 1 changes from A to B, and then other logic that fires if Field 1 changes from B to C. It may change from A to B and the trigger fires, and then a workflow changes that field again from B to C, we want that trigger to fire a second time. This is why I fear using logic that checks if it is the first or second invocation of the trigger within the same context will not work. – Chris Duncombe Oct 21 '14 at 14:59

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.