Adar Dembo / Jason Huang / Bret:

#1 sounds fine to me, but why do we have to introduce a new enum value? Shouldn't all instances of NOT_AUDITED behave this way? I don't see how throwing an exception like this would ever be better.

That depends.

Imagine a system where you have master versus transactional based data. You might elect to purge your transactional data because your system generates that data extremely fast while master data is meant to be around forever and grows at a much slower pace.

When you introduce an audited entity into the picture here, perhaps its acceptable to null out #NOT_AUDITED references to transactional relations but you want a hard failure in cases where a #NOT_AUDITED might be linked to a slower, more master data reference that you guarantee should exist in the main schema.

A configuration property wouldn't be good for this either. We ultimately need something we can specify at the property level to drive exactly how we're suppose to handle these situations per property.

One option is obviously a new enumeration value as Adam suggested. Another might be to introduce another attribute on the @Audited annotation like targetNotFoundAction=ERROR/IGNORE. We could set the default value to IGNORE, meaning we gracefully handle these cases but a user could opt to set it to ERROR, allowing those very specific corner cases to be controllable.

For #1, my concern is how to distinguish a missing relation from a null relation. I think it will be better to provide as much information as possible, for example, instead of returning a null, create an empty object and populate the id field into it so that by comparing the id field we know whether the relation has been changed. From auditor's perspective, it's good to know any change.

Envers already returns instances that are only guaranteed to be a partial view anyway. A lot of what gets populated highly depends on the environment with what properties and associations are being audited. So I'm not convinced returning null vs an instance with just the identifier makes any difference.

Imagine a situation where you have an entity that just requires an identifier and its attributes can all be null. How then would you differentiate those cases from the embedded place holder with just the identifier? It spins both ways imo.

Could envers use the existing "@NotFound" Hibernate annotation to infer that it should safely ignore cases where the auditing entity relationship is missing? Or possibly the implementation would be something similar? (Maybe there are cases where the not found should only apply to envers and not Hibernate overall)

This is indeed already supported.

In fact, if you use the mapping the user supplied in the description and add @NotFound(action=IGNORE), then you won't get the exception and the environment behaves gracefully by setting the field to null.

But as you pointed out, doing this does affect how Hibernate behaves too and thus if the ORM side were to fault for any reason, the error would be silently ignored when perhaps you'd rather it not.

But we can influence the same behavior specific just to Envers, we just need to determine the basis for the rule to use here.

I believe we either need a new enum value or a new attribute on the @Audited annotation as a described above. I think I might personally be partial to the targetNotFoundAction attribute.

Thoughts?