Commit a9ef29e0 authored by Guillaume Cornut's avatar Guillaume Cornut Committed by Jérémy Destin
Browse files

fix: Fix 400 pagination error by converting all spring bind error to BrAPI statuses.

parent 8384033f
......@@ -9,7 +9,7 @@ import fr.inra.urgi.faidare.domain.response.ApiResponseFactory;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.validation.BindException;
import org.springframework.validation.FieldError;
import org.springframework.validation.BindingResult;
import org.springframework.validation.ObjectError;
import org.springframework.web.HttpMediaTypeNotAcceptableException;
import org.springframework.web.HttpMediaTypeNotSupportedException;
......@@ -20,6 +20,7 @@ import org.springframework.web.bind.annotation.ExceptionHandler;
import java.util.ArrayList;
import java.util.List;
import java.util.stream.Collectors;
/**
* Brapi exception handling intercepting every spring web Exception and generate a correct BrAPI response out of it.
......@@ -41,30 +42,34 @@ public class BrapiExceptionHandler {
return ResponseEntity.status(httpStatus).body(body);
}
/**
* Generate BrAPI error response from Exception
*
* Automatically extracts javax.validation error message into the BrAPI status
*/
private static ResponseEntity<Object> createErrorResponse(
Exception exception, HttpStatus httpStatus
) {
List<BrapiStatus> statuses = new ArrayList<>();
BrapiPagination pagination = null;
if (exception instanceof BindException) {
// Exception occurring when a user provided parameter that did not pass spring validation
BindException bindException = (BindException) exception;
List<FieldError> fieldErrors = bindException.getFieldErrors();
statusFromFieldError(httpStatus, statuses, fieldErrors);
} else if (exception instanceof MethodArgumentNotValidException) {
// Exception occurring when a user provided request body parameter that did not pass spring validation
MethodArgumentNotValidException notValidException = (MethodArgumentNotValidException) exception;
ObjectError globalError = notValidException.getBindingResult().getGlobalError();
if (globalError != null) {
Exception e = new Exception(globalError.getDefaultMessage());
statuses.add(ApiResponseFactory.createStatus(e, httpStatus));
if (exception instanceof BindException || exception instanceof MethodArgumentNotValidException) {
BindingResult bindingResult;
if (exception instanceof BindException) {
// Exception occurring when a user provided parameter that did not pass spring validation
bindingResult = (BindException) exception;
} else {
// Exception occurring when a user provided request body parameter that did not pass spring validation
bindingResult = ((MethodArgumentNotValidException) exception).getBindingResult();
}
List<FieldError> fieldErrors = notValidException.getBindingResult().getFieldErrors();
statusFromFieldError(httpStatus, statuses, fieldErrors);
List<ObjectError> errors = bindingResult.getAllErrors();
errors.stream()
// Convert to Exception
.map(err -> new Exception(err.getDefaultMessage()))
// Convert to BrapiStatus
.map(ex -> ApiResponseFactory.createStatus(ex, httpStatus))
.collect(Collectors.toCollection(() -> statuses));
} else {
// Other exceptions
......@@ -79,18 +84,6 @@ public class BrapiExceptionHandler {
return createErrorResponse(httpStatus, statuses, pagination);
}
/**
* Convert spring field errors into BrAPI statuses
*/
private static void statusFromFieldError(HttpStatus httpStatus, List<BrapiStatus> statuses, List<FieldError> fieldErrors) {
if (fieldErrors != null) {
for (FieldError error : fieldErrors) {
Exception e = new Exception(error.getDefaultMessage());
statuses.add(ApiResponseFactory.createStatus(e, httpStatus));
}
}
}
/**
* Generates Brapi not found response
*/
......
......@@ -44,11 +44,16 @@ class BrapiExceptionHandlerTest {
" \"totalPages\": 0,\n" +
" \"currentPage\": 0\n" +
" },\n" +
" \"status\": [{\n" +
" \"name\": \"Bad Request: Page size cannot be above 1000\",\n" +
" \"code\": \"400\"\n" +
" }],\n" +
" \"datafiles\": []\n" +
" \"status\": [" +
" {" +
" \"name\":\"Bad Request: Page size cannot be above 1000\"," +
" \"code\":\"400\"" +
" },{" +
" \"name\":\"Bad Request: The result window (page x pageSize) cannot be over 10000. Please use an export API to download all the requested data.\"," +
" \"code\":\"400\"" +
" }" +
" ]," +
" \"datafiles\": []\n" +
" },\n" +
" \"result\": null\n" +
"}"));
......
......@@ -10,7 +10,6 @@ import org.springframework.test.context.junit.jupiter.SpringExtension;
import org.springframework.test.web.servlet.MockMvc;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.is;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;
......@@ -38,11 +37,21 @@ class CallsControllerTest {
@Test
void should_Fail_Page_Overflow() throws Exception {
mockMvc.perform(get("/brapi/v1/calls?pageSize=50000")
mockMvc.perform(get("/brapi/v1/calls?pageSize=100&page=2")
.contentType(MediaType.APPLICATION_JSON_UTF8))
.andExpect(status().isBadRequest())
.andExpect(jsonPath("$.metadata.status", hasSize(1)))
.andExpect(jsonPath("$.metadata.status[0].code", is("400")));
.andExpect(content().json("{" +
"\"metadata\":{" +
"\"pagination\":{\"pageSize\":100,\"currentPage\":2,\"totalCount\":26,\"totalPages\":1}," +
"\"status\":[{" +
"\"name\":\"Bad Request: The current page should be strictly less than the total number of pages.\"," +
"\"code\":\"400\"" +
"}]," +
"\"datafiles\":[]" +
"}," +
"\"result\":{\"data\":null}" +
"}"
));
}
@Test
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment