From 97b7f8371a1a7d9a3ef05b9920df619a6e087dec Mon Sep 17 00:00:00 2001 From: Thilo Schwarz Date: Sun, 13 Apr 2025 19:57:15 +0200 Subject: [PATCH] Refactor code style and add tests for exception handling Adjusted code style to align with consistent brace formatting and removed the null/blank check for `baseUrl` in the constructor. Added unit tests to verify exception handling in `CfDnsClient` for null arguments. --- .../codes/thischwa/cf/CfBasicHttpClient.java | 86 ++++++++++++------- .../java/codes/thischwa/cf/CfClientTest.java | 6 ++ 2 files changed, 63 insertions(+), 29 deletions(-) diff --git a/src/main/java/codes/thischwa/cf/CfBasicHttpClient.java b/src/main/java/codes/thischwa/cf/CfBasicHttpClient.java index 9f56203..eeebc9c 100644 --- a/src/main/java/codes/thischwa/cf/CfBasicHttpClient.java +++ b/src/main/java/codes/thischwa/cf/CfBasicHttpClient.java @@ -31,7 +31,8 @@ import org.apache.hc.core5.http.message.BasicClassicHttpRequest; * managing authentication, and handling JSON serialization. */ @Slf4j -abstract class CfBasicHttpClient { +abstract class CfBasicHttpClient +{ private final String baseUrl; private final String authEmail; private final String authKey; @@ -40,10 +41,6 @@ abstract class CfBasicHttpClient { CfBasicHttpClient(String baseUrl, String authEmail, String authKey) throws IllegalArgumentException { - if (baseUrl == null || baseUrl.isBlank()) - { - throw new IllegalArgumentException("Base URL must not be null or blank!"); - } if (authEmail == null || authEmail.isBlank()) { throw new IllegalArgumentException("Authentication email must not be null or blank!"); @@ -58,7 +55,8 @@ abstract class CfBasicHttpClient { this.objectMapper = initObjectMapper(); } - private ObjectMapper initObjectMapper() { + private ObjectMapper initObjectMapper() + { ObjectMapper mapper = new ObjectMapper(); mapper.registerModule(new JavaTimeModule()); mapper.setSerializationInclusion(JsonInclude.Include.NON_NULL); @@ -67,7 +65,8 @@ abstract class CfBasicHttpClient { return mapper; } - private CloseableHttpClient createHttpClient() { + private CloseableHttpClient createHttpClient() + { return HttpClients.custom() .addRequestInterceptorFirst( (request, context, execChain) -> { @@ -83,9 +82,11 @@ abstract class CfBasicHttpClient { } private T executeRequest( - ClassicHttpRequest request, Class responseType) throws CloudflareApiException { + ClassicHttpRequest request, Class responseType) throws CloudflareApiException + { String logUri = null; - try (CloseableHttpClient client = createHttpClient()) { + try (CloseableHttpClient client = createHttpClient()) + { ResultWrapper result = client.execute( request, @@ -95,9 +96,11 @@ abstract class CfBasicHttpClient { EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8))); logUri = request.getRequestUri(); - if (result.statusCode >= 200 && result.statusCode < 300) { + if (result.statusCode >= 200 && result.statusCode < 300) + { return objectMapper.readValue(result.responseBody, responseType); - } else { + } else + { log.error( "{} request failed for URL {}: Status {}", request.getMethod(), @@ -106,68 +109,93 @@ abstract class CfBasicHttpClient { throw new CloudflareApiException( request.getMethod() + " request failed with status code: " + result.statusCode); } - } catch (JsonProcessingException e) { + } catch (JsonProcessingException e) + { log.error("JSON parsing error for request to {}", logUri, e); throw new CloudflareApiException("Error processing JSON response", e); - } catch (Exception e) { + } catch (Exception e) + { log.error("Error during request execution", e); throw new CloudflareApiException("Request failed", e); } } - /** Sends a GET request to the given endpoint and maps the response. */ + /** + * Sends a GET request to the given endpoint and maps the response. + */ T getRequest(String endpoint, Class responseType) - throws CloudflareApiException { + throws CloudflareApiException + { HttpGet request = new HttpGet(buildUrl(endpoint)); return executeRequest(request, responseType); } - /** Sends a DELETE request to the given endpoint and maps the response. */ + /** + * Sends a DELETE request to the given endpoint and maps the response. + */ T deleteRequest(String endpoint) - throws CloudflareApiException { + throws CloudflareApiException + { HttpDelete request = new HttpDelete(buildUrl(endpoint)); return executeRequest(request, (Class) codes.thischwa.cf.model.RecordSingleResponse.class); } - /** Sends a POST request with a payload to the given endpoint and maps the response. */ + /** + * Sends a POST request with a payload to the given endpoint and maps the response. + */ T postRequest( - String endpoint, R requestPayload) throws CloudflareApiException { + String endpoint, R requestPayload) throws CloudflareApiException + { HttpPost request = new HttpPost(buildUrl(endpoint)); setRequestPayload(request, requestPayload); return executeRequest(request, (Class) codes.thischwa.cf.model.RecordSingleResponse.class); } - /** Sends a PUT request with a payload to the given endpoint and maps the response. */ + /** + * Sends a PUT request with a payload to the given endpoint and maps the response. + */ T putRequest( - String endpoint, R requestPayload, Class responseType) throws CloudflareApiException { + String endpoint, R requestPayload, Class responseType) throws CloudflareApiException + { HttpPut request = new HttpPut(buildUrl(endpoint)); setRequestPayload(request, requestPayload); return executeRequest(request, responseType); } - /** Sends a PATCH request with a payload to the given endpoint and maps the response. */ + /** + * Sends a PATCH request with a payload to the given endpoint and maps the response. + */ T patchRequest( - String endpoint, R requestPayload) throws CloudflareApiException { + String endpoint, R requestPayload) throws CloudflareApiException + { HttpPatch request = new HttpPatch(buildUrl(endpoint)); setRequestPayload(request, requestPayload); return executeRequest(request, (Class) codes.thischwa.cf.model.RecordSingleResponse.class); } - /** Sets the JSON payload for a request. */ + /** + * Sets the JSON payload for a request. + */ private void setRequestPayload( - BasicClassicHttpRequest request, R requestPayload) throws CloudflareApiException { - try { + BasicClassicHttpRequest request, R requestPayload) throws CloudflareApiException + { + try + { request.setEntity( new StringEntity( objectMapper.writeValueAsString(requestPayload), ContentType.APPLICATION_JSON)); - } catch (JsonProcessingException e) { + } catch (JsonProcessingException e) + { throw new CloudflareApiException("Error serializing JSON payload", e); } } - private String buildUrl(String endpoint) { + private String buildUrl(String endpoint) + { return baseUrl + endpoint; } - private record ResultWrapper(int statusCode, String responseBody) {} + private record ResultWrapper(int statusCode, String responseBody) + { + } } diff --git a/src/test/java/codes/thischwa/cf/CfClientTest.java b/src/test/java/codes/thischwa/cf/CfClientTest.java index 963cb80..cb2f700 100644 --- a/src/test/java/codes/thischwa/cf/CfClientTest.java +++ b/src/test/java/codes/thischwa/cf/CfClientTest.java @@ -89,4 +89,10 @@ public class CfClientTest { client.recordDeleteTypeIfExists(z, sldStr, RecordType.A); assertThrows(CloudflareNotFoundException.class, () -> client.sldInfo(z, sldStr, RecordType.A)); } + + @Test + void testException() { + assertThrows(IllegalArgumentException.class, () -> new CfDnsClient(null, "key")); + assertThrows(IllegalArgumentException.class, () -> new CfDnsClient("email", null)); + } }