4

I need to insert Monthly Revenue Forecast records for each Opportunity that's been inserted / updated, matches my criteria and caused the trigger to fire.

When I execute the below, in bulk via a test class, it causes an error

System.LimitException: Apex CPU time limit exceeded

I've already changed the lines after if(*list* size() > 0){ from for(Opportunity o : insOpps){ to use the SOQL query's instead.

And my trigger's not updating any fields on the Opportunity - and therefore causing the trigger to fire again.

So I'm guessing that either I've got my syntax wrong (but I am closing each For loop and have the DLM statements outside the loops) or my use of integers with the SOQL for loop is causing the issue, could you please suggest potential causes?

trigger MRFv2 on Opportunity (after insert, after update) {

    List<WEID__c> weids = WEID__c.getall().values();
    Set<Id> validRecordTypeIds = new Set<Id>();

    // add Opportunity Record Type Ids from Custom Setting to list of valid Ids      
    for(WEID__c weid: weids) {
        try {
            validRecordTypeIds.add(weid.WEOppId__c);
        } catch (System.StringException e) {
            System.debug(
                System.LoggingLevel.ERROR,
                'Invalid Record Type Id ' + weid.WEOppId__c);
        }
    }

    List<Opportunity> insOpps = new List<Opportunity>();
    List<Opportunity> upOpps = new List<Opportunity>();
    Map<Id,Integer> forecastDuration = new Map<Id,Integer>();
    Map<Integer,String> monthName = new Map<Integer,String>{1 => 'January', 2 => 'February', 3 => 'March', 4 => 'April', 5 => 'May', 6 => 'June', 7 => 'July', 8 => 'August', 9 => 'September', 10 => 'October', 11 => 'November', 12 => 'December'};
    List<Monthly_Revenue_Forecast__c> existMRFs = new List<Monthly_Revenue_Forecast__c>();
    List<Monthly_Revenue_Forecast__c> newMRFs = new List<Monthly_Revenue_Forecast__c>();
    Map<Id,Decimal> rampSett = new Map<Id,Decimal>();
    Map<Id,Decimal> rampTxnRev = new Map<Id,Decimal>();

    for(Opportunity o : Trigger.New){

        if(validRecordTypeIds.contains(o.RecordTypeId))
        {
            try{
                Opportunity oldO = Trigger.OldMap.get(o.Id);

                if(oldO.CloseDate != o.CloseDate
                   ||
                   oldO.Annual_Settlement_Value__c != o.Annual_Settlement_Value__c
                   ||
                   oldO.Implementation_Revenue__c != o.Implementation_Revenue__c
                   ||
                   oldO.Implementation_Revenue_as_a_of_FSR__c != oldO.Implementation_Revenue_as_a_of_FSR__c
                   ||
                   oldO.FSR_End_Date__c != o.FSR_End_Date__c
                   ||
                   oldO.Monthly_Txn_Revenue_POS__c != o.Monthly_Txn_Revenue_POS__c
                   ||
                   oldO.Annual_transaction_volume__c != o.Annual_transaction_volume__c)
                {
                    upOpps.add(o);
                    forecastDuration.put(o.Id, o.Implementation_Revenue__c.monthsBetween(o.FSR_End_Date__c));
                    system.debug('upOpps contains ' + upOpps);
                    system.debug('forecastDuration contains ' + forecastDuration);
                }
                //if old.Map is null, record has been inserted & MRF records should be added
            }catch(system.NullPointerException e){
                insOpps.add(o);
                forecastDuration.put(o.Id, o.Implementation_Revenue__c.monthsBetween(o.FSR_End_Date__c));
            }
        }
    }

    if(insOpps.size() > 0){

        for(Opportunity o : [SELECT Id,Name,Implementation_Revenue__c,Implementation_Revenue_as_a_of_FSR__c,
                                    Annual_Settlement_Value__c,Annual_transaction_volume__c,Monthly_Txn_Revenue_POS__c,
                                    CurrencyIsoCode 
                               FROM Opportunity
                              WHERE Id IN :insOpps])
        {

            Date ic = o.Implementation_Revenue__c;
            Integer icm = o.Implementation_Revenue__c.month();//DELETE?

            //Ramp calculations
            Decimal increment = o.Implementation_Revenue_as_a_of_FSR__c / 100;
            Integer rDur = (1 / increment).setscale(0,roundingMode.DOWN).intValue();
            Integer fcstDuration = forecastDuration.get(o.Id);
            Decimal monthlySett = o.Annual_Settlement_Value__c / fcstDuration;
            Decimal annualTxnRev = o.Annual_transaction_volume__c*o.Monthly_Txn_Revenue_POS__c;
            Decimal monthlyTxnRev = annualTxnRev/fcstDuration;

            //FSR calculations
            Integer fsrDur = forecastDuration.get(o.Id) - rDur;

            Decimal settLoopTotal = 0.00;
            Decimal txnrevLoopTotal = 0.00;

            //add Monthly Forecast records for 'ramp' months
            for(Integer m = 0; m < rDur; m++){

                Decimal ramp = increment*(m + 1);
                Decimal monthSett = monthlySett*ramp;
                Decimal monthTxnRev = monthlyTxnRev*ramp;

                Monthly_Revenue_Forecast__c fm = new Monthly_Revenue_Forecast__c();
                fm.Opportunity__c = o.Id;
                fm.CurrencyIsoCode = o.CurrencyIsoCode;
                fm.Name = monthName.get(ic.addMonths(m).month()) +' '+ ic.addMonths(m).year();
                fm.Revenue_Date__c = ic.addMonths(m + 1).toStartOfMonth() - 1;
                fm.Monthly_Settlement1__c = monthSett;
                fm.Transaction_Revenue__c = monthTxnRev;

                settLoopTotal = settLoopTotal + monthSett;
                txnrevLoopTotal = txnrevLoopTotal + monthTxnRev;

                newMRFs.add(fm);

                if(m == rDur - 1){
                    rampSett.put(o.Id,settLoopTotal);
                    rampTxnRev.put(o.Id,txnrevLoopTotal);
                    settLoopTotal = 0;
                    txnrevLoopTotal = 0;
                }
            }

            Decimal mnthlyFsrSett = (o.Annual_Settlement_Value__c - rampSett.get(o.Id)) / fsrDur;
            Decimal mnthlyFsrTxnRev = (annualTxnRev - rampTxnRev.get(o.Id)) / fsrDur;

            //add Monthly Forecast Records for rFSR Months
            for(Integer m = rDur; m < rDur + fsrDur; m++){

                Monthly_Revenue_Forecast__c fm = new Monthly_Revenue_Forecast__c();
                fm.Opportunity__c = o.Id;
                fm.CurrencyIsoCode = o.CurrencyIsoCode;
                fm.Name = monthName.get(ic.addMonths(m).month()) +' '+ ic.addMonths(m).year();
                fm.Revenue_Date__c = ic.addMonths(m + 1).toStartOfMonth() - 1;
                fm.Monthly_Settlement1__c = mnthlyFsrSett;
                fm.Transaction_Revenue__c = mnthlyFsrTxnRev;

                newMRFs.add(fm);
            }
        }
    }

    if(upOpps.size() > 0){

        for(Opportunity o : [SELECT Id,
                            (SELECT Id FROM Monthly_Revenue_Forecasts__r) FROM Opportunity
                              WHERE Id IN :upOpps])
        {               

            if(o.Monthly_Revenue_Forecasts__r.size()> 0){
                existMRFs.addAll(o.Monthly_Revenue_Forecasts__r);
            }
        }
        delete existMRFs;

        for(Opportunity o : [SELECT Id,Name,Implementation_Revenue__c,Implementation_Revenue_as_a_of_FSR__c,
                                    Annual_Settlement_Value__c,Annual_transaction_volume__c,Monthly_Txn_Revenue_POS__c,
                                    CurrencyIsoCode 
                               FROM Opportunity
                              WHERE Id IN :upOpps])
        {

            Date ic = o.Implementation_Revenue__c;
            Integer icm = o.Implementation_Revenue__c.month();//DELETE?

            //Ramp calculations
            Decimal increment = o.Implementation_Revenue_as_a_of_FSR__c / 100;
            Integer rDur = (1 / increment).setscale(0,roundingMode.DOWN).intValue();
            Integer fcstDuration = forecastDuration.get(o.Id);
            Decimal monthlySett = o.Annual_Settlement_Value__c / fcstDuration;
            Decimal annualTxnRev = o.Annual_transaction_volume__c*o.Monthly_Txn_Revenue_POS__c;
            Decimal monthlyTxnRev = annualTxnRev / fcstDuration;

            //FSR calculations
            Integer fsrDur = forecastDuration.get(o.Id) - rDur;

            Decimal settLoopTotal = 0.00;
            Decimal txnrevLoopTotal = 0.00;

            //add Monthly Forecast records for 'ramp' months
            for(Integer m = 0; m < rDur; m++){

                Decimal ramp = increment*(m + 1);
                Decimal monthSett = monthlySett*ramp;
                Decimal monthTxnRev = monthlyTxnRev*ramp;

                Monthly_Revenue_Forecast__c fm = new Monthly_Revenue_Forecast__c();
                fm.Opportunity__c = o.Id;
                fm.CurrencyIsoCode = o.CurrencyIsoCode;
                fm.Name = monthName.get(ic.addMonths(m).month()) +' '+ ic.addMonths(m).year();
                fm.Revenue_Date__c = ic.addMonths(m + 1).toStartOfMonth() - 1;
                fm.Monthly_Settlement1__c = monthSett;
                fm.Transaction_Revenue__c = monthTxnRev;

                settLoopTotal = settLoopTotal + monthSett;
                txnrevLoopTotal = txnrevLoopTotal + monthTxnRev;

                newMRFs.add(fm);

                if(m == rDur - 1){
                    rampSett.put(o.Id,settLoopTotal);
                    rampTxnRev.put(o.Id,txnrevLoopTotal);
                    settLoopTotal = 0;
                    txnrevLoopTotal = 0;
                }
            }

            Decimal mnthlyFsrSett = (o.Annual_Settlement_Value__c - rampSett.get(o.Id)) / fsrDur;
            Decimal mnthlyFsrTxnRev = (annualTxnRev - rampTxnRev.get(o.Id)) / fsrDur;

            //add Monthly Forecast Records for rFSR Months
            for(Integer m = rDur; m < rDur + fsrDur; m++){

                Monthly_Revenue_Forecast__c fm = new Monthly_Revenue_Forecast__c();
                fm.Opportunity__c = o.Id;
                fm.CurrencyIsoCode = o.CurrencyIsoCode;
                fm.Name = monthName.get(ic.addMonths(m).month()) +' '+ ic.addMonths(m).year();
                fm.Revenue_Date__c = ic.addMonths(m + 1).toStartOfMonth() - 1;
                fm.Monthly_Settlement1__c = mnthlyFsrSett;
                fm.Transaction_Revenue__c = mnthlyFsrTxnRev;

                //if test for empty list?
                newMRFs.add(fm);
            }
        }
    }
    insert newMRFS;
}

