Skip to content

Commit b2b716a

Browse files
vkryachkovkryachko
authored
Add a timeout test + encapsulate backend response. (#385)
1 parent ff714a2 commit b2b716a

File tree

5 files changed

+51
-33
lines changed

5 files changed

+51
-33
lines changed

‎transport/transport-backend-cct/src/main/java/com/google/android/datatransport/cct/CctTransportBackend.java

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
import com.google.android.datatransport.runtime.EventInternal;
3232
import com.google.android.datatransport.runtime.backends.BackendRequest;
3333
import com.google.android.datatransport.runtime.backends.BackendResponse;
34-
import com.google.android.datatransport.runtime.backends.BackendResponse.Status;
3534
import com.google.android.datatransport.runtime.backends.TransportBackend;
3635
import com.google.android.datatransport.runtime.time.Clock;
3736
import com.google.protobuf.ByteString;
@@ -82,6 +81,7 @@ final class CctTransportBackend implements TransportBackend {
8281
private final URL endPoint;
8382
private final Clock uptimeClock;
8483
private final Clock wallTimeClock;
84+
private final int readTimeout;
8585

8686
private static URL parseUrlOrThrow(String url) {
8787
try {
@@ -92,12 +92,22 @@ private static URL parseUrlOrThrow(String url) {
9292
}
9393

9494
CctTransportBackend(
95-
Context applicationContext, String url, Clock wallTimeClock, Clock uptimeClock) {
95+
Context applicationContext,
96+
String url,
97+
Clock wallTimeClock,
98+
Clock uptimeClock,
99+
int readTimeout) {
96100
this.connectivityManager =
97101
(ConnectivityManager) applicationContext.getSystemService(Context.CONNECTIVITY_SERVICE);
98102
this.endPoint = parseUrlOrThrow(url);
99103
this.uptimeClock = uptimeClock;
100104
this.wallTimeClock = wallTimeClock;
105+
this.readTimeout = readTimeout;
106+
}
107+
108+
CctTransportBackend(
109+
Context applicationContext, String url, Clock wallTimeClock, Clock uptimeClock) {
110+
this(applicationContext, url, wallTimeClock, uptimeClock, READ_TIME_OUT);
101111
}
102112

103113
@Override
@@ -186,7 +196,7 @@ private BatchedLogRequest getRequestBody(BackendRequest backendRequest) {
186196
private BackendResponse doSend(BatchedLogRequest requestBody) throws IOException {
187197
HttpURLConnection connection = (HttpURLConnection) endPoint.openConnection();
188198
connection.setConnectTimeout(CONNECTION_TIME_OUT);
189-
connection.setReadTimeout(READ_TIME_OUT);
199+
connection.setReadTimeout(readTimeout);
190200
connection.setDoOutput(true);
191201
connection.setInstanceFollowRedirects(false);
192202
connection.setRequestMethod("POST");
@@ -206,15 +216,15 @@ private BackendResponse doSend(BatchedLogRequest requestBody) throws IOException
206216
try {
207217
nextRequestMillis = LogResponse.parseFrom(inputStream).getNextRequestWaitMillis();
208218
} catch (InvalidProtocolBufferException e) {
209-
return BackendResponse.create(Status.NONTRANSIENT_ERROR, -1);
219+
return BackendResponse.fatalError();
210220
}
211221
}
212222
if (responseCode == 200) {
213-
return BackendResponse.create(Status.OK, nextRequestMillis);
223+
return BackendResponse.ok(nextRequestMillis);
214224
} else if (responseCode >= 500 || responseCode == 404) {
215-
return BackendResponse.create(Status.TRANSIENT_ERROR, -1);
225+
return BackendResponse.transientError();
216226
} else {
217-
return BackendResponse.create(Status.NONTRANSIENT_ERROR, -1);
227+
return BackendResponse.fatalError();
218228
}
219229
}
220230
}
@@ -226,7 +236,7 @@ public BackendResponse send(BackendRequest request) {
226236
return doSend(requestBody);
227237
} catch (IOException e) {
228238
LOGGER.log(Level.SEVERE, "Could not make request to the backend", e);
229-
return BackendResponse.create(Status.TRANSIENT_ERROR, -1);
239+
return BackendResponse.transientError();
230240
}
231241
}
232242

‎transport/transport-backend-cct/src/test/java/com/google/android/datatransport/cct/CctTransportBackendTest.java

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
import com.google.android.datatransport.runtime.EventInternal;
4040
import com.google.android.datatransport.runtime.backends.BackendRequest;
4141
import com.google.android.datatransport.runtime.backends.BackendResponse;
42-
import com.google.android.datatransport.runtime.backends.BackendResponse.Status;
4342
import com.google.android.datatransport.runtime.time.TestClock;
4443
import com.google.protobuf.ByteString;
4544
import java.util.Collections;
@@ -133,8 +132,7 @@ public void testSuccessLoggingRequest() {
133132
.setMobileSubtypeValue(activeNetworkInfo.getSubtype())
134133
.build()))));
135134

