1

The Salsesforce Source Scanner if flagging my DatabaseFLS class for "Query: Bulkify Apex Methods Using Collections In Methods" with 5 issues, and provides the below:

This best practice is about ensuring that Apex methods are structured to receive collections in the apex method signature. Executing queries, searches, or DML operations within an iteration adds the risk that the governor limits will be exceeded. More importantly, it is not an efficient design pattern with Apex. This is also true for any helper or utility methods an Apex request executes.

References: https://developer.salesforce.com/wiki/apex_code_best_practices https://developer.salesforce.com/docs/atlas.en-us.apexcode.meta/api/apex_limits_intro.htm https://developer.salesforce.com/docs/atlas.en-us.apexcode.meta/apexcode/apex_dev_guide.htm

The issue I see is that I am Bulkifying my DML operations so is this a false positive? or am I Just missing something obvious?

Bulkify Apex Methods Using Collections In Methods | Apex Code Quality | 5

global with sharing class DatabaseFLS {
    
    global static Boolean flsSelect (String flsSObject, String[] accessFields) {
        
        //Get Schema for the object
        SObjectType r = ((SObject)(Type.forName('Schema.' + flsSObject).newInstance())).getSObjectType();
        DescribeSObjectResult flsDescribe = r.getDescribe();
        Map<String,Schema.SObjectField> mfields = flsDescribe.fields.getMap();

        Boolean flsAccessibleFlag = true;

        //Check if each filed passed in the accessFields is accessible
        for(String fieldToCheck : accessFields)
        {        
            System.debug('Example - DatabaseFLS.flsInsert: get field: ' + mfields.get(fieldToCheck));
            if (!mfields.get(fieldToCheck).getDescribe().isAccessible()){
                flsAccessibleFlag = false;
                System.debug('Example - DatabaseFLS.flsInsert: '+ mfields.get(fieldToCheck) + ' isAccessible: false');
                break;
            } else {
                flsAccessibleFlag = true;
                System.debug('Example - DatabaseFLS.flsInsert: ' + mfields.get(fieldToCheck) + ' isAccessible: true');
            }
        }

        if (flsAccessibleFlag) {
            return flsAccessibleFlag;
        }
        else {
            return flsAccessibleFlag;
        }
    }
    
    global static Database.SaveResult flsInsert (sObject flsSObject) {
        List<SObject> flsSObjects = new List<SObject>();
        flsSObjects.add(flsSObject);
        List<Database.SaveResult> srlist = flsInsert(flsSObjects);
        System.debug('Example - DatabaseFLS.flsInsert: fls insert save result status: ' + srlist[0].isSuccess());
        return srlist[0];
    }

    global static List<Database.SaveResult> flsInsert (List<sObject> flsSObjects) {
        return flsInsert(flsSObjects, true);
    }

    global static List<Database.SaveResult> flsInsert (List<sObject> flsSObjects, Boolean allOrNone) {
        
        Boolean flsCreateableFlag = true;
        
        for (SObject so : flsSObjects){
            Schema.SObjectType r = so.getSObjectType();
                      
            DescribeSObjectResult flsDescribe = r.getDescribe();
            Map<String,Schema.SObjectField> sofields = flsDescribe.fields.getMap();
            Map<String, Object> mfields = so.getPopulatedFieldsAsMap();
            
            for (String fieldName : mfields.keySet()){
                System.debug('Example - DatabaseFLS.flsInsert: field name is ' + fieldName);
                SObjectField sfield = sofields.get(fieldName);
                schema.describefieldresult dfield = sfield.getDescribe();   
                System.debug('dfield: ' + dfield);

                if (!dfield.isCreateable()){
                    flsCreateableFlag = false;
                    System.debug('Example - DatabaseFLS.flsInsert: ' + dfield.getName() + ' isCreateable: false');
                    break;
                } else {
                    flsCreateableFlag = true;
                    System.debug('Example - DatabaseFLS.flsInsert: ' + dfield.getName() + ' isCreateable: true');
                }
            }
            
        }
        
        if (flsCreateableFlag) {
            return Database.insert(flsSObjects, allOrNone);
        }
        else {throw new ExampleCustomException('Example - DatabaseFLS.flsInsert: Error creating objects. Check that the user has the required rights.');
        }
    }

    global static Database.SaveResult flsUpdate (sObject flsSObject) {
        List<SObject> flsSObjects = new List<SObject>();
        flsSObjects.add(flsSObject);
        List<Database.SaveResult> srlist = flsUpdate(flsSObjects);
        return srlist[0];
    }

    global static List<Database.SaveResult> flsUpdate (List<sObject> flsSObjects) {
        return flsUpdate(flsSObjects, true);
    }

    global static List<Database.SaveResult> flsUpdate (List<sObject> flsSObjects, Boolean allOrNone) {
        Boolean flsUpdateableFlag = true;
        
        for (SObject so : flsSObjects){
            Schema.SObjectType r = so.getSObjectType();
            
            DescribeSObjectResult flsDescribe = r.getDescribe();
            
            Map<String,Schema.SObjectField> sofields = flsDescribe.fields.getMap();
            Map<String, Object> mfields = so.getPopulatedFieldsAsMap();
            
            for (String fieldName : mfields.keySet()){
                System.debug('Example - DatabaseFLS.flsUpdate: field name is ' + fieldName);
                SObjectField sfield = sofields.get(fieldName);
                schema.describefieldresult dfield = sfield.getDescribe();   
                System.debug('Example - DatabaseFLS.flsUpdate: dfield: ' + dfield);
                
                if(!dfield.isIdLookup()){
                    if (!dfield.isUpdateable()){ flsUpdateableFlag = false;
                        System.debug('Example - DatabaseFLS.flsUpdate: '+ dfield.getName() + ' isUpdateable: false');
                        break;
                    } else {
                        flsUpdateableFlag = true;
                        System.debug('Example - DatabaseFLS.flsUpdate: ' + dfield.getName() + ' isUpdateable: true');
                    }
                }
            }
        }

        if (flsUpdateableFlag) {
            return Database.update(flsSObjects, allOrNone);
        }
        else {  throw new ExampleCustomException('Example - DatabaseFLS.flsUpdate: Error updating objects. Check that the user has the required rights.');
        }
    }

    global static Database.DeleteResult flsDelete (sObject flsSObject) {
        if(null != flsSObject){
            List<SObject> flsSObjects = new List<SObject>();
            flsSObjects.add(flsSObject);
            List<Database.DeleteResult> delList = flsDelete(flsSObjects);
            return delList[0];
        }else{
            return null;
        }

    }

    global static List<Database.DeleteResult> flsDelete (List<sObject> flsSObjects) {
        return flsDelete(flsSObjects, true);
    }

    global static List<Database.DeleteResult> flsDelete (List<sObject> flsSObjects, Boolean allOrNone) {
        Boolean ableToDelete = true;
        String name = null;
        
        for(sObject obj: flsSObjects){
            Schema.DescribeSObjectResult flsDescribe = obj.getSObjectType().getDescribe();
            if (!flsDescribe.isDeletable()) {
                ableToDelete = false;
                name = flsDescribe.getName();
            }
        }
        if (ableToDelete) {
            return Database.delete(flsSObjects, allOrNone);
        }
        else { throw new ExampleCustomException('Example - DatabaseFLS.flsInsert: Error deleting [' + name + '] object. Check that the user has the required rights.');
        }
    }

    global static Database.SaveResult flsUpsert (sObject flsSObject) {
        List<SObject> flsSObjects = new List<SObject>();
        flsSObjects.add(flsSObject);
        List<Database.SaveResult> urlist = flsUpsert(flsSObjects);
        return urlist[0];
    }

    global static List<Database.SaveResult> flsUpsert (List<sObject> flsSObjects) {
        return flsUpsert(flsSObjects, true);
    }

    global static List<Database.SaveResult> flsUpsert (List<sObject> flsSObjects, Boolean allOrNone) {
        Boolean isCreateable = true;
        Boolean isUpdateable = true;
        
        List<SObject> updObjs = new List<SObject>();
        List<SObject> insObjs = new List<SObject>();
        
        for (SObject so : flsSObjects){
            if (so.Id != null)
                updObjs.add(so);
            else
                insObjs.add(so);
        }
        
        if (insObjs != null){
            isCreateable = getUpsertableFlag(insObjs, true);
            System.debug('Example - DatabaseFLS.flsInsert: isCreateable flag: ' + isCreateable);
        }
            
        
        if (updObjs != null){
            isUpdateable = getUpsertableFlag(updObjs, false);
            System.debug('Example - DatabaseFLS.flsInsert: isUpdateable flag: ' + isUpdateable);
        }
        
        if (isCreateable || isUpdateable) {
            List<Database.SaveResult> upsr = new List<Database.SaveResult>();
            
            if(isCreateable && (insObjs != null)){
                List<Database.SaveResult> sr = Database.insert(insObjs, allOrNone);
                upsr.addall(sr);
                System.debug('Example - DatabaseFLS.flsInsert: Invoked database.insert!');
            }
            
            if(isUpdateable && (updObjs != null)){
                List<Database.SaveResult> updr = Database.update(updObjs, allOrNone);
                upsr.addall(updr);
                System.debug('Example - DatabaseFLS.flsInsert: Invoked database.update!');
            }
            return upsr;
        } else {throw new ExampleCustomException('Example - DatabaseFLS.flsInsert: FLS Error upserting objects. Check that the user has the required rights.');
        }
    }

    global static Boolean getUpsertableFlag (List<sObject> upsertSObjects, Boolean isInsert){
        
        Boolean flsValidFlag = true;
        
        for (SObject so : upsertSObjects){
            Schema.SObjectType r = so.getSObjectType();
            DescribeSObjectResult flsDescribe = r.getDescribe();
            Map<String,Schema.SObjectField> sofields = flsDescribe.fields.getMap();
            Map<String, Object> mfields = so.getPopulatedFieldsAsMap();
            System.debug('Example - DatabaseFLS.flsInsert: mfield keyset size: '+ mfields.keySet().size());
            
            for (String fieldName : mfields.keySet()){
                System.debug('Example - DatabaseFLS.flsInsert: field name is ' + fieldName);
                SObjectField sfield = sofields.get(fieldName);
             
                schema.describefieldresult dfield = sfield.getDescribe();   
                System.debug('Example - DatabaseFLS.flsInsert: dfield: ' + dfield);

                if(!dfield.isIdLookup()){
                    if(isInsert){
                        if (dfield.isCreateable()){
                            flsValidFlag = true;
                            System.debug('Example - DatabaseFLS.flsInsert: ' + dfield.getName() + ' isCreateable: true');
                        } else { flsValidFlag = false;
                            System.debug('Example - DatabaseFLS.flsInsert: ' + dfield.getName() + ' isCreateable: false');
                            System.debug('Example - DatabaseFLS.flsInsert: dfield.isCreateable(): ' + dfield.isCreateable());
                            break;
                        }
                    } else {
                        if (dfield.isUpdateable()){
                            flsValidFlag = true;
                            System.debug('Example - DatabaseFLS.flsInsert: ' + dfield.getName() + ' isUpdateable: true');
                        } else {
                            flsValidFlag = false;
                            System.debug('Example - DatabaseFLS.flsInsert: ' + dfield.getName() + ' isUpdateable: false');
                            System.debug('Example - DatabaseFLS.flsInsert: dfield.isUpdateable(): ' + dfield.isUpdateable());
                            break;
                        }
                    }                
                }    
            }
        }
        return flsValidFlag;
    }

}
1
  • Which lines is the scanner flagging up?
    – fred
    Dec 22, 2023 at 14:19