UPDATE

When I reduce the number of records which are inserted / updated in my test Class, a different error message is displayed

Too many DML rows: 10001

UPDATE

I've now isolated the cause of the DML row issue to the second inner loop for the section of my code that starts if(upOpps.size() > 0){. If I comment out the section, see below, the trigger passes my test successfully.

            Decimal mnthlyFsrSett = (o.Annual_Settlement_Value__c - rampSett.get(o.Id)) / fsrDur;
            Decimal mnthlyFsrTxnRev = (annualTxnRev - rampTxnRev.get(o.Id)) / fsrDur;

            //add Monthly Forecast Records for rFSR Months
          /*  For(Integer m = rDur; m < rDur + fsrDur; m++){

                Monthly_Revenue_Forecast__c fm = new Monthly_Revenue_Forecast__c();
                fm.Opportunity__c = o.Id;
                fm.CurrencyIsoCode = o.CurrencyIsoCode;
                fm.Name = monthName.get(ic.addMonths(m).month()) +' '+ ic.addMonths(m).year();
                fm.Revenue_Date__c = ic.addMonths(m + 1).toStartOfMonth() - 1;
                fm.Monthly_Settlement1__c = mnthlyFsrSett;
                fm.Transaction_Revenue__c = mnthlyFsrTxnRev;

                //if test for empty list?
                newMRFs.add(fm);
            }*/
        }
    }
    insert newMRFS;
}