136-
assertEquals(response.getStatus(), Status.OK);
137-
assertEquals(response.getNextRequestWaitMillis(), 3);
135+
assertEquals(response, BackendResponse.ok(3));
138136
}
139137

140138
@Test
@@ -144,8 +142,7 @@ public void testUnsuccessfulLoggingRequest() {
144142
verify(
145143
postRequestedFor(urlEqualTo("/api"))
146144
.withHeader("Content-Type", equalTo("application/x-protobuf")));
147-
assertEquals(response.getStatus(), Status.TRANSIENT_ERROR);
148-
assertEquals(response.getNextRequestWaitMillis(), -1);
145+
assertEquals(response, BackendResponse.transientError());
149146
}
150147

151148
@Test
@@ -155,8 +152,7 @@ public void testServerErrorLoggingRequest() {
155152
verify(
156153
postRequestedFor(urlEqualTo("/api"))
157154
.withHeader("Content-Type", equalTo("application/x-protobuf")));
158-
assertEquals(response.getStatus(), Status.TRANSIENT_ERROR);
159-
assertEquals(response.getNextRequestWaitMillis(), -1);
155+
assertEquals(response, BackendResponse.transientError());
160156
}
161157

162158
@Test
@@ -172,8 +168,7 @@ public void testGarbageFromServer() {
172168
verify(
173169
postRequestedFor(urlEqualTo("/api"))
174170
.withHeader("Content-Type", equalTo("application/x-protobuf")));
175-
assertEquals(response.getStatus(), Status.NONTRANSIENT_ERROR);
176-
assertEquals(response.getNextRequestWaitMillis(), -1);
171+
assertEquals(response, BackendResponse.fatalError());
177172
}
178173

179174
@Test
@@ -183,7 +178,17 @@ public void testNonHandledResponseCode() {
183178
verify(
184179
postRequestedFor(urlEqualTo("/api"))
185180
.withHeader("Content-Type", equalTo("application/x-protobuf")));
186-
assertEquals(response.getStatus(), Status.NONTRANSIENT_ERROR);
187-
assertEquals(response.getNextRequestWaitMillis(), -1);
181+
assertEquals(response, BackendResponse.fatalError());
182+
}
183+
184+
@Test
185+
public void send_whenBackendResponseTimesOut_shouldReturnTransientError() {
186+
CctTransportBackend backend =
187+
new CctTransportBackend(
188+
RuntimeEnvironment.application, TEST_ENDPOINT, wallClock, uptimeClock, 300);
189+
stubFor(post(urlEqualTo("/api")).willReturn(aResponse().withFixedDelay(500)));
190+
BackendResponse response = backend.send(getBackendRequest());
191+
192+
assertEquals(response, BackendResponse.transientError());
188193
}
189194
}

