Consider adding DataAccessExceptionMapper to kiwi

[sleberknight]

I found this in one of our services (not including imports):

@Slf4j
public class DataAcccessExceptionMapper implements ExceptionMapper<DataAccessException> {

    private static final String MSG_OPTIMISTIC = "Unable to update data. You have a stale copy; refresh and try again."
    private static final String MSG_INVALID = "Unable to update data. Your data is invalid or a duplicate.";
    private static final String MSG_UNKNOWN =
            "Unable to complete your request; please report error %s and the current date/time to the administrators.";

    @Override
    public Response toResponse(DataAccessException e) {
        int code;
        String msg;

        if (e instanceof OptimisticLockingFailureException) {
            msg = MSG_OPTIMISTIC;
            code = 400;
            LOG.warn("{} - {}", msg, e.getMessage());
        } else if (e instanceof DataIntegrityViolationException) {
            msg = MSG_INVALID;
            code = 400;
            LOG.warn("{} - {}", msg, e.getMessage());
        } else {
            msg = f(MSG_UNKNOWN, ThreadLocalRandom.current().nextLong());
            code = 500;
            LOG.warn(msg, e);
        }

        return JaxrsExceptionMapper.buildResponse(new JaxrsException(msg, code));
    }
}

Things to note/consider:

• The three exception types are all Spring in the org.springframework.dao package, so might want to rename it something like SpringDataAccessExceptionMapper ? (though, see question below about one versus many mappers)
• Are there any other Spring data access exceptions that could, or should, go in this mapper?
• Would it be better to generate a UUID to go into the "unknown" error message instead of a random long?
• For the optimistic locking and data integrity violations, we log our custom message and only the exception message, but not the actual exception. Should we also include the exception as the last argument so the stack trace goes into the logs?
• If we add this, we'd maybe want to create other mappers for similar exceptions, e.g. from Hibernate, JDBI if it has any similar ones, Mongo, etc. I originally wanted to do the comparisons based on Strings, but then quickly realized you can't, since ExceptionMapper has a type parameter which is used by Jersey/JAX-RS/Jakarta EE to figure out which mapper to apply.
• If we want to support other libraries (e.g. Hibernate, JDBI, etc), is it better to have all of them in one ExceptionMapper or separate them?

[sleberknight]

Instead of using "instance of" checking, we could simply have a list of the FQCN for data access violation classes in various frameworks and libraries. We get the class name and compare against the ones we support. That way we could easily support the optimistic locking violations for the Spring, Hibernate, Spring Data, and whatever other libraries like JDBI which have their own exceptions.

[sleberknight]

No, can't do this because of the fifth bullet in the original description, which states:

I originally wanted to do the comparisons based on Strings, but then quickly realized you can't, since ExceptionMapper has a type parameter which is used by Jersey/JAX-RS/Jakarta EE to figure out which mapper to apply.

[sleberknight]

The Spring DataAccessException hierarchy contains a large number of exception subclasses of various types, broken down by the major categories of non-transient, transient, recoverable, and bulk operations. The non-transient and transient categories each have a number of exceptions underneath them. And there are even more if you also add in exceptions from Spring Data (Commons, MongoDB, etc.)

Because there are so many DataAccessException sub-types, trying to make a (more or less) generic DataAccessExceptionMapper will be complex and subjective. As a result I don't think we should attempt to implement this, especially given that over time we've moved away from Spring. So, I am closing this as "Won't Fix".