1

I am having an issue where I have a trigger that rolls up data from contacts to an account. This was working fine with no issues until we added a managed package app that is constantly checking and cleaning records with firmographic data. Looks liek the Batch APex job on this app runs every three minutes or so. With that said, I am getting 3 different errors which look to be related, one for UNABLE_TO_LOCK_ROW, Record Currently Unavilable and ENTITY_FAILED_IFLASTMODIFIED_ON_UPDATE, not on every time this trigger is called, but it is happenign multiple times each day.

I have added a catch block to attempt to retry processing the record multiple times if this error is caught, but I am unsure of how to test these catch blocks in a test class to get my code coverage up to 75% or greater.

Can anyone please take a few and look over the followign trigger and test class that I have so far, and make any recommendations on how to test for those catch blocks?

Trigger Code -

trigger PersonScoreRollup on Contact (after delete, after insert, after undelete, after update) {

Contact[] cons;
if (Trigger.isDelete) 
    cons = Trigger.old;
else
    cons = Trigger.new;

Set<ID> acctIds = new Set<ID>();
for (Contact con : cons) {
   acctIds.add(con.AccountId);
}

Map<ID, Contact> contactsForAccounts = new Map<ID, Contact>([SELECT Id ,AccountId, Person_Score__c FROM Contact WHERE AccountId IN :acctIds FOR UPDATE]);

Map<ID, Account> acctsToUpdate = new Map<ID, Account>([SELECT Id, Account_Score__c FROM Account WHERE Id IN :acctIds FOR UPDATE]);

for (Account acct : acctsToUpdate.values()) {
Set<Id> conIds = new Set<Id>();
Decimal totalValue = 0;
for (Contact con : contactsForAccounts.values()) {
    if (con.AccountId == acct.Id && con.Person_Score__c != NULL) {
        totalValue += con.Person_Score__c; 
    }
}
acct.Account_Score__c = totalValue;
}
if(acctsToUpdate.values().size() > 0) {
    try{
     update acctsToUpdate.values();   
    }catch(Exception e1){
        if(e1.getMessage().contains('UNABLE_TO_LOCK_ROW') || e1.getMessage().contains('Record Currently Unavailable') || e1.getMessage().contains('ENTITY_FAILED_IFLASTMODIFIED_ON_UPDATE')){
            try{
                update acctsToUpdate.values();
            }catch(Exception e2){
                if(e2.getMessage().contains('UNABLE_TO_LOCK_ROW') || e2.getMessage().contains('Record Currently Unavailable') || e2.getMessage().contains('ENTITY_FAILED_IFLASTMODIFIED_ON_UPDATE')){
                    try{
                        update acctsToUpdate.values();
                    }catch(Exception e3){

                    }
                }
             }
         }
     }
}
}

Test Class Code -

    @isTest
public class PersonScoreRollupTest {

    public static testmethod void tm1() {


        Account a1 = new Account();
        a1.Name = 'Test Account';
        a1.BillingStreet = '123 Anywhere Street';
        a1.BillingCity = 'Dallas';
        a1.BillingState = 'Texas';
        a1.BillingPostalCode = '75034';
        a1.Solution__c = 'HIPAA';
        a1.Channel__c = 'SMB';
        insert a1;


        Contact c1 = new Contact();
        c1.FirstName = 'Test';
        c1.LastName = 'tester';
        c1.AccountId = a1.Id;
        c1.Contact_Status__c = 'New';
        c1.LeadSource = 'LivePerson';
        c1.Referrer_Code__c = 'Test code';
        c1.Email = 'shawn.reichner@armor.com';
        c1.CurrencyIsoCode = 'USD';
        c1.Phone = '5709998888';
        c1.Person_Score__c = 30;
        insert c1;

        Contact c2 = new Contact();
        c2.FirstName = 'Test';
        c2.LastName = 'tester';
        c2.AccountId = a1.Id;
        c2.Contact_Status__c = 'New';
        c2.LeadSource = 'LivePerson';
        c2.Referrer_Code__c = 'Test code';
        c2.Email = 'shawn.reichner@armor.com';
        c2.CurrencyIsoCode = 'USD';
        c2.Phone = '5709998887';
        c2.Person_Score__c = 25;
        insert c2; 

        delete c1;



    }


}
  • 1
    I don't believe you can cause UNABLE_TO_LOCK_ROW in a test context (at least not reliably/repeatably). You'd be stuck with some messy dependency injection to fake it to obtain that coverage. Are you able to get more information about exactly what that managed package batch class does? It sounds like it's really the problem child here. – David Reed Jan 18 at 15:49
  • @DavidReed Yeah the application in question is Lean Data and it runs a batch process every few minutes on all Leads, and accounts to match leads to accounts and auto route them based on ABM best practices. This is seemingly the culprit as every few minutes batches of leads and accounts are called to do the research and auto routing if needed. It is a managed package so I can not see the actual code, but this is what I am seeing. – Shawn Jan 18 at 16:13
0

Revising my answer after better understanding the question...