‎transport/transport-runtime/src/main/java/com/google/android/datatransport/runtime/backends/BackendResponse.java

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public abstract class BackendResponse {
2626
public enum Status {
2727
OK,
2828
TRANSIENT_ERROR,
29-
NONTRANSIENT_ERROR,
29+
FATAL_ERROR,
3030
}
3131

3232
/** Status result of the backend call */
@@ -35,8 +35,15 @@ public enum Status {
3535
/** Time in millis to wait before attempting another request. */
3636
public abstract long getNextRequestWaitMillis();
3737

38-
/** Create a new instance of {@link BackendResponse}. */
39-
public static BackendResponse create(Status status, long nextRequestMillis) {
40-
return new AutoValue_BackendResponse(status, nextRequestMillis);
38+
public static BackendResponse transientError() {
39+
return new AutoValue_BackendResponse(Status.TRANSIENT_ERROR, -1);
40+
}
41+
42+
public static BackendResponse fatalError() {
43+
return new AutoValue_BackendResponse(Status.FATAL_ERROR, -1);
44+
}
45+
46+
public static BackendResponse ok(long nextRequestWaitMillis) {
47+
return new AutoValue_BackendResponse(Status.OK, nextRequestWaitMillis);
4148
}
4249
}

‎transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/backends/TestBackendFactory.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public EventInternal decorate(EventInternal event) {
3232

3333
@Override
3434
public BackendResponse send(BackendRequest backendRequest) {
35-
return BackendResponse.create(BackendResponse.Status.OK, 0);
35+
return BackendResponse.ok(0);
3636
}
3737
}
3838
}

‎transport/transport-runtime/src/test/java/com/google/android/datatransport/runtime/scheduling/jobscheduling/UploaderTest.java

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,7 @@ public void upload_noNetwork() {
9595

9696
@Test
9797
public void upload_yesNetwork() {
98-
when(mockBackend.send(any()))
99-
.thenReturn(BackendResponse.create(BackendResponse.Status.OK, 1000));
98+
when(mockBackend.send(any())).thenReturn(BackendResponse.ok(1000));
10099
when(uploader.isNetworkAvailable()).thenReturn(Boolean.TRUE);
101100
uploader.upload(BACKEND_NAME, 1, mockRunnable);
102101
verify(uploader, times(1)).logAndUpdateState(TRANSPORT_CONTEXT, 1);
@@ -105,25 +104,22 @@ public void upload_yesNetwork() {
105104

106105
@Test
107106
public void logAndUpdateStatus_okResponse() {
108-
when(mockBackend.send(any()))
109-
.thenReturn(BackendResponse.create(BackendResponse.Status.OK, 1000));
107+
when(mockBackend.send(any())).thenReturn(BackendResponse.ok(1000));
110108
uploader.logAndUpdateState(TRANSPORT_CONTEXT, 1);
111109
verify(store, times(1)).recordSuccess(any());
112110
verify(store, times(1)).recordNextCallTime(TRANSPORT_CONTEXT, 1002);
113111
}
114112

115113
@Test
116114
public void logAndUpdateStatus_nontransientResponse() {
117-
when(mockBackend.send(any()))
118-
.thenReturn(BackendResponse.create(BackendResponse.Status.NONTRANSIENT_ERROR, -1));
115+
when(mockBackend.send(any())).thenReturn(BackendResponse.fatalError());
119116
uploader.logAndUpdateState(TRANSPORT_CONTEXT, 1);
120117
verify(store, times(1)).recordSuccess(any());
121118
}
122119

123120
@Test
124121
public void logAndUpdateStatus_transientReponse() {
125-
when(mockBackend.send(any()))
126-
.thenReturn(BackendResponse.create(BackendResponse.Status.TRANSIENT_ERROR, -1));
122+
when(mockBackend.send(any())).thenReturn(BackendResponse.transientError());
127123
uploader.logAndUpdateState(TRANSPORT_CONTEXT, 1);
128124
verify(store, times(1)).recordFailure(any());
129125
verify(mockScheduler, times(1)).schedule(TRANSPORT_CONTEXT, 2);
@@ -137,7 +133,7 @@ public void logAndUpdateStatus_whenMoreEventsAvailableInStore_shouldReschedule()
137133
invocation -> {
138134
// store a new event
139135
store.persist(TRANSPORT_CONTEXT, EVENT);
140-
return BackendResponse.create(BackendResponse.Status.OK, 1000);
136+
return BackendResponse.ok(1000);
141137
});
142138
Iterable<PersistedEvent> persistedEvents = store.loadBatch(TRANSPORT_CONTEXT);
143139
uploader.logAndUpdateState(TRANSPORT_CONTEXT, 1);

0 commit comments

Comments
 (0)