+ Start a Discussion
Mhlangano KhumaloMhlangano Khumalo 

HELP with bulkification: I want to remove an SQL query inside a for loop.

I'm receiving a JSON string from another system in the format below. These records already exist in Salesforce. I want to only query these records in salesforce, set a boolean field to false in all of them and update.
{ "bacctList" : 
     [{
       "RC_Account_No__c" : "RC-2",
       "A_c_Number__c" : "111",
       "Bank_code__c" : "250655"
      },
      {
       "RC_Account_No__c" : "RC-1",
       "A_c_Number__c" : "222",
       "Bank_code__c" : "198765"
      }]
   }

The below code works fine, the issue is it's not bulkified.
@RestResource(urlMapping='/dosystem/bankaccounts/*')
global with sharing class BankAccountWebservice{
   
 @HttpPatch
    global static BankAccountWrapper doPatch(List<Bank_Account__c> bacctList) {
        RestRequest req = RestContext.request;
        RestResponse res = RestContext.response;
        BankAccountWrapper response = new BankAccountWrapper();
        
        List<Bank_Account__c> bankAccountsToUpdate= new List<Bank_Account__c>();

        for(integer i=0; i<bacctList.size(); i++){
            List<Bank_Account__c> ac = [SELECT Debit_Order_A_c__c,A_c_Number__c,Bank_code__c,RC_Account_No__c  
            FROM Bank_Account__c 
            WHERE (RC_Account_No__c =: bacctList[i].RC_Account_No__c AND  A_c_Number__c =: bacctList[i].A_c_Number__c AND Bank_code__c =: bacctList[i].Bank_code__c)];
            
            for(integer j=0; j< ac.size(); j++){
                ac[j].Debit_Order_A_c__c = false;       
                bankAccountsToUpdate.add(ac[j]);
             }    
        }
        
        try {
            update bankAccountsToUpdate;

            response.bacctList = bankAccountsToUpdate;
            response.status = 'Success';
            response.message = bacctList.size()+' Records Updated successfully';
        }
        catch(Exception exc) {
            res.StatusCode = 500;
            response.bacctList = null;
            response.status = 'Error';
            response.message = 'Your request failed with the following error: ' + exc.getMessage();
        }
        
        return response;
    }
    
    global class BankAccountWrapper {
        public List<Bank_Account__c> bacctList;
        public String status;
        public String message;
        
        public BankAccountWrapper(){
            bacctList = new List<Bank_Account__c>();
        }
    }   
}

The problem is the FOR loop. I tried using the IN clause in SOQL but the problem is im not getting the ID in the JSON string.
List<Bank_Account__c> bankAccountsToUpdate= new List<Bank_Account__c>();
        integer updatedRecordCount=0;
        
        List<Bank_Account__c> ac= [select Name,Debit_Order_A_c__c,A_c_Number__c,Bank_code__c From Bank_Account__c where Id IN : bacctList];
                    
		for(integer i=0; i<ac.size(); i++){                                    
			ac[i].Debit_Order_A_c__c = false;       
			bankAccountsToUpdate.add(ac[i]);
			updatedRecordCount++;                 
		
		}

PLEASE HELP.

 
Best Answer chosen by Mhlangano Khumalo
pconpcon
That's not going to work how you want it to work.  While that will work for the single use case, it will not work for the bulk.  For example if you have the following data coming in
 
{
    "bacctList" :
    [
        {
            "RC_Account_No__c" : "RC-2",
            "A_c_Number__c" : "111",
            "Bank_code__c" : "250655"
        }, {
            "RC_Account_No__c" : "RC-1",
            "A_c_Number__c" : "222",
            "Bank_code__c" : "198765"
        }
    ]
}

and you had the following data stored on your records
 
"Id": "A123"
"RC_Account_No__c" : "RC-2",
"A_c_Number__c" : "111",
"Bank_code__c" : "250655"

"Id": "B234"
"RC_Account_No__c" : "RC-1",
"A_c_Number__c" : "222",
"Bank_code__c" : "198765"

"Id": "C345"
"RC_Account_No__c" : "RC-2",
"A_c_Number__c" : "111",
"Bank_code__c" : "198765"

and your code ran then C345 would be updated and it shouldn't.

I now see your initial sample code and below is some updated code.  Also there was a typo in my initial code with the map declaration.
 
