fix: apply remarks from review

Author: juherr

steve-api/src/main/java/de/rwth/idsg/steve/web/api/ApiControllerAdvice.java

@@ -19,8 +19,6 @@
 package de.rwth.idsg.steve.web.api;
 
 import de.rwth.idsg.steve.SteveException;
-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.jspecify.annotations.Nullable;
@@ -59,9 +57,9 @@ public ApiErrorResponse handleBindException(HttpServletRequest req, BindExceptio
         return createResponse(url, HttpStatus.BAD_REQUEST, "Error understanding the request");
     }
 
-    @ExceptionHandler(NotFoundException.class)
+    @ExceptionHandler(SteveException.NotFound.class)
     @ResponseStatus(value = HttpStatus.NOT_FOUND)
-    public ApiErrorResponse handleNotFoundException(HttpServletRequest req, NotFoundException exception) {
+    public ApiErrorResponse handleNotFoundException(HttpServletRequest req, SteveException.NotFound exception) {
         String url = req.getRequestURL().toString();
         log.error("Request: {} raised following exception.", url, exception);
         return createResponse(url, HttpStatus.NOT_FOUND, exception.getMessage());
@@ -76,9 +74,9 @@ public ApiErrorResponse handleAlreadyExistsException(
         return createResponse(url, HttpStatus.UNPROCESSABLE_ENTITY, exception.getMessage());
     }
 
-    @ExceptionHandler(BadRequestException.class)
+    @ExceptionHandler(SteveException.BadRequest.class)
     @ResponseStatus(value = HttpStatus.BAD_REQUEST)
-    public ApiErrorResponse handleBadRequestException(HttpServletRequest req, BadRequestException exception) {
+    public ApiErrorResponse handleBadRequestException(HttpServletRequest req, SteveException.BadRequest exception) {
         String url = req.getRequestURL().toString();
         log.error("Request: {} raised following exception.", url, exception);
         return createResponse(url, HttpStatus.BAD_REQUEST, exception.getMessage());

steve-api/src/main/java/de/rwth/idsg/steve/web/api/OcppTagsRestController.java

@@ -18,10 +18,10 @@
  */
 package de.rwth.idsg.steve.web.api;
 
+import de.rwth.idsg.steve.SteveException;
 import de.rwth.idsg.steve.repository.dto.OcppTag.OcppTagOverview;
 import de.rwth.idsg.steve.service.OcppTagsService;
 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.OcppTagQueryFormForApi;
 import io.swagger.v3.oas.annotations.Operation;
@@ -186,7 +186,7 @@ private OcppTagOverview getOneInternal(int ocppTagPk) {
 
         var results = ocppTagsService.getOverview(params);
         if (results.isEmpty()) {
-            throw new NotFoundException("Could not find ocppTag with id " + ocppTagPk);
+            throw new SteveException.NotFound("Could not find ocppTag with id " + ocppTagPk);
         }
         return results.get(0);
     }

steve-api/src/main/java/de/rwth/idsg/steve/web/api/RemoteCommandsRestController.java

@@ -18,6 +18,7 @@
  */
 package de.rwth.idsg.steve.web.api;
 
+import de.rwth.idsg.steve.SteveException;
 import de.rwth.idsg.steve.ocpp.OcppVersion;
 import de.rwth.idsg.steve.repository.ChargePointRepository;
 import de.rwth.idsg.steve.repository.TaskStore;
@@ -29,7 +30,6 @@
 import de.rwth.idsg.steve.web.api.dto.ApiChargePointStart;
 import de.rwth.idsg.steve.web.api.dto.ApiChargePointStop;
 import de.rwth.idsg.steve.web.api.dto.ApiChargePointUnlock;
-import de.rwth.idsg.steve.web.api.exception.BadRequestException;
 import de.rwth.idsg.steve.web.dto.ocpp.RemoteStartTransactionParams;
 import de.rwth.idsg.steve.web.dto.ocpp.RemoteStopTransactionParams;
 import de.rwth.idsg.steve.web.dto.ocpp.UnlockConnectorParams;
@@ -143,7 +143,7 @@ public Integer postRemoteStartTx(@Valid ApiChargePointStart params) {
         // only one active task per charge box over api; to be discussed!
         if (activeTaskOnChargeBox(params.getChargeBoxId())) {
             var errMsg = String.format("Active task found on ChargeBox %s!", params.getChargeBoxId());
-            throw new BadRequestException(errMsg);
+            throw new SteveException.BadRequest(errMsg);
         }
 
         var transactionParams = new RemoteStartTransactionParams();
@@ -156,15 +156,15 @@ public Integer postRemoteStartTx(@Valid ApiChargePointStart params) {
                 var errMsg = String.format(
                         "Active transaction found for connector %s at ChargeBox %s!",
                         params.getConnectorId(), params.getChargeBoxId());
-                throw new BadRequestException(errMsg);
+                throw new SteveException.BadRequest(errMsg);
             }
         }
         transactionParams.setConnectorId(params.getConnectorId());
 
         var chargePoint = chargePointRepository
                 .getChargePointSelect(params.getChargeBoxId())
-                .orElseThrow(() ->
-                        new BadRequestException(String.format("ChargeBox %s not found!", params.getChargeBoxId())));
+                .orElseThrow(() -> new SteveException.BadRequest(
+                        String.format("ChargeBox %s not found!", params.getChargeBoxId())));
         transactionParams.setChargePointSelectList(Collections.singletonList(chargePoint));
 
         // Check if OCPP-Tag is allowed to use the connector? To be discussed and t.b.d.!
@@ -179,32 +179,33 @@ public Integer postRemoteStopTx(@Valid ApiChargePointStop params) {
         // only one active task per charge box over api; to be discussed!
         if (activeTaskOnChargeBox(params.getChargeBoxId())) {
             var errMsg = String.format("Active task found on ChargeBox %s!", params.getChargeBoxId());
-            throw new BadRequestException(errMsg);
+            throw new SteveException.BadRequest(errMsg);
         }
 
         var transactionParams = new RemoteStopTransactionParams();
 
         // set the ChargPointSelectionList, ensure the length is exactly one
         var chargePoint = chargePointRepository
                 .getChargePointSelect(params.getChargeBoxId())
-                .orElseThrow(() ->
-                        new BadRequestException(String.format("ChargeBox %s not found!", params.getChargeBoxId())));
+                .orElseThrow(() -> new SteveException.BadRequest(
+                        String.format("ChargeBox %s not found!", params.getChargeBoxId())));
         transactionParams.setChargePointSelectList(Collections.singletonList(chargePoint));
 
         // Get the transactionId of the active transaction on the connector.
         // If no transaction active don't send RemoteStop
         var transactionId = transactionRepository
                 .getActiveTransactionId(params.getChargeBoxId(), params.getConnectorId())
-                .orElseThrow(() -> new BadRequestException(String.format(
+                .orElseThrow(() -> new SteveException.BadRequest(String.format(
                         "No active transaction found for connector %s at ChargeBox %s!",
                         params.getConnectorId(), params.getChargeBoxId())));
         // check the user is allowed to stop this transaction (actual only the one who started it!)
         var ocppTag = transactionRepository
                 .getTransaction(transactionId)
-                .orElseThrow(() -> new BadRequestException(String.format("Transaction %s not found!", transactionId)))
+                .orElseThrow(
+                        () -> new SteveException.BadRequest(String.format("Transaction %s not found!", transactionId)))
                 .getOcppIdTag();
         if (!ocppTag.contentEquals(params.getOcppTag())) {
-            throw new BadRequestException("The transaction was authorised with another OCPP Tag!");
+            throw new SteveException.BadRequest("The transaction was authorised with another OCPP Tag!");
         }
         transactionParams.setTransactionId(transactionId);
 
@@ -217,14 +218,14 @@ public Integer postUnlockCon(@Valid ApiChargePointUnlock params) {
         // only one active task per charge box over api; to be discussed!
         if (activeTaskOnChargeBox(params.getChargeBoxId())) {
             var errMsg = String.format("Active task found on ChargeBox %s!", params.getChargeBoxId());
-            throw new BadRequestException(errMsg);
+            throw new SteveException.BadRequest(errMsg);
         }
 
         var transactionParams = new UnlockConnectorParams();
         var chargePoint = chargePointRepository
                 .getChargePointSelect(params.getChargeBoxId())
-                .orElseThrow(() ->
-                        new BadRequestException(String.format("ChargeBox %s not found!", params.getChargeBoxId())));
+                .orElseThrow(() -> new SteveException.BadRequest(
+                        String.format("ChargeBox %s not found!", params.getChargeBoxId())));
         transactionParams.setChargePointSelectList(Collections.singletonList(chargePoint));
 
         /* If a active transaction is found, don't unlock the connection. */
@@ -234,7 +235,7 @@ public Integer postUnlockCon(@Valid ApiChargePointUnlock params) {
             var errMsg = String.format(
                     "Active transaction found for connector %s at ChargeBox %s!",
                     params.getConnectorId(), params.getChargeBoxId());
-            throw new BadRequestException(errMsg);
+            throw new SteveException.BadRequest(errMsg);
         }
 
         transactionParams.setConnectorId(params.getConnectorId());

steve-api/src/main/java/de/rwth/idsg/steve/web/api/ReservationsRestController.java

@@ -21,7 +21,6 @@
 import de.rwth.idsg.steve.SteveException;
 import de.rwth.idsg.steve.repository.dto.Reservation;
 import de.rwth.idsg.steve.service.ReservationsService;
-import de.rwth.idsg.steve.web.api.exception.NotFoundException;
 import de.rwth.idsg.steve.web.dto.ReservationForm;
 import de.rwth.idsg.steve.web.dto.ReservationQueryForm;
 import io.swagger.v3.oas.annotations.tags.Tag;
@@ -59,8 +58,8 @@ public ResponseEntity<Reservation> addReservation(@Valid @RequestBody Reservatio
         var id = reservationsService.addReservation(form);
         var body = reservationsService
                 .getReservation(id)
-                .orElseThrow(
-                        () -> new SteveException("Reservation not found after creation, this should never happen"));
+                .orElseThrow(() -> new SteveException.InternalError(
+                        "Reservation not found after creation, this should never happen"));
         var location = ServletUriComponentsBuilder.fromCurrentRequest()
                 .path("/{id}")
                 .buildAndExpand(id)
@@ -72,7 +71,7 @@ public ResponseEntity<Reservation> addReservation(@Valid @RequestBody Reservatio
     public Reservation deleteReservation(@PathVariable int id) {
         var reservation = reservationsService
                 .getReservation(id)
-                .orElseThrow(() -> new NotFoundException(String.format("Reservation with id %d not found", id)));
+                .orElseThrow(() -> new SteveException.NotFound("Reservation with id %d not found", id));
         reservationsService.deleteReservation(id);
         return reservation;
     }

steve-api/src/main/java/de/rwth/idsg/steve/web/api/TasksRestController.java

@@ -18,10 +18,10 @@
  */
 package de.rwth.idsg.steve.web.api;
 
+import de.rwth.idsg.steve.SteveException;
 import de.rwth.idsg.steve.repository.TaskStore;
 import de.rwth.idsg.steve.web.api.dto.ApiTaskInfo;
 import de.rwth.idsg.steve.web.api.dto.ApiTaskList;
-import de.rwth.idsg.steve.web.api.exception.NotFoundException;
 import io.swagger.v3.oas.annotations.tags.Tag;
 import lombok.RequiredArgsConstructor;
 import org.springframework.http.MediaType;
@@ -81,7 +81,7 @@ public ApiTaskList clearFinished() {
     public ApiTaskInfo getTaskDetails(@RequestParam(name = "id") @Valid @NotNull @Positive Integer taskId) {
         var task = taskStore.get(taskId);
         if (task == null) {
-            throw new NotFoundException("Task not found: " + taskId);
+            throw new SteveException.NotFound("Task not found: " + taskId);
         }
         return new ApiTaskInfo(taskId, task);
     }

steve-api/src/main/java/de/rwth/idsg/steve/web/api/TransactionsRestController.java

@@ -18,9 +18,9 @@
  */
 package de.rwth.idsg.steve.web.api;
 
+import de.rwth.idsg.steve.SteveException;
 import de.rwth.idsg.steve.repository.TransactionRepository;
 import de.rwth.idsg.steve.repository.dto.Transaction;
-import de.rwth.idsg.steve.web.api.exception.BadRequestException;
 import de.rwth.idsg.steve.web.dto.TransactionQueryForm;
 import io.swagger.v3.oas.annotations.Operation;
 import io.swagger.v3.oas.annotations.tags.Tag;
@@ -62,7 +62,7 @@ public class TransactionsRestController {
     @GetMapping(value = "")
     public List<Transaction> get(@Valid @ParameterObject TransactionQueryForm.TransactionQueryFormForApi params) {
         if (params.isReturnCSV()) {
-            throw new BadRequestException("returnCSV=true is not supported for API calls");
+            throw new SteveException.BadRequest("returnCSV=true is not supported for API calls");
         }
 
         return transactionRepository.getTransactions(params);

steve-api/src/main/java/de/rwth/idsg/steve/web/api/exception/BadRequestException.java

@@ -18,9 +18,9 @@
  */
 package de.rwth.idsg.steve.web.api;
 
+import de.rwth.idsg.steve.SteveException;
 import de.rwth.idsg.steve.repository.dto.ChargePoint;
 import de.rwth.idsg.steve.service.ChargePointsService;
-import de.rwth.idsg.steve.web.api.exception.NotFoundException;
 import de.rwth.idsg.steve.web.dto.ChargePointForm;
 import jooq.steve.db.tables.records.ChargeBoxRecord;
 import org.junit.jupiter.api.BeforeEach;
@@ -89,7 +89,7 @@ public void testGet_withOneResult() {
     @Test
     @DisplayName("GET one: Entity not found, expected 404")
     public void testGetOne_notFound() {
-        when(chargePointsService.getDetails(anyInt())).thenThrow(new NotFoundException(""));
+        when(chargePointsService.getDetails(anyInt())).thenThrow(new SteveException.NotFound(""));
 
         assertThat(mockMvc.perform(get("/api/v1/chargeboxes/1"))).hasStatus(HttpStatus.NOT_FOUND);
     }

steve-api/src/main/java/de/rwth/idsg/steve/web/api/exception/NotFoundException.java

@@ -361,7 +361,9 @@ public void test14() throws Exception {
         form.setNote("note-1");
 
         // when
-        doThrow(new SteveException("failed")).when(ocppTagsService).updateOcppTag(any());
+        doThrow(new SteveException.InternalError("failed"))
+                .when(ocppTagsService)
+                .updateOcppTag(any());
 
         // then
         assertThat(mockMvc.perform(put("/api/v1/ocppTags/" + ocppTagPk)
@@ -411,7 +413,9 @@ public void test16() throws Exception {
 
         // when
         when(ocppTagsService.getOverview(any())).thenReturn(List.of(result));
-        doThrow(new SteveException("failed")).when(ocppTagsService).deleteOcppTag(ocppTagPk);
+        doThrow(new SteveException.InternalError("failed"))
+                .when(ocppTagsService)
+                .deleteOcppTag(ocppTagPk);
 
         // then
         assertThat(mockMvc.perform(delete("/api/v1/ocppTags/" + ocppTagPk)))