1 Answer 1

0

tl;dr

Methods that accept a single record parameter and perform DML are a viable risk to virtually any project or organization. You shouldn't ignore these warnings. They're there for a good reason.


These messages are generated from the single record parameter methods. While not strictly a show-stopping error, it should at least be a warning of code smell. From more than a decade of Apex code experience, I can say that it is far more likely that this method will later break something when someone decides they need some business logic to be made in bulk, than it is that the bulk operation will cause problems.

By bulkifying early, you'll likely save yourself, or another developer, later work. For example, you might create a method intended to be used on a Visualforce page, and it works on one record at a time. Later, another developer might need to duplicate that logic into a flow action or a batch class. At that point, governor limits may be exceeded on large API calls, which most developers and/or admins are not likely to test before deploying to production.

Consider this: method A calls the single record method, and later, method B calls the method A, and then even later, method C calls method B, and finally, still later, method D calls method C. Now, a later developer needs to make an invocable method. They find method D, take a glance, and figures it's safe to call inside a loop in an invocable method. They test it against a record, and everything looks fine. They then deploy this to production, where it immediately crashes an replication integration.

In that scenario, the developer didn't necessarily realize that DML was going to be called, since it was four layers removed. They could have researched all the code, but figured it wasn't necessary. The more layers that get piled on to code, the more likely it is that this kind of code will cause later problems, and the more layers, the more work it'll take to bulkify the entire process.

Whenever possible, don't assume your method won't later be called in a loop somehow. Bulkification should be considered a priority for every line of code you write. Of course, you could just say that some or all of these are false positives, because you already have an end design in mind. Just keep in mind that this design is a risk. Requirements change, projects evolve, etc.

I would strongly advise that you don't have single record methods if you can help it. There have been exactly zero times when I wrote bulkified code and then regretted not making it single record only, but there have been plenty of times when I wrote a non-bulkified method and later regretted it. These kinds of methods are ticking time bombs.

You must log in to answer this question.

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