How to write the superduper (=proper and stable) Custom Save method

Question

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() {
   ...
}

Answer 1

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);
}

Answer 2

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