5

First: I know this question has been posted many times so I apologize for making a new one, but I read over a lot of the old ones and they almost always seem to be that the SOQL query is inside of a loop. However, unless I'm totally blind, my query is not within a loop and I can't figure out what else I can do to optimize it further to avoid the governor limit.

The Ultimate Question: Why is this trigger being triggered in this context, and how can I optimize it so that I don't hit the Query limit?

Scenario:

We have a custom object called Account Administration.

When Account Administration is edited we send an outbound message to a receiver that updates an external SQL database with the new data from Account Admin. However, some of the important data from Account Admin is actually in formula fields that pull the data from the Account object that exists as its parent. Since formula changes don't trigger an edit/save, the outbound messages weren't going out when one of those important pieces changed on the account.

To combat this, we created a very basic trigger (Trigger.UpdateAccountAdmin) that gets a list of accounts where relevant fields have changed, and then does a SOQL query to get the list of Account Admin records that match the acctIds with relevant changes. We then just do a quick edit/save on each of those account admin records by way of setting a value to equal itself. This creates a save without modifying any actual record data, which triggers the outbound message as expected.

This trigger is live in Production and doing its job.


However, I am receiving the following Apex Test Failure when validating another ChangeSet against production:

Apex Test Failure - Too many SOQL Queries: 101 Stack Trace Trigger.UpdateAccountAdmin line 34

Class Name: AccountUpdateHandler_TEST

Method Name: updateUsedTrainingTimeUsingEventMapsTest

Error Message: System.LimitException: Too many SOQL queries: 101.

Stack Trace: Trigger.UpdateAccountAdmin: line 34, column 1


Stack Trace points to line 34 in Trigger.UpdateAccountAdmin:

//get Account Admins For Update
acctAdminToUpdate = [SELECT Id, Enable_Online_Registration__c FROM Account_Administration__c WHERE Account_Name__c IN :acctIds];

Full UpdateAccountAdmin Trigger:

Trigger UpdateAccountAdmin on Account (before insert, after update)
{
   boolean haveRelevantFieldsChanged(Account newAccount, Account oldAccount){
        System.debug('!! newAccount: '+newAccount);
        System.debug('!! oldAccount: '+oldAccount);
        if(newAccount.Email__c != oldAccount.Email__c 
        || newAccount.Billing_Status__c != oldAccount.Billing_Status__c 
        || newAccount.Customer_Number__c != oldAccount.Customer_Number__c 
        || newAccount.Delinquency_Date__c != oldAccount.Delinquency_Date__c 
        || newAccount.Parent.Customer_Number__c != oldAccount.Parent.Customer_Number__c 
        || newAccount.Parent.LEADS_Legacy_ID__c != oldAccount.Parent.LEADS_Legacy_ID__c 
        || newAccount.Password__c != oldAccount.Password__c 
        || newAccount.Support_Level__c != oldAccount.Support_Level__c){
        return true;
        }
        else{
        return false;
        }
    }

if (Trigger.isAfter && Trigger.isUpdate) {
    // Lists
    List<Id> acctIds = new List<Id>();
    List<Account_Administration__c> acctAdminToUpdate = new List<Account_Administration__c>();

    //Get Account IDs
    for(Account a :Trigger.new)
    {
        if(haveRelevantFieldsChanged(a, Trigger.oldMap.get(a.Id)))
            acctIds.add(a.Id); // Add IDs to List
    }

    //get Account Admins For Update
    acctAdminToUpdate = [SELECT Id, Enable_Online_Registration__c FROM Account_Administration__c WHERE Account_Name__c IN :acctIds];

    //iterate through Account Admins to update
    for(Account_Administration__c aa :acctAdminToUpdate)
    { 
        aa.Enable_Online_Registration__c = aa.Enable_Online_Registration__c; 
    }//trigger update on save

    //update Account Admin
    update acctAdminToUpdate;
    }
}

The Problem:

The Class (AccountUpdateHandler_TEST) and Method Name (updateUsedTrainingTimeUsingEventMapsTest) mentioned in the Test Failure don't even modify any of the relevant fields that are checked by the boolean in the Trigger that we use to determine if changes are relevant before adding them to the acctIds list, so I don't understand why it's even triggering in the first place.

