refactor (#910)

steve-community · Sep 30, 2022 · 6a69805 · 6a69805
1 parent d0ded00
commit 6a69805
Show file tree

Hide file tree

Showing 7 changed files with 58 additions and 44 deletions.
diff --git a/src/main/java/de/rwth/idsg/steve/web/api/ApiControllerAdvice.java b/src/main/java/de/rwth/idsg/steve/web/api/ApiControllerAdvice.java
@@ -19,6 +19,8 @@
 package de.rwth.idsg.steve.web.api;
 
 import de.rwth.idsg.steve.web.LocalDateTimeEditor;
+import de.rwth.idsg.steve.web.api.exception.BadRequestException;
+import de.rwth.idsg.steve.web.api.exception.NotFoundException;
 import lombok.Data;
 import lombok.extern.slf4j.Slf4j;
 import org.joda.time.DateTime;
@@ -27,77 +29,80 @@
 import org.springframework.http.HttpStatus;
 import org.springframework.validation.BindException;
 import org.springframework.web.bind.WebDataBinder;
-import org.springframework.web.bind.annotation.ControllerAdvice;
 import org.springframework.web.bind.annotation.ExceptionHandler;
 import org.springframework.web.bind.annotation.InitBinder;
+import org.springframework.web.bind.annotation.ResponseStatus;
+import org.springframework.web.bind.annotation.RestControllerAdvice;
 import org.springframework.web.method.annotation.MethodArgumentTypeMismatchException;
-import org.springframework.web.server.ResponseStatusException;
-import org.springframework.web.servlet.ModelAndView;
-import org.springframework.web.servlet.view.json.MappingJackson2JsonView;
 
 import javax.servlet.http.HttpServletRequest;
 
 /**
  * @author Sevket Goekay <sevketgokay@gmail.com>
  * @since 13.09.2022
  */
-@ControllerAdvice(basePackages = "de.rwth.idsg.steve.web.api")
+@RestControllerAdvice(basePackages = "de.rwth.idsg.steve.web.api")
 @Slf4j
 public class ApiControllerAdvice {
 
-    private final MappingJackson2JsonView jsonView = new MappingJackson2JsonView();
-
     @InitBinder
     public void binder(WebDataBinder binder) {
         binder.registerCustomEditor(String.class, new StringTrimmerEditor(true));
         binder.registerCustomEditor(LocalDateTime.class, new LocalDateTimeEditor());
     }
 
     @ExceptionHandler(BindException.class)
-    public ModelAndView handleBindException(HttpServletRequest req, BindException exception) {
-        StringBuffer url = req.getRequestURL();
+    @ResponseStatus(value = HttpStatus.BAD_REQUEST)
+    public ApiErrorResponse handleBindException(HttpServletRequest req, BindException exception) {
+        String url = req.getRequestURL().toString();
         log.error("Request: {} raised following exception.", url, exception);
         return createResponse(url, HttpStatus.BAD_REQUEST, "Error understanding the request");
     }
 
-    @ExceptionHandler(ResponseStatusException.class)
-    public ModelAndView handleResponseStatusException(HttpServletRequest req, ResponseStatusException exception) {
-        StringBuffer url = req.getRequestURL();
+    @ExceptionHandler(NotFoundException.class)
+    @ResponseStatus(value = HttpStatus.NOT_FOUND)
+    public ApiErrorResponse handleNotFoundException(HttpServletRequest req, NotFoundException exception) {
+        String url = req.getRequestURL().toString();
         log.error("Request: {} raised following exception.", url, exception);
-        return createResponse(url, exception.getStatus(), exception.getReason());
+        return createResponse(url, HttpStatus.NOT_FOUND, exception.getMessage());
+    }
+
+    @ExceptionHandler(BadRequestException.class)
+    @ResponseStatus(value = HttpStatus.BAD_REQUEST)
+    public ApiErrorResponse handleBadRequestException(HttpServletRequest req, BadRequestException exception) {
+        String url = req.getRequestURL().toString();
+        log.error("Request: {} raised following exception.", url, exception);
+        return createResponse(url, HttpStatus.BAD_REQUEST, exception.getMessage());
     }
 
     @ExceptionHandler(MethodArgumentTypeMismatchException.class)
-    public ModelAndView handleMethodArgumentTypeMismatchException(HttpServletRequest req, MethodArgumentTypeMismatchException exception) {
-        StringBuffer url = req.getRequestURL();
+    @ResponseStatus(value = HttpStatus.BAD_REQUEST)
+    public ApiErrorResponse handleMethodArgumentTypeMismatchException(HttpServletRequest req, MethodArgumentTypeMismatchException exception) {
+        String url = req.getRequestURL().toString();
         log.error("Request: {} raised following exception.", url, exception);
         return createResponse(url, HttpStatus.BAD_REQUEST, exception.getMessage());
     }
 
     @ExceptionHandler(Exception.class)
-    public ModelAndView handleException(HttpServletRequest req, Exception exception) {
-        StringBuffer url = req.getRequestURL();
+    @ResponseStatus(value = HttpStatus.INTERNAL_SERVER_ERROR)
+    public ApiErrorResponse handleException(HttpServletRequest req, Exception exception) {
+        String url = req.getRequestURL().toString();
         log.error("Request: {} raised following exception.", url, exception);
         return createResponse(url, HttpStatus.INTERNAL_SERVER_ERROR, exception.getMessage());
     }
 
-    private ModelAndView createResponse(StringBuffer url, HttpStatus status, String message) {
-        ModelAndView result = new ModelAndView(jsonView);
-        result.setStatus(status);
+    public static ApiErrorResponse createResponse(String url, HttpStatus status, String message) {
+        ApiErrorResponse result = new ApiErrorResponse();
 
-        result.addObject("timestamp", DateTime.now().toString());
-        result.addObject("status", status.value());
-        result.addObject("error", status.getReasonPhrase());
-        result.addObject("message", message);
-        result.addObject("path", url);
+        result.setTimestamp(DateTime.now().toString());
+        result.setStatus(status.value());
+        result.setError(status.getReasonPhrase());
+        result.setMessage(message);
+        result.setPath(url);
 
         return result;
     }
 
-    /**
-     * This is just here to be used by Swagger and for documentation purposes. It mirrors the fields used in
-     * {@link ApiControllerAdvice#createResponse(StringBuffer, HttpStatus, String)}
-     */
     @Data
     public static class ApiErrorResponse {
         private String timestamp;

diff --git a/src/main/java/de/rwth/idsg/steve/web/api/OcppTagsRestController.java b/src/main/java/de/rwth/idsg/steve/web/api/OcppTagsRestController.java
@@ -22,6 +22,7 @@
 import de.rwth.idsg.steve.repository.dto.OcppTag;
 import de.rwth.idsg.steve.service.OcppTagService;
 import de.rwth.idsg.steve.web.api.ApiControllerAdvice.ApiErrorResponse;
+import de.rwth.idsg.steve.web.api.exception.NotFoundException;
 import de.rwth.idsg.steve.web.dto.OcppTagForm;
 import de.rwth.idsg.steve.web.dto.OcppTagQueryForm;
 import io.swagger.annotations.ApiResponse;
@@ -40,7 +41,6 @@
 import org.springframework.web.bind.annotation.ResponseBody;
 import org.springframework.web.bind.annotation.ResponseStatus;
 import org.springframework.web.bind.annotation.RestController;
-import org.springframework.web.server.ResponseStatusException;
 
 import javax.validation.Valid;
 import java.util.Collections;
@@ -52,7 +52,7 @@
  */
 @Slf4j
 @RestController
-@RequestMapping(value = "/api/v1/ocppTags", produces = MediaType.APPLICATION_JSON_UTF8_VALUE)
+@RequestMapping(value = "/api/v1/ocppTags", produces = MediaType.APPLICATION_JSON_VALUE)
 @RequiredArgsConstructor
 public class OcppTagsRestController {
 
@@ -63,7 +63,6 @@ public class OcppTagsRestController {
         @ApiResponse(code = 200, message = "OK"),
         @ApiResponse(code = 400, message = "Bad Request", response = ApiErrorResponse.class),
         @ApiResponse(code = 401, message = "Unauthorized", response = ApiErrorResponse.class),
-        @ApiResponse(code = 403, message = "Forbidden", response = ApiErrorResponse.class),
         @ApiResponse(code = 500, message = "Internal Server Error", response = ApiErrorResponse.class)}
     )
     @GetMapping(value = "")
@@ -80,7 +79,6 @@ public List<OcppTag.Overview> get(OcppTagQueryForm params) {
         @ApiResponse(code = 200, message = "OK"),
         @ApiResponse(code = 400, message = "Bad Request", response = ApiErrorResponse.class),
         @ApiResponse(code = 401, message = "Unauthorized", response = ApiErrorResponse.class),
-        @ApiResponse(code = 403, message = "Forbidden", response = ApiErrorResponse.class),
         @ApiResponse(code = 404, message = "Not Found", response = ApiErrorResponse.class),
         @ApiResponse(code = 500, message = "Internal Server Error", response = ApiErrorResponse.class)}
     )
@@ -98,7 +96,6 @@ public OcppTag.Overview getOne(@PathVariable("ocppTagPk") Integer ocppTagPk) {
         @ApiResponse(code = 201, message = "Created"),
         @ApiResponse(code = 400, message = "Bad Request", response = ApiErrorResponse.class),
         @ApiResponse(code = 401, message = "Unauthorized", response = ApiErrorResponse.class),
-        @ApiResponse(code = 403, message = "Forbidden", response = ApiErrorResponse.class),
         @ApiResponse(code = 404, message = "Not Found", response = ApiErrorResponse.class),
         @ApiResponse(code = 500, message = "Internal Server Error", response = ApiErrorResponse.class)}
     )
@@ -120,7 +117,6 @@ public OcppTag.Overview create(@RequestBody @Valid OcppTagForm params) {
         @ApiResponse(code = 200, message = "OK"),
         @ApiResponse(code = 400, message = "Bad Request", response = ApiErrorResponse.class),
         @ApiResponse(code = 401, message = "Unauthorized", response = ApiErrorResponse.class),
-        @ApiResponse(code = 403, message = "Forbidden", response = ApiErrorResponse.class),
         @ApiResponse(code = 404, message = "Not Found", response = ApiErrorResponse.class),
         @ApiResponse(code = 500, message = "Internal Server Error", response = ApiErrorResponse.class)}
     )
@@ -141,7 +137,6 @@ public OcppTag.Overview update(@PathVariable("ocppTagPk") Integer ocppTagPk, @Re
         @ApiResponse(code = 200, message = "OK"),
         @ApiResponse(code = 400, message = "Bad Request", response = ApiErrorResponse.class),
         @ApiResponse(code = 401, message = "Unauthorized", response = ApiErrorResponse.class),
-        @ApiResponse(code = 403, message = "Forbidden", response = ApiErrorResponse.class),
         @ApiResponse(code = 404, message = "Not Found", response = ApiErrorResponse.class),
         @ApiResponse(code = 500, message = "Internal Server Error", response = ApiErrorResponse.class)}
     )
@@ -163,7 +158,7 @@ private OcppTag.Overview getOneInternal(int ocppTagPk) {
 
         List<OcppTag.Overview> results = ocppTagRepository.getOverview(params);
         if (results.isEmpty()) {
-            throw new ResponseStatusException(HttpStatus.NOT_FOUND, "Could not find this ocppTag");
+            throw new NotFoundException("Could not find this ocppTag");
         }
         return results.get(0);
     }

diff --git a/src/main/java/de/rwth/idsg/steve/web/api/TransactionsRestController.java b/src/main/java/de/rwth/idsg/steve/web/api/TransactionsRestController.java
@@ -21,18 +21,17 @@
 import de.rwth.idsg.steve.repository.TransactionRepository;
 import de.rwth.idsg.steve.repository.dto.Transaction;
 import de.rwth.idsg.steve.web.api.ApiControllerAdvice.ApiErrorResponse;
+import de.rwth.idsg.steve.web.api.exception.BadRequestException;
 import de.rwth.idsg.steve.web.dto.TransactionQueryForm;
 import io.swagger.annotations.ApiResponse;
 import io.swagger.annotations.ApiResponses;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
-import org.springframework.http.HttpStatus;
 import org.springframework.http.MediaType;
 import org.springframework.web.bind.annotation.GetMapping;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.ResponseBody;
 import org.springframework.web.bind.annotation.RestController;
-import org.springframework.web.server.ResponseStatusException;
 
 import javax.validation.Valid;
 import java.util.List;
@@ -43,7 +42,7 @@
  */
 @Slf4j
 @RestController
-@RequestMapping(value = "/api/v1/transactions", produces = MediaType.APPLICATION_JSON_UTF8_VALUE)
+@RequestMapping(value = "/api/v1/transactions", produces = MediaType.APPLICATION_JSON_VALUE)
 @RequiredArgsConstructor
 public class TransactionsRestController {
 
@@ -53,7 +52,6 @@ public class TransactionsRestController {
         @ApiResponse(code = 200, message = "OK"),
         @ApiResponse(code = 400, message = "Bad Request", response = ApiErrorResponse.class),
         @ApiResponse(code = 401, message = "Unauthorized", response = ApiErrorResponse.class),
-        @ApiResponse(code = 403, message = "Forbidden", response = ApiErrorResponse.class),
         @ApiResponse(code = 500, message = "Internal Server Error", response = ApiErrorResponse.class)}
     )
     @GetMapping(value = "")
@@ -62,7 +60,7 @@ public List<Transaction> get(@Valid TransactionQueryForm params) {
         log.debug("Read request for query: {}", params);
 
         if (params.isReturnCSV()) {
-            throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "returnCSV=true is not supported for API calls");
+            throw new BadRequestException("returnCSV=true is not supported for API calls");
         }
 
         var response = transactionRepository.getTransactions(params);

diff --git a/src/main/java/de/rwth/idsg/steve/web/api/exception/BadRequestException.java b/src/main/java/de/rwth/idsg/steve/web/api/exception/BadRequestException.java
@@ -0,0 +1,8 @@
+package de.rwth.idsg.steve.web.api.exception;
+
+public class BadRequestException extends RuntimeException {
+
+    public BadRequestException(String message) {
+        super(message);
+    }
+}
diff --git a/src/main/java/de/rwth/idsg/steve/web/api/exception/NotFoundException.java b/src/main/java/de/rwth/idsg/steve/web/api/exception/NotFoundException.java
@@ -0,0 +1,8 @@
+package de.rwth.idsg.steve.web.api.exception;
+
+public class NotFoundException extends RuntimeException {
+
+    public NotFoundException(String message) {
+        super(message);
+    }
+}
diff --git a/src/test/java/de/rwth/idsg/steve/web/api/OcppTagsRestControllerTest.java b/src/test/java/de/rwth/idsg/steve/web/api/OcppTagsRestControllerTest.java
@@ -49,7 +49,7 @@
 @ExtendWith(MockitoExtension.class)
 public class OcppTagsRestControllerTest {
 
-    private static final String CONTENT_TYPE = "application/json;charset=UTF-8";
+    private static final String CONTENT_TYPE = "application/json";
 
     private final ObjectMapper objectMapper = Jackson2ObjectMapperBuilder.json().build();
 

diff --git a/src/test/java/de/rwth/idsg/steve/web/api/TransactionRestControllerTest.java b/src/test/java/de/rwth/idsg/steve/web/api/TransactionRestControllerTest.java
@@ -41,7 +41,7 @@ public void setup() {
         mockMvc = MockMvcBuilders.standaloneSetup(new TransactionsRestController(transactionRepository))
             .setControllerAdvice(new ApiControllerAdvice())
             .setMessageConverters(new MappingJackson2HttpMessageConverter())
-            .alwaysExpect(content().contentType("application/json;charset=UTF-8"))
+            .alwaysExpect(content().contentType("application/json"))
             .build();
     }