@RestResource(urlMapping='/dosystem/bankaccounts/*')
global with sharing class BankAccountWebservice {
    @HttpPatch
    global static BankAccountWrapper doPatch(List<Bank_Account__c> bacctList) {
        RestRequest req = RestContext.request;
        RestResponse res = RestContext.response;
        BankAccountWrapper response = new BankAccountWrapper();

        List<Bank_Account__c> bankAccountsToUpdate = new List<Bank_Account__c>();

        Set<Id> accountIds = new Set<Id>();
        Set<String> acNumbers = new Set<String>();
        Set<String> bankCodes = new Set<String>();

        for (Bank_Account__c bac : bacctList) {
            accountIds.add(bac.RC_Account_No__c);
            acNumbers.add(bac.A_c_Number__c);
            bankCodes.add(bac.Bank_code__c);
        }

        Map<Id, Map<String, List<Bank_Account__c>>> bacMap = new Map<Id, Map<String, List<Bank_Account__c>>>();

        for (Bank_Account__c bac : [
            select Debit_Order_A_c__c,
                A_c_Number__c,
                Bank_code__c,
                RC_Account_No__c
            from Bank_Account__c
            where (
                RC_Account_No__c in :accountIds and
                (
                    A_c_Number__c in :acNumbers or
                    Bank_code__c in :bankCodes
                )
            )
        ]) {
            if (!bacMap.containsKey(bac.RC_Account_No__c)) {
                bacMap.put(bac.RC_Account_No__c, new Map<String, List<Bank_Account__c>());
            }

            String key = bac.A_c_Number__c + ':' + bac.Bank_code__c;

            if (!bacMap.get(bac.RC_Account_No__c).containsKey(key)) {
                bacMap.get(bac.RC_Account_No__c).put(key, new List<Bank_Account__c>());
            }

            bacMap.get(bac.RC_Account_No__c).get(key).add(bac);
        }

        for (Bank_Account__c bac : bacctList) {
            String key = bac.A_c_Number__c + ':' + bac.Bank_code__c;

            if (
                bacMap.containsKey(bac.RC_Account_No__c) &&
                bacMap.get(bac.RC_Account_No__c).containsKey(key)
            ) {
                for (Bank_Account__c toUpdate : bacMap.get(bac.RC_Account_No__c).get(key)) {
                    toUpdate.Debit_Order_A_c__c = false;
                    bankAccountsToUpdate.add(toUpdate);
                }
            }
        }

        try {
            update bankAccountsToUpdate;

            response.bacctList = bankAccountsToUpdate;
            response.status = 'Success';
            response.message = bacctList.size() + ' Records Updated successfully';
        } catch(Exception exc) {
            res.StatusCode = 500;
            response.bacctList = null;
            response.status = 'Error';
            response.message = 'Your request failed with the following error: ' + exc.getMessage();
        }

        return response;
    }

    global class BankAccountWrapper {
        public List<Bank_Account__c> bacctList;
        public String status;
        public String message;

        public BankAccountWrapper() {
            bacctList = new List<Bank_Account__c>();
        }
    }
}
NOTE: This code has not been tested and may contain typographical or logical errors

All Answers

pconpcon
Ok, so this is a bit of a doozy and may not work right off the bat.  I made some assumptions about the data types stored so if they aren't strings then you'll need to modify the code a bit.  What we're essentially doing is building up a map of all the possible accounts and then partitioning them based on the their combined key.
 
