[API-8040, API-7676]: Error handling and input validation improvements

src/main/java/com/siftscience/SiftClient.java

@@ -56,6 +56,8 @@ public SiftClient(String apiKey, String accountId) {
     }
 
     public SiftClient(String apiKey, String accountId, OkHttpClient okHttpClient) {
+        assertNotNull(apiKey, "API key must not be null");
+        assertNotNull(okHttpClient, "Http Client must not be null");
         this.apiKey = apiKey;
         this.accountId = accountId;
         this.httpClient = new HttpClient(okHttpClient);
@@ -95,16 +97,19 @@ public EventRequest buildRequest(FieldSet fields) {
     }
 
     public ApplyDecisionRequest buildRequest(ApplyDecisionFieldSet fields) {
+        assertAccountIdIsNotNull();
         setupApiKey(fields);
         return new ApplyDecisionRequest(baseUrl, getAccountId(), httpClient, fields);
     }
 
     public GetDecisionsRequest buildRequest(GetDecisionFieldSet fields) {
+        assertAccountIdIsNotNull();
         setupApiKey(fields);
         return new GetDecisionsRequest(baseUrl, getAccountId(), httpClient, fields);
     }
 
     public DecisionStatusRequest buildRequest(DecisionStatusFieldSet fields) {
+        assertAccountIdIsNotNull();
         setupApiKey(fields);
         return new DecisionStatusRequest(baseUrl, getAccountId(), httpClient, fields);
     }
@@ -130,26 +135,31 @@ public UserScoreRequest buildRequest(UserScoreFieldSet fields) {
     }
 
     public WorkflowStatusRequest buildRequest(WorkflowStatusFieldSet fields) {
+        assertAccountIdIsNotNull();
         setupApiKey(fields);
         return new WorkflowStatusRequest(baseUrl, getAccountId(), httpClient, fields);
     }
 
     public GetMerchantsRequest buildRequest(GetMerchantsFieldSet fields) {
+        assertAccountIdIsNotNull();
         setupApiKey(fields);
         return new GetMerchantsRequest(baseUrl, getAccountId(), httpClient, fields);
     }
 
     public GetMerchantRequest buildRequest(GetMerchantFieldSet fields) {
+        assertAccountIdIsNotNull();
         setupApiKey(fields);
         return new GetMerchantRequest(baseUrl, getAccountId(), httpClient, fields);
     }
 
     public CreateMerchantRequest buildRequest(CreateMerchantFieldSet fields) {
+        assertAccountIdIsNotNull();
         setupApiKey(fields);
         return new CreateMerchantRequest(baseUrl, getAccountId(), httpClient, fields);
     }
 
     public UpdateMerchantRequest buildRequest(UpdateMerchantFieldSet fields, String merchantId) {
+        assertAccountIdIsNotNull();
         setupApiKey(fields);
         return new UpdateMerchantRequest(baseUrl, getAccountId(), httpClient, fields, merchantId);
     }
@@ -172,4 +182,14 @@ public VerificationCheckRequest buildRequest(VerificationCheckFieldSet fields) {
     private void setupApiKey(FieldSet fields) {
         fields.setApiKey(getApiKey());
     }
+
+    private void assertAccountIdIsNotNull() {
+        assertNotNull(getAccountId(), "Account ID must not be null");
+    }
+
+    private void assertNotNull(Object value, String message) {
+        if (value == null) {
+            throw new IllegalArgumentException(message);
+        }
+    }
 }

src/main/java/com/siftscience/exception/SiftException.java

@@ -19,8 +19,10 @@ public SiftResponse getSiftResponse() {
     }
 
     private static String responseErrorMessage(SiftResponse response) {
-        if (response == null || response.getApiErrorMessage() == null) {
+        if (response == null) {
             return "Unexpected API error.";
+        } else if (response.getApiErrorMessage() == null) {
+            return "Unexpected API error " + response.getHttpStatusCode() + ".";
         }
         return response.getApiErrorMessage();
     }

src/test/java/com/siftscience/SiftClientTest.java

@@ -4,18 +4,27 @@
 import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR;
 import java.io.IOException;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.fail;
 import com.siftscience.exception.InvalidApiKeyException;
 import com.siftscience.exception.InvalidFieldException;
 import com.siftscience.exception.MissingFieldException;
 import com.siftscience.exception.RateLimitException;
 import com.siftscience.exception.ServerException;
+import com.siftscience.model.ApplyDecisionFieldSet;
+import com.siftscience.model.CreateMerchantFieldSet;
 import com.siftscience.model.CreateOrderFieldSet;
+import com.siftscience.model.DecisionStatusFieldSet;
+import com.siftscience.model.GetDecisionFieldSet;
+import com.siftscience.model.GetMerchantsFieldSet;
+import com.siftscience.model.UpdateMerchantFieldSet;
+import com.siftscience.model.WorkflowStatusFieldSet;
 import okhttp3.OkHttpClient;
 import okhttp3.mockwebserver.MockResponse;
 import okhttp3.mockwebserver.MockWebServer;
 import org.json.JSONException;
 import org.junit.After;
-import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.skyscreamer.jsonassert.JSONAssert;
@@ -93,11 +102,11 @@ public void testInvalidAPIKeyException() throws Exception {
         }
 
         // We should have gotten an exception.
-        Assert.assertNotNull(apiKeyException);
-        Assert.assertEquals("Invalid API key message.", apiKeyException.getLocalizedMessage());
+        assertNotNull(apiKeyException);
+        assertEquals("Invalid API key message.", apiKeyException.getLocalizedMessage());
 
         // Check that we can access the API key from the exception object.
-        Assert.assertEquals("INVALID_API_KEY",
+        assertEquals("INVALID_API_KEY",
                 apiKeyException.getSiftResponse().getRequestBody().getApiKey());
     }
 
@@ -141,8 +150,8 @@ public void testServerSideMissingFieldException() throws IOException {
         }
 
         // We should have gotten an exception.
-        Assert.assertNotNull(missingFieldException);
-        Assert.assertEquals("Missing user email message from server.",
+        assertNotNull(missingFieldException);
+        assertEquals("Missing user email message from server.",
                 missingFieldException.getLocalizedMessage());
     }
 
@@ -164,8 +173,8 @@ public void testInvalidCustomFieldKeyException() throws IOException {
         }
 
         // Should have thrown an exception.
-        Assert.assertNotNull(invalidFieldException);
-        Assert.assertEquals("Custom field \"$not_allowed\" may not begin with a dollar sign.",
+        assertNotNull(invalidFieldException);
+        assertEquals("Custom field \"$not_allowed\" may not begin with a dollar sign.",
                 invalidFieldException.getLocalizedMessage());
     }
 
@@ -211,8 +220,8 @@ public void testRateLimitException() throws IOException {
         }
 
         // We should have gotten an exception.
-        Assert.assertNotNull(rateLimitException);
-        Assert.assertEquals("Rate limit error message.", rateLimitException.getLocalizedMessage());
+        assertNotNull(rateLimitException);
+        assertEquals("Rate limit error message.", rateLimitException.getLocalizedMessage());
     }
 
     /**
@@ -243,8 +252,9 @@ public void testGenericServerErrorException() throws IOException {
         }
 
         // We should have gotten an exception.
-        Assert.assertNotNull(serverException);
-        Assert.assertEquals("Unexpected API error.", serverException.getLocalizedMessage());
+        assertNotNull(serverException);
+        assertEquals("Unexpected API error " + HTTP_INTERNAL_ERROR + ".",
+            serverException.getLocalizedMessage());
     }
 
     /**
@@ -276,8 +286,9 @@ public void testGenericServerErrorExceptionNonJson() throws IOException {
         }
 
         // We should have gotten an exception.
-        Assert.assertNotNull(serverException);
-        Assert.assertEquals("Unexpected API error.", serverException.getLocalizedMessage());
+        assertNotNull(serverException);
+        assertEquals("Unexpected API error " + HTTP_INTERNAL_ERROR + ".",
+            serverException.getLocalizedMessage());
     }
 
     /**
@@ -303,4 +314,92 @@ public void testNullsAreNotSerialized() throws JSONException {
         "}", fieldSet.toJson(), true);
     }
 
+    @Test
+    public void testFailsToCreateClientWhenApiKeyIsNull() {
+        assertIllegalArgumentWithMessage(
+            () -> new SiftClient(null, "YOUR_ACCOUNT_ID"),
+            "API key must not be null"
+        );
+    }
+
+    @Test
+    public void testFailsToCreateClientWhenHttpClientIsNull() {
+        assertIllegalArgumentWithMessage(
+            () -> new SiftClient("YOUR_API_KEY", "YOUR_ACCOUNT_ID",
+                (OkHttpClient) null),
+            "Http Client must not be null"
+        );
+    }
+
+    @Test
+    public void testFailsToBuildApplyDecisionRequest() {
+        SiftClient siftClient = new SiftClient("YOUR_API_KEY", null);
+        assertIllegalArgumentWithMessage(
+            () -> siftClient.buildRequest(new ApplyDecisionFieldSet()),
+            "Account ID must not be null"
+        );
+    }
+
+    @Test
+    public void testFailsToBuildGetDecisionRequest() {
+        SiftClient siftClient = new SiftClient("YOUR_API_KEY", null);
+        assertIllegalArgumentWithMessage(
+            () -> siftClient.buildRequest(new GetDecisionFieldSet()),
+            "Account ID must not be null"
+        );
+    }
+
+    @Test
+    public void testFailsToBuildDecisionStatusRequest() {
+        SiftClient siftClient = new SiftClient("YOUR_API_KEY", null);
+        assertIllegalArgumentWithMessage(
+            () -> siftClient.buildRequest(new DecisionStatusFieldSet()),
+            "Account ID must not be null"
+        );
+    }
+
+    @Test
+    public void testFailsToBuildWorkflowStatusRequest() {
+        SiftClient siftClient = new SiftClient("YOUR_API_KEY", null);
+        assertIllegalArgumentWithMessage(
+            () -> siftClient.buildRequest(new WorkflowStatusFieldSet()),
+            "Account ID must not be null"
+        );
+    }
+
+    @Test
+    public void testFailsToBuildGetMerchantsRequest() {
+        SiftClient siftClient = new SiftClient("YOUR_API_KEY", null);
+        assertIllegalArgumentWithMessage(
+            () -> siftClient.buildRequest(new GetMerchantsFieldSet()),
+            "Account ID must not be null"
+        );
+    }
+
+    @Test
+    public void testFailsToBuildCreateMerchantRequest() {
+        SiftClient siftClient = new SiftClient("YOUR_API_KEY", null);
+        assertIllegalArgumentWithMessage(
+            () -> siftClient.buildRequest(new CreateMerchantFieldSet()),
+            "Account ID must not be null"
+        );
+    }
+
+    @Test
+    public void testFailsToBuildUpdateMerchantRequest() {
+        SiftClient siftClient = new SiftClient("YOUR_API_KEY", null);
+        assertIllegalArgumentWithMessage(
+            () -> siftClient.buildRequest(new UpdateMerchantFieldSet(), "merchantID"),
+            "Account ID must not be null"
+        );
+    }
+
+    private void assertIllegalArgumentWithMessage(Runnable runnable, String message) {
+        try {
+            runnable.run();
+            fail("Excepted to throw IllegalArgumentException");
+        } catch (IllegalArgumentException exception) {
+            assertEquals(message, exception.getMessage());
+        }
+    }
 }