DATAREST-507 - Fixed IllegalArgumentException in RepositoryRestExceptionHandler.

A lot of cleanups in RepositoryRestExceptionHandler: prefer static methods where possible. Removed delegates where only one client was calling the method. Consistent argument ordering with optional (nullable) parameters last.

Silenced logging in unit tests for RepositoryRestExceptionHandler.

spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryRestExceptionHandler.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2014 the original author or authors.
+ * Copyright 2014-2015 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -69,7 +69,7 @@ public class RepositoryRestExceptionHandler {
 	 */
 	@ExceptionHandler
 	ResponseEntity<?> handleNotFound(ResourceNotFoundException o_O) {
-		return notFound();
+		return notFound(new HttpHeaders());
 	}
 
 	/**
@@ -80,7 +80,7 @@ public class RepositoryRestExceptionHandler {
 	 */
 	@ExceptionHandler
 	ResponseEntity<ExceptionMessage> handleNotReadable(HttpMessageNotReadableException o_O) {
-		return badRequest(o_O);
+		return badRequest(new HttpHeaders(), o_O);
 	}
 
 	/**
@@ -93,7 +93,8 @@ public class RepositoryRestExceptionHandler {
 	@ExceptionHandler({ InvocationTargetException.class, IllegalArgumentException.class, ClassCastException.class,
 			ConversionFailedException.class, NullPointerException.class })
 	ResponseEntity<ExceptionMessage> handleMiscFailures(Exception o_O) {
-		return errorResponse(null, HttpStatus.INTERNAL_SERVER_ERROR);
+
+		return errorResponse(HttpStatus.INTERNAL_SERVER_ERROR, new HttpHeaders(), null);
 	}
 
 	/**
@@ -105,9 +106,8 @@ public class RepositoryRestExceptionHandler {
 	@ExceptionHandler
 	ResponseEntity<RepositoryConstraintViolationExceptionMessage> handleRepositoryConstraintViolationException(
 			RepositoryConstraintViolationException o_O) {
-
+		return response(HttpStatus.BAD_REQUEST, new HttpHeaders(), new RepositoryConstraintViolationExceptionMessage(o_O,
-		return response(new HttpHeaders(), new RepositoryConstraintViolationExceptionMessage(o_O, messageSourceAccessor),
+				messageSourceAccessor));
-				HttpStatus.BAD_REQUEST);
 	}
 
 	/**
@@ -118,7 +118,7 @@ public class RepositoryRestExceptionHandler {
 	 */
 	@ExceptionHandler({ OptimisticLockingFailureException.class, DataIntegrityViolationException.class })
 	ResponseEntity<ExceptionMessage> handleConflict(Exception o_O) {
-		return errorResponse(null, o_O, HttpStatus.CONFLICT);
+		return errorResponse(HttpStatus.CONFLICT, new HttpHeaders(), o_O);
 	}
 
 	/**
@@ -134,7 +134,7 @@ public class RepositoryRestExceptionHandler {
 		HttpHeaders headers = new HttpHeaders();
 		headers.setAllow(o_O.getSupportedHttpMethods());
 
-		return new ResponseEntity<Void>(headers, HttpStatus.METHOD_NOT_ALLOWED);
+		return response(HttpStatus.METHOD_NOT_ALLOWED, headers);
 	}
 
 	/**
@@ -147,45 +147,37 @@ public class RepositoryRestExceptionHandler {
 	ResponseEntity<Void> handle(ETagDoesntMatchException o_O) {
 
 		HttpHeaders headers = o_O.getExpectedETag().addTo(new HttpHeaders());
-		return new ResponseEntity<Void>(headers, HttpStatus.PRECONDITION_FAILED);
+		return response(HttpStatus.PRECONDITION_FAILED, headers);
 	}
 
-	private <T> ResponseEntity<T> notFound() {
+	private static ResponseEntity<?> notFound(HttpHeaders headers) {
-		return notFound(new HttpHeaders(), null);
+		return response(HttpStatus.NOT_FOUND, headers, null);
 	}
 
-	private <T> ResponseEntity<T> notFound(HttpHeaders headers, T body) {
+	private static ResponseEntity<ExceptionMessage> badRequest(HttpHeaders headers, Exception throwable) {
-		return response(headers, body, HttpStatus.NOT_FOUND);
+		return errorResponse(HttpStatus.BAD_REQUEST, headers, throwable);
 	}
 
-	private <T extends Exception> ResponseEntity<ExceptionMessage> badRequest(T throwable) {
+	private static ResponseEntity<ExceptionMessage> errorResponse(HttpStatus status, HttpHeaders headers,
-		return badRequest(new HttpHeaders(), throwable);
+			Exception exception) {
-	}
-
-	private <T extends Exception> ResponseEntity<ExceptionMessage> badRequest(HttpHeaders headers, T throwable) {
-		return errorResponse(headers, throwable, HttpStatus.BAD_REQUEST);
-	}
-
-	private <T extends Exception> ResponseEntity<ExceptionMessage> errorResponse(T throwable, HttpStatus status) {
-		return errorResponse(new HttpHeaders(), throwable, status);
-	}
-
-	private <T extends Exception> ResponseEntity<ExceptionMessage> errorResponse(HttpHeaders headers,
-			Exception exception, HttpStatus status) {
 
 		if (null != exception && null != exception.getMessage()) {
 
 			LOG.error(exception.getMessage(), exception);
 
-			return response(headers, new ExceptionMessage(exception), status);
+			return response(status, headers, new ExceptionMessage(exception));
 
 		} else {
 
-			return response(headers, null, status);
+			return response(status, headers, null);
 		}
 	}
 
-	public <T> ResponseEntity<T> response(HttpHeaders headers, T body, HttpStatus status) {
+	private static <T> ResponseEntity<T> response(HttpStatus status, HttpHeaders headers) {
+		return response(status, headers, null);
+	}
+
+	private static <T> ResponseEntity<T> response(HttpStatus status, HttpHeaders headers, T body) {
 
 		Assert.notNull(headers, "Headers must not be null!");
 		Assert.notNull(status, "HttpStatus must not be null!");

spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/RepositoryRestExceptionHandlerUnitTests.java

@@ -18,12 +18,20 @@ package org.springframework.data.rest.webmvc;
 import static org.hamcrest.CoreMatchers.*;
 import static org.junit.Assert.*;
 
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
 import org.junit.Test;
+import org.slf4j.LoggerFactory;
 import org.springframework.context.support.StaticMessageSource;
+import org.springframework.dao.DataIntegrityViolationException;
 import org.springframework.data.rest.webmvc.support.ExceptionMessage;
+import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
 import org.springframework.http.converter.HttpMessageNotReadableException;
 
+import ch.qos.logback.classic.Level;
+import ch.qos.logback.classic.Logger;
+
 /**
  * Unit tests for {@link RepositoryRestExceptionHandler}.
  * 
@@ -33,6 +41,22 @@ public class RepositoryRestExceptionHandlerUnitTests {
 
 	static final RepositoryRestExceptionHandler HANDLER = new RepositoryRestExceptionHandler(new StaticMessageSource());
 
+	static Logger logger;
+	static Level logLevel;
+
+	@BeforeClass
+	public static void silenceLog() {
+
+		logger = (Logger) LoggerFactory.getLogger(RepositoryRestExceptionHandler.class);
+		logLevel = logger.getLevel();
+		logger.setLevel(Level.OFF);
+	}
+
+	@AfterClass
+	public static void enableLogging() {
+		logger.setLevel(logLevel);
+	}
+
 	/**
 	 * @see DATAREST-427
 	 */
@@ -42,6 +66,17 @@ public class RepositoryRestExceptionHandlerUnitTests {
 		ResponseEntity<ExceptionMessage> result = HANDLER
 				.handleNotReadable(new HttpMessageNotReadableException("Message!"));
 
-		assertThat(result.getStatusCode(), is(org.springframework.http.HttpStatus.BAD_REQUEST));
+		assertThat(result.getStatusCode(), is(HttpStatus.BAD_REQUEST));
+	}
+
+	/**
+	 * @see DATAREST-507
+	 */
+	@Test
+	public void handlesConflictCorrectly() {
+
+		ResponseEntity<ExceptionMessage> result = HANDLER.handleConflict(new DataIntegrityViolationException("Message!"));
+
+		assertThat(result.getStatusCode(), is(HttpStatus.CONFLICT));
 	}
 }