Skip to content

Commit

Permalink
Handle select Hibernate data exception classes in LoggingExceptionMap…
Browse files Browse the repository at this point in the history 
…per (#79)

* Handle select Hibernate data exception classes in LoggingExceptionMapper

* Handle Hibernate OptimisticEntityLockException and
  ConstraintViolationException classes
* Refactor internal logic to use a map from class name to a category
  which is then used to select the appropriate response status code.
  The only annoying thing is that we don't yet have Java's new
  exhaustive compiler check on the switch, so the default arm cannot
  be covered with tests.
* Refactor LoggingExceptionMapperTest to use the actual Spring and
  Hibernate classes, which allowed us to change the
  dataAccessExceptionResponse method to be private
* Add the Spring and Hibernate dependencies with test scope

Closes #48
  • Loading branch information
@sleberknight
sleberknight authored Jan 8, 2021
1 parent e0e49c7 commit c89ddd4
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 36 deletions.
26 changes: 26 additions & 0 deletions pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,11 @@

<!-- Versions for test dependencies -->
<curator.version>2.13.0</curator.version>
<hibernate.version>5.4.24.Final</hibernate.version>
<jackson.version>2.10.5</jackson.version>
<junit-pioneer.version>1.1.0</junit-pioneer.version>
<kiwi-test.version>0.13.0</kiwi-test.version>
<spring.version>5.3.0</spring.version>

<!-- Versions for plugins -->

Expand Down Expand Up @@ -284,6 +286,23 @@
</exclusions>
</dependency>

<dependency>
<groupId>org.hibernate</groupId>
<artifactId>hibernate-core</artifactId>
<version>${hibernate.version}</version>
<scope>test</scope>
<exclusions>
<exclusion>
<groupId>net.bytebuddy</groupId>
<artifactId>byte-buddy</artifactId>
</exclusion>
<exclusion>
<groupId>org.jboss.logging</groupId>
<artifactId>jboss-logging</artifactId>
</exclusion>
</exclusions>
</dependency>

<dependency>
<groupId>com.fasterxml.jackson.jaxrs</groupId>
<artifactId>jackson-jaxrs-json-provider</artifactId>
Expand Down Expand Up @@ -334,6 +353,13 @@
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.springframework</groupId>
<artifactId>spring-tx</artifactId>
<version>${spring.version}</version>
<scope>test</scope>
</dependency>

</dependencies>

</project>
53 changes: 35 additions & 18 deletions src/main/java/org/kiwiproject/dropwizard/util/exception/LoggingExceptionMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import com.google.common.annotations.VisibleForTesting;
import lombok.extern.slf4j.Slf4j;
import org.kiwiproject.jaxrs.exception.JaxrsBadRequestException;
import org.kiwiproject.jaxrs.exception.JaxrsConflictException;
import org.kiwiproject.jaxrs.exception.JaxrsException;
import org.kiwiproject.jaxrs.exception.JaxrsExceptionMapper;
import org.kiwiproject.jaxrs.exception.WebApplicationExceptionMapper;
Expand All @@ -13,6 +14,7 @@
import javax.ws.rs.core.Response;
import javax.ws.rs.ext.ExceptionMapper;
import javax.ws.rs.ext.Provider;
import java.util.Map;
import java.util.concurrent.ThreadLocalRandom;

/**
Expand All @@ -32,39 +34,54 @@ public class LoggingExceptionMapper<E extends Throwable> implements ExceptionMap
@VisibleForTesting
static final String MSG_DB_INVALID = "Unable to save data. Your data is invalid or not unique!";

private enum DataExceptionCategory {
OPTIMISTIC_LOCKING,
DATA_INTEGRITY
}

private static final Map<String, DataExceptionCategory> DATA_EXCEPTIONS = Map.of(
"org.springframework.dao.OptimisticLockingFailureException", DataExceptionCategory.OPTIMISTIC_LOCKING,
"org.hibernate.dialect.lock.OptimisticEntityLockException", DataExceptionCategory.OPTIMISTIC_LOCKING,
"org.springframework.dao.DataIntegrityViolationException", DataExceptionCategory.DATA_INTEGRITY,
"org.hibernate.exception.ConstraintViolationException", DataExceptionCategory.DATA_INTEGRITY
);

@Override
public Response toResponse(E exception) {
Response r;

if (exception instanceof WebApplicationException) {
// this shouldn't happen as we have a registered exception mapper for this class
r = new WebApplicationExceptionMapper().toResponse((WebApplicationException) exception);
} else {
var exceptionName = exception.getClass().getCanonicalName();

// don't want to load spring libraries to create it's own exception mapper but don't want a lot of
// services to have to create a duplicate DataAccessExceptionMapper either so ...
r = exceptionName.startsWith("org.springframework.dao.")
? dataAccessExceptionResponse(exceptionName, exception)
: logExceptionResponse(exception);
return new WebApplicationExceptionMapper().toResponse((WebApplicationException) exception);
}

return r;
return responseFor(exception);
}

@VisibleForTesting
Response dataAccessExceptionResponse(String className, E exception) {
private Response responseFor(E exception) {
var exceptionClassName = exception.getClass().getName();

if (DATA_EXCEPTIONS.containsKey(exceptionClassName)) {
var category = DATA_EXCEPTIONS.get(exceptionClassName);
return dataAccessExceptionResponse(exception, category);
}

return logExceptionResponse(exception);
}

private Response dataAccessExceptionResponse(E exception, DataExceptionCategory category) {
Response r;
switch (className) {
case "org.springframework.dao.OptimisticLockingFailureException":
r = JaxrsExceptionMapper.buildResponse(new JaxrsBadRequestException(MSG_DB_OPTIMISTIC));
switch (category) {
case OPTIMISTIC_LOCKING:
r = JaxrsExceptionMapper.buildResponse(new JaxrsConflictException(MSG_DB_OPTIMISTIC));
LOG.warn(MSG_DB_OPTIMISTIC, exception);
break;
case "org.springframework.dao.DataIntegrityViolationException":

case DATA_INTEGRITY:
r = JaxrsExceptionMapper.buildResponse(new JaxrsBadRequestException(MSG_DB_INVALID));
LOG.warn(MSG_DB_INVALID, exception);
break;

default:
LOG.warn("DataExceptionCategory {} is not handled! Using default handler.", category);
r = logExceptionResponse(exception);
}

Expand Down
65 changes: 47 additions & 18 deletions src/test/java/org/kiwiproject/dropwizard/util/exception/LoggingExceptionMapperTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,58 +2,87 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.kiwiproject.dropwizard.util.exception.ErrorMessageAssertions.assertAndGetErrorMessage;
import static org.kiwiproject.test.jaxrs.JaxrsTestHelper.assertBadRequest;
import static org.kiwiproject.test.jaxrs.JaxrsTestHelper.assertConflict;
import static org.kiwiproject.test.jaxrs.JaxrsTestHelper.assertInternalServerErrorResponse;

import org.hibernate.dialect.lock.OptimisticEntityLockException;
import org.hibernate.exception.ConstraintViolationException;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.dao.DataIntegrityViolationException;
import org.springframework.dao.OptimisticLockingFailureException;

import javax.ws.rs.WebApplicationException;
import java.sql.SQLException;

@SuppressWarnings("rawtypes")
@DisplayName("LoggingExceptionMapper")
class LoggingExceptionMapperTest {

private final LoggingExceptionMapper mapper = new LoggingExceptionMapper<>() {};
private final LoggingExceptionMapper mapper = new LoggingExceptionMapper<>() {
};

@SuppressWarnings("unchecked")
@Test
void shouldProcessAnyNonMappedException() {
void shouldProcess_AnyNonMappedException() {
var exception = new RuntimeException("oops");
var response = mapper.toResponse(exception);
var errorMessage = assertAndGetErrorMessage(response);

assertThat(response.getStatus()).isEqualTo(500);
assertInternalServerErrorResponse(response);
assertThat(errorMessage.getMessage()).startsWith("There was an error processing your request");
}

@SuppressWarnings("unchecked")
@Test
void shouldProcessSpringClasses() {
var className = "org.springframework.dao.OptimisticLockingFailureException";
var exception = new RuntimeException("oops");
var response = mapper.dataAccessExceptionResponse(className, exception);
void shouldProcess_SpringOptimisticLockingFailureException() {
var exception = new OptimisticLockingFailureException("optimistic lock error");
var response = mapper.toResponse(exception);
var errorMessage = assertAndGetErrorMessage(response);

assertConflict(response);
assertThat(errorMessage.getMessage()).isEqualTo(LoggingExceptionMapper.MSG_DB_OPTIMISTIC);
}

@SuppressWarnings("unchecked")
@Test
void shouldProcess_HibernateOptimisticEntityLockException() {
var entity = new Object();
var exception = new OptimisticEntityLockException(entity, "optimistic lock error");
var response = mapper.toResponse(exception);
var errorMessage = assertAndGetErrorMessage(response);

assertThat(response.getStatus()).isEqualTo(400);
assertConflict(response);
assertThat(errorMessage.getMessage()).isEqualTo(LoggingExceptionMapper.MSG_DB_OPTIMISTIC);
}

className = "org.springframework.dao.DataIntegrityViolationException";
response = mapper.dataAccessExceptionResponse(className, exception);
errorMessage = assertAndGetErrorMessage(response);
@SuppressWarnings("unchecked")
@Test
void shouldProcess_SpringDataIntegrityViolationException() {
var exception = new DataIntegrityViolationException("data integrity violation");
var response = mapper.toResponse(exception);
var errorMessage = assertAndGetErrorMessage(response);

assertThat(response.getStatus()).isEqualTo(400);
assertBadRequest(response);
assertThat(errorMessage.getMessage()).isEqualTo(LoggingExceptionMapper.MSG_DB_INVALID);
}

className = "org.springframework.dao.SomeOtherException";
response = mapper.dataAccessExceptionResponse(className, exception);
errorMessage = assertAndGetErrorMessage(response);
@SuppressWarnings("unchecked")
@Test
void shouldProcess_HibernateConstraintViolationException() {
var sqlException = new SQLException();
var exception = new ConstraintViolationException("constraint violation", sqlException, "constraint12345");
var response = mapper.toResponse(exception);
var errorMessage = assertAndGetErrorMessage(response);

assertThat(response.getStatus()).isEqualTo(500);
assertThat(errorMessage.getMessage()).startsWith("There was an error processing your request");
assertBadRequest(response);
assertThat(errorMessage.getMessage()).isEqualTo(LoggingExceptionMapper.MSG_DB_INVALID);
}

@SuppressWarnings("unchecked")
@Test
void shouldProcessWebApplicationException() {
void shouldProcess_WebApplicationException() {
var exception = new WebApplicationException("oops");
var response = mapper.toResponse(exception);
var errorMessage = assertAndGetErrorMessage(response);
Expand Down

0 comments on commit c89ddd4

Please sign in to comment.