From 697906a669d4f6a9ca138d294bd081cfaaff9bec Mon Sep 17 00:00:00 2001 From: Thilo Schwarz Date: Fri, 25 Apr 2025 18:56:35 +0200 Subject: [PATCH] Refactor response handling with ResponseValidator class Introduced a new ResponseValidator class to encapsulate response validation logic, improving code readability and maintainability. Updated CfDnsClient to delegate response validation to this new class. Adjusted tests to align with the refactor and ensure proper exception handling. --- .../java/codes/thischwa/cf/CfDnsClient.java | 51 ++++++++--------- .../codes/thischwa/cf/ResponseValidator.java | 56 +++++++++++++++++++ .../java/codes/thischwa/cf/CfClientTest.java | 11 ++-- 3 files changed, 86 insertions(+), 32 deletions(-) create mode 100644 src/main/java/codes/thischwa/cf/ResponseValidator.java diff --git a/src/main/java/codes/thischwa/cf/CfDnsClient.java b/src/main/java/codes/thischwa/cf/CfDnsClient.java index 71e168e..4a1987f 100644 --- a/src/main/java/codes/thischwa/cf/CfDnsClient.java +++ b/src/main/java/codes/thischwa/cf/CfDnsClient.java @@ -6,11 +6,9 @@ import codes.thischwa.cf.model.RecordEntity; import codes.thischwa.cf.model.RecordMultipleResponse; import codes.thischwa.cf.model.RecordSingleResponse; import codes.thischwa.cf.model.RecordType; -import codes.thischwa.cf.model.ResponseResultInfo; import codes.thischwa.cf.model.ZoneEntity; import codes.thischwa.cf.model.ZoneMultipleResponse; import java.util.List; -import java.util.stream.Collectors; import lombok.Setter; import lombok.extern.slf4j.Slf4j; @@ -18,6 +16,7 @@ import lombok.extern.slf4j.Slf4j; * CfDnsClient is a client interface to interact with Cloudflare DNS service. It allows managing DNS * records and zones within the Cloudflare system, including creating, updating, retrieving, and * deleting DNS records. + * *

Example: * 


  * // Create a new CfDnsClient instance