You've got to replicate your add-in app's behavior and then make sure the same exceptions get tested. So for example, lock a test record, test, unlock, test again. Here's some help with that.

  • Also, the error message could be added in the Assert... – m Peixoto Jan 18 at 16:00
  • I am sorry everyone, I am new to apex in this instance and am Not sure I am following what to actually do here. I looked at the example and it is not close to my example so I am not able to coordinate the way forward in my head. Is there any example using my code that you are able to provide to will show how to throw the error and then in the same test class (If possible) to try an update again without the error so that it covers the test lines? Thanks for any help you can provide a newbie to help me learn....Thanks again everyone! – Shawn Jan 18 at 16:07
  • 1
    @Shawn As a newbie, you've been thrown into a fairly complex architectural issue. The question lxalmida links has some very good answers that explain some of the (few and very messy) options to simulate an UNABLE_TO_LOCK_ROW error so you can cover your exception handlers. It's a fundamentally difficult problem, and I would suggest that your efforts are better spent fixing your managed package problem than trying to fake this exception. You'll likely encounter this problem again. – David Reed Jan 18 at 16:13
0

I think it may be more productive to try to fix the architecture that is leading to this error being thrown than to try to simulate it in test context (I'm not sure your retry mechanic will work, in any case). lxalmida linked a question discussing approaches for simulating UNABLE_TO_LOCK_ROW in a unit test, which you would need to do to cover your exception handlers. I hope if nothing else that question illuminates the fact that this is a complex situation and involves a lot of messy code to cover a line or two.

From your comment on the code architecture:

... application in question is Lean Data and it runs a batch process every few minutes on all Leads, and accounts to match leads to accounts and auto route them based on ABM best practices. This is seemingly the culprit as every few minutes batches of leads and accounts are called to do the research and auto routing if needed

I agree that this is almost certainly the culprit. Based solely on your description, that sounds like a really bad design on the part of Lean Data, and one that's likely to continue causing you problems. In fairness, I haven't used that application, so I cannot really comment on its behavior.

One thing we can do with the information we have is try to optimize your code to keep it performant and prevent it from trying to lock records it does not need to lock. The Record Locking Cheat Sheet is going to be important, as always in these situations!

One thing to note off the bat is that a Contact update operation locks the parent Account. This is called out on page 1 as a High risk of lock contention.

One of the core things we can do to reduce the risk of lock contention is to operate only on records that actually need to be changed. So here,

Set<ID> acctIds = new Set<ID>();
for (Contact con : cons) {
   acctIds.add(con.AccountId);
}

you're processing all of the Accounts for all of the Contacts in your trigger set. But based on what it looks like you're trying to do, you may not need to do so. If a Contact is being updated but its Person_Score__c field hasn't changed, or if it is deleted or newly inserted with a null Person_Score__c field, you have no action to do. So you do not need to include that Contact's Account in the operation, and reduce your lock contention potential by shrinking the number of additional records that need to be processed through the Account trigger.

Map<ID, Contact> contactsForAccounts = new Map<ID, Contact>([SELECT Id ,AccountId, Person_Score__c FROM Contact WHERE AccountId IN :acctIds FOR UPDATE]);

It isn't the source of this error (probably), but you don't need to lock your Contacts here - you aren't mutating them. You also don't need to explicitly lock the Accounts, because they're already locked per the Record Locking Cheat Sheet.

for (Account acct : acctsToUpdate.values()) {
    Set<Id> conIds = new Set<Id>();
    Decimal totalValue = 0;
    for (Contact con : contactsForAccounts.values()) {
        if (con.AccountId == acct.Id && con.Person_Score__c != NULL) {
            totalValue += con.Person_Score__c; 
        }
    }
    acct.Account_Score__c = totalValue;
}

I see a couple of ways to streamline this code. One is that you're performing a combinatoric search here - iterating over all Accounts, then iterating over all Contacts and seeing which ones match up. That's very inefficient, and it can lengthen the duration of your record locks - making the odds of lock contention higher by some unknowable small amount.

Instead, you can use a subquery on your Account query to get just its Contacts:

Map<ID, Account> acctsToUpdate = new Map<ID, Account>(
    [
        SELECT Id, Account_Score__c
               (SELECT Person_Score__c FROM Contacts WHERE Person_Score__c != NULL)
        FROM Account 
        WHERE Id IN :acctIds FOR UPDATE
    ]
);

Then, your loop simplifies:

for (Account acct : acctsToUpdate.values()) {
    Decimal totalValue = 0;
    for (Contact con : acct.Contacts) {
        totalValue += con.Person_Score__c; 
    }
    acct.Account_Score__c = totalValue;
}

Another route would be to write this as an aggregate query, which is even simpler:

 List<AggregateResult> agrs = [SELECT SUM(Person_Score__c) totalScore, AccountId FROM Contact WHERE AccountId IN :acctIds GROUP BY AccountId];

Then you can loop over your AggregateResults to do get('totalScore') and you've already got it correlated with the relevant AccountId, letting Salesforce do the work for you.

I don't know whether these changes will be enough to cure the occasional record locking errors. It's entirely possible they won't be, but I would recommend exhausting the options that are available and easy before either making deeper architectural interventions or spending a large amount of effort to cover these retry attempts.

  • David, Thank you for the additional information sir. Very much appreciated and I will try to follow your lead here and make those changes and we will see what happens. – Shawn Jan 18 at 17:28

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.