28

Custom Save actions in Apex are tricky!

Here are my requirements which I was unable to fulfill without harming the stability of my code:

  • Save for New and Edit (works with record with and without Id)
  • Saves a parent and related child records in one go
  • Provides ACID properties. If single DML fails all other will be rolled back
  • Displays DML errors as nice Page messages (no white exception screen)

1. Straight forward solution => Ugly exception instead of page messages

public void doSave() {

  upsert parent;

  for(Child__c child : children) {
      if(child.Id == null) {
          child.mdr_Parent__c = parent.Id;
      }
  }

  upsert children;
}

2. Convert exception to messages => No ACID properties

public PageReference doSave() {

    // Note: Without try/catch DML errors would be shown as ugly white error page
    try {
        upsert parent;

        for(Child__c child : children) {
            if(child.Id == null) {
                child.mdr_Parent__c = parent.Id;
            }
        }

        upsert children;
    }
    catch(Exception ex) {
        ApexPages.addMessages(ex);
    }

    PageReference result = (ApexPages.hasMessages()) ? null : new PageReference('/' + parent.Id);
    return result;
}

3. Add Database.rollback() => Instable, side effects, errors

The most elaborate solution leads to Data not available and Duplicate values errors under certain circumstances.

public PageReference doSave() {

    // Note: Required as below try/catch would prevent the platforms default behaviour to rollback on error
    Savepoint toBeforeState = Database.setSavepoint();

    // Note: Without try/catch DML errors would be shown as ugly white error page
    try {
        upsert parent;

        for(Child__c child : children) {
            if(child.Id == null) {
                child.mdr_Parent__c = parent.Id;
            }
        }

        upsert children;
    }
    catch(Exception ex) {
        ApexPages.addMessages(ex);
        Database.rollback(toBeforeState);

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

    PageReference result = (ApexPages.hasMessages()) ? null : new PageReference('/' + parent.Id);
    return result;
}

4. The perfect solution

public PageReference doSave() {
   ...
}
  • 2
    Good question! I've been grappling with this too. And no good answers either. – Caspar Harmer Apr 8 '15 at 9:15
  • 2
    this is disturbing as I use pattern 3 all the time except with inserts and updates, never upserts. – cropredy Apr 8 '15 at 18:04
  • 2
    @crop1645 Maybe my use of upsert is the root of all evil. Would you mind post your solution as an answer? – Robert Sösemann Apr 8 '15 at 18:34
  • 1
    I looked at your Data Not Available post and I would ask you to think about the state of the controller variables that are not rolled back when Database.rollback(..) is invoked and hence partial update ID fields may be still present on the VF page/controller when the user clicks Save the second time after an error – cropredy Apr 8 '15 at 18:49
  • 2
    Keith C's answer salesforce.stackexchange.com/a/57022/2602 was on the same point I was getting at – cropredy Apr 9 '15 at 17:38
5

I wouldn't necessarily call it the perfect solution, but here are some ideas I've used to make my code more maintainable.

Separate your Visualforce action method from your save method that performs DML operations. You never want your user to see the white exception screen so you want to catch all exceptions in your action method. But these exceptions should only occur when there's a bug.

If you're going to keep the user on the page after save, reset your controller's state upon a successful save. This allows you to pull in data that changed as a result of a trigger, for example.

public PageReference save() {
  Savepoint beforeSave = Database.setSavepoint();
  try {
    Boolean saveSuccessful = this.saveData();
    if (saveSuccessful) {
      if (this.shouldRedirect()) {
        return new PageReference('/' + this.parent.Id);
      } else {
        // If it was a new record, grab the new id and reinitialize the controller state
        this.parentId = this.parent.Id;
        this.reset();
      }
    }
  } catch (Exception ex) {
    ApexPages.addMessage(new ApexPages.Message(ApexPages.severity.FATAL, 'Unexpected error during save'));
    ApexPages.addMessages(ex);
    Database.rollback(beforeSave);
  }
}

When saving the records, I've found splitting up the inserts from the updates is helpful. It lets me keep track of records for which I need to reset the id if I have to rollback due to a DMLException caused by a validation rule, for example.

private Boolean saveData() {
  Savepoint beforeUpsert = Database.setSavepoint();
  GroupedForDML records = new GroupedForDML(this.recordsToUpsert());
  records.tryToInsert();
  records.trytoUpdate();
  if (records.errored) {
    Database.rollback(beforeUpsert);
    for (SObject o: records.toInsert) {
      // Unset Ids on any records successfully inserted, then rolled back due to the failed update.
      o.Id = null;
    }
    return false;
  }
  return true;
}

I've omitted the implementation of GroupedForDML for brevity. The only thing it does that's not obvious from its interface is that it adds the errors to the page messages when a DMLException is thrown in tryToInsert or tryToUpdate.

One other thing you will notice is that there's no code to set the parent id on the children after the parent is inserted. I prefer to use an External Id field on the parent object so the child is linked to the parent as soon as the child is instantiated.

The constructor generates a GUID for the external id and sets it on the parent if it's a new record. When a child is added to children, it's linked to parent either using the parent's id or a Parent__c object with the external id set.

public Example(ApexPages.StandardController controller) {
  this.parentId = controller.getId();
  // generate a GUID to use as the external id if this is a new parent
  this.parentGUID = guid();
  try {
    this.loadData();
  } catch (Exception ex) {
    ApexPages.addMessage(new ApexPages.Message(ApexPages.severity.FATAL, 'Unexpected error during initialization', 'loadData failed'));
    ApexPages.addMessages(ex);       
  }
}

private void loadData() {
  // load the data from the database
  this.parent = getData(this.parentId);
  if (this.parent.Id == null) {
    this.parent.External_Id__c = this.parentGUID;
  }
  this.children = this.parent.Children__r;
}

// Visualforce action to add a new child
public void addChild() {
  Child__c child = new Child__c();
  // Link the child to the parent by id or external id (assuming not all parents have external ids)
  if (this.parent.Id != null) {
    child.put('Parent__c', this.parent.Id);
  } else {
    child.putSObject(Child__c.Parent__c.getDescribe().getRelationshipName(), new Parent__c(External_Id__c = this.parent.External_Id__c));
  }
  this.children.add(child);
}
0

I think you are looking for Database.upsert, it performs upsert and collects all the DML Exceptions in the list<Database.UpsertResult> returned by it. You do not need a try...catch block and it also accepts a boolean parameter to allow partial success.

Read more about it here:

https://developer.salesforce.com/docs/atlas.en-us.apexcode.meta/apexcode/apex_methods_system_database.htm#apex_System_Database_upsert

  • Would using database methods instead of DML statements for a pattern 3 type custom save method avoid the Data not available and Duplicate value exceptions when calling Database.rollback() ? – martin Nov 18 '15 at 4:47

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.