Skip to content

Commit 162e766

Browse files
jaeoptjaeopt
committed
add more tests
1 parent 35a3876 commit 162e766

File tree

4 files changed

+195
-8
lines changed

4 files changed

+195
-8
lines changed

‎android-sdk/src/main/java/com/optimizely/ab/android/sdk/cmab/CMABClient.kt‎ renamed to ‎android-sdk/src/main/java/com/optimizely/ab/android/sdk/cmab/DefaultCmabClient.kt‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import java.net.HttpURLConnection
2121
import java.net.URL
2222

2323
@VisibleForTesting(otherwise = VisibleForTesting.PROTECTED)
24-
open class CMABClient(private val client: Client, private val logger: Logger) {
24+
open class DefaultCmabClient(private val client: Client, private val logger: Logger) {
2525

2626
@VisibleForTesting(otherwise = VisibleForTesting.PROTECTED)
2727
fun fetchDecision(
@@ -68,7 +68,7 @@ open class CMABClient(private val client: Client, private val logger: Logger) {
6868
} else {
6969
val errorMessage: String = java.lang.String.format(
7070
CmabClientHelper.CMAB_FETCH_FAILED,
71-
statusLine.getReasonPhrase()
71+
urlConnection.responseMessage
7272
)
7373
logger.error(errorMessage)
7474
throw CmabFetchException(errorMessage)
@@ -104,9 +104,11 @@ open class CMABClient(private val client: Client, private val logger: Logger) {
104104
var CONNECTION_TIMEOUT = 10 * 1000
105105
var READ_TIMEOUT = 60 * 1000
106106

107-
// the numerical base for the exponential backoff
108-
const val REQUEST_BACKOFF_TIMEOUT = 2
109-
// power the number of retries
110-
const val REQUEST_RETRIES_POWER = 3
107+
// cmab service retries only once with 1sec interval
108+
109+
// the numerical base for the exponential backoff (1 second)
110+
const val REQUEST_BACKOFF_TIMEOUT = 1
111+
// retry only once = 2 total attempts
112+
const val REQUEST_RETRIES_POWER = 2
111113
}
112114
}

‎android-sdk/src/test/java/com/optimizely/ab/android/sdk/cmab/DefaultCmabClientTest.java‎

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
// Copyright 2025, Optimizely, Inc. and contributors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// https://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package com.optimizely.ab.android.sdk.cmab;
16+
17+
import com.optimizely.ab.android.shared.Client;
18+
import org.junit.Before;
19+
import org.junit.Test;
20+
import org.junit.runner.RunWith;
21+
import org.mockito.ArgumentCaptor;
22+
import org.powermock.api.mockito.PowerMockito;
23+
import org.powermock.core.classloader.annotations.PowerMockIgnore;
24+
import org.powermock.core.classloader.annotations.PrepareForTest;
25+
import org.powermock.modules.junit4.PowerMockRunner;
26+
import org.slf4j.Logger;
27+
28+
import java.io.ByteArrayOutputStream;
29+
import java.io.IOException;
30+
import java.net.HttpURLConnection;
31+
import java.net.URL;
32+
import java.util.HashMap;
33+
import java.util.Map;
34+
35+
import static org.junit.Assert.*;
36+
import static org.mockito.ArgumentMatchers.*;
37+
import static org.mockito.Mockito.*;
38+
import static org.powermock.api.mockito.PowerMockito.mockStatic;
39+
import static org.powermock.api.mockito.PowerMockito.when;
40+
41+
/**
42+
* Tests for {@link DefaultCmabClient}
43+
*/
44+
@RunWith(PowerMockRunner.class)
45+
@PrepareForTest({CmabClientHelper.class})
46+
@PowerMockIgnore({"javax.net.ssl.*", "javax.security.*"})
47+
public class DefaultCmabClientTest {
48+
49+
private Client mockClient;
50+
private Logger mockLogger;
51+
private HttpURLConnection mockUrlConnection;
52+
private ByteArrayOutputStream mockOutputStream;
53+
54+
private DefaultCmabClient cmabClient;
55+
private String testRuleId = "test-rule-123";
56+
private String testUserId = "test-user-456";
57+
private String testCmabUuid = "test-uuid-789";
58+
private Map<String, Object> testAttributes;
59+
60+
@Before
61+
public void setup() {
62+
mockClient = mock(Client.class);
63+
mockLogger = mock(Logger.class);
64+
mockUrlConnection = mock(HttpURLConnection.class);
65+
mockOutputStream = mock(ByteArrayOutputStream.class);
66+
67+
cmabClient = new DefaultCmabClient(mockClient, mockLogger);
68+
69+
testAttributes = new HashMap<>();
70+
testAttributes.put("age", 25);
71+
testAttributes.put("country", "US");
72+
73+
// Mock static methods
74+
mockStatic(CmabClientHelper.class);
75+
}
76+
77+
@Test
78+
public void testFetchDecisionSuccess() throws Exception {
79+
// Mock successful HTTP response
80+
String mockResponseJson = "{\"variation_id\":\"variation_1\",\"status\":\"success\"}";
81+
when(mockClient.openConnection(any(URL.class))).thenReturn(mockUrlConnection);
82+
when(mockUrlConnection.getResponseCode()).thenReturn(200);
83+
when(mockClient.readStream(mockUrlConnection)).thenReturn(mockResponseJson);
84+
when(mockUrlConnection.getOutputStream()).thenReturn(mockOutputStream);
85+
when(mockClient.execute(any(Client.Request.class), eq(2), eq(2))).thenAnswer(invocation -> {
86+
Client.Request<String> request = invocation.getArgument(0);
87+
return request.execute();
88+
});
89+
90+
// Mock the helper methods
91+
when(CmabClientHelper.buildRequestJson(testUserId, testRuleId, testAttributes, testCmabUuid))
92+
.thenReturn("{\"user_id\":\"test-user-456\"}");
93+
when(CmabClientHelper.validateResponse(mockResponseJson)).thenReturn(true);
94+
when(CmabClientHelper.parseVariationId(mockResponseJson)).thenReturn("variation_1");
95+
96+
String result = cmabClient.fetchDecision(testRuleId, testUserId, testAttributes, testCmabUuid);
97+
98+
assertEquals("variation_1", result);
99+
verify(mockUrlConnection).setConnectTimeout(10000);
100+
verify(mockUrlConnection).setReadTimeout(60000);
101+
verify(mockUrlConnection).setRequestMethod("POST");
102+
verify(mockUrlConnection).setRequestProperty("content-type", "application/json");
103+
verify(mockUrlConnection).setDoOutput(true);
104+
verify(mockLogger).debug("Successfully fetched CMAB decision: {}", mockResponseJson);
105+
}
106+
107+
@Test
108+
public void testFetchDecisionConnectionFailure() throws Exception {
109+
// Mock connection failure
110+
when(mockClient.openConnection(any(URL.class))).thenReturn(null);
111+
when(mockClient.execute(any(Client.Request.class), eq(2), eq(2))).thenAnswer(invocation -> {
112+
Client.Request<String> request = invocation.getArgument(0);
113+
return request.execute();
114+
});
115+
116+
// Mock the helper methods
117+
when(CmabClientHelper.buildRequestJson(testUserId, testRuleId, testAttributes, testCmabUuid))
118+
.thenReturn("{\"user_id\":\"test-user-456\"}");
119+
120+
String result = cmabClient.fetchDecision(testRuleId, testUserId, testAttributes, testCmabUuid);
121+
122+
assertNull(result);
123+
}
124+
125+
@Test
126+
public void testConnectionTimeouts() throws Exception {
127+
when(mockClient.openConnection(any(URL.class))).thenReturn(mockUrlConnection);
128+
when(mockUrlConnection.getResponseCode()).thenReturn(200);
129+
when(mockClient.readStream(mockUrlConnection)).thenReturn("{\"variation_id\":\"test\"}");
130+
when(mockUrlConnection.getOutputStream()).thenReturn(mockOutputStream);
131+
when(mockClient.execute(any(Client.Request.class), anyInt(), anyInt())).thenAnswer(invocation -> {
132+
Client.Request<String> request = invocation.getArgument(0);
133+
return request.execute();
134+
});
135+
136+
when(CmabClientHelper.buildRequestJson(any(), any(), any(), any())).thenReturn("{}");
137+
when(CmabClientHelper.validateResponse(any())).thenReturn(true);
138+
when(CmabClientHelper.parseVariationId(any())).thenReturn("test");
139+
140+
cmabClient.fetchDecision(testRuleId, testUserId, testAttributes, testCmabUuid);
141+
142+
verify(mockUrlConnection).setConnectTimeout(10 * 1000); // 10 seconds
143+
verify(mockUrlConnection).setReadTimeout(60 * 1000); // 60 seconds
144+
}
145+
146+
@Test
147+
public void testRetryOnFailureWithRetryBackoff() throws Exception {
148+
when(mockClient.openConnection(any(URL.class))).thenReturn(mockUrlConnection);
149+
when(mockUrlConnection.getResponseCode()).thenReturn(500);
150+
when(mockUrlConnection.getResponseMessage()).thenReturn("Server Error");
151+
when(mockUrlConnection.getOutputStream()).thenReturn(mockOutputStream);
152+
153+
when(mockClient.execute(any(Client.Request.class), eq(1), eq(2))).thenReturn(null);
154+
155+
when(CmabClientHelper.buildRequestJson(any(), any(), any(), any())).thenReturn("{}");
156+
157+
String result = cmabClient.fetchDecision(testRuleId, testUserId, testAttributes, testCmabUuid);
158+
assertNull(result);
159+
160+
// Verify the retry configuration matches our constants
161+
verify(mockClient).execute(any(Client.Request.class), eq(DefaultCmabClient.REQUEST_BACKOFF_TIMEOUT), eq(DefaultCmabClient.REQUEST_RETRIES_POWER));
162+
assertEquals("REQUEST_BACKOFF_TIMEOUT should be 1", 1, DefaultCmabClient.REQUEST_BACKOFF_TIMEOUT);
163+
assertEquals("REQUEST_RETRIES_POWER should be 2", 2, DefaultCmabClient.REQUEST_RETRIES_POWER);
164+
}
165+
}

‎shared/src/androidTest/java/com/optimizely/ab/android/shared/ClientTest.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,20 @@ public void testExpBackoffFailure() {
145145
assertTrue(timeouts.contains(16));
146146
}
147147

148+
@Test
149+
public void testExpBackoffFailure_with_one_second_timeout() {
150+
Client.Request request = mock(Client.Request.class);
151+
when(request.execute()).thenReturn(null);
152+
// one second timeout is a corner case - pow(1, 4) = 1
153+
assertNull(client.execute(request, 1, 2));
154+
ArgumentCaptor<Integer> captor = ArgumentCaptor.forClass(Integer.class);
155+
verify(logger, times(2)).info(eq("Request failed, waiting {} seconds to try again"), captor.capture());
156+
List<Integer> timeouts = captor.getAllValues();
157+
assertTrue(timeouts.contains(1));
158+
assertTrue(timeouts.contains(1));
159+
}
160+
161+
148162
@Test
149163
public void testExpBackoffFailure_noRetriesWhenBackoffSetToZero() {
150164
Client.Request request = mock(Client.Request.class);

‎shared/src/main/java/com/optimizely/ab/android/shared/Client.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,9 +152,12 @@ public String readStream(@NonNull URLConnection urlConnection) {
152152
*/
153153
public <T> T execute(Request<T> request, int timeout, int power) {
154154
int baseTimeout = timeout;
155-
int maxTimeout = (int) Math.pow(baseTimeout, power);
156155
T response = null;
157-
while(timeout <= maxTimeout) {
156+
int attempts = 0;
157+
int maxAttempts = power + 1; // power represents retries, so total attempts = power + 1
158+
159+
while(attempts < maxAttempts) {
160+
attempts++;
158161
try {
159162
response = request.execute();
160163
} catch (Exception e) {
@@ -165,6 +168,9 @@ public <T> T execute(Request<T> request, int timeout, int power) {
165168
// retry is disabled when timeout set to 0
166169
if (timeout == 0) break;
167170

171+
// don't sleep if this was the last attempt
172+
if (attempts >= maxAttempts) break;
173+
168174
try {
169175
logger.info("Request failed, waiting {} seconds to try again", timeout);
170176
Thread.sleep(TimeUnit.MILLISECONDS.convert(timeout, TimeUnit.SECONDS));

0 commit comments

Comments
 (0)