@@ -40,10 +39,10 @@ import lombok.extern.slf4j.Slf4j;
 public class CfDnsClient extends CfBasicHttpClient {
   private static final String DEFAULT_BASEURL = "https://api.cloudflare.com/client/v4";
 
-  private boolean emptyResultThrowsException;
+  private final ResponseValidator responseValidator;
 
   /**
-   * Constructs a CfDnsClient instance for interacting with the Cloudflare DNS API.
+   * Constructs a new instance of {@code CfDnsClient}.
    *
    * @param authEmail The email address associated with the Cloudflare account, used for
    *                  authentication.
@@ -55,7 +54,7 @@ public class CfDnsClient extends CfBasicHttpClient {
   }
 
   /**
-   * Constructs a CfDnsClient instance for interacting with the Cloudflare DNS API.
+   * Constructs a new instance of {@code CfDnsClient}.
    *
    * @param baseUrl   The base URL of the Cloudflare API to be used for requests.
    * @param authEmail The email address associated with the Cloudflare account, used for
@@ -68,8 +67,19 @@ public class CfDnsClient extends CfBasicHttpClient {
   }
 
   /**
-   * Constructs a new instance of {@code CfDnsClient}, which facilitates interactions with the
-   * Cloudflare DNS API.
+   * Constructs a new instance of {@code CfDnsClient}.
+   *
+   * @param emptyResultThrowsException a boolean value indicating whether an exception should be
+   *                                   thrown when the result is empty
+   * @param authEmail                  the authentication email required for API access
+   * @param authKey                    the authentication key required for API access
+   */
+  public CfDnsClient(boolean emptyResultThrowsException, String authEmail, String authKey) {
+    this(emptyResultThrowsException, DEFAULT_BASEURL, authEmail, authKey);
+  }
+
+  /**
+   * Constructs a new instance of {@code CfDnsClient}.
    *
    * @param emptyResultThrowsException Specifies if an exception should be thrown when the API
    *                                   response is empty. Default is true.
@@ -82,11 +92,7 @@ public class CfDnsClient extends CfBasicHttpClient {
   public CfDnsClient(boolean emptyResultThrowsException, String baseUrl, String authEmail,
       String authKey) {
     super(baseUrl, authEmail, authKey);
-    this.emptyResultThrowsException = emptyResultThrowsException;
-  }
-
-  private static String buildFqdn(ZoneEntity zone, String sld) {
-    return sld + "." + zone.getName();
+    this.responseValidator = new ResponseValidator(emptyResultThrowsException);
   }
 
   /**
@@ -317,27 +323,16 @@ public class CfDnsClient extends CfBasicHttpClient {
     }
   }
 
+  private static String buildFqdn(ZoneEntity zone, String sld) {
+    return sld + "." + zone.getName();
+  }
+
   private void checkResponse(AbstractResponse resp) throws CloudflareApiException {
     checkResponse(resp, false);
   }
 
   private void checkResponse(AbstractResponse resp, boolean singleResultExpected)
       throws CloudflareApiException {
-    ResponseResultInfo resultInfo = resp.getResponseResultInfo();
-    if (!resultInfo.isSuccess()) {
-      String errors =
-          resultInfo.getErrors().stream().map(Object::toString).collect(Collectors.joining(", "));
-      throw new CloudflareApiException("Error in response: " + errors);
-    }
-
-    if (resp instanceof RecordMultipleResponse respMulti) {
-      if (singleResultExpected && respMulti.getResultInfo().getTotalCount() > 1) {
-        throw new CloudflareApiException(
-            "Unexpected result count: " + respMulti.getResultInfo().getTotalCount());
-      }
-      if (emptyResultThrowsException && respMulti.getResultInfo().getTotalCount() == 0) {
-        throw new CloudflareNotFoundException("No result found");
-      }
-    }
+    responseValidator.validate(resp, singleResultExpected);
   }
 }
diff --git a/src/main/java/codes/thischwa/cf/ResponseValidator.java b/src/main/java/codes/thischwa/cf/ResponseValidator.java
new file mode 100644
index 0000000..31a081d
--- /dev/null
+++ b/src/main/java/codes/thischwa/cf/ResponseValidator.java
@@ -0,0 +1,56 @@
+package codes.thischwa.cf;
+
+import codes.thischwa.cf.model.AbstractResponse;
+import codes.thischwa.cf.model.RecordMultipleResponse;
+import codes.thischwa.cf.model.ResponseResultInfo;
+import java.util.stream.Collectors;
+
+/**
+ * Validates API responses to ensure compliance with expected conditions, such as response success
+ * and result count.
+ *
+ * 
This class performs two primary validation tasks:
+ * 
    
+ * 
  • It checks whether the API response was successful by analyzing the associated response
+ * metadata. If the response indicates failure, an exception is thrown with descriptive error
+ * messages.
+ * 
  • It validates the number of results in the API response payload to detect unexpected counts.
+ * Depending on the configuration, discrepancies in result count or an empty result may trigger
+ * exceptions.
+ * 

+ */
+class ResponseValidator {
+  private final boolean emptyResultThrowsException;
+
+  ResponseValidator(boolean emptyResultThrowsException) {
+    this.emptyResultThrowsException = emptyResultThrowsException;
+  }
+
+  void validate(AbstractResponse resp, boolean singleResultExpected) throws CloudflareApiException {
+    validateResponseSuccess(resp);
+    validateResultCount(resp, singleResultExpected);
+  }
+
+  private void validateResponseSuccess(AbstractResponse resp) throws CloudflareApiException {
+    ResponseResultInfo resultInfo = resp.getResponseResultInfo();
+    if (!resultInfo.isSuccess()) {
+      String errors =
+          resultInfo.getErrors().stream().map(Object::toString).collect(Collectors.joining(", "));
+      throw new CloudflareApiException("Error in response: " + errors);
+    }
+  }
+
+  private void validateResultCount(AbstractResponse resp, boolean singleResultExpected)
+      throws CloudflareApiException {
+    if (resp instanceof RecordMultipleResponse respMulti) {
+      if (singleResultExpected && respMulti.getResultInfo().getTotalCount() > 1) {
+        throw new CloudflareApiException(
+            "Unexpected result count: " + respMulti.getResultInfo().getTotalCount());
+      }
+      if (emptyResultThrowsException && respMulti.getResultInfo().getTotalCount() == 0) {
+        throw new CloudflareNotFoundException("No result found");
+      }
+    }
+  }
+
+}
diff --git a/src/test/java/codes/thischwa/cf/CfClientTest.java b/src/test/java/codes/thischwa/cf/CfClientTest.java
index 47b31d8..606ece2 100644
--- a/src/test/java/codes/thischwa/cf/CfClientTest.java
+++ b/src/test/java/codes/thischwa/cf/CfClientTest.java
@@ -36,11 +36,14 @@ public class CfClientTest {
 
     assertThrows(CloudflareNotFoundException.class,
       () -> client.sldListAll(zList.get(0), "not-existing"));
+  }
 
-    client.setEmptyResultThrowsException(false);
-    rList = client.sldListAll(zList.get(0), "not-existing");
-    assertTrue(rList.isEmpty());
-    client.setEmptyResultThrowsException(true);
+  @Test
+  void testEmptyResultThrowsException() throws Exception {
+    List zList = client.zoneListAll();
+    CfDnsClient client = new CfDnsClient(true, API_EMAIL, API_KEY);
+    assertThrows(CloudflareNotFoundException.class,
+        () -> client.sldListAll(zList.get(0), "not-existing"));
   }
 
   @Test