[CAUSEWAY-3688] Audit trail should gracefully _attempt_ to capture values, but not fail fast.

• Type: Improvement
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 2.0.0-RC4
• Fix Version/s: 2.0.0
• Component/s: Ext Sec AuditTrail
• Assignee: Daniel Keir Haywood
• Reporter: Daniel Keir Haywood
• Created: 21/Feb/24 12:38
• Updated: 21/Feb/24 18:21
• Resolved: 21/Feb/24 18:21

Description

In Estatio, we have:

`LeaseContractFra` -> `LeaseContractFraCustomData`.  On the former:

@Programmatic
public CustomContractDataFra getCustomContractDataFra() {
    if (leaseContractRepository == null) return null; // for unit testing
    return leaseContractRepository.findCustomContractFraData(this).orElse(null);
} 

There's also a derived property 'getConditionPrecedentComment' on the LeaseContractFra which exposes one of the properties of the associated LeaseContractFraCustomData:

 

@PropertyLayout(multiLine = 5)
public String getConditionPrecedentComment() {
    return getCustomContractDataFra().getConditionPrecedentComment();
}    

 

Now the `LeaseContractFra` is the aggregate root ... when it is deleted it cascades the delete of the `LeaseContractFraCustomData` first, and then itself:

@Action(semantics = SemanticsOf.NON_IDEMPOTENT_ARE_YOU_SURE, domainEvent = RemoveEvent.class)
public void remove() {
    if (getCustomContractDataFra() != null) {
        repositoryService.removeAndFlush(getCustomContractDataFra());
    }
    // ...
    repositoryService.removeAndFlush(this);
} 

 

1. n the the audit trail component is called via the JDO "preDelete" callback of `LeaseContractFra`, it iterates over all of the properties of said object, including the derived `getConditionPrecedentComment`, and then NPEs.

Options:

1. put the burden of responsibility with the developer, either by
   1. exclude this derived property from the audit trail using `@Property(entityChangePublishing = DISABLED), or
   2. adding a null guard in the body of the derived property
2. change the audit trail to automatically ignore derived properties on the basis that they are after all derived and the audit trail will have captured the values of the underlying data that they are based upon.  (This is true in theory, but in practice figuring out the value of that derived property could be non-trivial; if nothing else, it makes the audit trail less valuable).
3. change the audit trail into a "best effort", ie it attempts to read the value of the property, but if it fails then it just continues.  In other words, this is a soft failure, similar to how if a `title()` method fails, then we render "Failed Title" but otherwise continue.

Recommendation: let's go with option #3