@RestResource(urlMapping='/dosystem/bankaccounts/*')
global with sharing class BankAccountWebservice {
    @HttpPatch
    global static BankAccountWrapper doPatch(List<Bank_Account__c> bacctList) {
        RestRequest req = RestContext.request;
        RestResponse res = RestContext.response;
        BankAccountWrapper response = new BankAccountWrapper();

        List<Bank_Account__c> bankAccountsToUpdate = new List<Bank_Account__c>();

        Set<Id> accountIds = new Set<Id>();
        Set<String> acNumbers = new Set<String>();
        Set<String> bankCodes = new Set<String>();

        for (Bank_Account__c bac : bacctList) {
            accountIds.add(bac.RC_Account_No__c);
            acNumbers.add(bac.A_c_Number__c);
            bankCodes.add(bac.Bank_code__c);
        }

        Map<Id, Map<String, List<Bank_Account__c>> bacMap = new Map<Id, Map<String, List<Bank_Account__c>>();

        for (Bank_Account__c bac : [
            select Debit_Order_A_c__c,
                A_c_Number__c,
                Bank_code__c,
                RC_Account_No__c
            from Bank_Account__c
            where (
                RC_Account_No__c in :accountIds and
                (   
                    A_c_Number__c in :acNumbers or
                    Bank_code__c in :bankCodes
                )
            )
        ]) {
            if (!bacMap.containsKey(bac.RC_Account_No__c)) {
                bacMap.put(bac.RC_Account_No__c, new Map<String, List<Bank_Account__c>());
            }

            String key = bac.A_c_Number__c + ':' + bac.Bank_code__c;

            if (!bacMap.get(bac.RC_Account_No__c).containsKey(key)) {
                bacMap.get(bac.RC_Account_No__c).put(key, new List<Bank_Account__c>());
            }

            bacMap.get(bac.RC_Account_No__c).get(key).add(bac);
        }

        for (Bank_Account__c bac : bacctList) {
            String key = bac.A_c_Number__c + ':' + bac.Bank_code__c;

            if (
                bacMap.containsKey(bac.RC_Account_No__c) &&
                bacMap.get(bac.RC_Account_No__c).containsKey(key)
            ) { 
                for (Bank_Account__c toUpdate : bacMap.get(bac.RC_Account_No__c).get(key)) {
                    toUpdate.Debit_Order_A_c__c = false;
                    bankAccountsToUpdate.add(toUpdate);
                }
            }
        }
        try {
            update bankAccountsToUpdate;

            response.bacctList = bankAccountsToUpdate;
            response.status = 'Success';
            response.message = bacctList.size() + ' Records Updated successfully';
        } catch(Exception exc) {
            res.StatusCode = 500;
            response.bacctList = null;
            response.status = 'Error';
            response.message = 'Your request failed with the following error: ' + exc.getMessage();
        }

        return response;
    }

    global class BankAccountWrapper {
        public List<Bank_Account__c> bacctList;
        public String status;
        public String message;

        public BankAccountWrapper() {
            bacctList = new List<Bank_Account__c>();
        }
    }
}

NOTE: This code has not been tested and may contain typographical or logical errors
Mhlangano KhumaloMhlangano Khumalo
Thanks, I'm busy trying the code. Will let you know when it works.
Mhlangano KhumaloMhlangano Khumalo
User-added image
I'm getting this error, what should l change?
Mhlangano KhumaloMhlangano Khumalo
@pcon, this is my code that works, taken for yours. Please have a look and tell me if its ok.
@RestResource(urlMapping='/dosystem/bankaccounts/*')
global with sharing class BankAccountWebservice {
    @HttpPatch
    global static BankAccountWrapper doPatch(List<Bank_Account__c> bacctList) {
        RestRequest req = RestContext.request;
        RestResponse res = RestContext.response;
        BankAccountWrapper response = new BankAccountWrapper();
        
        integer updatedRecordCount=0;  
        List<Bank_Account__c> bankAccountsToUpdate = new List<Bank_Account__c>();

        Set<String> accountIds = new Set<String>();
        Set<String> acNumbers = new Set<String>();
        Set<String> bankCodes = new Set<String>();

        for (Bank_Account__c bac : bacctList) {
            accountIds.add(bac.RC_Account_No__c);
            acNumbers.add(bac.A_c_Number__c);
            bankCodes.add(bac.Bank_code__c);
        }
    
        for (Bank_Account__c bac : [select Debit_Order_A_c__c, A_c_Number__c, Bank_code__c, RC_Account_No__c from Bank_Account__c where 
                RC_Account_No__c in :accountIds and A_c_Number__c in :acNumbers and Bank_code__c in :bankCodes                            
        ]) {
                bac.Debit_Order_A_c__c = false;      
                bankAccountsToUpdate.add(bac);
                updatedRecordCount++;                                                         
        }                        
        
        try {
            update bankAccountsToUpdate;

            response.bacctList = bankAccountsToUpdate;
            response.status = 'Success';
            response.message = bacctList.size() + ' Records Updated successfully';
        } catch(Exception exc) {
            res.StatusCode = 500;
            response.bacctList = null;
            response.status = 'Error';
            response.message = 'Your request failed with the following error: ' + exc.getMessage();
        }

        return response;
    }

    global class BankAccountWrapper {
        public List<Bank_Account__c> bacctList;
        public String status;
        public String message;

        public BankAccountWrapper() {
            bacctList = new List<Bank_Account__c>();
        }
    }
}

 
pconpcon
That's not going to work how you want it to work.  While that will work for the single use case, it will not work for the bulk.  For example if you have the following data coming in
 
