3

Having a problem with below code reaching too many queries (we have Hubspot and I'm receiving the error on HubSpot_Inc in my tests).I avoided using queries in loops, so looking for suggestions on how to improve. I don't necessarily need the pagination, but I don't think that is causing the issue. I'm relatively new to SF, so I know this might not be the best code. I was able to cover 100% in my test class (though it wasn't fun).

Also, when I run Opportunity queries (in the code below) the maximum result was about 3000.

Not 100% sure how to debug where the limit is being pushed

 public class SupportNotifyController {

/* Summary
   Custom contact controller that will create a contact list
   based upon the Account Opportunities that 'Closed Won'.
   A Visual Force page will let users filter by Product Family.
   Once filtered the contact list can be added to a campaign.
End Summary */

public List<Contact> contactList {get; set;}
public List<CampaignMember> bulletin {get; set;}
public List<Opportunity> oppt {get; set;}
public Campaign cp {get; set;}
public String selected {get; set;}
public Integer listCount {get; set;}
public Integer alistCount {get; set; }
public String campaigns {get; set; }
public Integer size{get;set;}
public Integer noOfRecords{get; set;}

//Get contact records for pagination
public ApexPages.StandardSetController contactRecords {
    get {
        if(contactRecords == null) {
            size = 100;
            contactRecords = new ApexPages.StandardSetController(getContacts());
            contactRecords.setPageSize(size);
            noOfRecords = contactRecords.getResultSize();
        }
        return contactRecords;
    }
    private set;
}

//Return records for paginations
public List<Contact> getContactPagination() {
    return (List<Contact>) contactRecords.getRecords();
}

//Method to filter contacts by Product Family chosen in VF page dropdown
public List<Contact> getFilterContacts(String option) {                
    if(selected == 'Hardware') {       
        Map<Id, Opportunity> opptMap = new Map<Id, Opportunity>([Select ID, AccountID From Opportunity o WHERE Id IN (Select OpportunityId From OpportunityLineItem WHERE Product_Family__c LIKE 'Hardware%') AND StageName='Closed Won']);
        oppt = opptMap.values();
    }
    else if(selected == 'Software') {
        Map<Id, Opportunity> opptMap = new Map<Id, Opportunity>([Select ID, AccountID From Opportunity o WHERE Id IN (Select OpportunityId From OpportunityLineItem WHERE Product_Family__c LIKE 'Software%') AND StageName='Closed Won']);
        oppt = opptMap.values();
    } 
    else if(selected == 'Renewal') {
        Map<Id, Opportunity> opptMap = new Map<Id, Opportunity>([Select ID, AccountID From Opportunity o WHERE Id IN (Select OpportunityId From OpportunityLineItem WHERE Product_Family__C='Renewal') AND StageName='Closed Won']);
        oppt = opptMap.values();
    } 
    else if(selected == 'Upgrade') {
        Map<Id, Opportunity> opptMap = new Map<Id, Opportunity>([Select ID, AccountID From Opportunity o WHERE Id IN (Select OpportunityId From OpportunityLineItem WHERE Product_Family__C='Upgrade') AND StageName='Closed Won']);
        oppt = opptMap.values();
    }           
    option = selected;           
    if (option == 'None' || option == null || oppt.size() <= 0) {
        return null;
    }            
    else {     
        Set<Id> accountSet = new Set<Id>();
        for(Opportunity o: oppt) {
            accountSet.Add(o.AccountID);
        }                                
        if(contactList == null) {
            contactList = new List<Contact>();
            List<Account> accountList = [SELECT Id, Name, (Select Id, Name, AccountId, Email, Phone, HasOptedOutOfEmail From Contacts WHERE HasOptedOutOfEmail = false) FROM Account WHERE ID in:accountSet];
            alistCount = accountList.size();
            for(Account a: accountList)  {                  
                contactList.addAll(a.Contacts);
            }                     
    }        
    return contactList;
    }
}

//Returns filtered list      
public List<Contact> getContacts() {
    return getFilterContacts(selected);
}

//Refreshes page based on Product Family filter       
public PageReference refreshSelected() {
    contactRecords = null;    
    contactList = null;               
    getContacts();                               
    if (getContacts() != null) {
        contactRecords.setPageNumber(1);
        listCount = getContacts().size();
    }
    else {
        listCount = 0; 
        alistCount = 0;           
    }
return null;            
}

//Filters campaign drop-down based upon only Support campaigns
public List<SelectOption> getItems() {
    List<SelectOption> options = new List<SelectOption>();
    for (Campaign cp: [SELECT Id, Name FROM Campaign WHERE Name LIKE 'SUPPORT%']) {
        options.add(new SelectOption(cp.Id, cp.Name));
    }        
    return options;
}

//Returns list of campaigns         
public String getCampaigns() {
    return campaigns;
}

//Sets campaigns    
public void setCampaigns(String campaigns) {
            this.campaigns = campaigns;
}

//Dummy page reference   
public PageReference Dummy() {
    return null;
}

//Add filtered contact list to campaign and opens campaign page       
public PageReference addToCampaign() {                
    if (contactList != null) {                                                      
        Set<Id> cpSet = new Set<Id>();
        bulletin = new List<CampaignMember>();           
        //Gets a list of current campaign members            
        List<CampaignMember> bulletinBefore = new List<CampaignMember>();                                     
        for (Campaign cp: [Select Id, (Select ContactId From CampaignMembers) From Campaign c WHERE Id = :campaigns]) {
            bulletinBefore.addAll(cp.CampaignMembers);              
        }
        //Delete them
        if (bulletinBefore.size() > 0) {   
                for (CampaignMember cm: bulletinBefore) {   
                    delete cm;
                }                       
         }

        //Add the contact list to the campaign                                                       
        for (Contact c: contactList) {
            CampaignMember newCM = new CampaignMember(
            CampaignId = campaigns,
            ContactId = c.Id,
            status = 'Sent' );                
            insert newCM;
        }  
        //Opens campaign page after completion                                            
        PageReference pageRef = new PageReference('/ui/campaign/CampaignManageMembersExistingTab?campaignId=' + campaigns);             
        pageRef.setRedirect(true);     
        return pageRef;    
    }  
    return null;              
}
}
2
  • Activate a debug log on the User who is executing the code. Setup -> Monitoring -> Debug Logs. It should tell you what line it errors on at least.
    – dphil
    Commented Apr 29, 2014 at 14:34
  • 1
    Doing database interactions (query or dml) should really not be done in a for-loop. This is very resource inefficient and salesforce will kill your code for it (=exception). I think Baskaran is currently updating his post to show you how you can do that :) Commented Apr 29, 2014 at 14:44

2 Answers 2

5

The first thing you can do is to limit you SOQL query using limit clause. For example,

select id, name from contact limit 100

And there are other issues in your code which you may want to bulkify. If you see the code below,

if (bulletinBefore.size() > 0) {   
            for (CampaignMember cm: bulletinBefore) {   
                delete cm;
            }                       
     }

    //Add the contact list to the campaign                                                       
    for (Contact c: contactList) {
        CampaignMember newCM = new CampaignMember(
        CampaignId = campaigns,
        ContactId = c.Id,
        status = 'Sent' );                
        insert newCM;
    }  

those two DML statements,

delete cm;
insert newCM;

should be taken out of the for loops and you need to use a list for the bulk update or delete.

Since you already knew the filters for the Product family as given in these queries,

if(selected == 'Hardware') {       
        Map<Id, Opportunity> opptMap = new Map<Id, Opportunity>([Select ID, AccountID From Opportunity o WHERE Id IN (Select OpportunityId From OpportunityLineItem WHERE Product_Family__c LIKE 'Hardware%') AND StageName='Closed Won']);
        oppt = opptMap.values();
    }
    else if(selected == 'Software') {
        Map<Id, Opportunity> opptMap = new Map<Id, Opportunity>([Select ID, AccountID From Opportunity o WHERE Id IN (Select OpportunityId From OpportunityLineItem WHERE Product_Family__c LIKE 'Software%') AND StageName='Closed Won']);

you can create one single SOQL query which will get all the opportunitylineitem related to the product family and build a map with the result. Say,

Select OpportunityId From OpportunityLineItem WHERE Product_Family__c LIKE 'Software%' and Product_Family__c in ('Software','hardware')

and use the map from the above query as a lookup. The issue is when the user selects multiple options for the search.

So, except for the above, you code seems to be good for me.

3
  • Oh yeah, I missed that with the insert and delete. I usually do that with a list, not sure why I added the loop. They can only select one option in the dropdown, so that is why I did the if/else statements. I was kind of surprised apex didn't have the 'switch' statement.
    – ajl7519
    Commented Apr 29, 2014 at 14:55
  • See my answer also. I'm mobile or I would have posted something similar, but the point is that calling DML repeatedly calls triggers repeatedly, and if they have queries, that's still a query in a loop.
    – sfdcfox
    Commented Apr 29, 2014 at 15:13
  • Thanks again everyone. I made suggested modifications and I was able to push to development.
    – ajl7519
    Commented Apr 29, 2014 at 17:29
7

To determine where the cause may be, you can use the debug logs.

First, go to Setup > monitoring > debug logs, set up a log for the user you are testing as, then modify the filters to set all categories to None, except Profiling, which should be set to Finest.

Next, recreate the error with the user you set up as the test user (this could be your user, if you can recreate the error).

Finally, go to the debug logs and check out the profiling information. It will tell you where the queries are being executed and how many times they were executed.

Just because you avoided queries in loops (as far as you can tell) doesn't mean there aren't bulkification issues elsewhere. For example, an inefficient set of DML operations might be calling triggers recursively, or more often than optimal.

Only by using the logs can you tell for certain which queries are problematic. If there are a bunch in the same area, it means you need to work backwards to figure out which loop is causing the problem.

Finally, it's possible that a large DML might call many trigger instances (e.g. trying to update 5000 records means the same trigger is called 25 times), in which case you may need batch processing, or query caching.

Without specific logs, just looking at your own code that you think may be a problem might not help. Note also that the other answer points out bulkification issues, which definitely will run into any random type of governor limits; it's always in your best interest to group queries and DML operations into as few units as possible.

Not the answer you're looking for? Browse other questions tagged .