Skip to content

Commit

Permalink
Remove usage of deprecated Backoff from google-http-java-client (#1221)
Browse files Browse the repository at this point in the history
  • Loading branch information
chingor13 authored Dec 28, 2018
1 parent dd80189 commit 2079c4c
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 84 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

package com.google.api.client.googleapis.batch;

import com.google.api.client.http.BackOffPolicy;
import com.google.api.client.http.GenericUrl;
import com.google.api.client.http.HttpBackOffUnsuccessfulResponseHandler;
import com.google.api.client.http.HttpExecuteInterceptor;
Expand Down Expand Up @@ -93,7 +92,6 @@ public void onFailure(GoogleJsonErrorContainer e, HttpHeaders responseHeaders) {
* @since 1.9
* @author [email protected] (Ravi Mistry)
*/
@SuppressWarnings("deprecation")
public final class BatchRequest {

/** The URL where batch requests are sent. */
Expand Down Expand Up @@ -220,12 +218,6 @@ public void execute() throws IOException {
HttpExecuteInterceptor originalInterceptor = batchRequest.getInterceptor();
batchRequest.setInterceptor(new BatchInterceptor(originalInterceptor));
int retriesRemaining = batchRequest.getNumberOfRetries();
BackOffPolicy backOffPolicy = batchRequest.getBackOffPolicy();

if (backOffPolicy != null) {
// Reset the BackOffPolicy at the start of each execute.
backOffPolicy.reset();
}

do {
retryAllowed = retriesRemaining > 0;
Expand Down Expand Up @@ -259,17 +251,6 @@ public void execute() throws IOException {
List<RequestInfo<?, ?>> unsuccessfulRequestInfos = batchResponse.unsuccessfulRequestInfos;
if (!unsuccessfulRequestInfos.isEmpty()) {
requestInfos = unsuccessfulRequestInfos;
// backOff if required.
if (batchResponse.backOffRequired && backOffPolicy != null) {
long backOffTime = backOffPolicy.getNextBackOffMillis();
if (backOffTime != BackOffPolicy.STOP) {
try {
sleeper.sleep(backOffTime);
} catch (InterruptedException exception) {
// ignore
}
}
}
} else {
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package com.google.api.client.googleapis.batch;

import com.google.api.client.googleapis.batch.BatchRequest.RequestInfo;
import com.google.api.client.http.BackOffPolicy;
import com.google.api.client.http.GenericUrl;
import com.google.api.client.http.HttpContent;
import com.google.api.client.http.HttpHeaders;
Expand All @@ -42,7 +41,6 @@
*
* @author [email protected] (Ravi Mistry)
*/
@SuppressWarnings("deprecation")
final class BatchUnparsedResponse {

/** The boundary used in the batch response to separate individual responses. */
Expand All @@ -60,9 +58,6 @@ final class BatchUnparsedResponse {
/** List of unsuccessful HTTP requests that can be retried. */
List<RequestInfo<?, ?>> unsuccessfulRequestInfos = new ArrayList<RequestInfo<?, ?>>();

/** Indicates if back off is required before the next retry. */
boolean backOffRequired;

/** The content Id the response is currently at. */
private int contentId = 0;

Expand Down Expand Up @@ -183,10 +178,6 @@ private <T, E> void parseAndCallback(
HttpHeaders responseHeaders = response.getHeaders();
HttpUnsuccessfulResponseHandler unsuccessfulResponseHandler =
requestInfo.request.getUnsuccessfulResponseHandler();
BackOffPolicy backOffPolicy = requestInfo.request.getBackOffPolicy();

// Reset backOff flag.
backOffRequired = false;

if (HttpStatusCodes.isSuccess(statusCode)) {
if (callback == null) {
Expand All @@ -207,12 +198,9 @@ private <T, E> void parseAndCallback(
if (!errorHandled) {
if (requestInfo.request.handleRedirect(response.getStatusCode(), response.getHeaders())) {
redirectRequest = true;
} else if (retrySupported && backOffPolicy != null
&& backOffPolicy.isBackOffRequired(response.getStatusCode())) {
backOffRequired = true;
}
}
if (retrySupported && (errorHandled || backOffRequired || redirectRequest)) {
if (retrySupported && (errorHandled || redirectRequest)) {
unsuccessfulRequestInfos.add(requestInfo);
} else {
if (callback == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
*
* @author [email protected] (Ravi Mistry)
*/
@SuppressWarnings("deprecation")
public class BatchRequestTest extends TestCase {

private static final String ROOT_URL = "http://www.test.com/";
Expand Down Expand Up @@ -301,19 +300,16 @@ private static class MockTransport extends MockHttpTransport {
final boolean testAuthenticationError;
boolean returnSuccessAuthenticatedContent;
boolean returnErrorAuthenticatedContent;
final boolean testExponentialBackOff;
final boolean testRedirect;
final boolean testBinary;
final boolean testMissingLength;
int actualCalls;
int callsBeforeSuccess;

MockTransport(boolean testServerError, boolean testAuthenticationError,
boolean testExponentialBackOff, boolean testRedirect, boolean testBinary,
MockTransport(boolean testServerError, boolean testAuthenticationError, boolean testRedirect, boolean testBinary,
boolean testMissingLength) {
this.testServerError = testServerError;
this.testAuthenticationError = testAuthenticationError;
this.testExponentialBackOff = testExponentialBackOff;
this.testRedirect = testRedirect;
this.testBinary = testBinary;
this.testMissingLength = testMissingLength;
Expand Down Expand Up @@ -355,8 +351,7 @@ public LowLevelHttpResponse execute() throws IOException {
+ "\"code\": " + ERROR_CODE + ", \"message\": \"" + ERROR_MSG + "\"}}");
ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
Writer responseContent = new OutputStreamWriter(outputStream, "ISO-8859-1");
if (returnSuccessAuthenticatedContent || (testExponentialBackOff && actualCalls > 1)
|| (testRedirect && actualCalls > 1)) {
if (returnSuccessAuthenticatedContent || (testRedirect && actualCalls > 1)) {
if (returnSuccessAuthenticatedContent || actualCalls == callsBeforeSuccess) {
responseContent.append("--" + RESPONSE_BOUNDARY + "\n")
.append("Content-Type: application/http\n")
Expand Down Expand Up @@ -483,13 +478,11 @@ public void intercept(HttpRequest request) {
private BatchRequest getBatchPopulatedWithRequests(boolean testServerError,
boolean testAuthenticationError,
boolean returnSuccessAuthenticatedContent,
boolean testExponentialBackOff,
boolean testRedirect,
boolean testBinary,
boolean testMissingLength) throws Exception {
transport = new MockTransport(testServerError,
testAuthenticationError,
testExponentialBackOff,
testRedirect,
testBinary,
testMissingLength);
Expand All @@ -514,9 +507,6 @@ private BatchRequest getBatchPopulatedWithRequests(boolean testServerError,
request2.setUnsuccessfulResponseHandler(
new MockUnsuccessfulResponseHandler(transport, returnSuccessAuthenticatedContent));
}
if (testExponentialBackOff) {
request2.setBackOffPolicy(new MockExponentialBackOffPolicy());
}

if (testBinary) {
batchRequest.queue(request1, MockData.Class1.class, ErrorOutput.ErrorBody.class,
Expand All @@ -532,7 +522,7 @@ private BatchRequest getBatchPopulatedWithRequests(boolean testServerError,

public void testQueueDatastructures() throws Exception {
BatchRequest batchRequest =
getBatchPopulatedWithRequests(false, false, false, false, false, false, false);
getBatchPopulatedWithRequests(false, false, false, false, false, false);
List<RequestInfo<?, ?>> requestInfos = batchRequest.requestInfos;

// Assert that the expected objects are queued.
Expand All @@ -552,7 +542,7 @@ public void testQueueDatastructures() throws Exception {

public void testExecute() throws Exception {
BatchRequest batchRequest =
getBatchPopulatedWithRequests(false, false, false, false, false, false, false);
getBatchPopulatedWithRequests(false, false, false, false, false, false);
batchRequest.execute();
// Assert callbacks have been invoked.
assertEquals(1, callback1.successCalls);
Expand All @@ -564,7 +554,7 @@ public void testExecute() throws Exception {

public void testExecuteWithError() throws Exception {
BatchRequest batchRequest =
getBatchPopulatedWithRequests(true, false, false, false, false, false, false);
getBatchPopulatedWithRequests(true, false, false, false, false, false);
batchRequest.execute();
// Assert callbacks have been invoked.
assertEquals(1, callback1.successCalls);
Expand Down Expand Up @@ -593,7 +583,7 @@ public void testExecuteWithVoidCallbackError() throws Exception {
}

public void subTestExecuteWithVoidCallback(boolean testServerError) throws Exception {
MockTransport transport = new MockTransport(testServerError, false, false, false, false, false);
MockTransport transport = new MockTransport(testServerError, false,false, false, false);
MockGoogleClient client = new MockGoogleClient.Builder(
transport, ROOT_URL, SERVICE_PATH, null, null).setApplicationName("Test Application")
.build();
Expand All @@ -617,7 +607,7 @@ public void subTestExecuteWithVoidCallback(boolean testServerError) throws Excep

public void testExecuteWithAuthenticationErrorThenSuccessCallback() throws Exception {
BatchRequest batchRequest =
getBatchPopulatedWithRequests(false, true, true, false, false, false, false);
getBatchPopulatedWithRequests(false, true, true, false, false, false);
batchRequest.execute();
// Assert callbacks have been invoked.
assertEquals(1, callback1.successCalls);
Expand All @@ -631,7 +621,7 @@ public void testExecuteWithAuthenticationErrorThenSuccessCallback() throws Excep

public void testExecuteWithAuthenticationErrorThenErrorCallback() throws Exception {
BatchRequest batchRequest =
getBatchPopulatedWithRequests(false, true, false, false, false, false, false);
getBatchPopulatedWithRequests(false, true, false, false, false, false);
batchRequest.execute();
// Assert callbacks have been invoked.
assertEquals(1, callback1.successCalls);
Expand All @@ -643,38 +633,9 @@ public void testExecuteWithAuthenticationErrorThenErrorCallback() throws Excepti
assertTrue(batchRequest.requestInfos.isEmpty());
}

public void testExecuteWithExponentialBackoffThenSuccessCallback() throws Exception {
BatchRequest batchRequest =
getBatchPopulatedWithRequests(true, false, false, true, false, false, false);
transport.callsBeforeSuccess = 2;
batchRequest.execute();
// Assert callbacks have been invoked.
assertEquals(1, callback1.successCalls);
assertEquals(1, callback2.successCalls);
// Assert transport called expected number of times.
assertEquals(2, transport.actualCalls);
// Assert requestInfos is empty after execute.
assertTrue(batchRequest.requestInfos.isEmpty());
}

public void testExecuteWithExponentialBackoffThenErrorCallback() throws Exception {
BatchRequest batchRequest =
getBatchPopulatedWithRequests(true, false, false, true, false, false, false);
transport.callsBeforeSuccess = 20;
batchRequest.execute();
// Assert callbacks have been invoked.
assertEquals(1, callback1.successCalls);
assertEquals(0, callback2.successCalls);
assertEquals(1, callback2.failureCalls);
// Assert transport called expected number of times.
assertEquals(11, transport.actualCalls);
// Assert requestInfos is empty after execute.
assertTrue(batchRequest.requestInfos.isEmpty());
}

public void testInterceptor() throws Exception {
BatchRequest batchRequest =
getBatchPopulatedWithRequests(true, false, false, true, false, false, false);
getBatchPopulatedWithRequests(true, false, false, false, false, false);
batchRequest.execute();
// Assert the top-level request initializer is called.
assertTrue(credential.initializerCalled);
Expand All @@ -683,7 +644,7 @@ public void testInterceptor() throws Exception {

public void testRedirect() throws Exception {
BatchRequest batchRequest =
getBatchPopulatedWithRequests(false, false, false, false, true, false, false);
getBatchPopulatedWithRequests(false, false, false, true, false, false);
transport.callsBeforeSuccess = 2;
batchRequest.execute();
// Assert transport called expected number of times.
Expand Down Expand Up @@ -820,7 +781,7 @@ public boolean retrySupported() {

public void testProtoExecute() throws Exception {
BatchRequest batchRequest =
getBatchPopulatedWithRequests(false, false, false, false, false, true, false);
getBatchPopulatedWithRequests(false, false, false, false, true, false);
batchRequest.execute();
// Assert callbacks have been invoked.
assertEquals(1, callback1.successCalls);
Expand All @@ -832,7 +793,7 @@ public void testProtoExecute() throws Exception {

public void testProtoExecuteWithError() throws Exception {
BatchRequest batchRequest =
getBatchPopulatedWithRequests(true, false, false, false, false, true, false);
getBatchPopulatedWithRequests(true, false, false, false, true, false);
batchRequest.execute();
// Assert callbacks have been invoked.
assertEquals(1, callback1.successCalls);
Expand All @@ -846,7 +807,7 @@ public void testProtoExecuteWithError() throws Exception {

public void testProtoExecuteWithoutLength() throws Exception {
BatchRequest batchRequest =
getBatchPopulatedWithRequests(false, false, false, false, false, true, true);
getBatchPopulatedWithRequests(false, false, false, false, true, true);
batchRequest.execute();
// Assert callbacks have been invoked.
assertEquals(1, callback1.successCalls);
Expand Down

0 comments on commit 2079c4c

Please sign in to comment.