From f45247371737f9413954088a3f2e7d3c7d00e65e Mon Sep 17 00:00:00 2001 From: Eldar Singin Date: Mon, 30 Dec 2024 14:18:09 +0200 Subject: [PATCH 1/3] [API-8040]: Include HTTP status code in response message for Sift exception --- .../siftscience/exception/SiftException.java | 4 +- .../java/com/siftscience/SiftRequestTest.java | 83 ++++++++++++------- 2 files changed, 55 insertions(+), 32 deletions(-) diff --git a/src/main/java/com/siftscience/exception/SiftException.java b/src/main/java/com/siftscience/exception/SiftException.java index e48103b0..d7a460ed 100644 --- a/src/main/java/com/siftscience/exception/SiftException.java +++ b/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(); } diff --git a/src/test/java/com/siftscience/SiftRequestTest.java b/src/test/java/com/siftscience/SiftRequestTest.java index 718321a3..20920804 100644 --- a/src/test/java/com/siftscience/SiftRequestTest.java +++ b/src/test/java/com/siftscience/SiftRequestTest.java @@ -1,54 +1,72 @@ package com.siftscience; +import static java.net.HttpURLConnection.HTTP_BAD_GATEWAY; +import static java.net.HttpURLConnection.HTTP_OK; +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.SiftException; import com.siftscience.model.ApplyDecisionFieldSet; import okhttp3.OkHttpClient; import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; import okhttp3.mockwebserver.RecordedRequest; -import org.junit.Assert; import org.junit.Test; -import static java.net.HttpURLConnection.HTTP_OK; public class SiftRequestTest { @Test public void testUserAgentHeader() throws Exception { + // given MockWebServer server = new MockWebServer(); - MockResponse response = new MockResponse(); - response.setResponseCode(HTTP_OK); - - server.enqueue(response); - server.start(); - String userId = "a_user_id"; - - // Create a new client and link it to the mock server. - SiftClient client = new SiftClient("YOUR_API_KEY", "YOUR_ACCOUNT_ID", - new OkHttpClient.Builder() - .addInterceptor(OkHttpUtils.urlRewritingInterceptor(server)) - .build()); - - // Build and execute the request against the mock server. - ApplyDecisionRequest request = client.buildRequest( - new ApplyDecisionFieldSet() - .setUserId(userId) - .setTime(System.currentTimeMillis())); + setUpMockResponseWithCode(server, HTTP_OK); + ApplyDecisionRequest request = createRequest(server, "a_user_id"); + // when request.send(); - // Verify the request. + // then RecordedRequest recordedRequest = server.takeRequest(); - Assert.assertEquals("SiftScience/v205 sift-java/3.17.0", recordedRequest.getHeader("User-Agent")); + assertEquals("SiftScience/v205 sift-java/3.17.0", + recordedRequest.getHeader("User-Agent")); } @Test public void testEnqueueRequests() throws Exception { + // given MockWebServer server = new MockWebServer(); - MockResponse response = new MockResponse(); - response.setResponseCode(HTTP_OK); + setUpMockResponseWithCode(server, HTTP_OK); + ApplyDecisionRequest request = createRequest(server, "a_user_id"); - server.enqueue(response); - server.start(); + // when + ApplyDecisionResponse receivedResponse = request.send(); + + // then + assertNotNull(receivedResponse); + } + + @Test + public void testThrowsExceptionWhenReceivedServerError() throws Exception { + // given + MockWebServer server = new MockWebServer(); + setUpMockResponseWithCode(server, HTTP_BAD_GATEWAY); + ApplyDecisionRequest request = createRequest(server, "a_user_id"); + + // when + try { + request.send(); + fail("request.send() is expected to throw SiftException"); + } catch (SiftException siftException) { + // then + assertEquals("Unexpected API error " + HTTP_BAD_GATEWAY + ".", + siftException.getMessage()); + } + } + + private ApplyDecisionRequest createRequest(MockWebServer server, String userId) { // Create a new client and link it to the mock server. SiftClient client = new SiftClient("YOUR_API_KEY", "YOUR_ACCOUNT_ID", new OkHttpClient.Builder() @@ -57,14 +75,17 @@ public void testEnqueueRequests() throws Exception { client.enqueueRequests(); // Build and execute the request against the mock server. - ApplyDecisionRequest request = client.buildRequest( + return client.buildRequest( new ApplyDecisionFieldSet() - .setUserId("a_user_id") + .setUserId(userId) .setTime(System.currentTimeMillis())); + } - ApplyDecisionResponse receivedResponse = request.send(); + private void setUpMockResponseWithCode(MockWebServer server, int httpCode) throws IOException { + MockResponse response = new MockResponse(); + response.setResponseCode(httpCode); - // verify - Assert.assertNotNull(receivedResponse); + server.enqueue(response); + server.start(); } } From e4fba45f4914a0e8678c1ff7fdd5f0b9fc2fb4ac Mon Sep 17 00:00:00 2001 From: Eldar Singin Date: Mon, 30 Dec 2024 14:22:50 +0200 Subject: [PATCH 2/3] [API-8040]: Include HTTP status code in response message for Sift exception --- .../java/com/siftscience/SiftClientTest.java | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/src/test/java/com/siftscience/SiftClientTest.java b/src/test/java/com/siftscience/SiftClientTest.java index 569c0efc..60711aa6 100644 --- a/src/test/java/com/siftscience/SiftClientTest.java +++ b/src/test/java/com/siftscience/SiftClientTest.java @@ -4,6 +4,8 @@ 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 com.siftscience.exception.InvalidApiKeyException; import com.siftscience.exception.InvalidFieldException; import com.siftscience.exception.MissingFieldException; @@ -15,7 +17,6 @@ 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 +94,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 +142,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 +165,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 +212,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 +244,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 +278,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()); } /** From 039a70a2ff9a35434d1a5f85d9449adeb59e27c5 Mon Sep 17 00:00:00 2001 From: Eldar Singin Date: Mon, 30 Dec 2024 15:19:39 +0200 Subject: [PATCH 3/3] [API-7676]: Sift Java SDK should fail when initialized with null api key --- src/main/java/com/siftscience/SiftClient.java | 20 ++++ .../java/com/siftscience/SiftClientTest.java | 96 +++++++++++++++++++ 2 files changed, 116 insertions(+) diff --git a/src/main/java/com/siftscience/SiftClient.java b/src/main/java/com/siftscience/SiftClient.java index 5b3e471c..e90071b6 100644 --- a/src/main/java/com/siftscience/SiftClient.java +++ b/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); + } + } } diff --git a/src/test/java/com/siftscience/SiftClientTest.java b/src/test/java/com/siftscience/SiftClientTest.java index 60711aa6..d87db9e6 100644 --- a/src/test/java/com/siftscience/SiftClientTest.java +++ b/src/test/java/com/siftscience/SiftClientTest.java @@ -6,12 +6,20 @@ 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; @@ -306,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()); + } + } }