3

I am working on an app which we plan to publish on app exchange. As per pre-requisite requirement by salesforce, any app before getting listed on app exchange should go through code review from checkmarx.we submitted our code base to checkmarx and get one of warning reported by checkmarx. The error says "Bulkify Apex Methods - Using Collections in methods".

I'm getting checkmarx warning below line:

public void sendEmail(Purchase_order__c orders,Attachment attachment){

Controller:

    public with sharing class PurchaseorderContrllor {
        private ApexPages.StandardController sc;
        public String projectid{get;set;}
        public String subprojectid{get;set;}
        public String purchaseordersId{get;set;}
        public String markID{get;set;}
        public string contenttype{get;set;}
        public decimal a1{get;set;}
        public string a2{get;set;}
        public string a3{get;set;}
        //public transient List<Attachment> attachment {get;set;} 
        public musqot__Purchase_order_setting__c pos{get;set;}
        public integer i; 
        public List<Purchase_article__c> PA_Temp = new List<Purchase_article__c>();
        public Purchase_order__c orders{get;set;}
        public Static Boolean notWildInvoicePurchOrdr;
        public PurchaseorderContrllor (ApexPages.StandardController controller) {
            This.sc = sc; 

        projectid = ApexPages.currentPage().getParameters().get('planid');   
        subprojectid= ApexPages.currentPage().getParameters().get('prjId'); 
        markID=ApexPages.currentPage().getParameters().get('markID');  
        orders= new Purchase_order__c ();
        orders.Project__c= projectid;  
        orders.subproject__c= subprojectid;
        orders.Marketing_activity__c=markID;          
    }

    public List<SelectOption> PAList
    {
        get
        {
            PA_Temp = [Select u.Name, u.Id From Purchase_article__c u order by u.Name limit 250];
            PAList = new List<SelectOption>(); 
            for(Purchase_article__c temp : PA_Temp)
            {
                PAList.add(new SelectOption(temp.Id, temp.Name));
            }
            return PAList;
        }
        set;
    }

    public Pagereference insertOrders(){
        integer i;
        try{
            if (Purchase_order__c.sObjectType.getDescribe().isCreateable())
            {
                notWildInvoicePurchOrdr = true;
                pos=[select id,PO_Startnumber__c,musqot__PO_Lastnumber__c ,musqot__PO_Prefix__c from musqot__Purchase_order_setting__c limit 250];
                if (pos.musqot__PO_Prefix__c!=null){                
                a1=pos.musqot__PO_Lastnumber__c+1;
                a2=pos.musqot__PO_Prefix__c;      
                a3=a2 +''+''+''+a1;
                orders.musqot__Purchase_order_name__c=a3; 

                /* if(orders.Send_date__c==system.today()){
                orders.Purchase_order_send__c =orders.Send_date__c;
                orders.Send_date__c=null; 
                insert orders;
                }
                else */        
                     }        
                else{
                a1=pos.musqot__PO_Lastnumber__c+1;
                a2 =''+a1;
                orders.musqot__Purchase_order_name__c = a2;
                }

                insert orders;

                i=[select count() from musqot__Purchase_order__c limit 250];
                if(i==i++){
                    pos=[select id,PO_Startnumber__c,musqot__PO_Lastnumber__c ,musqot__PO_Prefix__c from musqot__Purchase_order_setting__c limit 250];
                    pos.musqot__PO_Lastnumber__c=a1 ;

                    update pos;
                }

            }

            //Attachment Code
            User u = [select Id from user where id=:userinfo.getuserid()];

            attachment.OwnerId = u.Id;
            attachment.ParentId = orders.Id; // the record the file is attached to
            //attachment.OwnerId = u.id;
            attachment.ContentType=contentType;
            if(attachment.body<>null){
                if (Schema.sObjectType.Attachment.fields.body.isCreateable()){

                    attachment.Name = orders.Purchase_order_name__c;
                    //attachment.Body=null;
                    insert attachment;

                }                  
            }
            sendEmail(orders,attachment);
public Attachment attachment {
        get {
            if (attachment == null)
                attachment = new Attachment();
            return attachment;
        }
        set;
    }

    public void sendEmail(Purchase_order__c orders,Attachment attachment){
            if(orders.Send_date__c ==System.today() && orders.Supplier_email__c != null){ 

                EmailTemplate template = [SELECT Id, name,Subject, HtmlValue, Body FROM EmailTemplate WHERE name= 'Purchaseorder'];
                Contact cnt = new Contact();
                cnt.LastName = 'Supplier';
                cnt.Email = orders.Supplier_email__c;
                insert cnt;
                Messaging.SingleEmailMessage mail = new Messaging.SingleEmailMessage();
                mail.setWhatId(orders.Id);
                mail.setTargetObjectId(cnt.Id);
                mail.setTemplateId(template.Id);
                mail.saveAsActivity = false;

                  if(attachment.body<>null){

                    List<Messaging.Emailfileattachment> fileAttachments = new List<Messaging.Emailfileattachment>();

                    Messaging.Emailfileattachment efa = new Messaging.Emailfileattachment();
                    efa.setFileName(attachment.Name);
                    efa.setBody(attachment.Body);
                    efa.setContentType(attachment.ContentType);
                    fileAttachments.add(efa);
                    mail.setFileAttachments(fileAttachments);
                }

                Messaging.sendEmail(new Messaging.SingleEmailMessage[] { mail }); 
                delete cnt;           
            }       
         }
     } 
2

That is a warning rather than a specific error. The impact mostly depends on how the sendEmail method is called. E.g. If it is never called in a loop per Purchase_order__c and/or attachment you should be fine. If this is the case they you can just provide justification to Salesforce along this line.

However, if it is call multiple times per transaction then you should bulkify it. The simplest way to start this process is to rework the signature of the method to something like:

public void sendEmail(List<Purchase_order__c> orders, Attachment attachment){ // ... }

Then work through the method altering how the Purchase_order__c records are used. The most important part would be ensuring the SOQL query against Purchase_order__c only occurs once and that you only call Messaging.sendEmail once passing it the complete collection of email messages.

9
  • but, still we got the security warning from checkmarx...
    – fsales572
    Feb 20 '15 at 8:09
  • unfortunately i found one deleted trigger with same name sendEmail on Purchase_order__c object.....
    – fsales572
    Feb 20 '15 at 8:12
  • trigger deleted from UI, but still it shows in package--->View Deleted Components section
    – fsales572
    Feb 20 '15 at 8:15
  • trigger sendEmail on Purchase_order__c (after insert, after update) { ------------------------------------------------------------------------------------- List<Messaging.Emailfileattachment> fileAttachments = new List<Messaging.Emailfileattachment>(); for (Attachment a : [select Name, Body, BodyLength from Attachment where ParentId = :e.Id]) { }
    – fsales572
    Feb 20 '15 at 8:25
  • in the trigger i had written soql query inside for loop... is this the reason for getting security warning from checkmarx????????
    – fsales572
    Feb 20 '15 at 8:27

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.