{
    "bacctList" :
    [
        {
            "RC_Account_No__c" : "RC-2",
            "A_c_Number__c" : "111",
            "Bank_code__c" : "250655"
        }, {
            "RC_Account_No__c" : "RC-1",
            "A_c_Number__c" : "222",
            "Bank_code__c" : "198765"
        }
    ]
}

and you had the following data stored on your records
 
"Id": "A123"
"RC_Account_No__c" : "RC-2",
"A_c_Number__c" : "111",
"Bank_code__c" : "250655"

"Id": "B234"
"RC_Account_No__c" : "RC-1",
"A_c_Number__c" : "222",
"Bank_code__c" : "198765"

"Id": "C345"
"RC_Account_No__c" : "RC-2",
"A_c_Number__c" : "111",
"Bank_code__c" : "198765"

and your code ran then C345 would be updated and it shouldn't.

I now see your initial sample code and below is some updated code.  Also there was a typo in my initial code with the map declaration.
 
@RestResource(urlMapping='/dosystem/bankaccounts/*')
global with sharing class BankAccountWebservice {
    @HttpPatch
    global static BankAccountWrapper doPatch(List<Bank_Account__c> bacctList) {
        RestRequest req = RestContext.request;
        RestResponse res = RestContext.response;
        BankAccountWrapper response = new BankAccountWrapper();

        List<Bank_Account__c> bankAccountsToUpdate = new List<Bank_Account__c>();

        Set<Id> accountIds = new Set<Id>();
        Set<String> acNumbers = new Set<String>();
        Set<String> bankCodes = new Set<String>();

        for (Bank_Account__c bac : bacctList) {
            accountIds.add(bac.RC_Account_No__c);
            acNumbers.add(bac.A_c_Number__c);
            bankCodes.add(bac.Bank_code__c);
        }

        Map<Id, Map<String, List<Bank_Account__c>>> bacMap = new Map<Id, Map<String, List<Bank_Account__c>>>();

        for (Bank_Account__c bac : [
            select Debit_Order_A_c__c,
                A_c_Number__c,
                Bank_code__c,
                RC_Account_No__c
            from Bank_Account__c
            where (
                RC_Account_No__c in :accountIds and
                (
                    A_c_Number__c in :acNumbers or
                    Bank_code__c in :bankCodes
                )
            )
        ]) {
            if (!bacMap.containsKey(bac.RC_Account_No__c)) {
                bacMap.put(bac.RC_Account_No__c, new Map<String, List<Bank_Account__c>());
            }

            String key = bac.A_c_Number__c + ':' + bac.Bank_code__c;

            if (!bacMap.get(bac.RC_Account_No__c).containsKey(key)) {
                bacMap.get(bac.RC_Account_No__c).put(key, new List<Bank_Account__c>());
            }

            bacMap.get(bac.RC_Account_No__c).get(key).add(bac);
        }

        for (Bank_Account__c bac : bacctList) {
            String key = bac.A_c_Number__c + ':' + bac.Bank_code__c;

            if (
                bacMap.containsKey(bac.RC_Account_No__c) &&
                bacMap.get(bac.RC_Account_No__c).containsKey(key)
            ) {
                for (Bank_Account__c toUpdate : bacMap.get(bac.RC_Account_No__c).get(key)) {
                    toUpdate.Debit_Order_A_c__c = false;
                    bankAccountsToUpdate.add(toUpdate);
                }
            }
        }

        try {
            update bankAccountsToUpdate;

            response.bacctList = bankAccountsToUpdate;
            response.status = 'Success';
            response.message = bacctList.size() + ' Records Updated successfully';
        } catch(Exception exc) {
            res.StatusCode = 500;
            response.bacctList = null;
            response.status = 'Error';
            response.message = 'Your request failed with the following error: ' + exc.getMessage();
        }

        return response;
    }

    global class BankAccountWrapper {
        public List<Bank_Account__c> bacctList;
        public String status;
        public String message;

        public BankAccountWrapper() {
            bacctList = new List<Bank_Account__c>();
        }
    }
}
NOTE: This code has not been tested and may contain typographical or logical errors
This was selected as the best answer
Mhlangano KhumaloMhlangano Khumalo
Thanks @pcon on line 38 i put a close angle bracket like below & it compiled.
bacMap.put(bac.RC_Account_No__c, new Map<String, List<Bank_Account__c>>());

 
Mhlangano KhumaloMhlangano Khumalo
@pcon, Thank you so much for your assistance, especially for taking your time to look at the code, work on it and for sticking through until the solution worked. I've tested and it works like a charm. Thanks, much appreciated.