2

I ran the checkmarx security scanner and I got 1 item flagged as a "Bulkify Apex Methods" but I'm not 100% sure I get what's the issue there.

Here's a screenshot of the Checkmarx issue.

Can you please explain what exactly is wrong?

Here's a code snippet

InvoiceTriggerHandler:

 public static void CancelInvoice(list < Invoice__c > CancelledInvoices ) 
    {

        //Get all the normal line items from the cancelled invoices
        set<Id> InvoicesId = new map<Id, Invoice__c>(CancelledInvoices).keySet();
        list<Invoice_Line_Item__c> LineItemsToUpdate = AZ_DataAccess.getLineItemsFromInvoice(InvoicesId);

        //Cancel each line item by changing their record types
        for (Invoice_Line_Item__c lineitem : LineItemsToUpdate)
        {
            if (lineitem.RecordTypeId == LineItemRecordType)
                lineitem.RecordTypeId = CancelledRecordType;
        }

        //Update the line items to reflect the change on all of their associated elements.
        update LineItemsToUpdate;
    }

DataAccess

 public static list<Invoice_Line_Item__c> getLineItemsFromInvoice(set<Id> Invoices)
    {
       ....

        return [SELECT
                    Id,
                    Amount__c,
                    Description__c,
                    Expense__c,
                    Expense__r.Name,
                    Invoice__c,
                    Invoice_Item_Type__c,
                    Milestone__c,
                    Milestone__r.Name,
                    Project__c,
                    Project__r.Name,
                    Quantity__c,
                    Rate__c,
                    RecordTypeId,
                    Role__c,
                    Tax__c,
                    Tax__r.Name,
                    Tax__r.Tax_Reference_Number__c,
                    Total_Charge__c
                FROM
                    Invoice_Line_Item__c
                WHERE
                    Invoice__c IN :Invoices];
    }
  • 2
    Seems like a false positive. The only thing I might change is to move your RecordType check into your WHERE clause. Any chance you are calling CancelInvoice within a loop? – Adrian Larson Dec 23 '15 at 18:45
  • @AdrianLarson CancelInvoice is called on Trigger.Delete and I pass the trigger.old List. – raym0nd Dec 23 '15 at 18:50
  • @AdrianLarson The getLineItemsFromInvoice is a generic function that's why I dont filter it based on recordtype – raym0nd Dec 23 '15 at 18:53
0

One thing that you can try is avoid use of getLineItemsFromInvoice and use query in loop header. This will improve performance of your code. I don't see it as security issue but bulkification is also not security issue as well so worth trying.

So, here is was i suggest.

public static void CancelInvoice(list < Invoice__c > CancelledInvoices ) 
{
set<Id> InvoicesId = new map<Id, Invoice__c>(CancelledInvoices).keySet();

//When you run a query in loop header and accept objects in list and then use this list to loop through it need lesser CUP cycles and heap to process the data. ultimetly it improves the performace and bulikifiy processing 
for (list<Invoice_Line_Item__c> LineItemsToScan = [SELECT Id,  Amount__c, Description__c, Expense__c, Expense__r.Name,  Invoice__c, Invoice_Item_Type__c, Milestone__c,  Milestone__r.Name, Project__c, Project__r.Name, Quantity__c, Rate__c, RecordTypeId, Role__c, Tax__c, Tax__r.Name, Tax__r.Tax_Reference_Number__c, Total_Charge__c FROM  Invoice_Line_Item__c WHERE Invoice__c IN :Invoices])
{
    for(Invoice_Line_Item__c lineitem :  LineItemsToScan){
        if (lineitem.RecordTypeId == LineItemRecordType){
            lineitem.RecordTypeId = CancelledRecordType;
            LineItemsToUpdate.add(lineitem)
        }
    }
}
update LineItemsToUpdate;

}

Here is link which explain above approach in much details: https://developer.salesforce.com/page/Troubleshooting_Apex_Performance_Problems

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.