Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove usage of deprecated Backoff from google-http-java-client #1221

Merged
merged 1 commit into from
Dec 28, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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