Security Review Scanner help

Question

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];
    }

Answer 1

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