This is pretty odd as the inner loop is a copy from the if(insOpps.size() > 0){ section, which does have code coverage.

The size of the upOpps list is 0, if I add the debug line just before or just after if(upOpps.size() > 0){ whereas insOpps size is 200.

But my test class inserts the 200 Opportunities and then does update them - I have code coverage for the lines after the upOpps size if test.

Test Class

        //create Opportunitys to update, in bulk
    List<Opportunity> opportunitys = new List<Opportunity>();

    for (Integer i = 0; i < 200; i++) {

        Opportunity o = new Opportunity();
        o.name = 'Test Opp' + i;
        o.RecordTypeId = '012700000005qif';
        o.OwnerId = u.Id;

        o.StageName = '1) Suspect';
        o.CloseDate = date.newInstance(2015,06,15);
        o.Implementation_Revenue__c = date.newInstance(2015, 08, 15);
        o.FSR_End_Date__c = date.newInstance(2016, 08, 15);
        o.Implementation_Revenue_as_a_of_FSR__c = 40;

        //cover Forecast validation rules
        o.Monthly_Txn_Revenue_POS__c = 0;
        o.Annual_transaction_volume__c = 0;

        //cover Credit Agreement Details Required validation rule
        o.Credit_Limit__c = 100.00;
        o.Date_Credit_Limit_Approved__c = date.newInstance(1901, 01, 01);
        o.Payment_Terms_Days__c = 5;
        o.Billing_Cycle__c = 'Daily';

        //cover Revenue Values Required validation rule
        o.Annual_Settlement_Value__c = 100.00;
        o.Average_transaction_Value_POS__c = 1.00;

        opportunitys.add(o);
    }
    insert opportunitys;
    system.debug('opportunitys contains ' + opportunitys);

    for(Opportunity upOppsDate : opportunitys){
        upOppsDate.CloseDate = date.newInstance(2015,07,15);
    }
    update opportunitys;
  • I've experienced this limit when using the Math class, which eats up CPU. Have you considered using Flows, pre-calculating, or using Custom field formulas to reference? Does this happen with 20 or 200 records? – James Jun 23 '15 at 14:26
  • @James I could use custom formula fields..but there might be another cause of the issue - when I reduce the number of records in the test to 20, I get a different error 'Too many DML rows: 10001' so I must have included a DML statement in a loop but I can't see where - could this be linked to using the SOQL query to find the Opportunities to relate the MRF records to? – Alex S Jun 23 '15 at 14:50
  • I would like at using batch apex to handle the Too many DML rows: 10001 error: developer.salesforce.com/docs/atlas.en-us.apexcode.meta/… – Girbot Jun 23 '15 at 15:19
  • 1
    @AlexS, i recommend putting debug logs to include the size of all Lists and work with 1 record to test the scale ( if 1 record creates 100 new records, you'll definitely hit issues. I suspect a list is loaded with too manay rows, so find a way to reduce this and make even more efficient. as Girbot mentioned, should you require more than 10K DML rows, you'll need to leverage batch APEX. – James Jun 23 '15 at 15:22
  • @Girbot thanks for the suggestion, the test definitely isn't supposed to create 10,001 records so something else is causing the issue. – Alex S Jun 23 '15 at 15:23
1

There are a number of issues with your code. So many that I don't want to go into detail with them all. First is that it would be very helpful to you to separate your initial test for data into an

If(Trigger.isInsert)
{
   for(Opportunity o : Trigger.New)
   {
      forecastDuration.put(o.Id, o.Implementation_Revenue__c.monthsBetween(o.FSR_End_Date__c));

   {

    upOpps.addAll(trigger.new);
{ 

// there's nothing in trigger.old to test against, so no need in AfterInsert

If(Trigger.isUpdate)
{
   for(Opportunity o : Trigger.New)
   {

       // record selection code here
   {

{

Opportunity oldO = Trigger.OldMap.get(o.Id);

For you selection criteria in AfterUpdate:

// try{
//              Opportunity oldO = Trigger.OldMap.get(o.Id);

You don't want to use a try-catch block in your trigger. This is cpu intensive. With separate blocks, this is not necessary. Test it and see. If fields aren't required or validated, do a null test instead.

// record selection code 

for(Opportunity o : Trigger.New)
{
   if(oldO.CloseDate != o.CloseDate
      ||
      oldO.Annual_Settlement_Value__c != o.Annual_Settlement_Value__c
      ||
      oldO.Implementation_Revenue__c != o.Implementation_Revenue__c
      ||
      oldO.Implementation_Revenue_as_a_of_FSR__c != oldO.Implementation_Revenue_as_a_of_FSR__c
      ||
      oldO.FSR_End_Date__c != o.FSR_End_Date__c
      ||
      oldO.Monthly_Txn_Revenue_POS__c != o.Monthly_Txn_Revenue_POS__c
      ||
      oldO.Annual_transaction_volume__c != o.Annual_transaction_volume__c)
      {
      upOpps.add(o);
      forecastDuration.put(o.Id, o.Implementation_Revenue__c.monthsBetween(o.FSR_End_Date__c));

{

This is SIX OR conditions to test at the same time. It might behoove you to break these up into 6 If conditions where you add the ID's to a set. Once you have them, at the end, add all of them to a single set. Let's call it OupIds for our purposes.

You can convert OupIds to a list from trigger.newmap to get your list of opportunities and create your map for the oppIds that are in the set should you need to. I suspect that a test will show that doing it this way will prove faster, but don't have comparable data at the moment I can test with to prove one way or another. I can only guarantee that it will be faster than repeating the try-catch that you're using.

for(Opportunity o : [SELECT Id,Name,Implementation_Revenue__c,Implementation_Revenue_as_a_of_FSR__c,
                                    Annual_Settlement_Value__c,Annual_transaction_volume__c,Monthly_Txn_Revenue_POS__c,
                                    CurrencyIsoCode 
                               FROM Opportunity
                              WHERE Id IN :insOpps])
        {

You don't need to run this query. You will already have all of this info. It should be contained in trigger.new or trigger.newmap for the Ids that you've determined need to be processed.

I would run the following loop on OupIds, not on the object and pull the data from trigger.newmap.

For(Id oid:OupIds)
{

   Date ic = trigger.newmap.get(oid).Implementation_Revenue__c;
   Integer icm = trigger.newmap.get(oid).Implementation_Revenue__c.month();//DELETE?

   //Ramp calculations
   Decimal increment = trigger.newmap.get(oid).Implementation_Revenue_as_a_of_FSR__c / 100;

   // You're inside of a loop, don't define each of these variables each time through!!! 
   // Initialize them once and only once outside the loop instead

   Integer rDur = (1 / increment).setscale(0,roundingMode.DOWN).intValue();
   Integer fcstDuration = forecastDuration.get(oid);
   Decimal monthlySett = trigger.newmap.get(oid).Annual_Settlement_Value__c / fcstDuration;
   Decimal annualTxnRev = trigger.newmap.get(oid)..Annual_transaction_volume__c*trigger.newmap.get(oid).Monthly_Txn_Revenue_POS__c;
   Decimal monthlyTxnRev = annualTxnRev/fcstDuration;

   //FSR calculations
   Integer fsrDur = forecastDuration.get(oId) - rDur;

   Decimal settLoopTotal = 0.00;
   Decimal txnrevLoopTotal = 0.00;

   //add Monthly Forecast records for 'ramp' months
   for(Integer m = 0; m < rDur; m++){

   Decimal ramp = increment*(m + 1);
   Decimal monthSett = monthlySett*ramp;
   Decimal monthTxnRev = monthlyTxnRev*ramp;

   // ### same comments here on these variables, you're recreating them within an inner loop!!!

   Monthly_Revenue_Forecast__c fm = new Monthly_Revenue_Forecast__c();
   fm.Opportunity__c = oId;
   fm.CurrencyIsoCode = trigger.newmap.get(oid).CurrencyIsoCode;
   fm.Name = monthName.get(ic.addMonths(m).month()) +' '+ ic.addMonths(m).year();
   fm.Revenue_Date__c = ic.addMonths(m + 1).toStartOfMonth() - 1;
   fm.Monthly_Settlement1__c = monthSett;
   fm.Transaction_Revenue__c = monthTxnRev;

   settLoopTotal = settLoopTotal + monthSett;
   txnrevLoopTotal = txnrevLoopTotal + monthTxnRev;

   newMRFs.add(fm);

   if(m == rDur - 1){
      rampSett.put(oid,settLoopTotal);
      rampTxnRev.put(oid,txnrevLoopTotal);
      settLoopTotal = 0;
      txnrevLoopTotal = 0;
   }
}

I'm certain you can see where these comments would apply to the remaining portions of your code.

  • Thanks so just to clarify, you think it's mainly the calculations inside the loop which are causing the issue? Do I need to store each 'calculation' (the decimals, integers etc.) in a map, with their Opportunity Id so that I can reference the from the Integer loops or..? – Alex S Jun 23 '15 at 17:10
  • First, I was responding to the CPU time limit exceeded and either you hadn't updated with the "too many DML", or I didn't see it. Off hand, I'm not seeing where you're repeatedly accessing the same values between inner and outer loops. Feel free to point them out to me. If there's someplace you're repeating the same calculation on same numbers in the outer loop again in the inner loop, then obviously a map would be beneficial. And I think it's a combination of selections, declarations and calculations, but more declarations & calculations. – crmprogdev Jun 23 '15 at 17:32
  • It's the same calculation but different numbers each time so if I need to do those calculations outside the loops, I think I need somewhere to store the results (whereas obviously they're populating the fields of the MRF records immediately at the moment), what would you recommend? I'll come back to you about accessing the same values in the inner & outer loops, I should be able to work out how to manage that. – Alex S Jun 23 '15 at 17:44
  • 1
    If different numbers each time and you're writing them to a list<object>, no need to put in a separate map unless map<id,object> since you can access the current state of the object from the loop. Just use that instance of the list<object> or map<id,object> to retrieve values as needed. You don't want to duplicate effort if you're exceeding cpu time and it's already in memory. If not writing directly to object or map of object, then separate maps would make sense if you need it later. – crmprogdev Jun 23 '15 at 18:08
  • Am I right in thinking that I can't write the results of the calculations to an object - unless they're populating a field? So I'll use maps i.e. map<id,integer> to store variable icm for example. Just wanted to check that I'm not missing anything there. – Alex S Jun 24 '15 at 10:10

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.