diff --git a/lapis2/src/main/kotlin/org/genspectrum/lapis/auth/DataOpennessAuthorizationFilter.kt b/lapis2/src/main/kotlin/org/genspectrum/lapis/auth/DataOpennessAuthorizationFilter.kt index 994305ea7..64c9bf20f 100644 --- a/lapis2/src/main/kotlin/org/genspectrum/lapis/auth/DataOpennessAuthorizationFilter.kt +++ b/lapis2/src/main/kotlin/org/genspectrum/lapis/auth/DataOpennessAuthorizationFilter.kt @@ -12,13 +12,13 @@ import org.genspectrum.lapis.controller.ACCESS_KEY_PROPERTY import org.genspectrum.lapis.controller.AGGREGATED_ROUTE import org.genspectrum.lapis.controller.AMINO_ACID_INSERTIONS_ROUTE import org.genspectrum.lapis.controller.AMINO_ACID_MUTATIONS_ROUTE -import org.genspectrum.lapis.controller.LapisError import org.genspectrum.lapis.controller.LapisErrorResponse import org.genspectrum.lapis.controller.NUCLEOTIDE_INSERTIONS_ROUTE import org.genspectrum.lapis.controller.NUCLEOTIDE_MUTATIONS_ROUTE import org.genspectrum.lapis.util.CachedBodyHttpServletRequest import org.springframework.http.HttpStatus import org.springframework.http.MediaType +import org.springframework.http.ProblemDetail import org.springframework.stereotype.Component import org.springframework.web.filter.OncePerRequestFilter @@ -54,10 +54,10 @@ abstract class DataOpennessAuthorizationFilter(protected val objectMapper: Objec response.writer.write( objectMapper.writeValueAsString( LapisErrorResponse( - LapisError( - "Forbidden", - result.message, - ), + ProblemDetail.forStatus(HttpStatus.FORBIDDEN).also { + it.title = HttpStatus.FORBIDDEN.reasonPhrase + it.detail = result.message + }, ), ), ) diff --git a/lapis2/src/main/kotlin/org/genspectrum/lapis/controller/ExceptionHandler.kt b/lapis2/src/main/kotlin/org/genspectrum/lapis/controller/ExceptionHandler.kt index cd5d528a0..55e85efc6 100644 --- a/lapis2/src/main/kotlin/org/genspectrum/lapis/controller/ExceptionHandler.kt +++ b/lapis2/src/main/kotlin/org/genspectrum/lapis/controller/ExceptionHandler.kt @@ -4,7 +4,9 @@ import mu.KotlinLogging import org.genspectrum.lapis.model.SiloNotImplementedError import org.genspectrum.lapis.silo.SiloException import org.springframework.http.HttpStatus +import org.springframework.http.HttpStatusCode import org.springframework.http.MediaType +import org.springframework.http.ProblemDetail import org.springframework.http.ResponseEntity import org.springframework.web.bind.annotation.ControllerAdvice import org.springframework.web.bind.annotation.ExceptionHandler @@ -20,85 +22,60 @@ class ExceptionHandler : ResponseEntityExceptionHandler() { @ExceptionHandler(Throwable::class) @ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR) fun handleUnexpectedException(e: Throwable): ErrorResponse { - log.error(e) { "Caught unexpected exception: ${e.message}" } + log.warn(e) { "Caught unexpected exception: ${e.message}" } - return ResponseEntity - .status(HttpStatus.INTERNAL_SERVER_ERROR) - .contentType(MediaType.APPLICATION_JSON) - .body( - LapisErrorResponse( - LapisError( - "Unexpected error", - "${e.message}", - ), - ), - ) + return responseEntity(HttpStatus.INTERNAL_SERVER_ERROR, e.message) } @ExceptionHandler(IllegalArgumentException::class) @ResponseStatus(HttpStatus.BAD_REQUEST) fun handleIllegalArgumentException(e: IllegalArgumentException): ErrorResponse { - log.error(e) { "Caught IllegalArgumentException: ${e.message}" } + log.warn(e) { "Caught IllegalArgumentException: ${e.message}" } - return ResponseEntity - .status(HttpStatus.BAD_REQUEST) - .contentType(MediaType.APPLICATION_JSON) - .body( - LapisErrorResponse( - LapisError( - "Bad request", - "${e.message}", - ), - ), - ) + return responseEntity(HttpStatus.BAD_REQUEST, e.message) } @ExceptionHandler(AddForbiddenToOpenApiDocsHelper::class) @ResponseStatus(HttpStatus.FORBIDDEN) fun handleForbiddenException(e: AddForbiddenToOpenApiDocsHelper): ErrorResponse { - return ResponseEntity - .status(HttpStatus.FORBIDDEN) - .contentType(MediaType.APPLICATION_JSON) - .body( - LapisErrorResponse( - LapisError( - "Forbidden", - "${e.message}", - ), - ), - ) + return responseEntity(HttpStatus.FORBIDDEN, e.message) } @ExceptionHandler(SiloException::class) fun handleSiloException(e: SiloException): ErrorResponse { - log.error(e) { "Caught SiloException: ${e.statusCode} - ${e.message}" } + log.warn(e) { "Caught SiloException: ${e.statusCode} - ${e.message}" } - return ResponseEntity - .status(e.statusCode) - .contentType(MediaType.APPLICATION_JSON) - .body( - LapisErrorResponse( - LapisError( - e.title, - e.message, - ), - ), - ) + return responseEntity(e.statusCode, e.title, e.message) } @ExceptionHandler(SiloNotImplementedError::class) @ResponseStatus(HttpStatus.NOT_IMPLEMENTED) fun handleNotImplementedError(e: SiloNotImplementedError): ErrorResponse { - log.error(e) { "Caught SiloNotImplementedError: ${e.message}" } + log.warn(e) { "Caught SiloNotImplementedError: ${e.message}" } + + return responseEntity(HttpStatus.NOT_IMPLEMENTED, e.message) + } + + private fun responseEntity(httpStatus: HttpStatus, detail: String?) = + responseEntity(httpStatus, httpStatus.reasonPhrase, detail) + + private fun responseEntity(httpStatus: HttpStatusCode, title: String, detail: String?) = + responseEntity(httpStatus.value(), title, detail) + + private fun responseEntity( + httpStatus: Int, + title: String, + detail: String?, + ): ErrorResponse { return ResponseEntity - .status(HttpStatus.NOT_IMPLEMENTED) + .status(httpStatus) .contentType(MediaType.APPLICATION_JSON) .body( LapisErrorResponse( - LapisError( - "Not implemented", - "${e.message}", - ), + ProblemDetail.forStatus(httpStatus).also { + it.title = title + it.detail = detail + }, ), ) } @@ -106,5 +83,3 @@ class ExceptionHandler : ResponseEntityExceptionHandler() { /** This is not yet actually thrown, but makes "403 Forbidden" appear in OpenAPI docs. */ class AddForbiddenToOpenApiDocsHelper(message: String) : Exception(message) - -data class LapisError(val title: String, val message: String) diff --git a/lapis2/src/main/kotlin/org/genspectrum/lapis/controller/LapisResponse.kt b/lapis2/src/main/kotlin/org/genspectrum/lapis/controller/LapisResponse.kt index c8a06fbc5..7a9aa8b0a 100644 --- a/lapis2/src/main/kotlin/org/genspectrum/lapis/controller/LapisResponse.kt +++ b/lapis2/src/main/kotlin/org/genspectrum/lapis/controller/LapisResponse.kt @@ -1,5 +1,7 @@ package org.genspectrum.lapis.controller +import org.springframework.http.ProblemDetail + data class LapisResponse(val data: Data) -data class LapisErrorResponse(val error: LapisError) +data class LapisErrorResponse(val error: ProblemDetail) diff --git a/lapis2/src/test/kotlin/org/genspectrum/lapis/auth/ProtectedDataAuthorizationTest.kt b/lapis2/src/test/kotlin/org/genspectrum/lapis/auth/ProtectedDataAuthorizationTest.kt index a560a53e5..6c0b31444 100644 --- a/lapis2/src/test/kotlin/org/genspectrum/lapis/auth/ProtectedDataAuthorizationTest.kt +++ b/lapis2/src/test/kotlin/org/genspectrum/lapis/auth/ProtectedDataAuthorizationTest.kt @@ -23,7 +23,7 @@ private const val notAuthorizedToAccessEndpointError = """ { "error" : { "title": "Forbidden", - "message": "You are not authorized to access /aggregated." + "detail": "You are not authorized to access /aggregated." } } """ @@ -55,7 +55,7 @@ class ProtectedDataAuthorizationTest(@Autowired val mockMvc: MockMvc) { { "error" : { "title": "Forbidden", - "message": "An access key is required to access /aggregated." + "detail": "An access key is required to access /aggregated." } } """, @@ -74,7 +74,7 @@ class ProtectedDataAuthorizationTest(@Autowired val mockMvc: MockMvc) { { "error" : { "title": "Forbidden", - "message": "An access key is required to access /aggregated." + "detail": "An access key is required to access /aggregated." } } """, diff --git a/lapis2/src/test/kotlin/org/genspectrum/lapis/controller/ExceptionHandlerTest.kt b/lapis2/src/test/kotlin/org/genspectrum/lapis/controller/ExceptionHandlerTest.kt index 71464962e..31768346f 100644 --- a/lapis2/src/test/kotlin/org/genspectrum/lapis/controller/ExceptionHandlerTest.kt +++ b/lapis2/src/test/kotlin/org/genspectrum/lapis/controller/ExceptionHandlerTest.kt @@ -55,8 +55,8 @@ class ExceptionHandlerTest(@Autowired val mockMvc: MockMvc) { """ { "error": { - "title": "Unexpected error", - "message": "SomeMessage" + "title": "Internal Server Error", + "detail": "SomeMessage" } } """, @@ -66,7 +66,7 @@ class ExceptionHandlerTest(@Autowired val mockMvc: MockMvc) { @Test fun `Passes through exception with status code from SILO`() { - every { validControllerCall() } throws SiloException(123, "SomeTitle", "SomeMessage",) + every { validControllerCall() } throws SiloException(123, "SomeTitle", "SomeMessage") mockMvc.perform(get(validRoute)) .andExpect(status().`is`(123)) @@ -77,7 +77,7 @@ class ExceptionHandlerTest(@Autowired val mockMvc: MockMvc) { { "error": { "title": "SomeTitle", - "message": "SomeMessage" + "detail": "SomeMessage" } } """, @@ -97,8 +97,8 @@ class ExceptionHandlerTest(@Autowired val mockMvc: MockMvc) { """ { "error": { - "title": "Bad request", - "message": "SomeMessage" + "title": "Bad Request", + "detail": "SomeMessage" } } """, @@ -118,8 +118,8 @@ class ExceptionHandlerTest(@Autowired val mockMvc: MockMvc) { """ { "error": { - "title": "Not implemented", - "message": "SomeMessage" + "title": "Not Implemented", + "detail": "SomeMessage" } } """, diff --git a/lapis2/src/test/kotlin/org/genspectrum/lapis/controller/LapisControllerCommonFieldsTest.kt b/lapis2/src/test/kotlin/org/genspectrum/lapis/controller/LapisControllerCommonFieldsTest.kt index d0bf09384..188871592 100644 --- a/lapis2/src/test/kotlin/org/genspectrum/lapis/controller/LapisControllerCommonFieldsTest.kt +++ b/lapis2/src/test/kotlin/org/genspectrum/lapis/controller/LapisControllerCommonFieldsTest.kt @@ -149,7 +149,7 @@ class LapisControllerCommonFieldsTest( mockMvc.perform(request) .andExpect(status().isBadRequest) - .andExpect(jsonPath("\$.error.message").value("orderByField must have a string property \"field\"")) + .andExpect(jsonPath("\$.error.detail").value("orderByField must have a string property \"field\"")) } @Test @@ -209,7 +209,7 @@ class LapisControllerCommonFieldsTest( mockMvc.perform(request) .andExpect(status().isBadRequest) - .andExpect(jsonPath("\$.error.message").value("limit must be a number or null")) + .andExpect(jsonPath("\$.error.detail").value("limit must be a number or null")) } @Test @@ -271,7 +271,7 @@ class LapisControllerCommonFieldsTest( mockMvc.perform(request) .andExpect(status().isBadRequest) - .andExpect(jsonPath("\$.error.message").value("offset must be a number or null")) + .andExpect(jsonPath("\$.error.detail").value("offset must be a number or null")) } @Test diff --git a/lapis2/src/test/kotlin/org/genspectrum/lapis/controller/LapisControllerTest.kt b/lapis2/src/test/kotlin/org/genspectrum/lapis/controller/LapisControllerTest.kt index e65fd7a2e..f153a67c0 100644 --- a/lapis2/src/test/kotlin/org/genspectrum/lapis/controller/LapisControllerTest.kt +++ b/lapis2/src/test/kotlin/org/genspectrum/lapis/controller/LapisControllerTest.kt @@ -234,9 +234,9 @@ class LapisControllerTest(@Autowired val mockMvc: MockMvc) { mockMvc.perform(request) .andExpect(status().isBadRequest) - .andExpect(jsonPath("\$.error.title").value("Bad request")) + .andExpect(jsonPath("\$.error.title").value("Bad Request")) .andExpect( - jsonPath("\$.error.message").value("minProportion must be a number"), + jsonPath("\$.error.detail").value("minProportion must be a number"), ) }