chore: refactor JacksonBodyHandler to remove unchecked operation

src/main/java/dev/tomr/hcloud/http/HetznerCloudHttpClient.java

@@ -3,6 +3,8 @@
 import dev.tomr.hcloud.HetznerCloud;
 import dev.tomr.hcloud.http.exception.HetznerApiException;
 import dev.tomr.hcloud.http.model.HetznerErrorResponse;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
 import java.net.URI;
@@ -15,6 +17,7 @@
 
 public class HetznerCloudHttpClient {
 
+    private static final Logger log = LoggerFactory.getLogger(HetznerCloudHttpClient.class);
     private static HetznerCloudHttpClient instance;
 
     private final HttpClient httpClient;
@@ -49,7 +52,7 @@ public static HetznerCloudHttpClient getInstance() {
      * @throws IOException Exception passed from HTTP client
      * @throws InterruptedException Exception passed from HTTP client
      */
-    public <T extends HetznerJsonObject> T sendHttpRequest(Class<T> clazz, String endpoint, RequestVerb requestVerb, String apiKey) throws IOException, InterruptedException {
+    public <T extends HetznerJsonObject> T sendHttpRequest(Class<T> clazz, String endpoint, RequestVerb requestVerb, String apiKey) throws IOException, InterruptedException, IllegalAccessException {
         return sendHttpRequest(clazz, endpoint, requestVerb, apiKey, null);
     }
 
@@ -65,16 +68,20 @@ public <T extends HetznerJsonObject> T sendHttpRequest(Class<T> clazz, String en
      * @throws IOException Exception passed from HTTP client
      * @throws InterruptedException Exception passed from HTTP client
      */
-    public <T extends HetznerJsonObject> T sendHttpRequest(Class<T> clazz, String endpoint, RequestVerb requestVerb, String apiKey, String body) throws IOException, InterruptedException {
+    public <T extends HetznerJsonObject> T sendHttpRequest(Class<T> clazz, String endpoint, RequestVerb requestVerb, String apiKey, String body) throws IOException, InterruptedException, IllegalAccessException {
         HttpRequest request = createHttpRequest(endpoint, requestVerb, apiKey, body);
-        HttpResponse<T> response = httpClient.send(request, new JacksonBodyHandler<>(clazz));
+        HttpResponse<HetznerResult<T, HetznerErrorResponse>> response = httpClient.send(request, new JacksonBodyHandler<>(clazz, HetznerErrorResponse.class));
 
-        switch (response.statusCode()) {
-            case 200, 201, 204 -> {
-                return (T) response.body();
+        switch (Integer.parseInt(Integer.toString(response.statusCode()).substring(0, 1))) {
+            case 2 -> {
+                if (response.statusCode() == 204) {
+                    log.info("Response was 204, continuing...");
+                    return null;
+                }
+                return response.body().getSuccessObject();
             }
             default -> {
-                HetznerErrorResponse errorResponse = (HetznerErrorResponse) response.body();
+                HetznerErrorResponse errorResponse = response.body().getFailureObject();
                 throw new HetznerApiException(errorResponse);
             }
         }

src/main/java/dev/tomr/hcloud/http/HetznerResult.java

@@ -0,0 +1,31 @@
+package dev.tomr.hcloud.http;
+
+public class HetznerResult<T, U> {
+    private final T successObject;
+    private final U failureObject;
+
+    public HetznerResult(T successObject, U failureObject) {
+        this.successObject = successObject;
+        this.failureObject = failureObject;
+    }
+
+    public boolean isSuccess() {
+        return successObject != null;
+    }
+
+    public T getSuccessObject() throws IllegalAccessException {
+        if (isSuccess()) {
+            return successObject;
+        } else {
+            throw new IllegalAccessException("No success object!");
+        }
+    }
+
+    public U getFailureObject() throws IllegalAccessException {
+        if (!isSuccess()) {
+            return failureObject;
+        } else {
+            throw new IllegalAccessException("No failure object!");
+        }
+    }
+}

src/main/java/dev/tomr/hcloud/http/JacksonBodyHandler.java

@@ -1,56 +1,67 @@
 package dev.tomr.hcloud.http;
 
 import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.JsonMappingException;
 import dev.tomr.hcloud.HetznerCloud;
 import dev.tomr.hcloud.http.model.HetznerErrorResponse;
 
+import java.io.IOException;
+import java.io.InputStream;
 import java.net.http.HttpResponse;
 import java.nio.charset.StandardCharsets;
+import java.util.function.Function;
 
 /**
  * Internal helper to handle the converting of a returned HTTP response's body, to an Object, with Jackson ObjectMapper
  * @param <T>
  */
-public class JacksonBodyHandler<T> implements HttpResponse.BodyHandler<T> {
+public class JacksonBodyHandler<T, U> implements HttpResponse.BodyHandler<HetznerResult<T, U>> {
 
     private final Class<T> clazz;
+    private final Class<U> errorClass;
 
-    public JacksonBodyHandler(Class<T> clazz) {
+    private int statusCode;
+
+    public JacksonBodyHandler(Class<T> clazz, Class<U> errorClass) {
         this.clazz = clazz;
+        this.errorClass = errorClass;
     }
 
     @Override
-    public HttpResponse.BodySubscriber<T> apply(HttpResponse.ResponseInfo responseInfo) {
-        switch (responseInfo.statusCode()) {
-            case 200, 201 -> {
-                return jsonPayload(clazz);
+    public HttpResponse.BodySubscriber<HetznerResult<T, U>> apply(HttpResponse.ResponseInfo responseInfo) {
+        Function<InputStream, HetznerResult<T, U>> mapper;
+        statusCode = responseInfo.statusCode();
+        switch (Integer.parseInt(Integer.toString(statusCode).substring(0, 1))) {
+            case 2 -> {
+                mapper = this::parseSuccess;
             }
             default -> {
-                return apiErrorJson();
+                mapper = this::parseError;
             }
         }
+        return HttpResponse.BodySubscribers.mapping(HttpResponse.BodySubscribers.ofInputStream(), mapper);
     }
 
-    public static <T> HttpResponse.BodySubscriber<T> jsonPayload(Class<T> targetClass) {
-        HttpResponse.BodySubscriber<String> bodyString = HttpResponse.BodySubscribers.ofString(StandardCharsets.UTF_8);
-        return HttpResponse.BodySubscribers.mapping(bodyString, (String b) -> {
-            try {
-                return HetznerCloud.getObjectMapper().readValue(b, targetClass);
-            } catch (JsonProcessingException e) {
-                throw new RuntimeException(e);
-            }
-        });
+    private HetznerResult<T, U> parseSuccess(InputStream input) {
+        return new HetznerResult<>(parseJson(input, clazz), null);
     }
 
-    public static HttpResponse.BodySubscriber apiErrorJson() {
-        HttpResponse.BodySubscriber<String> bodyString = HttpResponse.BodySubscribers.ofString(StandardCharsets.UTF_8);
-        return HttpResponse.BodySubscribers.mapping(bodyString, (String b) -> {
-            try {
-                return HetznerCloud.getObjectMapper().readValue(b, HetznerErrorResponse.class);
-            } catch (JsonProcessingException e) {
+    private HetznerResult<T, U> parseError(InputStream input) {
+        return new HetznerResult<>(null, parseJson(input, errorClass));
+    }
+
+
+    private <R> R parseJson(InputStream input, Class<R> clazz) {
+        try {
+            return HetznerCloud.getObjectMapper().readValue(input, clazz);
+        } catch (IOException e) {
+            // this is actually just bad, but handles 204 properly. I do **not** like this though, need to revisit
+            if (statusCode == 204) {
+                return null;
+            } else {
                 throw new RuntimeException(e);
             }
-        });
+        }
     }
 
 }

src/main/java/dev/tomr/hcloud/service/server/ServerService.java

@@ -99,7 +99,7 @@ private CompletableFuture<Void> scheduleHttpRequest(String host, String apiKey)
                 } else {
                     throw new RuntimeException("No updated values??");
                 }
-            } catch (InterruptedException | IOException e) {
+            } catch (InterruptedException | IOException | IllegalAccessException e) {
                 throw new RuntimeException(e);
             }
         }, serviceManager.getExecutor());

src/test/java/dev/tomr/hcloud/component/http/HttpClientComponentTest.java

@@ -12,6 +12,8 @@
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 
 import java.io.IOException;
 
@@ -32,7 +34,7 @@ public class HttpClientComponentTest {
 
     @Test
     @DisplayName("HTTP Client can make a successful GET request and map to class")
-    public void testHttpClientAndMapping() throws IOException, InterruptedException {
+    public void testHttpClientAndMapping() throws IOException, InterruptedException, IllegalAccessException {
         wireMockExtension.stubFor(get("/test").willReturn(ok(objectMapper.writeValueAsString(new TestModel(1, 1, "sunt aut facere repellat provident occaecati excepturi optio reprehenderit", "quia et suscipit\nsuscipit recusandae consequuntur expedita et cum\nreprehenderit molestiae ut ut quas totam\nnostrum rerum est autem sunt rem eveniet architecto")))));
 
         HetznerCloudHttpClient client = new HetznerCloudHttpClient();
@@ -47,7 +49,7 @@ public void testHttpClientAndMapping() throws IOException, InterruptedException
 
     @Test
     @DisplayName("HTTP Client can make a successful POST request with Body and map to response class")
-    void testHttpClientAndMappingWithBody() throws IOException, InterruptedException {
+    void testHttpClientAndMappingWithBody() throws IOException, InterruptedException, IllegalAccessException {
         wireMockExtension.stubFor(post("/test").willReturn(ok(objectMapper.writeValueAsString(new TestModel(2, 1, "some title", "some body")))));
         HetznerCloudHttpClient client = new HetznerCloudHttpClient();
 
@@ -64,7 +66,7 @@ void testHttpClientAndMappingWithBody() throws IOException, InterruptedException
 
     @Test
     @DisplayName("HTTP Client can make a successful PUT request with Body and map to response class")
-    void testHttpClientAndMappingWithBodyPUT() throws IOException, InterruptedException {
+    void testHttpClientAndMappingWithBodyPUT() throws IOException, InterruptedException, IllegalAccessException {
         wireMockExtension.stubFor(put("/test").willReturn(ok(objectMapper.writeValueAsString(new TestModel(2, 1, "some title", "some body")))));
         HetznerCloudHttpClient client = new HetznerCloudHttpClient();
 
@@ -111,4 +113,15 @@ void testHttpClientThrowsHetznerApiExceptionWhenBadJsonResponse() {
 
         assertThrows(IOException.class, () -> client.sendHttpRequest(TestModel.class, HOST + "badResponseWithXml", RequestVerb.GET, ""));
     }
+
+    @Test
+    @DisplayName("HTTP Client handles 204 no content correctly")
+    void httpClientHandles204NoContent() throws IOException, InterruptedException, IllegalAccessException {
+        // this test is needed because 204 does not support handling a body
+        // todo refactor how we handle HTTP Status codes we **know** will never supply a body, i.e. 204 - this implementation leaves a lot to be desired
+        wireMockExtension.stubFor(get("/test").willReturn(aResponse().withStatus(204)));
+        HetznerCloudHttpClient client = new HetznerCloudHttpClient();
+
+        assertDoesNotThrow(() -> client.sendHttpRequest(TestModel.class, HOST + "test", RequestVerb.GET, ""));
+    }
 }

src/test/java/dev/tomr/hcloud/http/HetznerResultTest.java

@@ -0,0 +1,56 @@
+package dev.tomr.hcloud.http;
+
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+public class HetznerResultTest {
+
+    @Test
+    @DisplayName("HetznerResult will disallow access to the failure object, when a success object exists")
+    void hetznerResultWillDisallowAccessToFailureWhenSuccessExists() {
+        HetznerResult<String, String> hetznerResult = new HetznerResult<>("success", "failure");
+        assertThrows(IllegalAccessException.class, hetznerResult::getFailureObject);
+    }
+
+    @Test
+    @DisplayName("Returns given success object")
+    void returnsGivenSuccessObject() throws IllegalAccessException {
+        HetznerResult<String, String> hetznerResult = new HetznerResult<>("success", null);
+        assertEquals(hetznerResult.getSuccessObject(), "success");
+    }
+
+    @Test
+    @DisplayName("Returns given failure object")
+    void returnsGivenFailureObject() throws IllegalAccessException {
+        HetznerResult<String, String> hetznerResult = new HetznerResult<>(null, "failure");
+        assertEquals(hetznerResult.getFailureObject(), "failure");
+    }
+
+    @Test
+    @DisplayName("Returns whether it has a success object correctly")
+    void returnsWhetherItHasASuccessObject() {
+        HetznerResult<String, String> hetznerResult = new HetznerResult<>("success", null);
+        assertTrue(hetznerResult.isSuccess());
+
+        HetznerResult<String, String> hetznerResult2 = new HetznerResult<>(null, "failure");
+        assertFalse(hetznerResult2.isSuccess());
+    }
+
+    @Test
+    @DisplayName("Throws IllegalAccessException when trying to access a non existent success object")
+    void throwsIllegalAccessExceptionWhenTryingToAccessNonExistentSuccessObject() {
+        HetznerResult<Object, Object> hetznerResult = new HetznerResult<>(null, new Object());
+
+        assertThrows(IllegalAccessException.class, hetznerResult::getSuccessObject);
+    }
+
+    @Test
+    @DisplayName("Throws IllegalAccessException when trying to access a non existent failure object")
+    void throwsIllegalAccessExceptionWhenTryingToAccessNonExistentFailureObject() {
+        HetznerResult<Object, Object> hetznerResult = new HetznerResult<>(new Object(), null);
+
+        assertThrows(IllegalAccessException.class, hetznerResult::getFailureObject);
+    }
+}

src/test/java/dev/tomr/hcloud/service/server/ServerServiceTest.java

@@ -110,7 +110,7 @@ void taskIsScheduledWhenServerNameOrLabelUpdateCalledForFirstTime() {
             serverService.serverNameOrLabelUpdate("name", "name", server);
 
             verify(hetznerCloudHttpClient, timeout(2000).times(1)).sendHttpRequest(eq(ServerDTO.class), eq("http://host/server/1"), any(RequestVerb.class), eq("key1234"),  eq("{\"name\":\"name\"}"));
-        } catch (IOException | InterruptedException e) {
+        } catch (IOException | InterruptedException | IllegalAccessException e) {
             throw new RuntimeException(e);
         }
     }
@@ -165,7 +165,7 @@ void taskIsScheduledWhenServerNameOrLabelUpdateCalledForFirstTimeWithLabels() {
             serverService.serverNameOrLabelUpdate("labels", Map.of("label", "value"), server);
 
             verify(hetznerCloudHttpClient, timeout(2000).times(1)).sendHttpRequest(eq(ServerDTO.class), eq("http://host/server/1"), any(RequestVerb.class), eq("key1234"),  eq("{\"labels\":{\"label\":\"value\"}}"));
-        } catch (IOException | InterruptedException e) {
+        } catch (IOException | InterruptedException | IllegalAccessException e) {
             throw new RuntimeException(e);
         }
     }
@@ -221,7 +221,7 @@ void taskUsesExtraFieldsChangedAfterFirstInvocation() {
             serverService.serverNameOrLabelUpdate("labels", Map.of("l", "v"), server);
 
             verify(hetznerCloudHttpClient, timeout(2000).times(1)).sendHttpRequest(eq(ServerDTO.class), eq("http://host/server/1"), any(RequestVerb.class), eq("key1234"),  eq("{\"labels\":{\"label\":\"value\"},\"name\":\"name\"}"));
-        } catch (IOException | InterruptedException e) {
+        } catch (IOException | InterruptedException | IllegalAccessException e) {
             throw new RuntimeException(e);
         }
     }
@@ -279,7 +279,7 @@ void whenHttpClientThrowsGracefully() {
 
             verify(hetznerCloudHttpClient, timeout(2000).times(0)).sendHttpRequest(eq(ServerDTO.class), eq("http://host/server/1"), any(RequestVerb.class), eq("key1234"),  eq("{\"labels\":{\"label\":\"value\"},\"name\":\"name\"}"));
             verify(serviceManager, timeout(2000).times(1)).closeExecutor();
-        } catch (IOException | InterruptedException e) {
+        } catch (IOException | InterruptedException | IllegalAccessException e) {
             throw new RuntimeException(e);
         }
     }
@@ -337,7 +337,7 @@ void cancelMethodPreventsTheRequestBeingSent() {
 
             verify(hetznerCloudHttpClient, timeout(2000).times(0)).sendHttpRequest(any(), any(), any(), any(), any());
             verify(serviceManager, timeout(2000).times(1)).closeExecutor();
-        } catch (IOException | InterruptedException e) {
+        } catch (IOException | InterruptedException | IllegalAccessException e) {
             throw new RuntimeException(e);
         }
     }