1

In my controller extension I have a custom save method that

  • works for New and Edit scenarios
  • also saves modified child records

The below code works in all unit tests but starts to show strange behavior in customer orgs (duplicate values, workflow errors).

private void save() {
    // Note: Provide ACID behavior
    Savepoint toBeforeState = Database.setSavepoint();

    try {
        upsert record;
        saveChildrenOfTypeA();
        saveChildrenOfTypeB();
    }
    catch(Exception ex) {
        ApexPages.addMessages(ex);
        Database.rollback(toBeforeState);

        // Note: See http://salesforce.stackexchange.com/questions/57020/data-not-available-when-using-database-rollback-in-custom-save-action
        removeInvalidIds();

        throw ex;
    }
}

It's just a feeling, but from browsing the web, I think maybe

  1. The use of upsert instead of insert and update
  2. The try/catch with the database rollback
  3. The re-throwing of the exception

is wrong.

What do you think?!

  • 1
    I don't follow the need for the try-catch block Robert. Are you getting the dupes in the child records, the Master records, or both? – crmprogdev Mar 18 '15 at 13:25
  • @crmprogdev: Would changes in the parent record be rolled back if anything in the child saving goes wrong without the try/catch? Are you saying I could get this without any rollback and try catch? – Robert Sösemann Mar 18 '15 at 13:32
  • The way you have it set up, everything would be rolled back. I see no reason for the try catch. You're trying to upsert a record. If it fails, it fails and you'll get an error, correct? You can't save a child record in a M-D relationship if the master doesn't exist. So no need for the try-catch that I can see. That at least applies for the insert portion. You can always do a database upsert and get the results. If true, then proceed with saving the childrecords. – crmprogdev Mar 18 '15 at 13:45
  • @crmprogdev but how would I get ACID when updating parent plus childs? An error in the childs needs to revert the changes in the parent? – Robert Sösemann Mar 18 '15 at 13:49
  • 1
    Will post an answer with code for you. – crmprogdev Mar 18 '15 at 14:06
1

I think something like this might make it easier to debug where your problem lies.

private void save() {
    // Note: Provide ACID behavior
    Savepoint toBeforeState = Database.setSavepoint();

    string failedIn = '';
    string error = '';
    try {
        failedIn = 'Parent';
        upsert record;
        failedIn = 'TypeA';
        saveChildrenOfTypeA();
        failedIn = 'TypeB';
        saveChildrenOfTypeB();
        failedIn = null;
    }
    catch(Exception ex) {
        error = ex.getMessage();
    }

    if(failedIn != null)
    {
        ApexPages.addMessages('Failed In:' +failedIn+' Error:'+error);
        Database.rollback(toBeforeState);

        // Note: See http://salesforce.stackexchange.com/questions/57020/data-not-available-when-using-database-rollback-in-custom-save-action
        removeInvalidIds();
    }
}

That way you can make sure that the problem is not the try catch or the fact that you are throwing an exception.

Good luck!

Luis Luciani

Edit:

Sorry for the confusion. Let me try to explain myself.

You need both features. The try/catch will prevent the execution from stopping and throwing an ugly error as soon as one is encountered. The savepoint and rollback you need so that you do not leave any half processed changes on the DB. If you were not using the try/catch, then you would not need the savepoints, because the platform will handle the rollback for you, even if the first change succeeded and a subsequent one failed.

In conclusion, seems like you have 2 options. Get rid of the try/catch and the savepoints, or keep both and do some debugging to determine why you are ending up with dupes (In which case I would start with the code provided above).

Hope that helps.

| improve this answer | |
  • I was not looking for debugging tips but someone to tell me if one needs Savepoints and try/catch contructs to get ACID behavior. Sorry that's a -1. – Robert Sösemann Mar 18 '15 at 14:40
  • 1
    You do need both. The try catch will prevent the execution from stopping and throwing an ugly error as soon as one is encountered. The savepoint and rollback you need so that you do not leave any half processed changes on the DB. – slashApex Mar 18 '15 at 15:10
0

I believe this is the type of thing that you need to do which I was trying to explain in comments and apparently not doing a very good job of doing. The database.upsert(list<sObject>,true) requires that all records be either inserted or updated in order for there to be success.

Whether or not you choose to get the record Id's is entirely your choice. As you can see, at any point at which you've failed to successfully upsert all your records, you automatically do a rollback. You can add an Apex.Page Messages at that point if you desire to.

Using this method, there's no need for a try-catch block. The method assumes the use of lists and would need to be modified if you're using individual records.

Note: Edited to correct locations of 1st and 3rd Database.rollbacks.

private void save() {
    // Note: Provide ACID behavior
    Savepoint toBeforeState = Database.setSavepoint();    

    if(record.IsEmpty() == false)
    {
        list<Database.UpsertResult>uResults = Database.upsert(record, true);   

        // Iterate through each returned result
        for (Database.uResults ur : urList) 
        {
            if (ur.isSuccess()) 
            {
               // Operation was successful, so get the ID of the record that was processed
               System.debug('Successfully inserted record. record ID: ' + ur.getId());
            }else{
               // Operation failed, so get all errors                
               for(Database.Error err : ur.getErrors()) 
               {
                  System.debug('The following error has occurred.');                    
                  System.debug(err.getStatusCode() + ': ' + err.getMessage());
                  System.debug('record fields that affected this error: ' + err.getFields());
               }
                Database.rollback(toBeforeState); 
            }
        }  
    }   


    if(ChildrenOfTypeA.IsEmpty() == false)
    {
        list<Database.upsertResult>uResultsA = Database.upsert(ChildrenOfTypeA, true);    
        // Iterate through each returned result
        for (Database.uResultA urA : urListA) 
        {
            if (urA.isSuccess()) {
               // Operation was successful, so get the ID of the record that was processed
               System.debug('Successfully inserted record. record ID: ' + urA.getId());
            }else {
                // Operation failed, so get all errors                
                for(Database.Error err : urA.getErrors()) 
                {
                    System.debug('The following error has occurred.');                    
                    System.debug(err.getStatusCode() + ': ' + err.getMessage());
                    System.debug('record fields that affected this error: ' + err.getFields());
                }
                Database.rollback(toBeforeState);
            }
        }
     }

    if(ChildrenOfTypeB.IsEmpty() == false)
    {
    list<Database.upsertResult>uResultsB = Database.upsert(ChildrenOfTypeB, true);

       // Iterate through each returned result
       for (Database.uResultA urB : urListB) 
       {
           if (urB.isSuccess()) 
           {
               // Operation was successful, so get the ID of the record that was processed
               System.debug('Successfully inserted record. record ID: ' + urB.getId());
           }else {
               // Operation failed, so get all errors                
               for(Database.Error err : urB.getErrors()) 
               {
                   System.debug('The following error has occurred.');                    
                   System.debug(err.getStatusCode() + ': ' + err.getMessage());
                   System.debug('record fields that affected this error: ' + err.getFields());
               }
               Database.rollback(toBeforeState);
           }
       }
   }
}
| improve this answer | |
0

I think problems will arise from rethrowing the exception that you just catched and converted to an PageMessage.

| improve this answer | |

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.