AccountUpdateHandler_TEST.updateUsedTrainingTimeUsingEventMapsTest:

@isTest
private class AccountUpdateHandler_TEST {

    //tests method: updateUsedTrainingTimeUsingEventMaps(Map<Id,Event> oldEventMap, Map<Id,Event> newEventMap)
    @isTest static void updateUsedTrainingTimeUsingEventMapsTest() {

        // Create Custom Settings for MSI TriggerControl
        MSITriggerControlHelper.create();

        // Implement test code

        Product2 product = new Product2(Name = 'Non-Account WhatId');
        insert product;

        Account account = new Account(Name = 'Test Account');
        account.Total_Training_Minutes__c = 0;
        insert account;

        Event event = new Event();
        event.DurationInMinutes = 60;
        event.ActivityDateTime = Datetime.now();

        insert event;

        // If the event was not associated with an account, don't bother looking at this event
        update event;

        // If the account associated with this event could not be retrieved, don't bother looking at this event
        // WhatId is set to a product, so no related event was returned in query
        event.WhatId = product.Id;
        update event;

        // Move on to the next event if:
        // (a) the event does not contain training or
        // (b) the event length was not specified
        event.WhatId = account.Id;
        update event;

         // If the account has no training minutes left
         //move on to the next event
        event.Activity__c = 'Onsite Training';
        update event;

        // If the event is 'newly' completed (i.e., the status is moving from 'Open' to 'Completed', we add the account id and the training minutes 
        // used to a map and move on to the next event
        // Check if the amount of minutes we are about to claim as 'used' will exceed the total number of training minutes remaining on the account
        event.Event_length_in_Minutes__c = '60'; 
        update event;
        account.Total_Training_Minutes__c = 30;
        update account;
        update event;

        //usedMinutes >= account.Total_Training_Minutes__c
        event.Status__c = 'Open';
        event.Event_length_in_Minutes__c = '240';
        update event;
        event.Status__c = 'Completed';  
        update event;

        //usedMinutes < account.Total_Training_Minutes__c
        account.Total_Training_Minutes__c = 600;
        update account;
        event.Status__c = 'Open';
        event.Event_length_in_Minutes__c = '60';
        update event;
        event.Status__c = 'Completed';  
        update event;

        // Another scenario is if the status of the event was previously completed but the event minutes has changed.
        // If the status has stayed as 'completed' and there was no change in the event length, move on to the next event
        event.Status__c = 'Completed';   
        update event;

        Account account2 = new Account(Name = 'Test Account');
        account2.Total_Training_Minutes__c = 600;
        insert account2;
        event.WhatId = account2.Id;
        update event;

        //account.Used_Training_Minutes__c == null
        event.Event_length_in_Minutes__c = '240';
        update event;

        //account.Used_Training_Minutes__c >= previousEventLength
        account2.Used_Training_Minutes__c = 600;
        update account2;
        event.Event_length_in_Minutes__c = '60';
        update event;

        //account.Used_Training_Minutes__c < previousEventLength
        account2.Used_Training_Minutes__c = 10;
        update account2;
        event.Event_length_in_Minutes__c = '240';
        update event;

        //// Check if the amount of minutes we are about to claim as 'used' will exceed the total number of training minutes remaining on the account

        //usedMinutes >= account.Total_Training_Minutes__c
        account2.Used_Training_Minutes__c = 600;
        account2.Total_Training_Minutes__c = 50;
        update account2;

        event.Event_length_in_Minutes__c = '60';
        update event;


    }   

The Ultimate Question (Repeated from Top): Why is this trigger being triggered in this context, and how can I optimize it so that I don't hit the Query limit?

