Skip to content

Commit c75fcaa

Browse files
esingin-siftesingin-sift
authored
[API-8040, API-7676]: Error handling and input validation improvements (#114)
* [API-8040]: Include HTTP status code in response message for Sift exception * [API-7676]: Sift Java SDK should fail when initialized with null api key
1 parent 630fd87 commit c75fcaa

File tree

4 files changed

+188
-46
lines changed

4 files changed

+188
-46
lines changed

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ public SiftClient(String apiKey, String accountId) {
5656
}
5757

5858
public SiftClient(String apiKey, String accountId, OkHttpClient okHttpClient) {
59+
assertNotNull(apiKey, "API key must not be null");
60+
assertNotNull(okHttpClient, "Http Client must not be null");
5961
this.apiKey = apiKey;
6062
this.accountId = accountId;
6163
this.httpClient = new HttpClient(okHttpClient);
@@ -95,16 +97,19 @@ public EventRequest buildRequest(FieldSet fields) {
9597
}
9698

9799
public ApplyDecisionRequest buildRequest(ApplyDecisionFieldSet fields) {
100+
assertAccountIdIsNotNull();
98101
setupApiKey(fields);
99102
return new ApplyDecisionRequest(baseUrl, getAccountId(), httpClient, fields);
100103
}
101104

102105
public GetDecisionsRequest buildRequest(GetDecisionFieldSet fields) {
106+
assertAccountIdIsNotNull();
103107
setupApiKey(fields);
104108
return new GetDecisionsRequest(baseUrl, getAccountId(), httpClient, fields);
105109
}
106110

107111
public DecisionStatusRequest buildRequest(DecisionStatusFieldSet fields) {
112+
assertAccountIdIsNotNull();
108113
setupApiKey(fields);
109114
return new DecisionStatusRequest(baseUrl, getAccountId(), httpClient, fields);
110115
}
@@ -130,26 +135,31 @@ public UserScoreRequest buildRequest(UserScoreFieldSet fields) {
130135
}
131136

132137
public WorkflowStatusRequest buildRequest(WorkflowStatusFieldSet fields) {
138+
assertAccountIdIsNotNull();
133139
setupApiKey(fields);
134140
return new WorkflowStatusRequest(baseUrl, getAccountId(), httpClient, fields);
135141
}
136142

137143
public GetMerchantsRequest buildRequest(GetMerchantsFieldSet fields) {
144+
assertAccountIdIsNotNull();
138145
setupApiKey(fields);
139146
return new GetMerchantsRequest(baseUrl, getAccountId(), httpClient, fields);
140147
}
141148

142149
public GetMerchantRequest buildRequest(GetMerchantFieldSet fields) {
150+
assertAccountIdIsNotNull();
143151
setupApiKey(fields);
144152
return new GetMerchantRequest(baseUrl, getAccountId(), httpClient, fields);
145153
}
146154

147155
public CreateMerchantRequest buildRequest(CreateMerchantFieldSet fields) {
156+
assertAccountIdIsNotNull();
148157
setupApiKey(fields);
149158
return new CreateMerchantRequest(baseUrl, getAccountId(), httpClient, fields);
150159
}
151160

152161
public UpdateMerchantRequest buildRequest(UpdateMerchantFieldSet fields, String merchantId) {
162+
assertAccountIdIsNotNull();
153163
setupApiKey(fields);
154164
return new UpdateMerchantRequest(baseUrl, getAccountId(), httpClient, fields, merchantId);
155165
}
@@ -172,4 +182,14 @@ public VerificationCheckRequest buildRequest(VerificationCheckFieldSet fields) {
172182
private void setupApiKey(FieldSet fields) {
173183
fields.setApiKey(getApiKey());
174184
}
185+
186+
private void assertAccountIdIsNotNull() {
187+
assertNotNull(getAccountId(), "Account ID must not be null");
188+
}
189+
190+
private void assertNotNull(Object value, String message) {
191+
if (value == null) {
192+
throw new IllegalArgumentException(message);
193+
}
194+
}
175195
}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,10 @@ public SiftResponse getSiftResponse() {
1919
}
2020

2121
private static String responseErrorMessage(SiftResponse response) {
22-
if (response == null || response.getApiErrorMessage() == null) {
22+
if (response == null) {
2323
return "Unexpected API error.";
24+
} else if (response.getApiErrorMessage() == null) {
25+
return "Unexpected API error " + response.getHttpStatusCode() + ".";
2426
}
2527
return response.getApiErrorMessage();
2628
}

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

Lines changed: 113 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,27 @@
44
import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR;
55
import java.io.IOException;
66

7+
import static org.junit.Assert.assertEquals;
8+
import static org.junit.Assert.assertNotNull;
9+
import static org.junit.Assert.fail;
710
import com.siftscience.exception.InvalidApiKeyException;
811
import com.siftscience.exception.InvalidFieldException;
912
import com.siftscience.exception.MissingFieldException;
1013
import com.siftscience.exception.RateLimitException;
1114
import com.siftscience.exception.ServerException;
15+
import com.siftscience.model.ApplyDecisionFieldSet;
16+
import com.siftscience.model.CreateMerchantFieldSet;
1217
import com.siftscience.model.CreateOrderFieldSet;
18+
import com.siftscience.model.DecisionStatusFieldSet;
19+
import com.siftscience.model.GetDecisionFieldSet;
20+
import com.siftscience.model.GetMerchantsFieldSet;
21+
import com.siftscience.model.UpdateMerchantFieldSet;
22+
import com.siftscience.model.WorkflowStatusFieldSet;
1323
import okhttp3.OkHttpClient;
1424
import okhttp3.mockwebserver.MockResponse;
1525
import okhttp3.mockwebserver.MockWebServer;
1626
import org.json.JSONException;
1727
import org.junit.After;
18-
import org.junit.Assert;
1928
import org.junit.Before;
2029
import org.junit.Test;
2130
import org.skyscreamer.jsonassert.JSONAssert;
@@ -93,11 +102,11 @@ public void testInvalidAPIKeyException() throws Exception {
93102
}
94103

95104
// We should have gotten an exception.
96-
Assert.assertNotNull(apiKeyException);
97-
Assert.assertEquals("Invalid API key message.", apiKeyException.getLocalizedMessage());
105+
assertNotNull(apiKeyException);
106+
assertEquals("Invalid API key message.", apiKeyException.getLocalizedMessage());
98107

99108
// Check that we can access the API key from the exception object.
100-
Assert.assertEquals("INVALID_API_KEY",
109+
assertEquals("INVALID_API_KEY",
101110
apiKeyException.getSiftResponse().getRequestBody().getApiKey());
102111
}
103112

@@ -141,8 +150,8 @@ public void testServerSideMissingFieldException() throws IOException {
141150
}
142151

143152
// We should have gotten an exception.
144-
Assert.assertNotNull(missingFieldException);
145-
Assert.assertEquals("Missing user email message from server.",
153+
assertNotNull(missingFieldException);
154+
assertEquals("Missing user email message from server.",
146155
missingFieldException.getLocalizedMessage());
147156
}
148157

@@ -164,8 +173,8 @@ public void testInvalidCustomFieldKeyException() throws IOException {
164173
}
165174

166175
// Should have thrown an exception.
167-
Assert.assertNotNull(invalidFieldException);
168-
Assert.assertEquals("Custom field \"$not_allowed\" may not begin with a dollar sign.",
176+
assertNotNull(invalidFieldException);
177+
assertEquals("Custom field \"$not_allowed\" may not begin with a dollar sign.",
169178
invalidFieldException.getLocalizedMessage());
170179
}
171180

@@ -211,8 +220,8 @@ public void testRateLimitException() throws IOException {
211220
}
212221

213222
// We should have gotten an exception.
214-
Assert.assertNotNull(rateLimitException);
215-
Assert.assertEquals("Rate limit error message.", rateLimitException.getLocalizedMessage());
223+
assertNotNull(rateLimitException);
224+
assertEquals("Rate limit error message.", rateLimitException.getLocalizedMessage());
216225
}
217226

218227
/**
@@ -243,8 +252,9 @@ public void testGenericServerErrorException() throws IOException {
243252
}
244253

245254
// We should have gotten an exception.
246-
Assert.assertNotNull(serverException);
247-
Assert.assertEquals("Unexpected API error.", serverException.getLocalizedMessage());
255+
assertNotNull(serverException);
256+
assertEquals("Unexpected API error " + HTTP_INTERNAL_ERROR + ".",
257+
serverException.getLocalizedMessage());
248258
}
249259

250260
/**
@@ -276,8 +286,9 @@ public void testGenericServerErrorExceptionNonJson() throws IOException {
276286
}
277287

278288
// We should have gotten an exception.
279-
Assert.assertNotNull(serverException);
280-
Assert.assertEquals("Unexpected API error.", serverException.getLocalizedMessage());
289+
assertNotNull(serverException);
290+
assertEquals("Unexpected API error " + HTTP_INTERNAL_ERROR + ".",
291+
serverException.getLocalizedMessage());
281292
}
282293

283294
/**
@@ -303,4 +314,92 @@ public void testNullsAreNotSerialized() throws JSONException {
303314
"}", fieldSet.toJson(), true);
304315
}
305316

317+
@Test
318+
public void testFailsToCreateClientWhenApiKeyIsNull() {
319+
assertIllegalArgumentWithMessage(
320+
() -> new SiftClient(null, "YOUR_ACCOUNT_ID"),
321+
"API key must not be null"
322+
);
323+
}
324+
325+
@Test
326+
public void testFailsToCreateClientWhenHttpClientIsNull() {
327+
assertIllegalArgumentWithMessage(
328+
() -> new SiftClient("YOUR_API_KEY", "YOUR_ACCOUNT_ID",
329+
(OkHttpClient) null),
330+
"Http Client must not be null"
331+
);
332+
}
333+
334+
@Test
335+
public void testFailsToBuildApplyDecisionRequest() {
336+
SiftClient siftClient = new SiftClient("YOUR_API_KEY", null);
337+
assertIllegalArgumentWithMessage(
338+
() -> siftClient.buildRequest(new ApplyDecisionFieldSet()),
339+
"Account ID must not be null"
340+
);
341+
}
342+
343+
@Test
344+
public void testFailsToBuildGetDecisionRequest() {
345+
SiftClient siftClient = new SiftClient("YOUR_API_KEY", null);
346+
assertIllegalArgumentWithMessage(
347+
() -> siftClient.buildRequest(new GetDecisionFieldSet()),
348+
"Account ID must not be null"
349+
);
350+
}
351+
352+
@Test
353+
public void testFailsToBuildDecisionStatusRequest() {
354+
SiftClient siftClient = new SiftClient("YOUR_API_KEY", null);
355+
assertIllegalArgumentWithMessage(
356+
() -> siftClient.buildRequest(new DecisionStatusFieldSet()),
357+
"Account ID must not be null"
358+
);
359+
}
360+
361+
@Test
362+
public void testFailsToBuildWorkflowStatusRequest() {
363+
SiftClient siftClient = new SiftClient("YOUR_API_KEY", null);
364+
assertIllegalArgumentWithMessage(
365+
() -> siftClient.buildRequest(new WorkflowStatusFieldSet()),
366+
"Account ID must not be null"
367+
);
368+
}
369+
370+
@Test
371+
public void testFailsToBuildGetMerchantsRequest() {
372+
SiftClient siftClient = new SiftClient("YOUR_API_KEY", null);
373+
assertIllegalArgumentWithMessage(
374+
() -> siftClient.buildRequest(new GetMerchantsFieldSet()),
375+
"Account ID must not be null"
376+
);
377+
}
378+
379+
@Test
380+
public void testFailsToBuildCreateMerchantRequest() {
381+
SiftClient siftClient = new SiftClient("YOUR_API_KEY", null);
382+
assertIllegalArgumentWithMessage(
383+
() -> siftClient.buildRequest(new CreateMerchantFieldSet()),
384+
"Account ID must not be null"
385+
);
386+
}
387+
388+
@Test
389+
public void testFailsToBuildUpdateMerchantRequest() {
390+
SiftClient siftClient = new SiftClient("YOUR_API_KEY", null);
391+
assertIllegalArgumentWithMessage(
392+
() -> siftClient.buildRequest(new UpdateMerchantFieldSet(), "merchantID"),
393+
"Account ID must not be null"
394+
);
395+
}
396+
397+
private void assertIllegalArgumentWithMessage(Runnable runnable, String message) {
398+
try {
399+
runnable.run();
400+
fail("Excepted to throw IllegalArgumentException");
401+
} catch (IllegalArgumentException exception) {
402+
assertEquals(message, exception.getMessage());
403+
}
404+
}
306405
}

0 commit comments

Comments
 (0)