  • 1
    Any chance updates to the Account_Administration__c object will cause updates to the Account object? Also, does the test currently fail in production? Not clear based on reading your OP, though perhaps I didn't read it carefully enough. – Adrian Larson Feb 9 '16 at 17:00
  • Hi @AdrianLarson thanks for your reply. We don't have any triggers firing directly from the Account Administration Sobject (the outbound message is via workflow rules), and my understanding is no trigger means nothing could be firing so I don't think updates on Account_Administration__c are updating Account. The AccountUpdateHandler_TEST class runs 2/2 Test Methods successfully in production when I go directly to the class to run the tests. updateUsedTrainingTimeUsingEventMapsTest passes in that context. – Morgan Marchese Feb 9 '16 at 17:05
  • 3
    SOQL Limits is an aggregate, meaning that it may not be this trigger it could of just broke it.. – EricSSH Feb 9 '16 at 17:25
  • 1
    Derek's suggestion that your test method is too big is accurate. It appears you've got 5 or 6 'scenarios' that you're running through with the test data. Each of those individual scenarios should be its own little test. At that point it would be easy to wrap the code that is being tested (i.e. an individual update call which will execute your trigger logic) in Test.startTest() and Test.stopTest(). This will isolate your test setup from the logic being run by the code you're testing. Related Question: When to use Test.startTest – Mark Pond Feb 9 '16 at 18:00
  • 1
    Thanks @MarkPond, always a pleasure. I've marked Derek's answer as correct as I have already begun implementing some of his suggestions and noticed an improvement in my test methods passing. – Morgan Marchese Feb 9 '16 at 21:18
5

Your trigger looks fine to me. Just one quick tip. You don't actually need to do anything to the collection of Account_Administration__c records before calling DML update. You will cause your trigger to fire (assuming you have a before/after update trigger on Account_Administration__c) simply by virtue of calling update <collection>.

You could go so far as to just issue an update on the result set of a query.

update [SELECT Id, Enable_Online_Registration__c FROM Account_Administration__c WHERE Account_Name__c IN :acctIds];

Now, on to your failing test.

From the look of it, you're doing a ton of setup. I didn't count exactly how many dml updates were issued, but it's not a small number.

Each of those updates causes triggers to run, which probably contribute to the number of SOQL queries being run before your test proper even begins.

I have two suggestions:

  1. see if you can consolidate some of those Event updates
  2. wrap your actual test code inside Test.startTest() and Test.stopTest()

The second suggestion in particular will help you out. It is used to separate test setup from the actual test that you're trying to run. It also separates the governor limits, giving you a fresh set for use in your test proper.

+edit:

On further inspection, your unit test is also incomplete. There aren't any assertions being made, so you're not really testing anything. You're getting the code coverage, yes, but covered != tested.

I could still look over this test some more; but, after my second reading, I get the feeling that instead of being a single test, this should be broken up into multiple tests (each testing only one of the event updates, querying the account after the test, and making assertions against the result state of your account).

|improve this answer|||||
  • Thanks for your insight Derek, definitely didn't know I could do the DML update without changing the record, I will make that change now. Regarding my failing test, you mentioned that each DML update causes the trigger to run. Can I assume that means that even though the DML update is irrelevant to the context of the trigger, it's still doing the SOQL call even if the acctIds list is empty because no relevant fields have changed? – Morgan Marchese Feb 9 '16 at 17:16
  • 1
    @MorganMarch Every call to insert, update, delete, or undelete causes any relevant before and after triggers to fire. – Adrian Larson Feb 9 '16 at 17:21
  • 1
    @MorganMarch Yes, and no. Only the DML on Accounts will cause the query to run, but again, you have all the update event DML which may be making queries as well. If the trigger for Account is run, however, you don't have any condition surrounding your query for Account_Administration__c records to prevent it from being run if acctIds is empty, so the query will be run. You're not running your Account_Administration__c query 101 times, it just happens to be the query that put you over the 100 query limit. – Derek F Feb 9 '16 at 17:27
  • @DerekF - Learning a lot here, very thankful for your time! Good to know that its just THAT query hitting the limit rather than that query being run 100 times, that will help diagnosis. I did notice that I'm not doing a simple size check on acctIds before doing the query, so that is an easy improvement. I can then also do a size check on acctAdminToUpdate before updating, to avoid unnecessary DML updates. And I see what you're saying in your edit regarding splitting up the tests and making Assertions. Looks like lots of room for me to improve still, will get started on this now. Thank you! – Morgan Marchese Feb 9 '16 at 17:32

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.